diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index d919aeecb7..8d5a1ec8df 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -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) diff --git a/lib/spack/spack/test/patch.py b/lib/spack/spack/test/patch.py index 0a91a0847c..41c4ef0621 100644 --- a/lib/spack/spack/test/patch.py +++ b/lib/spack/spack/test/patch.py @@ -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)