PR#1084: Remove dangling -debug* RPMs from final MMD - fm-orchestrator

fm-orchestrator

#1084 Remove dangling -debug* RPMs from final MMD

Merged 5 years ago by jkaluza. Opened 5 years ago by lucarval.

lucarval/fm-orchestrator remove-dangling-debug-rpms into master

Remove dangling -debug* RPMs from final MMD

Luiz Carvalho • 5 years ago

086ed4a

module_build_service/builder/KojiContentGenerator.py

file modified

+64 -5

		`@@ -35,6 +35,7 @@`
		`import tempfile`
		`import time`
		`from io import open`
		`+ from collections import defaultdict`
		`import kobo.rpmlib`

		`from six import text_type`
		`@@ -52,6 +53,25 @@`
		`return KojiModuleBuilder.get_session(config, owner)`


		`+ def strip_suffixes(s, suffixes):`
		`+ """`
		`+ Helper function to remove suffixes from given string.`
		`+`
		`+ At most, a single suffix is removed to avoid removing portions`
		`+ of the string that were not originally at its end.`
		`+`
		`+ :param str s: String to operate on`
		`+ :param iter<str> tokens: Iterable of suffix strings`
		`+ :rtype: str`
		`+ :return: String without any of the given suffixes`
		`+ """`
		`+ for suffix in suffixes:`
		`+ if s.endswith(suffix):`
		`+ s = s[:-len(suffix)]`
		`+ break`
		`+ return s`
		`+`
		`+`
		`def koji_retrying_multicall_map(args, *kwargs):`
		`"""`
		`Wrapper around KojiModuleBuilder.koji_retrying_multicall_map, because`
		`@@ -541,9 +561,27 @@`
		`# Modulemd.SimpleSet into which we will add licenses of all RPMs.`
		`rpm_licenses = Modulemd.SimpleSet()`

		`+ # RPM name suffixes that imply relationship to another RPM`
		`+ group_suffixes = ("-debuginfo", "-debugsource")`
		`+`
		`+ # Grouping of "main" RPM with the corresponding debuginfo and debugsource RPMs`
		`+ grouped_rpms = defaultdict(list)`
		`+ source_rpms = {}`
		`+ non_devel_source_rpms = {}`
		`+ for nevra, rpm in self.rpms_dict.items():`
		`+ name = strip_suffixes(rpm["name"], group_suffixes)`
		`+ grouped_rpms[name].append((nevra, rpm))`
		`+ if rpm["arch"] == "src":`
		`+ source_rpms[name] = nevra`
		`+`
		# Check each RPM in `self.rpms_dict` to find out if it can be included in mmd
		`# for this architecture.`
		`for nevra, rpm in self.rpms_dict.items():`
		`+ # Filter out debug and source RPMs, these will later be included if`
		`+ # the "main" RPM is included`
		`+ if rpm["name"].endswith(group_suffixes) or rpm["arch"] == "src":`
		`+ continue`
		`+`
		`# Filter out RPMs which will never end up in final modulemd:`
		# - the architecture of an RPM is not multilib architecture for `arch`.
		`# - the architecture of an RPM is not the final mmd architecture.`
		`@@ -560,6 +598,16 @@`
		`continue`

		`should_include = self._should_include_rpm(rpm, mmd, arch, multilib_arches)`
		`+`
		`+ # A source RPM should only be included in -devel module, if the "main" RPM`
		`+ # has been completed excluded from non-devel module. Track which source`
		`+ # RPMs would've been included in non-devel module to create a complement`
		`+ # list for -devel modules.`
		`+ if should_include:`
		`+ source_rpm = source_rpms.get(rpm["name"])`
		`+ if source_rpm:`
		`+ non_devel_source_rpms[rpm["name"]] = source_rpm`
		`+`
		`if self.devel and should_include:`
		`# In case this is a -devel module, we want to skip any RPMs which would normally be`
		`# included in a module, and only keep those which wouldn't be included, because`
		`@@ -570,12 +618,23 @@`
		`# really should include and skip the others.`
		`continue`

		`- # Add RPM to packages.`
		`- rpm_artifacts.add(nevra)`
		`+ # Add RPM and its related RPMs to packages.`
		`+ for related_nevra, related_rpm in grouped_rpms[rpm["name"]]:`
		`+ if related_rpm["arch"] != rpm["arch"]:`
		`+ continue`
		`+`
		`+ rpm_artifacts.add(related_nevra)`
		`+ # Not all RPMs have licenses (for example debuginfo packages).`
		`+ license = related_rpm.get("license")`
		`+ if license:`
		`+ rpm_licenses.add(license)`

		`- # Not all RPMs have licenses (for example debuginfo packages).`
		`- if "license" in rpm and rpm["license"]:`
		`- rpm_licenses.add(rpm["license"])`
		`+ if self.devel:`
		`+ for source_nevra in set(source_rpms.values()) - set(non_devel_source_rpms.values()):`
		`+ rpm_artifacts.add(source_nevra)`
		`+ else:`
		`+ for source_nevra in non_devel_source_rpms.values():`
		`+ rpm_artifacts.add(source_nevra)`

		`mmd.set_content_licenses(rpm_licenses)`
		`mmd.set_rpm_artifacts(rpm_artifacts)`

