rpm: Refine prefix matching for sub RPMs to reduce fragility (#879)

* rpm: Refine prefix matching for sub RPMs to reduce fragility

The current prefix matching for sub RPMs can potentially fail in two
ways:

- if we have two overlapping prefixes we may match the shorter prefix
  against the wrong RPM

- the shorter prefix will match repeatedly even after finding its
  target

This change keeps track of which sub RPMs have already match so avoid
double matching a shorter prefix.  Additionally, we sort the sub RPM
list by name length in reverse order.  This ensures that we prefer to
match the longest prefixes first and avoid double matching with the
same prefix.

* Expand subrpm test for similarly named RPMs

This change expands the basic subrpm test to account for multiple
subrpms that have matching prefixes.

pkg/make_rpm.py

@@ -446,13 +446,20 @@ def CallRpmBuild(self, dirname, rpmbuild_args, debuginfo_type):
   def SaveResult(self, out_file, subrpm_out_files):
     """Save the result RPM out of the temporary working directory."""
     if self.rpm_paths:
+      subrpms_seen = set()
+      sorted_subrpm_out_files = sorted(
+         subrpm_out_files, key=lambda n: len(n[1]), reverse=True)
+
       for p in self.rpm_paths:
          is_subrpm = False
 
-         for subrpm_name, subrpm_out_file in subrpm_out_files:
-            subrpm_prefix = self.name + '-' + subrpm_name
+         for subrpm_name, subrpm_out_file in sorted_subrpm_out_files:
+            if subrpm_name in subrpms_seen:
+               continue
 
+            subrpm_prefix = self.name + '-' + subrpm_name
             if os.path.basename(p).startswith(subrpm_prefix):
+               subrpms_seen.add(subrpm_name)
                shutil.copy(p, subrpm_out_file)
                is_subrpm = True
                if self.debug or True:

tests/rpm/BUILD

@@ -517,6 +517,16 @@ pkg_sub_rpm(
     ],
 )
 
+pkg_sub_rpm(
+    name = "sub_rpm2",
+    package_name = "test_sub_rpm2",
+    description = "Test subrpm2 description",
+    summary = "Test subrpm2",
+    srcs = [
+        ":test_sub_rpm_files",
+    ],
+)
+
 genrule(
     name = "test_sub_rpm_main_file_input",
     outs = ["test_sub_rpm_main_file_input.txt"],
@@ -543,6 +553,7 @@ pkg_rpm(
     ],
     subrpms = [
         ":sub_rpm",
+        ":sub_rpm2",
     ],
 )
 
@@ -559,6 +570,9 @@ genrule(
     echo "===== sub RPM ======" >> $@
     rpm -qpi --list $${RPMS[1]} | \
         grep -v 'Build Date' | grep -v 'Build Host' | grep -v 'Relocations' >> $@
+    echo "===== sub RPM ======" >> $@
+    rpm -qpi --list $${RPMS[2]} | \
+        grep -v 'Build Date' | grep -v 'Build Host' | grep -v 'Relocations' >> $@
     """,
 )
 

tests/rpm/test_sub_rpm_contents.txt.golden

@@ -28,3 +28,18 @@ Summary     : Test subrpm
 Description :
 Test subrpm description
 /test_sub_rpm_file_input.txt
+===== sub RPM ======
+Name        : test_sub_rpm_main-test_sub_rpm2
+Version     : 1
+Release     : 0
+Architecture: noarch
+Install Date: (not installed)
+Group       : Unspecified
+Size        : 17
+License     : Apache License, v2.0
+Signature   : (none)
+Source RPM  : test_sub_rpm_main-1-0.src.rpm
+Summary     : Test subrpm2
+Description :
+Test subrpm2 description
+/test_sub_rpm_file_input.txt