spec lookup: separate module

lib/spack/spack/cmd/__init__.py

@@ -26,6 +26,7 @@
 import spack.paths
 import spack.repo
 import spack.spec
+import spack.spec_lookup
 import spack.spec_parser
 import spack.store
 import spack.traverse as traverse
@@ -211,7 +212,8 @@ def _concretize_spec_pairs(
     ):
         # Get all the concrete specs
         ret = [
-            concrete or (abstract if abstract.concrete else abstract.lookup_hash())
+            concrete
+            or (abstract if abstract.concrete else spack.spec_lookup.lookup_hash(abstract))
             for abstract, concrete in to_concretize
         ]
 

lib/spack/spack/cmd/diff.py

@@ -11,6 +11,7 @@
 import spack.cmd
 import spack.environment as ev
 import spack.solver.asp as asp
+import spack.spec_lookup
 import spack.util.spack_json as sjson
 from spack.cmd.common import arguments
 
@@ -210,7 +211,7 @@ def diff(parser, args):
     specs = []
     for spec in spack.cmd.parse_specs(args.specs):
         # If the spec has a hash, check it before disambiguating
-        spec.replace_hash()
+        spack.spec_lookup.replace_hash(spec)
         if spec.concrete:
             specs.append(spec)
         else:

lib/spack/spack/concretize.py

@@ -199,10 +199,12 @@ def concretize_one(spec: Union[str, Spec], tests: TestsType = False) -> Spec:
             the packages in the list, if True activate 'test' dependencies for all packages.
     """
     from spack.solver.asp import Solver, SpecBuilder
+    from spack.spec_lookup import replace_hash
 
     if isinstance(spec, str):
         spec = Spec(spec)
-    spec = spec.lookup_hash()
+
+    replace_hash(spec)
 
     if spec.concrete:
         return spec.copy()

lib/spack/spack/error.py

@@ -202,3 +202,10 @@ class MirrorError(SpackError):
 
     def __init__(self, msg, long_msg=None):
         super().__init__(msg, long_msg)
+
+
+class InvalidHashError(SpecError):
+    def __init__(self, spec, hash):
+        msg = f"No spec with hash {hash} could be found to match {spec}."
+        msg += " Either the hash does not exist, or it does not match other spec constraints."
+        super().__init__(msg)

lib/spack/spack/solver/asp.py

@@ -39,6 +39,7 @@
 import spack.repo
 import spack.solver.splicing
 import spack.spec
+import spack.spec_lookup
 import spack.store
 import spack.util.crypto
 import spack.util.libc
@@ -3774,7 +3775,7 @@ def execute_explicit_splices(self):
 
                     # The first iteration, we need to replace the abstract hash
                     if not replacement.concrete:
-                        replacement.replace_hash()
+                        spack.spec_lookup.replace_hash(replacement)
                     current_spec = current_spec.splice(replacement, transitive)
             new_key = NodeArgument(id=key.id, pkg=current_spec.name)
             specs[new_key] = current_spec
@@ -4133,7 +4134,7 @@ def solve_with_stats(
           setup_only (bool): if True, stop after setup and don't solve (default False).
           allow_deprecated (bool): allow deprecated version in the solve
         """
-        specs = [s.lookup_hash() for s in specs]
+        specs = [spack.spec_lookup.lookup_hash(s) for s in specs]
         reusable_specs = self._check_input_and_extract_concrete_specs(specs)
         reusable_specs.extend(self.selector.reusable_specs(specs))
         setup = SpackSolverSetup(tests=tests)