tests/test_content_generator.py

file modified

+61 -11

		`@@ -439,14 +439,14 @@`

		`@pytest.mark.parametrize("devel", (False, True))`
		`def test_fill_in_rpms_list(self, devel):`
		`+ self._add_test_rpm("dhcp-libs-12:4.3.5-5.module_2118aef6.src", "dhcp")`
		`self._add_test_rpm("dhcp-libs-12:4.3.5-5.module_2118aef6.x86_64", "dhcp")`
		`self._add_test_rpm("dhcp-libs-12:4.3.5-5.module_2118aef6.i686", "dhcp")`
		`self._add_test_rpm("dhcp-libs-12:4.3.5-5.module_2118aef6.s390x", "dhcp")`
		`- self._add_test_rpm("dhcp-libs-12:4.3.5-5.module_2118aef6.src", "dhcp")`
		`+ self._add_test_rpm("perl-Tangerine-12:4.3.5-5.module_2118aef6.src", "perl-Tangerine")`
		`self._add_test_rpm("perl-Tangerine-12:4.3.5-5.module_2118aef6.x86_64", "perl-Tangerine")`
		`self._add_test_rpm("perl-Tangerine-12:4.3.5-5.module_2118aef6.i686", "perl-Tangerine")`
		`self._add_test_rpm("perl-Tangerine-12:4.3.5-5.module_2118aef6.s390x", "perl-Tangerine")`
		`- self._add_test_rpm("perl-Tangerine-12:4.3.5-5.module_2118aef6.src", "perl-Tangerine")`

		`self.cg.devel = devel`
		`mmd = self.cg.module.mmd()`
		`@@ -455,10 +455,11 @@`
		`if not devel:`
		`# Only x86_64 packages should be filled in, because we requested x86_64 arch.`
		`assert set(mmd.get_rpm_artifacts().get()) == set([`
		`- "dhcp-libs-12:4.3.5-5.module_2118aef6.x86_64",`
		`"dhcp-libs-12:4.3.5-5.module_2118aef6.src",`
		`+ "dhcp-libs-12:4.3.5-5.module_2118aef6.x86_64",`
		`+ "perl-Tangerine-12:4.3.5-5.module_2118aef6.src",`
		`"perl-Tangerine-12:4.3.5-5.module_2118aef6.x86_64",`
		`- "perl-Tangerine-12:4.3.5-5.module_2118aef6.src"])`
		`+ ])`
		`else:`
		`# The i686 packages are filtered out in normal packages, because multilib`
		`# is not enabled for them - therefore we want to include them in -devel.`
		`@@ -467,8 +468,10 @@`
		`"perl-Tangerine-12:4.3.5-5.module_2118aef6.i686"])`

		`def test_fill_in_rpms_exclusivearch(self):`
		`+ self._add_test_rpm("dhcp-libs-12:4.3.5-5.module_2118aef6.src", "dhcp")`
		`self._add_test_rpm("dhcp-libs-12:4.3.5-5.module_2118aef6.noarch", "dhcp",`
		`exclusivearch=["x86_64"])`
		`+ self._add_test_rpm("perl-Tangerine-12:4.3.5-5.module_2118aef6.src", "perl-Tangerine")`
		`self._add_test_rpm("perl-Tangerine-12:4.3.5-5.module_2118aef6.noarch", "perl-Tangerine",`
		`exclusivearch=["ppc64le"])`

		`@@ -478,11 +481,15 @@`
		`# Only dhcp-libs should be filled in, because perl-Tangerine has different`
		`# exclusivearch.`
		`assert set(mmd.get_rpm_artifacts().get()) == set([`
		`- "dhcp-libs-12:4.3.5-5.module_2118aef6.noarch"])`
		`+ "dhcp-libs-12:4.3.5-5.module_2118aef6.src",`
		`+ "dhcp-libs-12:4.3.5-5.module_2118aef6.noarch",`
		`+ ])`

		`def test_fill_in_rpms_excludearch(self):`
		`+ self._add_test_rpm("dhcp-libs-12:4.3.5-5.module_2118aef6.src", "dhcp")`
		`self._add_test_rpm("dhcp-libs-12:4.3.5-5.module_2118aef6.noarch", "dhcp",`
		`excludearch=["x86_64"])`
		`+ self._add_test_rpm("perl-Tangerine-12:4.3.5-5.module_2118aef6.src", "perl-Tangerine")`
		`self._add_test_rpm("perl-Tangerine-12:4.3.5-5.module_2118aef6.noarch", "perl-Tangerine",`
		`excludearch=["ppc64le"])`

		`@@ -491,14 +498,20 @@`

		`# Only perl-Tangerine should be filled in, because dhcp-libs is excluded from x86_64.`
		`assert set(mmd.get_rpm_artifacts().get()) == set([`
		`- "perl-Tangerine-12:4.3.5-5.module_2118aef6.noarch"])`
		`+ "perl-Tangerine-12:4.3.5-5.module_2118aef6.src",`
		`+ "perl-Tangerine-12:4.3.5-5.module_2118aef6.noarch",`
		`+ ])`

		`@pytest.mark.parametrize("devel", (False, True))`
		`def test_fill_in_rpms_rpm_whitelist(self, devel):`
		`+ self._add_test_rpm("dhcp-libs-12:4.3.5-5.module_2118aef6.src", "dhcp",`
		`+ koji_srpm_name="python27-dhcp")`
		`self._add_test_rpm("dhcp-libs-12:4.3.5-5.module_2118aef6.x86_64", "dhcp",`
		`koji_srpm_name="python27-dhcp")`
		`self._add_test_rpm("dhcp-libs-12:4.3.5-5.module_2118aef6.i686", "dhcp",`
		`koji_srpm_name="python27-dhcp")`
		`+ self._add_test_rpm("perl-Tangerine-12:4.3.5-5.module_2118aef6.src", "perl-Tangerine",`
		`+ koji_srpm_name="foo-perl-Tangerine")`
		`self._add_test_rpm("perl-Tangerine-12:4.3.5-5.module_2118aef6.x86_64", "perl-Tangerine",`
		`koji_srpm_name="foo-perl-Tangerine")`
		`self._add_test_rpm("perl-Tangerine-12:4.3.5-5.module_2118aef6.i686", "perl-Tangerine",`
		`@@ -516,19 +529,37 @@`
		`# Only x86_64 dhcp-libs should be filled in, because only python27-dhcp is whitelisted`
		`# srpm name.`
		`assert set(mmd.get_rpm_artifacts().get()) == set([`
		`- "dhcp-libs-12:4.3.5-5.module_2118aef6.x86_64"])`
		`+ "dhcp-libs-12:4.3.5-5.module_2118aef6.src",`
		`+ "dhcp-libs-12:4.3.5-5.module_2118aef6.x86_64",`
		`+ ])`
		`else:`
		`assert set(mmd.get_rpm_artifacts().get()) == set([`
		`"dhcp-libs-12:4.3.5-5.module_2118aef6.i686",`
		`+ "perl-Tangerine-12:4.3.5-5.module_2118aef6.src",`
		`"perl-Tangerine-12:4.3.5-5.module_2118aef6.x86_64",`
		`- "perl-Tangerine-12:4.3.5-5.module_2118aef6.i686"])`
		`+ "perl-Tangerine-12:4.3.5-5.module_2118aef6.i686",`
		`+ ])`

		`@pytest.mark.parametrize("devel", (False, True))`
		`def test_fill_in_rpms_list_filters(self, devel):`
		`+ self._add_test_rpm("dhcp-libs-12:4.3.5-5.module_2118aef6.src", "dhcp")`
		`self._add_test_rpm("dhcp-libs-12:4.3.5-5.module_2118aef6.x86_64", "dhcp")`
		`+ self._add_test_rpm("dhcp-libs-debuginfo-12:4.3.5-5.module_2118aef6.x86_64", "dhcp")`
		`+ self._add_test_rpm("dhcp-libs-debugsource-12:4.3.5-5.module_2118aef6.x86_64", "dhcp")`
		`self._add_test_rpm("dhcp-libs-12:4.3.5-5.module_2118aef6.i686", "dhcp")`
		`+ self._add_test_rpm("dhcp-libs-debuginfo-12:4.3.5-5.module_2118aef6.i686", "dhcp")`
		`+ self._add_test_rpm("dhcp-libs-debugsource-12:4.3.5-5.module_2118aef6.i686", "dhcp")`
		`+ self._add_test_rpm("perl-Tangerine-12:4.3.5-5.module_2118aef6.src", "perl-Tangerine")`
		`self._add_test_rpm("perl-Tangerine-12:4.3.5-5.module_2118aef6.x86_64", "perl-Tangerine")`
		`+ self._add_test_rpm("perl-Tangerine-debuginfo-12:4.3.5-5.module_2118aef6.x86_64",`
		`+ "perl-Tangerine")`
		`+ self._add_test_rpm("perl-Tangerine-debugsource-12:4.3.5-5.module_2118aef6.x86_64",`
		`+ "perl-Tangerine")`
		`self._add_test_rpm("perl-Tangerine-12:4.3.5-5.module_2118aef6.i686", "perl-Tangerine")`
		`+ self._add_test_rpm("perl-Tangerine-debuginfo-12:4.3.5-5.module_2118aef6.i686",`
		`+ "perl-Tangerine")`
		`+ self._add_test_rpm("perl-Tangerine-debugsource-12:4.3.5-5.module_2118aef6.i686",`
		`+ "perl-Tangerine")`

		`self.cg.devel = devel`
		`mmd = self.cg.module.mmd()`
		`@@ -541,19 +572,35 @@`
		`if not devel:`
		`# Only x86_64 perl-Tangerine should be filled in, because dhcp-libs is filtered out.`
		`assert set(mmd.get_rpm_artifacts().get()) == set([`
		`- "perl-Tangerine-12:4.3.5-5.module_2118aef6.x86_64"])`
		`+ "perl-Tangerine-12:4.3.5-5.module_2118aef6.src",`
		`+ "perl-Tangerine-12:4.3.5-5.module_2118aef6.x86_64",`
		`+ "perl-Tangerine-debuginfo-12:4.3.5-5.module_2118aef6.x86_64",`
		`+ "perl-Tangerine-debugsource-12:4.3.5-5.module_2118aef6.x86_64",`
		`+ ])`
		`else:`
		`assert set(mmd.get_rpm_artifacts().get()) == set([`
		`+ "dhcp-libs-12:4.3.5-5.module_2118aef6.src",`
		`"dhcp-libs-12:4.3.5-5.module_2118aef6.x86_64",`
		`+ "dhcp-libs-debuginfo-12:4.3.5-5.module_2118aef6.x86_64",`
		`+ "dhcp-libs-debugsource-12:4.3.5-5.module_2118aef6.x86_64",`
		`"dhcp-libs-12:4.3.5-5.module_2118aef6.i686",`
		`- "perl-Tangerine-12:4.3.5-5.module_2118aef6.i686"])`
		`+ "dhcp-libs-debuginfo-12:4.3.5-5.module_2118aef6.i686",`
		`+ "dhcp-libs-debugsource-12:4.3.5-5.module_2118aef6.i686",`
		`+ "perl-Tangerine-12:4.3.5-5.module_2118aef6.i686",`
		`+ "perl-Tangerine-debuginfo-12:4.3.5-5.module_2118aef6.i686",`
		`+ "perl-Tangerine-debugsource-12:4.3.5-5.module_2118aef6.i686",`
		`+ ])`

		`@pytest.mark.parametrize("devel", (False, True))`
		`def test_fill_in_rpms_list_multilib(self, devel):`
		`+ self._add_test_rpm("dhcp-libs-12:4.3.5-5.module_2118aef6.src", "dhcp",`
		`+ multilib=["x86_64"])`
		`self._add_test_rpm("dhcp-libs-12:4.3.5-5.module_2118aef6.x86_64", "dhcp",`
		`multilib=["x86_64"])`
		`self._add_test_rpm("dhcp-libs-12:4.3.5-5.module_2118aef6.i686", "dhcp",`
		`multilib=["x86_64"])`
		`+ self._add_test_rpm("perl-Tangerine-12:4.3.5-5.module_2118aef6.src", "perl-Tangerine",`
		`+ multilib=["ppc64le"])`
		`self._add_test_rpm("perl-Tangerine-12:4.3.5-5.module_2118aef6.x86_64", "perl-Tangerine",`
		`multilib=["ppc64le"])`
		`self._add_test_rpm("perl-Tangerine-12:4.3.5-5.module_2118aef6.i686", "perl-Tangerine",`
		`@@ -567,9 +614,12 @@`
		`# Only i686 package for dhcp-libs should be added, because perl-Tangerine does not have`
		`# multilib set.`
		`assert set(mmd.get_rpm_artifacts().get()) == set([`
		`+ "dhcp-libs-12:4.3.5-5.module_2118aef6.src",`
		`"dhcp-libs-12:4.3.5-5.module_2118aef6.x86_64",`
		`"dhcp-libs-12:4.3.5-5.module_2118aef6.i686",`
		`- "perl-Tangerine-12:4.3.5-5.module_2118aef6.x86_64"])`
		`+ "perl-Tangerine-12:4.3.5-5.module_2118aef6.src",`
		`+ "perl-Tangerine-12:4.3.5-5.module_2118aef6.x86_64",`
		`+ ])`
		`else:`
		`assert set(mmd.get_rpm_artifacts().get()) == set([`
		`"perl-Tangerine-12:4.3.5-5.module_2118aef6.i686"])`

