From 2968ae667f62f859f42d08897b0bca273265158d Mon Sep 17 00:00:00 2001 From: Peter Scheibel Date: Thu, 1 Sep 2022 11:04:01 -0700 Subject: New command, `spack change`, to change existing env specs (#31995) If you have an environment like ``` $ cat spack.yaml spack: specs: [openmpi@4.1.0+cuda] ``` this PR provides a new command `spack change` that you can use to adjust environment specs from the command line: ``` $ spack change openmpi~cuda $ cat spack.yaml spack: specs: [openmpi@4.1.0~cuda] ``` in other words, this allows you to tweak the details of environment specs from the command line. Notes: * This is only allowed for environments that do not define matrices * This is possible but not anticipated to be needed immediately * If this were done, it should probably only be done for "named"/not-anonymous specs (i.e. we can change `openmpi+cuda` but not spec like `+cuda` or `@4.0.1~cuda`) --- lib/spack/spack/cmd/change.py | 51 ++++++++++++++++++++++ lib/spack/spack/environment/environment.py | 52 ++++++++++++++++++++++ lib/spack/spack/spec.py | 43 +++++++++++++++++++ lib/spack/spack/spec_list.py | 7 +++ lib/spack/spack/test/cmd/env.py | 30 +++++++++++++ lib/spack/spack/test/env.py | 69 ++++++++++++++++++++++++++++-- lib/spack/spack/test/spec_semantics.py | 14 ++++++ share/spack/spack-completion.bash | 11 ++++- 8 files changed, 273 insertions(+), 4 deletions(-) create mode 100644 lib/spack/spack/cmd/change.py diff --git a/lib/spack/spack/cmd/change.py b/lib/spack/spack/cmd/change.py new file mode 100644 index 0000000000..17d71f33c5 --- /dev/null +++ b/lib/spack/spack/cmd/change.py @@ -0,0 +1,51 @@ +# Copyright 2013-2022 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +import spack.cmd +import spack.cmd.common.arguments as arguments + +description = "change an existing spec in an environment" +section = "environments" +level = "long" + + +def setup_parser(subparser): + subparser.add_argument( + "-l", + "--list-name", + dest="list_name", + default="specs", + help="name of the list to remove specs from", + ) + subparser.add_argument( + "--match-spec", + dest="match_spec", + help="if name is ambiguous, supply a spec to match", + ) + subparser.add_argument( + "-a", + "--all", + action="store_true", + help="change all matching specs (allow changing more than one spec)", + ) + arguments.add_common_arguments(subparser, ["specs"]) + + +def change(parser, args): + env = spack.cmd.require_active_env(cmd_name="change") + + with env.write_transaction(): + if args.match_spec: + match_spec = spack.spec.Spec(args.match_spec) + else: + match_spec = None + for spec in spack.cmd.parse_specs(args.specs): + env.change_existing_spec( + spec, + list_name=args.list_name, + match_spec=match_spec, + allow_changing_multiple_specs=args.all, + ) + env.write() diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index 3df8e4f1df..a27977c956 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -1073,6 +1073,58 @@ class Environment(object): return bool(not existing) + def change_existing_spec( + self, + change_spec, + list_name=user_speclist_name, + match_spec=None, + allow_changing_multiple_specs=False, + ): + """ + Find the spec identified by `match_spec` and change it to `change_spec`. + + Arguments: + change_spec (spack.spec.Spec): defines the spec properties that + need to be changed. This will not change attributes of the + matched spec unless they conflict with `change_spec`. + list_name (str): identifies the spec list in the environment that + should be modified + match_spec (spack.spec.Spec): if set, this identifies the spec + that should be changed. If not set, it is assumed we are + looking for a spec with the same name as `change_spec`. + """ + if not (change_spec.name or (match_spec and match_spec.name)): + raise ValueError( + "Must specify a spec name to identify a single spec" + " in the environment that will be changed" + ) + match_spec = match_spec or Spec(change_spec.name) + + list_to_change = self.spec_lists[list_name] + if list_to_change.is_matrix: + raise SpackEnvironmentError( + "Cannot directly change specs in matrices:" + " specify a named list that is not a matrix" + ) + + matches = list(x for x in list_to_change if x.satisfies(match_spec)) + if len(matches) == 0: + raise ValueError( + "There are no specs named {0} in {1}".format(match_spec.name, list_name) + ) + elif len(matches) > 1 and not allow_changing_multiple_specs: + raise ValueError("{0} matches multiple specs".format(str(match_spec))) + + new_speclist = SpecList(list_name) + for i, spec in enumerate(list_to_change): + if spec.satisfies(match_spec): + new_speclist.add(Spec.override(spec, change_spec)) + else: + new_speclist.add(spec) + + self.spec_lists[list_name] = new_speclist + self.update_stale_references() + def remove(self, query_spec, list_name=user_speclist_name, force=False): """Remove specs from an environment that match a query_spec""" query_spec = Spec(query_spec) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 8a463c2f78..b7ff676fe2 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -284,6 +284,22 @@ class ArchSpec(object): self.platform, self.os, self.target = platform_tuple + @staticmethod + def override(init_spec, change_spec): + if init_spec: + new_spec = init_spec.copy() + else: + new_spec = ArchSpec() + if change_spec.platform: + new_spec.platform = change_spec.platform + # TODO: if the platform is changed to something that is incompatible + # with the current os, we should implicitly remove it + if change_spec.os: + new_spec.os = change_spec.os + if change_spec.target: + new_spec.target = change_spec.target + return new_spec + def _autospec(self, spec_like): if isinstance(spec_like, ArchSpec): return spec_like @@ -2242,6 +2258,33 @@ class Spec(object): raise spack.error.SpecError("Couldn't parse dependency types in spec.") yield dep_name, dep_hash, list(deptypes), hash_type + @staticmethod + def override(init_spec, change_spec): + # TODO: this doesn't account for the case where the changed spec + # (and the user spec) have dependencies + new_spec = init_spec.copy() + package_cls = spack.repo.path.get_pkg_class(new_spec.name) + if change_spec.versions and not change_spec.versions == spack.version.ver(":"): + new_spec.versions = change_spec.versions + for variant, value in change_spec.variants.items(): + if variant in package_cls.variants: + if variant in new_spec.variants: + new_spec.variants.substitute(value) + else: + new_spec.variants[variant] = value + else: + raise ValueError("{0} is not a variant of {1}".format(variant, new_spec.name)) + if change_spec.compiler: + new_spec.compiler = change_spec.compiler + if change_spec.compiler_flags: + for flagname, flagvals in change_spec.compiler_flags.items(): + new_spec.compiler_flags[flagname] = flagvals + if change_spec.architecture: + new_spec.architecture = ArchSpec.override( + new_spec.architecture, change_spec.architecture + ) + return new_spec + @staticmethod def from_literal(spec_dict, normal=True): """Builds a Spec from a dictionary containing the spec literal. diff --git a/lib/spack/spack/spec_list.py b/lib/spack/spack/spec_list.py index aac099eb4d..ffbf2bfaf9 100644 --- a/lib/spack/spack/spec_list.py +++ b/lib/spack/spack/spec_list.py @@ -34,6 +34,13 @@ class SpecList(object): self._constraints = None self._specs = None + @property + def is_matrix(self): + for item in self.specs_as_yaml_list: + if isinstance(item, dict): + return True + return False + @property def specs_as_yaml_list(self): if self._expanded_list is None: diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index d849931dea..e465060392 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -43,6 +43,7 @@ pytestmark = [ env = SpackCommand("env") install = SpackCommand("install") add = SpackCommand("add") +change = SpackCommand("change") remove = SpackCommand("remove") concretize = SpackCommand("concretize") stage = SpackCommand("stage") @@ -71,6 +72,35 @@ def test_add(): assert Spec("mpileaks") in e.user_specs +def test_change_match_spec(): + env("create", "test") + + e = ev.read("test") + with e: + add("mpileaks@2.1") + add("mpileaks@2.2") + + change("--match-spec", "mpileaks@2.2", "mpileaks@2.3") + + assert not any(x.satisfies("mpileaks@2.2") for x in e.user_specs) + assert any(x.satisfies("mpileaks@2.3") for x in e.user_specs) + + +def test_change_multiple_matches(): + env("create", "test") + + e = ev.read("test") + with e: + add("mpileaks@2.1") + add("mpileaks@2.2") + add("libelf@0.8.12%clang") + + change("--match-spec", "mpileaks", "-a", "mpileaks%gcc") + + assert all(x.satisfies("%gcc") for x in e.user_specs if x.name == "mpileaks") + assert any(x.satisfies("%clang") for x in e.user_specs if x.name == "libelf") + + def test_env_add_virtual(): env("create", "test") diff --git a/lib/spack/spack/test/env.py b/lib/spack/spack/test/env.py index c1b36eb5dd..60056e397a 100644 --- a/lib/spack/spack/test/env.py +++ b/lib/spack/spack/test/env.py @@ -4,16 +4,19 @@ # SPDX-License-Identifier: (Apache-2.0 OR MIT) """Test environment internals without CLI""" +import sys import pytest +from six import StringIO import spack.environment as ev import spack.spec - -@pytest.mark.skipif( - str(spack.platforms.host()) == "windows", reason="Not supported on Windows (yet)" +pytestmark = pytest.mark.skipif( + sys.platform == "win32", reason="Envs are not supported on windows" ) + + def test_hash_change_no_rehash_concrete(tmpdir, mock_packages, config): # create an environment env_path = tmpdir.mkdir("env_dir").strpath @@ -43,6 +46,66 @@ def test_hash_change_no_rehash_concrete(tmpdir, mock_packages, config): assert read_in.specs_by_hash[read_in.concretized_order[0]]._hash == new_hash +def test_env_change_spec(tmpdir, mock_packages, config): + env_path = tmpdir.mkdir("env_dir").strpath + env = ev.Environment(env_path) + env.write() + + spec = spack.spec.Spec("mpileaks@2.1~shared+debug") + env.add(spec) + env.write() + + change_spec = spack.spec.Spec("mpileaks@2.2") + env.change_existing_spec(change_spec) + (spec,) = env.added_specs() + assert spec == spack.spec.Spec("mpileaks@2.2~shared+debug") + + change_spec = spack.spec.Spec("mpileaks~debug") + env.change_existing_spec(change_spec) + (spec,) = env.added_specs() + assert spec == spack.spec.Spec("mpileaks@2.2~shared~debug") + + +_test_matrix_yaml = """\ +env: + definitions: + - compilers: ["%gcc", "%clang"] + - desired_specs: ["mpileaks@2.1"] + specs: + - matrix: + - [$compilers] + - [$desired_specs] +""" + + +def test_env_change_spec_in_definition(tmpdir, mock_packages, config, mutable_mock_env_path): + initial_yaml = StringIO(_test_matrix_yaml) + e = ev.create("test", initial_yaml) + e.concretize() + e.write() + + assert any(x.satisfies("mpileaks@2.1%gcc") for x in e.user_specs) + + e.change_existing_spec(spack.spec.Spec("mpileaks@2.2"), list_name="desired_specs") + e.write() + + assert any(x.satisfies("mpileaks@2.2%gcc") for x in e.user_specs) + assert not any(x.satisfies("mpileaks@2.1%gcc") for x in e.user_specs) + + +def test_env_change_spec_in_matrix_raises_error( + tmpdir, mock_packages, config, mutable_mock_env_path +): + initial_yaml = StringIO(_test_matrix_yaml) + e = ev.create("test", initial_yaml) + e.concretize() + e.write() + + with pytest.raises(spack.environment.SpackEnvironmentError) as error: + e.change_existing_spec(spack.spec.Spec("mpileaks@2.2")) + assert "Cannot directly change specs in matrices" in str(error) + + def test_activate_should_require_an_env(): with pytest.raises(TypeError): ev.activate(env="name") diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py index d417adaa18..b543eae9a8 100644 --- a/lib/spack/spack/test/spec_semantics.py +++ b/lib/spack/spack/test/spec_semantics.py @@ -1111,6 +1111,20 @@ class TestSpecSematics(object): with pytest.raises(spack.spec.SpliceError, match="will not provide the same virtuals."): spec.splice(dep, transitive) + def test_spec_override(self): + init_spec = Spec("a foo=baz foobar=baz cflags=-O3 cxxflags=-O1") + change_spec = Spec("a foo=fee cflags=-O2") + new_spec = Spec.override(init_spec, change_spec) + new_spec.concretize() + assert "foo=fee" in new_spec + # This check fails without concretizing: apparently if both specs are + # abstract, then the spec will always be considered to satisfy + # 'variant=value' (regardless of whether it in fact does). + assert "foo=baz" not in new_spec + assert "foobar=baz" in new_spec + assert new_spec.compiler_flags["cflags"] == ["-O2"] + assert new_spec.compiler_flags["cxxflags"] == ["-O1"] + @pytest.mark.regression("3887") @pytest.mark.parametrize("spec_str", ["git", "hdf5", "py-flake8"]) diff --git a/share/spack/spack-completion.bash b/share/spack/spack-completion.bash index 4cfde14e7a..58e94c036a 100755 --- a/share/spack/spack-completion.bash +++ b/share/spack/spack-completion.bash @@ -337,7 +337,7 @@ _spack() { then SPACK_COMPREPLY="-h --help -H --all-help --color -c --config -C --config-scope -d --debug --timestamp --pdb -e --env -D --env-dir -E --no-env --use-env-repo -k --insecure -l --enable-locks -L --disable-locks -m --mock -b --bootstrap -p --profile --sorted-profile --lines -v --verbose --stacktrace -V --version --print-shell-vars" else - SPACK_COMPREPLY="activate add arch audit blame bootstrap build-env buildcache cd checksum ci clean clone commands compiler compilers concretize config containerize create deactivate debug dependencies dependents deprecate dev-build develop diff docs edit env extensions external fetch find gc gpg graph help info install license list load location log-parse maintainers make-installer mark mirror module patch pkg providers pydoc python reindex remove rm repo resource restage solve spec stage style tags test test-env tutorial undevelop uninstall unit-test unload url verify versions view" + SPACK_COMPREPLY="activate add arch audit blame bootstrap build-env buildcache cd change checksum ci clean clone commands compiler compilers concretize config containerize create deactivate debug dependencies dependents deprecate dev-build develop diff docs edit env extensions external fetch find gc gpg graph help info install license list load location log-parse maintainers make-installer mark mirror module patch pkg providers pydoc python reindex remove rm repo resource restage solve spec stage style tags test test-env tutorial undevelop uninstall unit-test unload url verify versions view" fi } @@ -585,6 +585,15 @@ _spack_cd() { fi } +_spack_change() { + if $list_options + then + SPACK_COMPREPLY="-h --help -l --list-name --match-spec -a --all" + else + _all_packages + fi +} + _spack_checksum() { if $list_options then -- cgit v1.2.3-70-g09d2