experimental refactoring of hash functions

scheibelp · tgamblin · commit 35befe8db938 · 2019-07-19T18:04:58.000-07:00
diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
@@ -876,7 +876,7 @@ def install(self, user_spec, concrete_spec=None, **install_args):
             # spec might be in the user_specs, but not installed.
             # TODO: Redo name-based comparison for old style envs
             spec = next(s for s in self.user_specs if s.satisfies(user_spec))
-            concrete = self.specs_by_hash.get(spec.dag_hash(all_deps=True))
+            concrete = self.specs_by_hash.get(spec.build_hash())
             if not concrete:
                 concrete = spec.concretized()
                 self._add_concrete_spec(spec, concrete)
@@ -988,7 +988,7 @@ def _add_concrete_spec(self, spec, concrete, new=True):
         # update internal lists of specs
         self.concretized_user_specs.append(spec)
 
-        h = concrete.dag_hash(all_deps=True)
+        h = concrete.build_hash()
         self.concretized_order.append(h)
         self.specs_by_hash[h] = concrete
 
@@ -1103,7 +1103,7 @@ def _to_lockfile_dict(self):
         concrete_specs = {}
         for spec in self.specs_by_hash.values():
             for s in spec.traverse():
-                dag_hash_all = s.dag_hash(all_deps=True)
+                dag_hash_all = s.build_hash()
                 if dag_hash_all not in concrete_specs:
                     spec_dict = s.to_node_dict(all_deps=True)
                     spec_dict[s.name]['hash'] = s.dag_hash()
@@ -1164,7 +1164,7 @@ def _read_lockfile_dict(self, d):
         self.specs_by_hash = {}
         for _, spec in specs_by_hash.items():
             dag_hash = spec.dag_hash()
-            build_hash = spec.dag_hash(all_deps=True)
+            build_hash = spec.build_hash()
             if dag_hash in root_hashes:
                 old_hash_to_new[dag_hash] = build_hash
 
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
@@ -1286,34 +1286,41 @@ def prefix(self):
     def prefix(self, value):
         self._prefix = Prefix(value)
 
-    def dag_hash(self, length=None, all_deps=False):
+    def dag_hash(self, length=None):
         """Return a hash of the entire spec DAG, including connectivity."""
         if not self.concrete:
-            h = self._dag_hash(all_deps=all_deps)
+            h = self._dag_hash(all_deps=False)
             # An upper bound of None is equivalent to len(h). An upper bound of
             # 0 produces the empty string
             return h[:length]
 
         if not self._hash:
             self._hash = self._dag_hash(all_deps=False)
 
+        return self._hash[:length]
+
+    def build_hash(self):
+        if not self.concrete:
+            return self._dag_hash(all_deps=True)
+
         if not self._build_hash:
             self._build_hash = self._dag_hash(all_deps=True)
 
-        h = self._build_hash if all_deps else self._hash
-        return h[:length]
+        return self._build_hash
 
     def _dag_hash(self, all_deps=False):
         yaml_text = syaml.dump(
             self.to_node_dict(all_deps=all_deps),
             default_flow_style=True,
             width=maxint)
-        sha = hashlib.sha1(yaml_text.encode('utf-8'))
 
+        return self._encode_hash_content(yaml_text.encode('utf-8'))
+
+    def _encode_hash_content(self, bytes):
+        sha = hashlib.sha1(bytes)
         b32_hash = base64.b32encode(sha.digest()).lower()
         if sys.version_info[0] >= 3:
             b32_hash = b32_hash.decode('utf-8')
-
         return b32_hash
 
     def dag_hash_bit_prefix(self, bits):
@@ -1329,13 +1336,8 @@ def full_hash(self, length=None):
                 self.to_node_dict(hash_function=lambda s: s.full_hash()),
                 default_flow_style=True, width=maxint)
             package_hash = self.package.content_hash()
-            sha = hashlib.sha1(yaml_text.encode('utf-8') + package_hash)
-
-            b32_hash = base64.b32encode(sha.digest()).lower()
-            if sys.version_info[0] >= 3:
-                b32_hash = b32_hash.decode('utf-8')
-
-            self._full_hash = b32_hash
+            self._full_hash = self._encode_hash_content(
+                yaml_text.encode('utf-8') + package_hash)
 
         return self._full_hash[:length]
 
@@ -1387,7 +1389,10 @@ def to_node_dict(self, hash_function=None, all_deps=False):
         deps = self.dependencies_dict(deptype=deptypes)
         if deps:
             if hash_function is None:
-                hash_function = lambda s: s.dag_hash(all_deps=all_deps)
+                if all_deps:
+                    hash_function = lambda s: s.build_hash()
+                else:
+                    hash_function = lambda s: s.dag_hash()
             d['dependencies'] = syaml_dict([
                 (name,
                  syaml_dict([
@@ -1408,7 +1413,7 @@ def to_dict(self, all_deps=False):
             node = s.to_node_dict(all_deps=all_deps)
             node[s.name]['hash'] = s.dag_hash()
             if all_deps:
-                node[s.name]['build_hash'] = s.dag_hash(all_deps=True)
+                node[s.name]['build_hash'] = s.build_hash()
             node_list.append(node)
 
         return syaml_dict([('spec', node_list)])
diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py
@@ -676,8 +676,8 @@ def test_read_old_lock_and_write_new(tmpdir):
         # When the lockfile is rewritten, it should adopt the new hash scheme
         # which accounts for all dependencies, including build dependencies
         assert hashes == set([
-            x.dag_hash(all_deps=True),
-            y.dag_hash(all_deps=True)])
+            x.build_hash(),
+            y.build_hash()])
 
 
 @pytest.mark.usefixtures('config')