lucarval commented 5 years ago

If an RPM is not included, its correspnding -debug* RPMs should also not
be included.

Internal ref: FACTORY-3263

Signed-off-by: Luiz Carvalho lucarval@redhat.com

jkaluza commented on line 13 of module_build_service/builder/KojiContentGenerator.py 5 years ago

Would "strip_suffix" explain the fact that "At most, a single suffix is removed" better? Just thinking about it, not a review blocker or even request to really change it.

rebased onto 3e5eb3f20d89c85b328ae479d063e924379c45dd

5 years ago

lucarval commented 5 years ago

Would "strip_suffix" explain the fact that "At most, a single suffix is removed" better? Just thinking about it, not a review blocker or even request to really change it.

Re-worded to: "At most, a single suffix is removed to avoid removing portions of the string that were not originally at its end."

jkaluza commented on line 56 of module_build_service/builder/KojiContentGenerator.py 5 years ago

The str.endswith method accepts tuple, so you can use this instead:

if rpm["name"].endswith(group_suffixes) or rpm["arch"] == "src":

Edited 5 years ago by jkaluza

jkaluza commented on line 5 of tests/test_content_generator.py 5 years ago

Given we are handling src RPMs differently now, could you also add some test SRPM here?

jkaluza commented 5 years ago

Finished the review :)

