From c97d058ce3ff0ffc4b1de063daca78cecff23d62 Mon Sep 17 00:00:00 2001 From: scheibelp Date: Wed, 6 Jun 2018 18:28:25 -0700 Subject: Fix bug where patches specified by dependents were not applied (#8272) Fixes #7885 #7193 added the patches_to_apply function to collect patches which are then applied in Package.do_patch. However this only collects patches that are associated with the Package object and does not include Spec-related patches (which are applied by dependents, added in #5476). Spec.patches already collects patches from the package as well as those applied by dependents, so the Package.patches_to_apply function isn't necessary. All uses of Package.patches_to_apply are replaced with Package.spec.patches. This also updates Package.content_hash to require the associated spec to be concrete: Spec.patches is only set after concretization. Before this PR, it was possible for Package.content_hash to be valid before concretizing the associated Spec if all patches were associated with the Package (vs. being applied by dependents). This behavior was unreliable though so the change is unlikely to be disruptive. --- lib/spack/spack/package.py | 19 +++++++------------ lib/spack/spack/test/packages.py | 6 ++++++ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index 2b5299197f..73a872545c 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -40,7 +40,6 @@ import functools import glob import hashlib import inspect -import itertools import os import re import shutil @@ -1129,7 +1128,7 @@ class PackageBase(with_metaclass(PackageMeta, object)): # Apply all the patches for specs that match this one patched = False - for patch in self.patches_to_apply(): + for patch in patches: try: with working_dir(self.stage.source_path): patch.apply(self.stage) @@ -1173,19 +1172,15 @@ class PackageBase(with_metaclass(PackageMeta, object)): else: touch(no_patches_file) - def patches_to_apply(self): - """If the patch set does not change between two invocations of spack, - then all patches in the set will be applied in the same order""" - patchesToApply = itertools.chain.from_iterable( - patch_list - for spec, patch_list in self.patches.items() - if self.spec.satisfies(spec)) - return list(patchesToApply) - def content_hash(self, content=None): """Create a hash based on the sources and logic used to build the package. This includes the contents of all applied patches and the contents of applicable functions in the package subclass.""" + if not self.spec.concrete: + err_msg = ("Cannot invoke content_hash on a package" + " if the associated spec is not concrete") + raise spack.error.SpackError(err_msg) + hashContent = list() source_id = fs.for_package_version(self, self.version).source_id() if not source_id: @@ -1199,7 +1194,7 @@ class PackageBase(with_metaclass(PackageMeta, object)): else: hashContent.append(source_id.encode('utf-8')) hashContent.extend(':'.join((p.sha256, str(p.level))).encode('utf-8') - for p in self.patches_to_apply()) + for p in self.spec.patches) hashContent.append(package_hash(self.spec, content)) return base64.b32encode( hashlib.sha256(bytes().join(sorted(hashContent))).digest()).lower() diff --git a/lib/spack/spack/test/packages.py b/lib/spack/spack/test/packages.py index 2d9878ed96..9fae9decfb 100644 --- a/lib/spack/spack/test/packages.py +++ b/lib/spack/spack/test/packages.py @@ -71,6 +71,8 @@ class TestPackage(object): def test_content_hash_all_same_but_patch_contents(self): spec1 = Spec("hash-test1@1.1") spec2 = Spec("hash-test2@1.1") + spec1.concretize() + spec2.concretize() content1 = package_content(spec1) content1 = content1.replace(spec1.package.__class__.__name__, '') content2 = package_content(spec2) @@ -81,6 +83,8 @@ class TestPackage(object): def test_content_hash_different_variants(self): spec1 = Spec("hash-test1@1.2 +variantx") spec2 = Spec("hash-test2@1.2 ~variantx") + spec1.concretize() + spec2.concretize() content1 = package_content(spec1) content1 = content1.replace(spec1.package.__class__.__name__, '') content2 = package_content(spec2) @@ -91,6 +95,8 @@ class TestPackage(object): def test_all_same_but_archive_hash(self): spec1 = Spec("hash-test1@1.3") spec2 = Spec("hash-test2@1.3") + spec1.concretize() + spec2.concretize() content1 = package_content(spec1) content1 = content1.replace(spec1.package.__class__.__name__, '') content2 = package_content(spec2) -- cgit v1.2.3-70-g09d2