"spack diff": add ignore option for dependencies (#41711)

* add trim function to `Spec` and `--ignore` option to 'spack diff'

Allows user to compare two specs while ignoring the sub-DAG of a particular dependency, e.g.

spack diff --ignore=mpi --ignore=zlib trilinos/abcdef trilinos/fedcba

to focus on differences closer to the root of the software stack

lib/spack/spack/cmd/diff.py

@@ -44,6 +44,9 @@ def setup_parser(subparser):
         action="append",
         help="select the attributes to show (defaults to all)",
     )
+    subparser.add_argument(
+        "--ignore", action="append", help="omit diffs related to these dependencies"
+    )
 
 
 def shift(asp_function):
@@ -54,7 +57,7 @@ def shift(asp_function):
     return asp.AspFunction(first, rest)
 
 
-def compare_specs(a, b, to_string=False, color=None):
+def compare_specs(a, b, to_string=False, color=None, ignore_packages=None):
     """
     Generate a comparison, including diffs (for each side) and an intersection.
 
@@ -73,6 +76,14 @@ def compare_specs(a, b, to_string=False, color=None):
     if color is None:
         color = get_color_when()
 
+    a = a.copy()
+    b = b.copy()
+
+    if ignore_packages:
+        for pkg_name in ignore_packages:
+            a.trim(pkg_name)
+            b.trim(pkg_name)
+
     # Prepare a solver setup to parse differences
     setup = asp.SpackSolverSetup()
 
@@ -209,7 +220,7 @@ def diff(parser, args):
 
     # Calculate the comparison (c)
     color = False if args.dump_json else get_color_when()
-    c = compare_specs(specs[0], specs[1], to_string=True, color=color)
+    c = compare_specs(specs[0], specs[1], to_string=True, color=color, ignore_packages=args.ignore)
 
     # Default to all attributes
     attributes = args.attribute or ["all"]

lib/spack/spack/spec.py

@@ -4728,6 +4728,20 @@ def build_spec(self):
     def build_spec(self, value):
         self._build_spec = value
 
+    def trim(self, dep_name):
+        """
+        Remove any package that is or provides `dep_name` transitively
+        from this tree. This can also remove other dependencies if
+        they are only present because of `dep_name`.
+        """
+        for spec in list(self.traverse()):
+            new_dependencies = _EdgeMap()  # A new _EdgeMap
+            for pkg_name, edge_list in spec._dependencies.items():
+                for edge in edge_list:
+                    if (dep_name not in edge.virtuals) and (not dep_name == edge.spec.name):
+                        new_dependencies.add(edge)
+            spec._dependencies = new_dependencies
+
     def splice(self, other, transitive):
         """Splices dependency "other" into this ("target") Spec, and return the
         result as a concrete Spec.

lib/spack/spack/test/cmd/diff.py

@@ -10,12 +10,150 @@
 import spack.main
 import spack.store
 import spack.util.spack_json as sjson
+from spack.test.conftest import create_test_repo
 
 install_cmd = spack.main.SpackCommand("install")
 diff_cmd = spack.main.SpackCommand("diff")
 find_cmd = spack.main.SpackCommand("find")
 
 
+_p1 = (
+    "p1",
+    """\
+class P1(Package):
+    version("1.0")
+
+    variant("p1var", default=True)
+    variant("usev1", default=True)
+
+    depends_on("p2")
+    depends_on("v1", when="+usev1")
+""",
+)
+
+
+_p2 = (
+    "p2",
+    """\
+class P2(Package):
+    version("1.0")
+
+    variant("p2var", default=True)
+
+    depends_on("p3")
+""",
+)
+
+
+_p3 = (
+    "p3",
+    """\
+class P3(Package):
+    version("1.0")
+
+    variant("p3var", default=True)
+""",
+)
+
+_i1 = (
+    "i1",
+    """\
+class I1(Package):
+    version("1.0")
+
+    provides("v1")
+
+    variant("i1var", default=True)
+
+    depends_on("p3")
+    depends_on("p4")
+""",
+)
+
+_i2 = (
+    "i2",
+    """\
+class I2(Package):
+    version("1.0")
+
+    provides("v1")
+
+    variant("i2var", default=True)
+
+    depends_on("p3")
+    depends_on("p4")
+""",
+)
+
+
+_p4 = (
+    "p4",
+    """\
+class P4(Package):
+    version("1.0")
+
+    variant("p4var", default=True)
+""",
+)
+
+
+# Note that the hash of p1 will differ depending on the variant chosen
+# we probably always want to omit that from diffs
+@pytest.fixture
+def _create_test_repo(tmpdir, mutable_config):
+    """
+    p1____
+    |     \
+    p2     v1
+    | ____/ |
+    p3      p4
+
+    i1 and i2 provide v1 (and both have the same dependencies)
+
+    All packages have an associated variant
+    """
+    yield create_test_repo(tmpdir, [_p1, _p2, _p3, _i1, _i2, _p4])
+
+
+@pytest.fixture
+def test_repo(_create_test_repo, monkeypatch, mock_stage):
+    with spack.repo.use_repositories(_create_test_repo) as mock_repo_path:
+        yield mock_repo_path
+
+
+def test_diff_ignore(test_repo):
+    specA = spack.spec.Spec("p1+usev1").concretized()
+    specB = spack.spec.Spec("p1~usev1").concretized()
+
+    c1 = spack.cmd.diff.compare_specs(specA, specB, to_string=False)
+
+    def match(function, name, args):
+        limit = len(args)
+        return function.name == name and list(args[:limit]) == list(function.args[:limit])
+
+    def find(function_list, name, args):
+        return any(match(f, name, args) for f in function_list)
+
+    assert find(c1["a_not_b"], "node_os", ["p4"])
+
+    c2 = spack.cmd.diff.compare_specs(specA, specB, ignore_packages=["v1"], to_string=False)
+
+    assert not find(c2["a_not_b"], "node_os", ["p4"])
+    assert find(c2["intersect"], "node_os", ["p3"])
+
+    # Check ignoring changes on multiple packages
+
+    specA = spack.spec.Spec("p1+usev1 ^p3+p3var").concretized()
+    specA = spack.spec.Spec("p1~usev1 ^p3~p3var").concretized()
+
+    c3 = spack.cmd.diff.compare_specs(specA, specB, to_string=False)
+    assert find(c3["a_not_b"], "variant_value", ["p3", "p3var"])
+
+    c4 = spack.cmd.diff.compare_specs(specA, specB, ignore_packages=["v1", "p3"], to_string=False)
+    assert not find(c4["a_not_b"], "node_os", ["p4"])
+    assert not find(c4["a_not_b"], "variant_value", ["p3"])
+
+
 def test_diff_cmd(install_mockery, mock_fetch, mock_archive, mock_packages):
     """Test that we can install two packages and diff them"""
 

