From 49ed41b028596d2314c219d174e533f8142ad6e4 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Wed, 3 Nov 2021 14:02:06 -0700 Subject: 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`. --- lib/spack/spack/cmd/diff.py | 4 ++-- lib/spack/spack/solver/asp.py | 14 +++++++++++--- lib/spack/spack/test/cmd/diff.py | 33 ++++++++++++++++++++++++++------- 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'] -- cgit v1.2.3-70-g09d2