@@ -4170,7 +4171,7 @@ def solve_in_rounds(
             tests (bool): add test dependencies to the solve
             allow_deprecated (bool): allow deprecated version in the solve
         """
-        specs = [s.lookup_hash() for s in specs]
+        specs = [spack.spec_lookup.lookup_hash(s) for s in specs]
         reusable_specs = self._check_input_and_extract_concrete_specs(specs)
         reusable_specs.extend(self.selector.reusable_specs(specs))
         setup = SpackSolverSetup(tests=tests)

lib/spack/spack/spec.py

@@ -106,8 +106,6 @@
 import spack.version as vn
 import spack.version.git_ref_lookup
 
-from .enums import InstallRecordStatus
-
 __all__ = [
     "CompilerSpec",
     "Spec",
@@ -128,8 +126,6 @@
     "UnsatisfiableArchitectureSpecError",
     "UnsatisfiableProviderSpecError",
     "UnsatisfiableDependencySpecError",
-    "AmbiguousHashError",
-    "InvalidHashError",
     "SpecDeprecatedError",
 ]
 
@@ -2170,66 +2166,6 @@ def process_hash_bit_prefix(self, bits):
         """Get the first <bits> bits of the DAG hash as an integer type."""
         return spack.util.hash.base32_prefix_bits(self.process_hash(), bits)
 
-    def _lookup_hash(self):
-        """Lookup just one spec with an abstract hash, returning a spec from the the environment,
-        store, or finally, binary caches."""
-        import spack.binary_distribution
-        import spack.environment
-
-        active_env = spack.environment.active_environment()
-
-        # First env, then store, then binary cache
-        matches = (
-            (active_env.all_matching_specs(self) if active_env else [])
-            or spack.store.STORE.db.query(self, installed=InstallRecordStatus.ANY)
-            or spack.binary_distribution.BinaryCacheQuery(True)(self)
-        )
-
-        if not matches:
-            raise InvalidHashError(self, self.abstract_hash)
-
-        if len(matches) != 1:
-            raise AmbiguousHashError(
-                f"Multiple packages specify hash beginning '{self.abstract_hash}'.", *matches
-            )
-
-        return matches[0]
-
-    def lookup_hash(self):
-        """Given a spec with an abstract hash, return a copy of the spec with all properties and
-        dependencies by looking up the hash in the environment, store, or finally, binary caches.
-        This is non-destructive."""
-        if self.concrete or not any(node.abstract_hash for node in self.traverse()):
-            return self
-
-        spec = self.copy(deps=False)
-        # root spec is replaced
-        if spec.abstract_hash:
-            spec._dup(self._lookup_hash())
-            return spec
-
-        # Get dependencies that need to be replaced
-        for node in self.traverse(root=False):
-            if node.abstract_hash:
-                spec._add_dependency(node._lookup_hash(), depflag=0, virtuals=())
-
-        # reattach nodes that were not otherwise satisfied by new dependencies
-        for node in self.traverse(root=False):
-            if not any(n.satisfies(node) for n in spec.traverse()):
-                spec._add_dependency(node.copy(), depflag=0, virtuals=())
-
-        return spec
-
-    def replace_hash(self):
-        """Given a spec with an abstract hash, attempt to populate all properties and dependencies
-        by looking up the hash in the environment, store, or finally, binary caches.
-        This is destructive."""
-
-        if not any(node for node in self.traverse(order="post") if node.abstract_hash):
-            return
-
-        self._dup(self.lookup_hash())
-
     def to_node_dict(self, hash=ht.dag_hash):
         """Create a dictionary representing the state of this Spec.
 
@@ -3132,7 +3068,7 @@ def constrain(self, other, deps=True):
             if not self.abstract_hash or other.abstract_hash.startswith(self.abstract_hash):
                 self.abstract_hash = other.abstract_hash
             elif not self.abstract_hash.startswith(other.abstract_hash):
-                raise InvalidHashError(self, other.abstract_hash)
+                raise spack.error.InvalidHashError(self, other.abstract_hash)
 
         if not (self.name == other.name or (not self.name) or (not other.name)):
             raise UnsatisfiableSpecNameError(self.name, other.name)
@@ -5339,21 +5275,6 @@ def __init__(self, spec):
         super().__init__(msg)
 
 
-class AmbiguousHashError(spack.error.SpecError):
-    def __init__(self, msg, *specs):
-        spec_fmt = "{namespace}.{name}{@version}{%compiler}{compiler_flags}"
-        spec_fmt += "{variants}{ arch=architecture}{/hash:7}"
-        specs_str = "\n  " + "\n  ".join(spec.format(spec_fmt) for spec in specs)
-        super().__init__(msg + specs_str)
-
-
-class InvalidHashError(spack.error.SpecError):
-    def __init__(self, spec, hash):
-        msg = f"No spec with hash {hash} could be found to match {spec}."
-        msg += " Either the hash does not exist, or it does not match other spec constraints."
-        super().__init__(msg)
-
-
 class SpecFilenameError(spack.error.SpecError):
     """Raised when a spec file name is invalid."""
 

lib/spack/spack/spec_list.py

@@ -5,6 +5,7 @@
 from typing import List
 
 import spack.spec
+import spack.spec_lookup
 import spack.variant
 from spack.error import SpackError
 from spack.spec import Spec
@@ -230,7 +231,7 @@ def _expand_matrix_constraints(matrix_config):
             pass
 
         # Resolve abstract hashes for exclusion criteria
-        if any(test_spec.lookup_hash().satisfies(x) for x in excludes):
+        if any(spack.spec_lookup.lookup_hash(test_spec).satisfies(x) for x in excludes):
             continue
 
         if sigil:

lib/spack/spack/spec_lookup.py

@@ -0,0 +1,79 @@
+# Copyright Spack Project Developers. See COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+import spack.binary_distribution
+import spack.environment
+import spack.error
+import spack.spec
+import spack.store
+
+from .enums import InstallRecordStatus
+
+
+def _lookup_hash(spec: spack.spec.Spec):
+    """Lookup just one spec with an abstract hash, returning a spec from the the environment,
+    store, or finally, binary caches."""
+
+    active_env = spack.environment.active_environment()
+
+    # First env, then store, then binary cache
+    matches = (
+        (active_env.all_matching_specs(spec) if active_env else [])
+        or spack.store.STORE.db.query(spec, installed=InstallRecordStatus.ANY)
+        or spack.binary_distribution.BinaryCacheQuery(True)(spec)
+    )
+
+    if not matches:
+        raise spack.error.InvalidHashError(spec, spec.abstract_hash)
+
+    if len(matches) != 1:
+        raise AmbiguousHashError(
+            f"Multiple packages specify hash beginning '{spec.abstract_hash}'.", *matches
+        )
+
+    return matches[0]
+
+
+def lookup_hash(spec: spack.spec.Spec) -> spack.spec.Spec:
+    """Given a spec with an abstract hash, return a copy of the spec with all properties and
+    dependencies by looking up the hash in the environment, store, or finally, binary caches.
+    This is non-destructive."""
+    if spec.concrete or not any(node.abstract_hash for node in spec.traverse()):
+        return spec
+
+    spec = spec.copy(deps=False)
+    # root spec is replaced
+    if spec.abstract_hash:
+        spec._dup(_lookup_hash(spec))
+        return spec
+
+    # Get dependencies that need to be replaced
+    for node in spec.traverse(root=False):
+        if node.abstract_hash:
+            spec._add_dependency(_lookup_hash(node), depflag=0, virtuals=())
+
+    # reattach nodes that were not otherwise satisfied by new dependencies
+    for node in spec.traverse(root=False):
+        if not any(n.satisfies(node) for n in spec.traverse()):
+            spec._add_dependency(node.copy(), depflag=0, virtuals=())
+
+    return spec
+
+
+def replace_hash(spec: spack.spec.Spec) -> None:
+    """Given a spec with an abstract hash, attempt to populate all properties and dependencies
+    by looking up the hash in the environment, store, or finally, binary caches.
+    This is destructive."""
+
+    if not any(node for node in spec.traverse(order="post") if node.abstract_hash):
+        return
+
+    spec._dup(lookup_hash(spec))
+
+
+class AmbiguousHashError(spack.error.SpecError):
+    def __init__(self, msg, *specs):
+        spec_fmt = "{namespace}.{name}{@version}{%compiler}{compiler_flags}"
+        spec_fmt += "{variants}{ arch=architecture}{/hash:7}"
+        specs_str = "\n  " + "\n  ".join(spec.format(spec_fmt) for spec in specs)
+        super().__init__(msg + specs_str)

lib/spack/spack/test/spec_semantics.py

@@ -427,9 +427,9 @@ def test_mismatched_constrain_spec_by_hash(self, default_mock_concretization, da
         """Test that Specs specified only by their incompatible hashes fail appropriately."""
         lhs = "/" + database.query_one("callpath ^mpich").dag_hash()
         rhs = "/" + database.query_one("callpath ^mpich2").dag_hash()
-        with pytest.raises(spack.spec.InvalidHashError):
+        with pytest.raises(spack.error.InvalidHashError):
             Spec(lhs).constrain(Spec(rhs))
-        with pytest.raises(spack.spec.InvalidHashError):
+        with pytest.raises(spack.error.InvalidHashError):
             Spec(lhs[:7]).constrain(Spec(rhs))
 
     @pytest.mark.parametrize(

lib/spack/spack/test/spec_syntax.py

@@ -11,9 +11,11 @@
 import spack.binary_distribution
 import spack.cmd
 import spack.concretize
+import spack.error
 import spack.platforms.test
 import spack.repo
 import spack.spec
+from spack.spec_lookup import AmbiguousHashError, lookup_hash, replace_hash
 from spack.spec_parser import (
     UNIX_FILENAME,
     WINDOWS_FILENAME,
@@ -26,7 +28,7 @@
 
 FAIL_ON_WINDOWS = pytest.mark.xfail(
     sys.platform == "win32",
-    raises=(SpecTokenizationError, spack.spec.InvalidHashError),
+    raises=(SpecTokenizationError, spack.error.InvalidHashError),
     reason="Unix style path on Windows",
 )
 
@@ -782,22 +784,22 @@ def test_spec_by_hash(database, monkeypatch, config):
 
     hash_str = f"/{mpileaks.dag_hash()}"
     parsed_spec = SpecParser(hash_str).next_spec()
-    parsed_spec.replace_hash()
+    replace_hash(parsed_spec)
     assert parsed_spec == mpileaks
 
     short_hash_str = f"/{mpileaks.dag_hash()[:5]}"
     parsed_spec = SpecParser(short_hash_str).next_spec()
-    parsed_spec.replace_hash()
+    replace_hash(parsed_spec)
     assert parsed_spec == mpileaks
 
     name_version_and_hash = f"{mpileaks.name}@{mpileaks.version} /{mpileaks.dag_hash()[:5]}"
     parsed_spec = SpecParser(name_version_and_hash).next_spec()
-    parsed_spec.replace_hash()
+    replace_hash(parsed_spec)
     assert parsed_spec == mpileaks
 
     b_hash = f"/{b.dag_hash()}"
     parsed_spec = SpecParser(b_hash).next_spec()
-    parsed_spec.replace_hash()
+    replace_hash(parsed_spec)
     assert parsed_spec == b
 
 
@@ -811,7 +813,7 @@ def test_dep_spec_by_hash(database, config):
     assert "zmpi" in mpileaks_zmpi
 
     mpileaks_hash_fake = SpecParser(f"mpileaks ^/{fake.dag_hash()} ^zmpi").next_spec()
-    mpileaks_hash_fake.replace_hash()
+    replace_hash(mpileaks_hash_fake)
     assert "fake" in mpileaks_hash_fake
     assert mpileaks_hash_fake["fake"] == fake
     assert "zmpi" in mpileaks_hash_fake
@@ -820,7 +822,7 @@ def test_dep_spec_by_hash(database, config):
     mpileaks_hash_zmpi = SpecParser(
         f"mpileaks %{mpileaks_zmpi.compiler} ^ /{zmpi.dag_hash()}"
     ).next_spec()
-    mpileaks_hash_zmpi.replace_hash()
+    replace_hash(mpileaks_hash_zmpi)
     assert "zmpi" in mpileaks_hash_zmpi
     assert mpileaks_hash_zmpi["zmpi"] == zmpi
     assert mpileaks_zmpi.compiler.satisfies(mpileaks_hash_zmpi.compiler)
@@ -828,7 +830,7 @@ def test_dep_spec_by_hash(database, config):
     mpileaks_hash_fake_and_zmpi = SpecParser(
         f"mpileaks ^/{fake.dag_hash()[:4]} ^ /{zmpi.dag_hash()[:5]}"
     ).next_spec()
-    mpileaks_hash_fake_and_zmpi.replace_hash()
+    replace_hash(mpileaks_hash_fake_and_zmpi)
     assert "zmpi" in mpileaks_hash_fake_and_zmpi
     assert mpileaks_hash_fake_and_zmpi["zmpi"] == zmpi
 
@@ -888,13 +890,13 @@ def test_ambiguous_hash(mutable_database):
 
     # ambiguity in first hash character
     s1 = SpecParser("/x").next_spec()
-    with pytest.raises(spack.spec.AmbiguousHashError):
+    with pytest.raises(AmbiguousHashError):
-        s1.lookup_hash()
+        lookup_hash(s1)
 
     # ambiguity in first hash character AND spec name
     s2 = SpecParser("pkg-a/x").next_spec()
-    with pytest.raises(spack.spec.AmbiguousHashError):
+    with pytest.raises(AmbiguousHashError):
-        s2.lookup_hash()
+        lookup_hash(s2)
 
 
 @pytest.mark.db
@@ -903,24 +905,24 @@ def test_invalid_hash(database, config):
     mpich = database.query_one("mpich")
 
     # name + incompatible hash
-    with pytest.raises(spack.spec.InvalidHashError):
+    with pytest.raises(spack.error.InvalidHashError):
         parsed_spec = SpecParser(f"zmpi /{mpich.dag_hash()}").next_spec()
-        parsed_spec.replace_hash()
+        replace_hash(parsed_spec)
-    with pytest.raises(spack.spec.InvalidHashError):
+    with pytest.raises(spack.error.InvalidHashError):
         parsed_spec = SpecParser(f"mpich /{zmpi.dag_hash()}").next_spec()
-        parsed_spec.replace_hash()
+        replace_hash(parsed_spec)
 
     # name + dep + incompatible hash
-    with pytest.raises(spack.spec.InvalidHashError):
+    with pytest.raises(spack.error.InvalidHashError):
         parsed_spec = SpecParser(f"mpileaks ^zmpi /{mpich.dag_hash()}").next_spec()
-        parsed_spec.replace_hash()
+        replace_hash(parsed_spec)
 
 
 def test_invalid_hash_dep(database, config):
     mpich = database.query_one("mpich")
     hash = mpich.dag_hash()
-    with pytest.raises(spack.spec.InvalidHashError):
+    with pytest.raises(spack.error.InvalidHashError):
-        spack.spec.Spec(f"callpath ^zlib/{hash}").replace_hash()
+        replace_hash(spack.spec.Spec(f"callpath ^zlib/{hash}"))
 
 
 @pytest.mark.db
@@ -933,9 +935,8 @@ def test_nonexistent_hash(database, config):
     hashes = [s._hash for s in specs]
     assert no_such_hash not in [h[: len(no_such_hash)] for h in hashes]
 
-    with pytest.raises(spack.spec.InvalidHashError):
+    with pytest.raises(spack.error.InvalidHashError):
-        parsed_spec = SpecParser(f"/{no_such_hash}").next_spec()
+        replace_hash(SpecParser(f"/{no_such_hash}").next_spec())
-        parsed_spec.replace_hash()
 
 
 @pytest.mark.parametrize(
@@ -966,7 +967,7 @@ def test_disambiguate_hash_by_spec(spec1, spec2, constraint, mock_packages, monk
     else:
         spec = spack.spec.Spec("/spec" + constraint)
 
-    assert spec.lookup_hash() == spec1_concrete
+    assert lookup_hash(spec) == spec1_concrete
 
 
 @pytest.mark.parametrize(