From cb588d933c8ec8e7901d71600a0bd5499080c6e6 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Wed, 23 Oct 2024 00:07:48 -0700 Subject: [PATCH] remove straggling uses of hash_types.package_hash - [x] don't allow calling `spec.package_hash()` on abstract specs - [x] get rid of last few uses of `ht.package_hash`, which was removed. Signed-off-by: Todd Gamblin --- lib/spack/spack/package_base.py | 4 ++-- lib/spack/spack/spec.py | 29 +++++++++++++++++--------- lib/spack/spack/test/spec_semantics.py | 12 +++++++++-- 3 files changed, 31 insertions(+), 14 deletions(-) diff --git a/lib/spack/spack/package_base.py b/lib/spack/spack/package_base.py index 10ff01e2758..a9ca94c406c 100644 --- a/lib/spack/spack/package_base.py +++ b/lib/spack/spack/package_base.py @@ -47,6 +47,7 @@ import spack.store import spack.url import spack.util.environment +import spack.util.package_hash as ph import spack.util.path import spack.util.spack_yaml as syaml import spack.util.web @@ -55,7 +56,6 @@ from spack.install_test import PackageTest, TestSuite from spack.solver.version_order import concretization_version_order from spack.stage import DevelopStage, ResourceStage, Stage, StageComposite, compute_stage_name -from spack.util.package_hash import package_hash from spack.version import GitVersion, StandardVersion FLAG_HANDLER_RETURN_TYPE = Tuple[ @@ -1818,7 +1818,7 @@ def artifact_hashes(self, content=None): hashes["patches"] = [p.sha256 for p in self.spec.patches] # package.py contents - hashes["package_hash"] = package_hash(self.spec, source=content) + hashes["package_hash"] = ph.package_hash(self.spec, source=content) return hashes diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 2fa1bffe85b..32653e01956 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -1478,7 +1478,7 @@ def __init__( for h in ht.hashes: setattr(self, h.attr, None) - # dictionary of source artifact hashes, set at concretization time + # hash of package.py at the time of concretization self._package_hash = None # dictionary of source artifact hashes, set at concretization time @@ -2009,14 +2009,17 @@ def _cached_hash(self, hash, length=None): def package_hash(self): """Compute the hash of the contents of the package for this node""" + if not self.concrete: + raise ValueError("Spec is not concrete: " + str(self)) + # Concrete specs with the old DAG hash did not have the package hash, so we do # not know what the package looked like at concretization time - if self.concrete and not self._package_hash: + if not self._package_hash: raise ValueError( "Cannot call package_hash() on concrete specs with the old dag_hash()" ) - return self._cached_hash(ht.package_hash) + return self._package_hash def dag_hash(self, length=None): """This is Spack's default hash, used to identify installations. @@ -4235,7 +4238,11 @@ def _splice_detach_and_add_dependents(self, replacement, context): for ancestor in ancestors_in_context: # Only set it if it hasn't been spliced before ancestor._build_spec = ancestor._build_spec or ancestor.copy() - ancestor.clear_cached_hashes(ignore=(ht.package_hash.attr,)) + + # reset all hashes *except* package and artifact hashes (since we are not + # rebuilding the spec) + ancestor.clear_cached_hashes(content_hashes=False) + for edge in ancestor.edges_to_dependencies(depflag=dt.BUILD): if edge.depflag & ~dt.BUILD: edge.depflag &= ~dt.BUILD @@ -4429,16 +4436,18 @@ def mask_build_deps(in_spec): return spec - def clear_cached_hashes(self, ignore=()): + def clear_cached_hashes(self, content_hashes=True): """ Clears all cached hashes in a Spec, while preserving other properties. """ for h in ht.hashes: - if h.attr not in ignore: - if hasattr(self, h.attr): - setattr(self, h.attr, None) - self._package_hash = None - self._artifact_hashes = None + if hasattr(self, h.attr): + setattr(self, h.attr, None) + + if content_hashes: + self._package_hash = None + self._artifact_hashes = None + self._dunder_hash = None def __hash__(self): diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py index a821c53f2fb..33e074aadb9 100644 --- a/lib/spack/spack/test/spec_semantics.py +++ b/lib/spack/spack/test/spec_semantics.py @@ -15,6 +15,7 @@ import spack.solver.asp import spack.spec import spack.store +import spack.util.package_hash as ph import spack.variant import spack.version as vn from spack.error import SpecError, UnsatisfiableSpecError @@ -1640,20 +1641,27 @@ def test_spec_installed(default_mock_concretization, database): assert not spec.installed +def test_cannot_call_dag_hash_on_abstract_spec(): + with pytest.raises(ValueError, match="Spec is not concrete"): + Spec("pkg-a").package_hash() + + @pytest.mark.regression("30678") def test_call_dag_hash_on_old_dag_hash_spec(mock_packages, default_mock_concretization): # create a concrete spec a = default_mock_concretization("pkg-a") dag_hashes = {spec.name: spec.dag_hash() for spec in a.traverse()} + for spec in a.traverse(): + assert dag_hashes[spec.name] == spec.dag_hash() + assert spec.package_hash() == ph.package_hash(spec) + # make it look like an old DAG hash spec with no package hash on the spec. for spec in a.traverse(): assert spec.concrete spec._package_hash = None for spec in a.traverse(): - assert dag_hashes[spec.name] == spec.dag_hash() - with pytest.raises(ValueError, match="Cannot call package_hash()"): spec.package_hash()