From 5d50ad3941ebeee95d972baff265e31be22a380a Mon Sep 17 00:00:00 2001 From: Peter Scheibel Date: Tue, 19 Dec 2023 16:37:44 -0800 Subject: "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 | 15 ++- lib/spack/spack/spec.py | 14 +++ lib/spack/spack/test/cmd/diff.py | 138 ++++++++++++++++++++++++ lib/spack/spack/test/concretize_requirements.py | 30 ++---- lib/spack/spack/test/conftest.py | 21 ++++ lib/spack/spack/test/spec_semantics.py | 11 ++ share/spack/spack-completion.bash | 2 +- share/spack/spack-completion.fish | 4 +- 8 files changed, 208 insertions(+), 27 deletions(-) diff --git a/lib/spack/spack/cmd/diff.py b/lib/spack/spack/cmd/diff.py index f6a5c5ce7a..0e20d7c30b 100644 --- a/lib/spack/spack/cmd/diff.py +++ b/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"] diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 600ce988d7..270cf5eaf2 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -4728,6 +4728,20 @@ class Spec: 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. diff --git a/lib/spack/spack/test/cmd/diff.py b/lib/spack/spack/test/cmd/diff.py index b90c61fbc8..9a901b9cbc 100644 --- a/lib/spack/spack/test/cmd/diff.py +++ b/lib/spack/spack/test/cmd/diff.py @@ -10,12 +10,150 @@ import spack.config 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""" diff --git a/lib/spack/spack/test/concretize_requirements.py b/lib/spack/spack/test/concretize_requirements.py index 529d481b2f..0932793b6f 100644 --- a/lib/spack/spack/test/concretize_requirements.py +++ b/lib/spack/spack/test/concretize_requirements.py @@ -16,6 +16,7 @@ import spack.util.spack_yaml as syaml 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 @@ packages: 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 @@ packages: - 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) diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py index c832c4d1d5..9d3ef7652d 100644 --- a/lib/spack/spack/test/conftest.py +++ b/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) diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py index 87ed1e4b3f..97855ed5bb 100644 --- a/lib/spack/spack/test/spec_semantics.py +++ b/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 diff --git a/share/spack/spack-completion.bash b/share/spack/spack-completion.bash index 85358fbdcc..6ebba8e1ee 100755 --- a/share/spack/spack-completion.bash +++ b/share/spack/spack-completion.bash @@ -999,7 +999,7 @@ _spack_develop() { _spack_diff() { if $list_options then - SPACK_COMPREPLY="-h --help --json --first -a --attribute" + SPACK_COMPREPLY="-h --help --json --first -a --attribute --ignore" else _all_packages fi diff --git a/share/spack/spack-completion.fish b/share/spack/spack-completion.fish index a05cb6c658..cce1622217 100755 --- a/share/spack/spack-completion.fish +++ b/share/spack/spack-completion.fish @@ -1405,7 +1405,7 @@ complete -c spack -n '__fish_spack_using_command develop' -s f -l force -r -f -a complete -c spack -n '__fish_spack_using_command develop' -s f -l force -r -d 'remove any files or directories that block cloning source code' # spack diff -set -g __fish_spack_optspecs_spack_diff h/help json first a/attribute= +set -g __fish_spack_optspecs_spack_diff h/help json first a/attribute= ignore= complete -c spack -n '__fish_spack_using_command_pos_remainder 0 diff' -f -a '(__fish_spack_installed_specs)' complete -c spack -n '__fish_spack_using_command diff' -s h -l help -f -a help complete -c spack -n '__fish_spack_using_command diff' -s h -l help -d 'show this help message and exit' @@ -1415,6 +1415,8 @@ complete -c spack -n '__fish_spack_using_command diff' -l first -f -a load_first complete -c spack -n '__fish_spack_using_command diff' -l first -d 'load the first match if multiple packages match the spec' complete -c spack -n '__fish_spack_using_command diff' -s a -l attribute -r -f -a attribute complete -c spack -n '__fish_spack_using_command diff' -s a -l attribute -r -d 'select the attributes to show (defaults to all)' +complete -c spack -n '__fish_spack_using_command diff' -l ignore -r -f -a ignore +complete -c spack -n '__fish_spack_using_command diff' -l ignore -r -d 'omit diffs related to these dependencies' # spack docs set -g __fish_spack_optspecs_spack_docs h/help -- cgit v1.2.3-70-g09d2