almalinux/pungi
14
1
Fork 0
Code Issues Pull Requests Releases Activity

Correct fix for volume ID substition sorting by length

Browse Source 
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>
This commit is contained in:
Adam Williamson 2018-02-23 14:35:07 -08:00
parent caed78e11a
commit a1d559fb93
2 changed files with 8 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
pungi/util.py
View File

@ -325,7 +325,7 @@ def _apply_substitutions(compose, volid):
substitutions = compose.conf['volume_id_substitutions'].items() substitutions = compose.conf['volume_id_substitutions'].items()
# processing should start with the longest pattern, otherwise, we could # processing should start with the longest pattern, otherwise, we could
# unexpectedly replace a substring of that longest pattern # 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) volid = volid.replace(k, v)
return volid return volid

7
tests/test_util.py
View File

@ -229,6 +229,11 @@ class TestVolumeIdGenerator(unittest.TestCase):
('Fedora-WorkstationOstree-ostree-x86_64-Rawhide', 'Fedora-WS-ostree-x86_64-rawh'), ('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', 'x86_64-compose_id-20160107'),
('x86_64-compose_id-20160107-Alpha', 'x86_64-compose_id-20160107-A'), ('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: for volid, expected in all_keys:
conf = { conf = {
@ -237,6 +242,8 @@ class TestVolumeIdGenerator(unittest.TestCase):
'WorkstationOstree': 'WS', 'WorkstationOstree': 'WS',
'Workstation': 'WS', 'Workstation': 'WS',
'Alpha': 'A', 'Alpha': 'A',
'zzzaaaaaazzz': 'zzz',
'aaaaaa': 'aaa',
} }
} }
c = compose.Compose(conf, self.tmp_dir) c = compose.Compose(conf, self.tmp_dir)