summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTodd Gamblin <tgamblin@llnl.gov>2021-11-03 14:02:06 -0700
committerTodd Gamblin <tgamblin@llnl.gov>2021-11-05 00:15:47 -0700
commit49ed41b028596d2314c219d174e533f8142ad6e4 (patch)
tree1c54bf24210d344b098d369be6c5d7a5d4b1c6ba
parent3e3e84ba3055431947c11f78a4ca8f7a1234208e (diff)
downloadspack-49ed41b028596d2314c219d174e533f8142ad6e4.tar.gz
spack-49ed41b028596d2314c219d174e533f8142ad6e4.tar.bz2
spack-49ed41b028596d2314c219d174e533f8142ad6e4.tar.xz
spack-49ed41b028596d2314c219d174e533f8142ad6e4.zip
spack diff: more flexible tests, restore transitive diff with spec_clauses
In switching to hash facts for concrete specs, we lost the transitive facts from dependencies. This was fine for solves, because they were implied by the imposed constraints from every hash. However, for `spack diff`, we want to see what the hashes mean, so we need another mode for `spec_clauses()` to show that. This adds a `expand_hashes` argument to `spec_clauses()` that allows us to output *both* the hashes and their implications on dependencies. We use this mode in `spack diff`.
-rw-r--r--lib/spack/spack/cmd/diff.py4
-rw-r--r--lib/spack/spack/solver/asp.py14
-rw-r--r--lib/spack/spack/test/cmd/diff.py33
3 files changed, 39 insertions, 12 deletions
diff --git a/lib/spack/spack/cmd/diff.py b/lib/spack/spack/cmd/diff.py
index 85b73b6545..8d7b99e834 100644
--- a/lib/spack/spack/cmd/diff.py
+++ b/lib/spack/spack/cmd/diff.py
@@ -68,8 +68,8 @@ def compare_specs(a, b, to_string=False, color=None):
# Prepare a solver setup to parse differences
setup = asp.SpackSolverSetup()
- a_facts = set(t for t in setup.spec_clauses(a, body=True))
- b_facts = set(t for t in setup.spec_clauses(b, body=True))
+ a_facts = set(t for t in setup.spec_clauses(a, body=True, expand_hashes=True))
+ b_facts = set(t for t in setup.spec_clauses(b, body=True, expand_hashes=True))
# We want to present them to the user as simple key: values
intersect = sorted(a_facts.intersection(b_facts))
diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 0c483d8dfc..1e604ed406 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -1081,7 +1081,7 @@ class SpackSolverSetup(object):
raise RuntimeError(msg)
return clauses
- def _spec_clauses(self, spec, body=False, transitive=True):
+ def _spec_clauses(self, spec, body=False, transitive=True, expand_hashes=False):
"""Return a list of clauses for a spec mandates are true.
Arguments:
@@ -1089,7 +1089,15 @@ class SpackSolverSetup(object):
body (bool): if True, generate clauses to be used in rule bodies
(final values) instead of rule heads (setters).
transitive (bool): if False, don't generate clauses from
- dependencies (default True)
+ dependencies (default True)
+ expand_hashes (bool): if True, descend into hashes of concrete specs
+ (default False)
+
+ Normally, if called with ``transitive=True``, ``spec_clauses()`` just generates
+ hashes for the dependency requirements of concrete specs. If ``expand_hashes``
+ is ``True``, we'll *also* output all the facts implied by transitive hashes,
+ which are redundant during a solve but useful outside of one (e.g.,
+ for spec ``diff``).
"""
clauses = []
@@ -1200,7 +1208,7 @@ class SpackSolverSetup(object):
for dep in spec.traverse(root=False):
if spec.concrete:
clauses.append(fn.hash(dep.name, dep.dag_hash()))
- else:
+ if not spec.concrete or expand_hashes:
clauses.extend(
self._spec_clauses(dep, body, transitive=False)
)
diff --git a/lib/spack/spack/test/cmd/diff.py b/lib/spack/spack/test/cmd/diff.py
index 949e046b56..48435df2a6 100644
--- a/lib/spack/spack/test/cmd/diff.py
+++ b/lib/spack/spack/test/cmd/diff.py
@@ -61,12 +61,30 @@ def test_load_first(install_mockery, mock_fetch, mock_archive, mock_packages):
output = diff_cmd('--json', 'mpileaks', 'mpileaks')
result = sjson.load(output)
- assert len(result['a_not_b']) == 0
- assert len(result['b_not_a']) == 0
+ assert not result['a_not_b']
+ assert not result['b_not_a']
assert 'mpileaks' in result['a_name']
assert 'mpileaks' in result['b_name']
- assert "intersect" in result and len(result['intersect']) > 50
+
+ # spot check attributes in the intersection to ensure they describe the spec
+ assert "intersect" in result
+ assert all(["node", dep] in result["intersect"] for dep in (
+ "mpileaks", "callpath", "dyninst", "libelf", "libdwarf", "mpich"
+ ))
+ assert all(
+ len([diff for diff in result["intersect"] if diff[0] == attr]) == 6
+ for attr in (
+ "version",
+ "node_target",
+ "node_platform",
+ "node_os",
+ "node_compiler",
+ "node_compiler_version",
+ "node",
+ "hash",
+ )
+ )
# After we install another version, it should ask us to disambiguate
install_cmd('mpileaks+debug')
@@ -87,7 +105,8 @@ def test_load_first(install_mockery, mock_fetch, mock_archive, mock_packages):
"mpileaks/{0}".format(no_debug_hash))
result = sjson.load(output)
- assert len(result['a_not_b']) == 1
- assert len(result['b_not_a']) == 1
- assert result['a_not_b'][0] == ['variant_value', 'mpileaks debug True']
- assert result['b_not_a'][0] == ['variant_value', 'mpileaks debug False']
+ assert ['hash', 'mpileaks %s' % debug_hash] in result['a_not_b']
+ assert ['variant_value', 'mpileaks debug True'] in result['a_not_b']
+
+ assert ['hash', 'mpileaks %s' % no_debug_hash] in result['b_not_a']
+ assert ['variant_value', 'mpileaks debug False'] in result['b_not_a']