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

@@ -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)

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"])

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

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

8 months 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."

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

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

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

Finished the review :)

rebased onto 105a84370369fd2ba3d778a1fb26a18258edea45

8 months ago

rebased onto 45b769b888d081a154c3bd5f124e316bc0ddf40b

8 months 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

8 months ago

rebased onto 086ed4a

8 months ago

Tests are passing locally, merging.

Pull-Request has been merged by jkaluza

8 months ago