From 2968ae667f62f859f42d08897b0bca273265158d Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
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 ++++++
7 files changed, 263 insertions(+), 3 deletions(-)
create mode 100644 lib/spack/spack/cmd/change.py
(limited to 'lib')
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"])
--
cgit v1.2.3-70-g09d2