summaryrefslogtreecommitdiff
path: root/lib
diff options
context:
space:
mode:
Diffstat (limited to 'lib')
-rw-r--r--lib/spack/spack/architecture.py8
-rw-r--r--lib/spack/spack/spec.py8
-rw-r--r--lib/spack/spack/test/spec_yaml.py73
3 files changed, 81 insertions, 8 deletions
diff --git a/lib/spack/spack/architecture.py b/lib/spack/spack/architecture.py
index 7552795cd2..378fb5d5d9 100644
--- a/lib/spack/spack/architecture.py
+++ b/lib/spack/spack/architecture.py
@@ -351,10 +351,10 @@ class OperatingSystem(object):
return (self.name, self.version)
def to_dict(self):
- return {
- 'name': self.name,
- 'version': self.version
- }
+ return syaml_dict([
+ ('name', self.name),
+ ('version', self.version)
+ ])
@key_ordering
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index c553da796d..f983e5c559 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -1506,10 +1506,10 @@ class Spec(object):
d['parameters'] = params
if self.external:
- d['external'] = {
- 'path': self.external_path,
- 'module': self.external_module
- }
+ d['external'] = syaml.syaml_dict([
+ ('path', self.external_path),
+ ('module', self.external_module),
+ ])
if not self._concrete:
d['concrete'] = False
diff --git a/lib/spack/spack/test/spec_yaml.py b/lib/spack/spack/test/spec_yaml.py
index 7fd2a36469..96bed17b78 100644
--- a/lib/spack/spack/test/spec_yaml.py
+++ b/lib/spack/spack/test/spec_yaml.py
@@ -8,13 +8,20 @@
YAML format preserves DAG information in the spec.
"""
+import ast
+import inspect
import os
from collections import Iterable, Mapping
+import pytest
+
+import spack.architecture
import spack.hash_types as ht
+import spack.spec
import spack.util.spack_json as sjson
import spack.util.spack_yaml as syaml
+import spack.version
from spack import repo
from spack.spec import Spec, save_dependency_spec_yamls
@@ -204,6 +211,72 @@ def test_ordered_read_not_required_for_consistent_dag_hash(
assert spec.full_hash() == round_trip_reversed_json_spec.full_hash()
+@pytest.mark.parametrize("module", [
+ spack.spec,
+ spack.architecture,
+ spack.version,
+])
+def test_hashes_use_no_python_dicts(module):
+ """Coarse check to make sure we don't use dicts in Spec.to_node_dict().
+
+ Python dicts are not guaranteed to iterate in a deterministic order
+ (at least not in all python versions) so we need to use lists and
+ syaml_dicts. syaml_dicts are ordered and ensure that hashes in Spack
+ are deterministic.
+
+ This test is intended to handle cases that are not covered by the
+ consistency checks above, or that would be missed by a dynamic check.
+ This test traverses the ASTs of functions that are used in our hash
+ algorithms, finds instances of dictionaries being constructed, and
+ prints out the line numbers where they occur.
+
+ """
+ class FindFunctions(ast.NodeVisitor):
+ """Find a function definition called to_node_dict."""
+ def __init__(self):
+ self.nodes = []
+
+ def visit_FunctionDef(self, node): # noqa
+ if node.name in ("to_node_dict", "to_dict", "to_dict_or_value"):
+ self.nodes.append(node)
+
+ class FindDicts(ast.NodeVisitor):
+ """Find source locations of dicts in an AST."""
+ def __init__(self, filename):
+ self.nodes = []
+ self.filename = filename
+
+ def add_error(self, node):
+ self.nodes.append(
+ "Use syaml_dict instead of dict at %s:%s:%s"
+ % (self.filename, node.lineno, node.col_offset)
+ )
+
+ def visit_Dict(self, node): # noqa
+ self.add_error(node)
+
+ def visit_Call(self, node): # noqa
+ name = None
+ if isinstance(node.func, ast.Name):
+ name = node.func.id
+ elif isinstance(node.func, ast.Attribute):
+ name = node.func.attr
+
+ if name == 'dict':
+ self.add_error(node)
+
+ find_functions = FindFunctions()
+ module_ast = ast.parse(inspect.getsource(module))
+ find_functions.visit(module_ast)
+
+ find_dicts = FindDicts(module.__file__)
+ for node in find_functions.nodes:
+ find_dicts.visit(node)
+
+ # fail with offending lines if we found some dicts.
+ assert [] == find_dicts.nodes
+
+
def reverse_all_dicts(data):
"""Descend into data and reverse all the dictionaries"""
if isinstance(data, dict):