hlrs-software-stack/spack
7
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Hotfix: maintain patch order while fixing hash

Browse source 
fixes #5587

In trying to preserve patch ordering, #5476 made equality inconsistent
for the added 'patches' variant. This commit maintains the original
weak ordering of patch applications while preserving consistency of
comparisons. The ordering DOES NOT enter the hashing mechanism. It's
supposed to be a hotfix, while we think of a cleaner and more-permanent
solution.
This commit is contained in:
Massimiliano Culpo 2017-10-04 20:39:25 +02:00 committed by scheibelp
parent 2b7a37ed99
commit 5fa1191d17
2 changed files with 29 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

32
lib/spack/spack/spec.py
View file

@ -1812,10 +1812,12 @@ def concretize(self):
for patch in patch_list:
patches.append(patch.sha256)
if patches:
# Special-case: keeps variant values unique but ordered.
s.variants['patches'] = MultiValuedVariant('patches', ())
mvar = s.variants['patches']
mvar._value = mvar._original_value = tuple(dedupe(patches))
mvar = s.variants.setdefault(
'patches', MultiValuedVariant('patches', ())
)
mvar.value = patches
# FIXME: Monkey patches mvar to store patches order
mvar._patches_in_order_of_appearance = patches
# Apply patches required on dependencies by depends_on(..., patch=...)
for dspec in self.traverse_edges(deptype=all,
@ -1832,15 +1834,13 @@ def concretize(self):
for patch in patch_list:
patches.append(patch.sha256)
if patches:
# note that we use a special multi-valued variant and
# keep the patches ordered.
if 'patches' not in dspec.spec.variants:
mvar = MultiValuedVariant('patches', ())
dspec.spec.variants['patches'] = mvar
else:
mvar = dspec.spec.variants['patches']
mvar._value = mvar._original_value = tuple(
dedupe(list(mvar._value) + patches))
mvar = dspec.spec.variants.setdefault(
'patches', MultiValuedVariant('patches', ())
)
mvar.value = mvar.value + tuple(patches)
# FIXME: Monkey patches mvar to store patches order
l = getattr(mvar, '_patches_in_order_of_appearance', [])
mvar._patches_in_order_of_appearance = dedupe(l + patches)
for s in self.traverse():
if s.external_module:
@ -2523,7 +2523,11 @@ def patches(self):
return []
patches = []
for sha256 in self.variants['patches'].value:
# FIXME: The private attribute below is attached after
# FIXME: concretization to store the order of patches somewhere.
# FIXME: Needs to be refactored in a cleaner way.
for sha256 in self.variants['patches']._patches_in_order_of_appearance:
patch = self.package.lookup_patch(sha256)
if patch:
patches.append(patch)

20
lib/spack/spack/test/patch.py
View file

@ -99,9 +99,11 @@ def test_patch_in_spec(builtin_mock, config):
spec.concretize()
assert 'patches' in list(spec.variants.keys())
# foo, bar, baz
assert (('b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c',
'7d865e959b2466918c9863afca942d0fb89d7c9ac0c99bafc3749504ded97730',
# Here the order is bar, foo, baz. Note that MV variants order
# lexicographically based on the hash, not on the position of the
# patch directive.
assert (('7d865e959b2466918c9863afca942d0fb89d7c9ac0c99bafc3749504ded97730',
'b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c',
'bf07a7fbb825fc0aae7bf4a1177b2b31fcf8a3feeaf7092761e18c859ee52a9c') ==
spec.variants['patches'].value)
@ -131,8 +133,8 @@ def test_multiple_patched_dependencies(builtin_mock, config):
# URL patches
assert 'patches' in list(spec['fake'].variants.keys())
# urlpatch.patch, urlpatch.patch.gz
assert (('abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234',
'1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd') ==
assert (('1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd',
'abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234') ==
spec['fake'].variants['patches'].value)
@ -159,8 +161,8 @@ def test_conditional_patched_dependencies(builtin_mock, config):
# URL patches
assert 'patches' in list(spec['fake'].variants.keys())
# urlpatch.patch, urlpatch.patch.gz
assert (('abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234',
'1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd') ==
assert (('1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd',
'abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234') ==
spec['fake'].variants['patches'].value)
@ -187,6 +189,6 @@ def test_conditional_patched_deps_with_conditions(builtin_mock, config):
# URL patches
assert 'patches' in list(spec['fake'].variants.keys())
# urlpatch.patch, urlpatch.patch.gz
assert (('abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234',
'1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd') ==
assert (('1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd',
'abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234') ==
spec['fake'].variants['patches'].value)