lib/spack/spack/test/concretize_requirements.py

@@ -16,6 +16,7 @@
 import spack.version
 from spack.solver.asp import InternalConcretizerError, UnsatisfiableSpecError
 from spack.spec import Spec
+from spack.test.conftest import create_test_repo
 from spack.util.url import path_to_file_url
 
 pytestmark = [
@@ -92,30 +93,13 @@ class U(Package):
 
 
 @pytest.fixture
-def create_test_repo(tmpdir, mutable_config):
-    repo_path = str(tmpdir)
-    repo_yaml = tmpdir.join("repo.yaml")
-    with open(str(repo_yaml), "w") as f:
-        f.write(
-            """\
-repo:
-  namespace: testcfgrequirements
-"""
-        )
-
-    packages_dir = tmpdir.join("packages")
-    for pkg_name, pkg_str in [_pkgx, _pkgy, _pkgv, _pkgt, _pkgu]:
-        pkg_dir = packages_dir.ensure(pkg_name, dir=True)
-        pkg_file = pkg_dir.join("package.py")
-        with open(str(pkg_file), "w") as f:
-            f.write(pkg_str)
-
-    yield spack.repo.Repo(repo_path)
+def _create_test_repo(tmpdir, mutable_config):
+    yield create_test_repo(tmpdir, [_pkgx, _pkgy, _pkgv, _pkgt, _pkgu])
 
 
 @pytest.fixture
-def test_repo(create_test_repo, monkeypatch, mock_stage):
-    with spack.repo.use_repositories(create_test_repo) as mock_repo_path:
+def test_repo(_create_test_repo, monkeypatch, mock_stage):
+    with spack.repo.use_repositories(_create_test_repo) as mock_repo_path:
         yield mock_repo_path
 
 
@@ -530,7 +514,7 @@ def test_oneof_ordering(concretize_scope, test_repo):
     assert s2.satisfies("@2.5")
 
 
-def test_reuse_oneof(concretize_scope, create_test_repo, mutable_database, fake_installs):
+def test_reuse_oneof(concretize_scope, _create_test_repo, mutable_database, fake_installs):
     conf_str = """\
 packages:
   y:
@@ -538,7 +522,7 @@ def test_reuse_oneof(concretize_scope, create_test_repo, mutable_database, fake_
     - one_of: ["@2.5", "%gcc"]
 """
 
-    with spack.repo.use_repositories(create_test_repo):
+    with spack.repo.use_repositories(_create_test_repo):
         s1 = Spec("y@2.5%gcc").concretized()
         s1.package.do_install(fake=True, explicit=True)
 

lib/spack/spack/test/conftest.py

@@ -1976,3 +1976,24 @@ def mock_modules_root(tmp_path, monkeypatch):
     """Sets the modules root to a temporary directory, to avoid polluting configuration scopes."""
     fn = functools.partial(_root_path, path=str(tmp_path))
     monkeypatch.setattr(spack.modules.common, "root_path", fn)
+
+
+def create_test_repo(tmpdir, pkg_name_content_tuples):
+    repo_path = str(tmpdir)
+    repo_yaml = tmpdir.join("repo.yaml")
+    with open(str(repo_yaml), "w") as f:
+        f.write(
+            """\
+repo:
+  namespace: testcfgrequirements
+"""
+        )
+
+    packages_dir = tmpdir.join("packages")
+    for pkg_name, pkg_str in pkg_name_content_tuples:
+        pkg_dir = packages_dir.ensure(pkg_name, dir=True)
+        pkg_file = pkg_dir.join("package.py")
+        with open(str(pkg_file), "w") as f:
+            f.write(pkg_str)
+
+    return spack.repo.Repo(repo_path)

lib/spack/spack/test/spec_semantics.py

@@ -1288,6 +1288,17 @@ def test_call_dag_hash_on_old_dag_hash_spec(mock_packages, default_mock_concreti
             spec.package_hash()
 
 
+def test_spec_trim(mock_packages, config):
+    top = Spec("dt-diamond").concretized()
+    top.trim("dt-diamond-left")
+    remaining = set(x.name for x in top.traverse())
+    assert set(["dt-diamond", "dt-diamond-right", "dt-diamond-bottom"]) == remaining
+
+    top.trim("dt-diamond-right")
+    remaining = set(x.name for x in top.traverse())
+    assert set(["dt-diamond"]) == remaining
+
+
 @pytest.mark.regression("30861")
 def test_concretize_partial_old_dag_hash_spec(mock_packages, config):
     # create an "old" spec with no package hash