#857 Correct fix for volume ID substition sorting by length
Merged 6 years ago by lsedlar. Opened 6 years ago by adamwill.
adamwill/pungi subst-sort-correct  into  master

file modified
+1 -1
@@ -325,7 +325,7 @@ 

      substitutions = compose.conf['volume_id_substitutions'].items()

      # processing should start with the longest pattern, otherwise, we could

      # unexpectedly replace a substring of that longest pattern

-     for k, v in sorted(substitutions, reverse=True):

+     for k, v in sorted(substitutions, key=lambda x: len(x[0]), reverse=True):

          volid = volid.replace(k, v)

      return volid

  

file modified
+7
@@ -229,6 +229,11 @@ 

              ('Fedora-WorkstationOstree-ostree-x86_64-Rawhide', 'Fedora-WS-ostree-x86_64-rawh'),

              ('x86_64-compose_id-20160107', 'x86_64-compose_id-20160107'),

              ('x86_64-compose_id-20160107-Alpha', 'x86_64-compose_id-20160107-A'),

+             # These test the case where one substitution is a subset

+             # of the other, but sorts alphabetically ahead of it, to

+             # make sure we're correctly sorting by length

+             ('Fedora-zzzaaaaaazzz-Rawhide', 'Fedora-zzz-rawh'),

+             ('Fedora-aaaaaa-Rawhide', 'Fedora-aaa-rawh'),

          ]

          for volid, expected in all_keys:

              conf = {
@@ -237,6 +242,8 @@ 

                      'WorkstationOstree': 'WS',

                      'Workstation': 'WS',

                      'Alpha': 'A',

+                     'zzzaaaaaazzz': 'zzz',

+                     'aaaaaa': 'aaa',

                  }

              }

              c = compose.Compose(conf, self.tmp_dir)

The previous attempt - caed78e - is not really correct. It sorts
the dict item tuples according to the alphabetical sort order of
the first item of each tuple (reversed). This will always work
when both substitutions start with the same characters, as in
the case of two strings that start with the same characters but
have a different length, the shorter one sorts alphabetically
first, and we reverse that. But it is not safe if the shorter
substitution doesn't start with the same characters, as in the
case I put in the tests: we should sort 'zzzaaaaaazzz' before
'aaaaaa' (and hence apply the 'zzzaaaaaazzz' substitution to a
volume ID that contains that string and not the 'aaaaaa' one),
but the previous commit did not.

Signed-off-by: Adam Williamson awilliam@redhat.com

rebased onto a1d559f

6 years ago

Pull-Request has been merged by lsedlar

6 years ago

@adamwill thanks for the correction.