rebased onto 105a84370369fd2ba3d778a1fb26a18258edea45

5 years ago

rebased onto 45b769b888d081a154c3bd5f124e316bc0ddf40b

5 years ago

lucarval commented 5 years ago

The str.endswith method accepts tuple, so you can use this instead:
if rpm["name"].endswith(group_suffixes) or rpm["arch"] == "src":

Thanks! Did not know about this, much cleaner now.

Given we are handling src RPMs differently now, could you also add some test SRPM here?

I've enhanced the existing tests instead. Otherwise, I'd have to add a complicated test that would essentially duplicate the existing ones.

Please take another look.

rebased onto 854a54e7f299c0ef0552758ef03acb77f305d0e1

5 years ago

rebased onto 086ed4a

5 years ago

jkaluza commented 5 years ago

Tests are passing locally, merging.

Pull-Request has been merged by jkaluza

5 years ago

Metadata

Assignee

None

Tags

No Tags

Flags

jenkins #240

failure

Build failed (commit: 086ed4a3)

5 years ago

jenkins #239

failure

Build failed (commit: 086ed4a3)

5 years ago

jenkins #238

failure

Build failed (commit: 086ed4a3)

5 years ago

jenkins #241

success (100%)

Build successful (commit: 086ed4a3)

5 years ago

Changes Summary 2

+64 -5

file changed

module_build_service/builder/KojiContentGenerator.py

+61 -11

file changed

tests/test_content_generator.py

fm-orchestrator

Source Code

#1084 Remove dangling -debug* RPMs from final MMD Merged 5 years ago by jkaluza. Opened 5 years ago by lucarval. lucarval/fm-orchestrator remove-dangling-debug-rpms into master

Metadata

Flags

Changes Summary 2

#1084 Remove dangling -debug* RPMs from final MMD

Merged 5 years ago by jkaluza. Opened 5 years ago by lucarval.

lucarval/fm-orchestrator remove-dangling-debug-rpms into master