Merge branch 'UCL-RITS-hash_stability' into bugfix/hash-collision

2 files changed, 51 insertions, 15 deletions

diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index f742ca4616..ac77abd57b 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -119,6 +119,7 @@ from spack.build_environment import get_path_from_module, load_module
 from spack.util.naming import mod_to_class
 from spack.util.prefix import Prefix
 from spack.util.string import *
+from spack.util.spack_yaml import syaml_dict
 from spack.version import *
 from spack.provider_index import ProviderIndex
 
@@ -911,22 +912,29 @@ class Spec(object):
             return b32_hash
 
     def to_node_dict(self):
-        d = {}
+        ordered_dict = lambda d: syaml_dict(sorted(d.items()))
 
-        params = dict((name, v.value) for name, v in self.variants.items())
-        params.update(dict((name, value)
-                           for name, value in self.compiler_flags.items()))
+        d = syaml_dict()
+
+        params = syaml_dict(sorted(
+            (name, v.value) for name, v in self.variants.items()))
+        params.update(ordered_dict(self.compiler_flags))
 
         if params:
             d['parameters'] = params
 
-        if self.dependencies():
-            deps = self.dependencies_dict(deptype=('link', 'run'))
-            d['dependencies'] = dict(
-                (name, {
-                    'hash': dspec.spec.dag_hash(),
-                    'type': [str(s) for s in dspec.deptypes]})
-                for name, dspec in deps.items())
+        deps = self.dependencies_dict(deptype=('link', 'run'))
+        if deps:
+            d['dependencies'] = syaml_dict(sorted((
+                (
+                    name,
+                    ordered_dict({
+                        'hash': dspec.spec.dag_hash(),
+                        'type': sorted([str(s) for s in dspec.deptypes])
+                    })
+                )
+                for name, dspec in deps.items()
+            )))
 
         if self.namespace:
             d['namespace'] = self.namespace
@@ -934,15 +942,15 @@ class Spec(object):
         if self.architecture:
             # TODO: Fix the target.to_dict to account for the tuple
             # Want it to be a dict of dicts
-            d['arch'] = self.architecture.to_dict()
+            d['arch'] = ordered_dict(self.architecture.to_dict())
 
         if self.compiler:
-            d.update(self.compiler.to_dict())
+            d['compiler'] = syaml_dict(self.compiler.to_dict()['compiler'])
 
         if self.versions:
-            d.update(self.versions.to_dict())
+            d.update(ordered_dict(self.versions.to_dict()))
 
-        return {self.name: d}
+        return syaml_dict({self.name: d})
 
     def to_yaml(self, stream=None):
         node_list = []
diff --git a/lib/spack/spack/test/spec_dag.py b/lib/spack/spack/test/spec_dag.py
index 8f61c7ac76..5c2731041c 100644
--- a/lib/spack/spack/test/spec_dag.py
+++ b/lib/spack/spack/test/spec_dag.py
@@ -495,3 +495,31 @@ class SpecDagTest(MockPackagesTest):
 
         traversal = dag.traverse(deptype='run')
         self.assertEqual([x.name for x in traversal], names)
+
+    def test_using_ordered_dict(self):
+        """ Checks that dicts are ordered
+
+            Necessary to make sure that dag_hash is stable across python
+            versions and processes.
+        """
+        def descend_and_check(iterable, level=0):
+            from spack.util.spack_yaml import syaml_dict
+            from collections import Iterable, Mapping
+            if isinstance(iterable, Mapping):
+                self.assertTrue(isinstance(iterable, syaml_dict))
+                return descend_and_check(iterable.values(), level=level + 1)
+            max_level = level
+            for value in iterable:
+                if isinstance(value, Iterable) and not isinstance(value, str):
+                    nlevel = descend_and_check(value, level=level + 1)
+                    if nlevel > max_level:
+                        max_level = nlevel
+            return max_level
+
+        specs = ['mpileaks ^zmpi', 'dttop', 'dtuse']
+        for spec in specs:
+            dag = Spec(spec)
+            dag.normalize()
+            level = descend_and_check(dag.to_node_dict())
+            # level just makes sure we are doing something here
+            self.assertTrue(level >= 5)