spack · tldahlgren · Sep 16, 2022 · Aug 27, 2022 · trws · Sep 15, 2022
diff --git a/lib/spack/spack/hash_types.py b/lib/spack/spack/hash_types.py
@@ -44,7 +44,7 @@ def __call__(self, spec):
 
 #: Hash descriptor used only to transfer a DAG, as is, across processes
 process_hash = SpecHashDescriptor(
-    deptype=("build", "link", "run", "test"), package_hash=False, name="process_hash"
+    deptype=("build", "link", "run", "test"), package_hash=True, name="process_hash"
 )
 
 

diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
@@ -1445,6 +1445,9 @@ class Body(object):
 
         # dependencies
         if spec.concrete:
+            # older specs do not have package hashes, so we have to do this carefully
+            if getattr(spec, "_package_hash", None):
+                clauses.append(fn.package_hash(spec.name, spec._package_hash))
             clauses.append(fn.hash(spec.name, spec.dag_hash()))
 
         # add all clauses from dependencies

diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
@@ -4056,6 +4056,9 @@ def _cmp_node(self):
         yield self.compiler_flags
         yield self.architecture
 
+        # this is not present on older specs
+        yield getattr(self, "_package_hash", None)
+
     def eq_node(self, other):
         """Equality with another spec, not including dependencies."""
         return (other is not None) and lang.lazy_eq(self._cmp_node, other._cmp_node)
@@ -4065,6 +4068,16 @@ def _cmp_iter(self):
         for item in self._cmp_node():
             yield item
 
+        # This needs to be in _cmp_iter so that no specs with different process hashes
+        # are considered the same by `__hash__` or `__eq__`.
+        #
+        # TODO: We should eventually unify the `_cmp_*` methods with `to_node_dict` so
+        # TODO: there aren't two sources of truth, but this needs some thought, since
+        # TODO: they exist for speed.  We should benchmark whether it's really worth
+        # TODO: having two types of hashing now that we use `json` instead of `yaml` for
+        # TODO: spec hashing.
+        yield self.process_hash() if self.concrete else None
+
         def deps():
             for dep in sorted(itertools.chain.from_iterable(self._dependencies.values())):
                 yield dep.spec.name

diff --git a/lib/spack/spack/test/cmd/diff.py b/lib/spack/spack/test/cmd/diff.py
@@ -86,6 +86,7 @@ def test_load_first(install_mockery, mock_fetch, mock_archive, mock_packages):
             "node_compiler",
             "node_compiler_version",
             "node",
+            "package_hash",
             "hash",
         )
     )

diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py
@@ -1257,3 +1257,25 @@ def test_concretize_partial_old_dag_hash_spec(mock_packages, config):
 def test_unsupported_compiler():
     with pytest.raises(UnsupportedCompilerError):
         Spec("gcc%fake-compiler").validate_or_raise()
+
+
+def test_package_hash_affects_dunder_and_dag_hash(mock_packages, config):
+    a1 = Spec("a").concretized()
+    a2 = Spec("a").concretized()
+
+    assert hash(a1) == hash(a2)
+    assert a1.dag_hash() == a2.dag_hash()
+    assert a1.process_hash() == a2.process_hash()
+
+    a1.clear_cached_hashes()
+    a2.clear_cached_hashes()
+
+    # tweak the dag hash of one of these specs
+    new_hash = "00000000000000000000000000000000"
+    if new_hash == a1._package_hash:
+        new_hash = "11111111111111111111111111111111"
+    a1._package_hash = new_hash
+
+    assert hash(a1) != hash(a2)
+    assert a1.dag_hash() != a2.dag_hash()
+    assert a1.process_hash() != a2.process_hash()
-Original file line number
+Diff line change
@@ Expand Up @@
                 "node_compiler",
                 "node_compiler_version",
                 "node",
+                "package_hash",
                 "hash",
             )
         )
@@ Expand Down @@