summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPeter Scheibel <scheibel1@llnl.gov>2023-12-18 00:47:53 -0800
committerGitHub <noreply@github.com>2023-12-18 00:47:53 -0800
commit14c7bfe9cedaa779717852aef6520761ffe95cd3 (patch)
treeb533c09969bb4422aea41521274251f6f6782624
parented52b505d4132f411f15d9764d4bd014c76af8dd (diff)
downloadspack-14c7bfe9cedaa779717852aef6520761ffe95cd3.tar.gz
spack-14c7bfe9cedaa779717852aef6520761ffe95cd3.tar.bz2
spack-14c7bfe9cedaa779717852aef6520761ffe95cd3.tar.xz
spack-14c7bfe9cedaa779717852aef6520761ffe95cd3.zip
`spack develop`: convert to config (#35273)
Convert the 'develop' section of an environment to a dedicated configuration section. This means for example that instead of having to define `develop` specs in the `spack.yaml`, the environment can `include:` another `develop.yaml` configuration which specifies which specs should be developed in the environment. This change is not expected to be disruptive given that existing environment `spack.yaml` files will conform to the new schema. (Update 11/28/2023) I have implemented the `develop`/`undevelop` commands in terms of more-generic modification functions added to the `config` module: `change_or_add` and `update_all`. It is assumed that the semantics added here (described in 11/18 update) would be desirable to extend to other config update actions (e.g. adding compilers, changing package requirements, adding mirrors). (Update 11/18/2023) I have updated this such that `spack develop`, and `spack undevelop` to potentially modify all writable scopes, like https://github.com/spack/spack/pull/41147. https://github.com/spack/spack/pull/35307 will be useful for modifying included scopes, but generally speaking specifying a `--scope` will not be required for `spack develop`: `spack develop` will add new develop specs to whatever scope already has develop specs defined, or to the highest-priority writable scope (which should be the env scope). TODOs: - [x] If you `spack undevelop` a package which is mentioned at multiple layers of configuration, then currently this would only modify one of them. That's not technically a new issue (has always existed for configuration modification), but may be confusing to users when presented via an interface other than `spack config set` - [x] Need to add (or confirm) the ability to modify individual config files by providing a path (rather than using a scope identifier as a key to retrieve associated config). - [x] `spack develop` adds new develop specs to the scope that defines them (potentially skipping higher priority scopes to e.g. augment included scope files) --------- Co-authored-by: scheibelp <scheibelp@users.noreply.github.com> Co-authored-by: Todd Gamblin <tgamblin@llnl.gov>
-rw-r--r--lib/spack/spack/cmd/develop.py78
-rw-r--r--lib/spack/spack/cmd/undevelop.py50
-rw-r--r--lib/spack/spack/config.py79
-rw-r--r--lib/spack/spack/environment/environment.py221
-rw-r--r--lib/spack/spack/schema/develop.py34
-rw-r--r--lib/spack/spack/schema/env.py15
-rw-r--r--lib/spack/spack/schema/merged.py2
-rw-r--r--lib/spack/spack/test/cmd/develop.py12
-rw-r--r--lib/spack/spack/test/cmd/env.py50
-rw-r--r--lib/spack/spack/test/cmd/undevelop.py4
-rw-r--r--lib/spack/spack/test/config.py28
-rwxr-xr-xshare/spack/spack-completion.fish6
12 files changed, 368 insertions, 211 deletions
diff --git a/lib/spack/spack/cmd/develop.py b/lib/spack/spack/cmd/develop.py
index f515352175..ca3ab36e4d 100644
--- a/lib/spack/spack/cmd/develop.py
+++ b/lib/spack/spack/cmd/develop.py
@@ -45,10 +45,41 @@ def setup_parser(subparser):
arguments.add_common_arguments(subparser, ["spec"])
-def develop(parser, args):
- env = spack.cmd.require_active_env(cmd_name="develop")
+def _update_config(spec, path):
+ find_fn = lambda section: spec.name in section
+
+ entry = {"spec": str(spec)}
+ if path != spec.name:
+ entry["path"] = path
+
+ def change_fn(section):
+ section[spec.name] = entry
+
+ spack.config.change_or_add("develop", find_fn, change_fn)
+
+
+def _retrieve_develop_source(spec, abspath):
+ # "steal" the source code via staging API. We ask for a stage
+ # to be created, then copy it afterwards somewhere else. It would be
+ # better if we can create the `source_path` directly into its final
+ # destination.
+ pkg_cls = spack.repo.PATH.get_pkg_class(spec.name)
+ # We construct a package class ourselves, rather than asking for
+ # Spec.package, since Spec only allows this when it is concrete
+ package = pkg_cls(spec)
+ if isinstance(package.stage[0].fetcher, spack.fetch_strategy.GitFetchStrategy):
+ package.stage[0].fetcher.get_full_repo = True
+ # If we retrieved this version before and cached it, we may have
+ # done so without cloning the full git repo; likewise, any
+ # mirror might store an instance with truncated history.
+ package.stage[0].disable_mirrors()
+ package.stage.steal_source(abspath)
+
+
+def develop(parser, args):
if not args.spec:
+ env = spack.cmd.require_active_env(cmd_name="develop")
if args.clone is False:
raise SpackError("No spec provided to spack develop command")
@@ -66,7 +97,7 @@ def develop(parser, args):
# Both old syntax `spack develop pkg@x` and new syntax `spack develop pkg@=x`
# are currently supported.
spec = spack.spec.parse_with_version_concrete(entry["spec"])
- env.develop(spec=spec, path=path, clone=True)
+ _retrieve_develop_source(spec, abspath)
if not env.dev_specs:
tty.warn("No develop specs to download")
@@ -81,12 +112,16 @@ def develop(parser, args):
version = spec.versions.concrete_range_as_version
if not version:
raise SpackError("Packages to develop must have a concrete version")
-
spec.versions = spack.version.VersionList([version])
- # default path is relative path to spec.name
+ # If user does not specify --path, we choose to create a directory in the
+ # active environment's directory, named after the spec
path = args.path or spec.name
- abspath = spack.util.path.canonicalize_path(path, default_wd=env.path)
+ if not os.path.isabs(path):
+ env = spack.cmd.require_active_env(cmd_name="develop")
+ abspath = spack.util.path.canonicalize_path(path, default_wd=env.path)
+ else:
+ abspath = path
# clone default: only if the path doesn't exist
clone = args.clone
@@ -96,15 +131,24 @@ def develop(parser, args):
if not clone and not os.path.exists(abspath):
raise SpackError("Provided path %s does not exist" % abspath)
- if clone and os.path.exists(abspath):
- if args.force:
- shutil.rmtree(abspath)
- else:
- msg = "Path %s already exists and cannot be cloned to." % abspath
- msg += " Use `spack develop -f` to overwrite."
- raise SpackError(msg)
-
+ if clone:
+ if os.path.exists(abspath):
+ if args.force:
+ shutil.rmtree(abspath)
+ else:
+ msg = "Path %s already exists and cannot be cloned to." % abspath
+ msg += " Use `spack develop -f` to overwrite."
+ raise SpackError(msg)
+
+ _retrieve_develop_source(spec, abspath)
+
+ # Note: we could put develop specs in any scope, but I assume
+ # users would only ever want to do this for either (a) an active
+ # env or (b) a specified config file (e.g. that is included by
+ # an environment)
+ # TODO: when https://github.com/spack/spack/pull/35307 is merged,
+ # an active env is not required if a scope is specified
+ env = spack.cmd.require_active_env(cmd_name="develop")
+ tty.debug("Updating develop config for {0} transactionally".format(env.name))
with env.write_transaction():
- changed = env.develop(spec, path, clone)
- if changed:
- env.write()
+ _update_config(spec, path)
diff --git a/lib/spack/spack/cmd/undevelop.py b/lib/spack/spack/cmd/undevelop.py
index 8f3c19cd1a..7159708796 100644
--- a/lib/spack/spack/cmd/undevelop.py
+++ b/lib/spack/spack/cmd/undevelop.py
@@ -17,21 +17,51 @@ def setup_parser(subparser):
subparser.add_argument(
"-a", "--all", action="store_true", help="remove all specs from (clear) the environment"
)
+
arguments.add_common_arguments(subparser, ["specs"])
-def undevelop(parser, args):
- env = spack.cmd.require_active_env(cmd_name="undevelop")
+def _update_config(specs_to_remove, remove_all=False):
+ def change_fn(dev_config):
+ modified = False
+ for spec in specs_to_remove:
+ if spec.name in dev_config:
+ tty.msg("Undevelop: removing {0}".format(spec.name))
+ del dev_config[spec.name]
+ modified = True
+ if remove_all and dev_config:
+ dev_config.clear()
+ modified = True
+ return modified
+ spack.config.update_all("develop", change_fn)
+
+
+def undevelop(parser, args):
+ remove_specs = None
+ remove_all = False
if args.all:
- specs = env.dev_specs.keys()
+ remove_all = True
else:
- specs = spack.cmd.parse_specs(args.specs)
+ remove_specs = spack.cmd.parse_specs(args.specs)
+ # TODO: when https://github.com/spack/spack/pull/35307 is merged,
+ # an active env is not required if a scope is specified
+ env = spack.cmd.require_active_env(cmd_name="undevelop")
with env.write_transaction():
- changed = False
- for spec in specs:
- tty.msg("Removing %s from environment %s development specs" % (spec, env.name))
- changed |= env.undevelop(spec)
- if changed:
- env.write()
+ _update_config(remove_specs, remove_all)
+
+ updated_all_dev_specs = set(spack.config.get("develop"))
+ remove_spec_names = set(x.name for x in remove_specs)
+
+ if remove_all:
+ not_fully_removed = updated_all_dev_specs
+ else:
+ not_fully_removed = updated_all_dev_specs & remove_spec_names
+
+ if not_fully_removed:
+ tty.msg(
+ "The following specs could not be removed as develop specs"
+ " - see `spack config blame develop` to locate files requiring"
+ f" manual edits: {', '.join(not_fully_removed)}"
+ )
diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index cd1be71c9d..58fcbfb6c8 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -70,6 +70,7 @@ SECTION_SCHEMAS = {
"compilers": spack.schema.compilers.schema,
"concretizer": spack.schema.concretizer.schema,
"definitions": spack.schema.definitions.schema,
+ "develop": spack.schema.develop.schema,
"mirrors": spack.schema.mirrors.schema,
"repos": spack.schema.repos.schema,
"packages": spack.schema.packages.schema,
@@ -927,6 +928,84 @@ def scopes():
return CONFIG.scopes
+def writable_scopes() -> List[ConfigScope]:
+ """
+ Return list of writable scopes
+ """
+ return list(
+ reversed(
+ list(
+ x
+ for x in CONFIG.scopes.values()
+ if not isinstance(x, (InternalConfigScope, ImmutableConfigScope))
+ )
+ )
+ )
+
+
+def writable_scope_names() -> List[str]:
+ return list(x.name for x in writable_scopes())
+
+
+def matched_config(cfg_path):
+ return [(scope, get(cfg_path, scope=scope)) for scope in writable_scope_names()]
+
+
+def change_or_add(section_name, find_fn, update_fn):
+ """Change or add a subsection of config, with additional logic to
+ select a reasonable scope where the change is applied.
+
+ Search through config scopes starting with the highest priority:
+ the first matching a criteria (determined by ``find_fn``) is updated;
+ if no such config exists, find the first config scope that defines
+ any config for the named section; if no scopes define any related
+ config, then update the highest-priority config scope.
+ """
+ configs_by_section = matched_config(section_name)
+
+ found = False
+ for scope, section in configs_by_section:
+ found = find_fn(section)
+ if found:
+ break
+
+ if found:
+ update_fn(section)
+ spack.config.set(section_name, section, scope=scope)
+ return
+
+ # If no scope meets the criteria specified by ``find_fn``,
+ # then look for a scope that has any content (for the specified
+ # section name)
+ for scope, section in configs_by_section:
+ if section:
+ update_fn(section)
+ found = True
+ break
+
+ if found:
+ spack.config.set(section_name, section, scope=scope)
+ return
+
+ # If no scopes define any config for the named section, then
+ # modify the highest-priority scope.
+ scope, section = configs_by_section[0]
+ update_fn(section)
+ spack.config.set(section_name, section, scope=scope)
+
+
+def update_all(section_name, change_fn):
+ """Change a config section, which may have details duplicated
+ across multiple scopes.
+ """
+ configs_by_section = matched_config("develop")
+
+ for scope, section in configs_by_section:
+ modified = change_fn(section)
+ if modified:
+ spack.config.set(section_name, section, scope=scope)
+
+
def _validate_section_name(section):
"""Exit if the section is not a valid section."""
if section not in SECTION_SCHEMAS:
diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py
index 5d6273506e..2721ddb828 100644
--- a/lib/spack/spack/environment/environment.py
+++ b/lib/spack/spack/environment/environment.py
@@ -16,7 +16,7 @@ import time
import urllib.parse
import urllib.request
import warnings
-from typing import Any, Dict, Iterable, List, Optional, Set, Tuple, Union
+from typing import Dict, Iterable, List, Optional, Set, Tuple, Union
import llnl.util.filesystem as fs
import llnl.util.tty as tty
@@ -307,7 +307,7 @@ def create(
def create_in_dir(
- manifest_dir: Union[str, pathlib.Path],
+ root: Union[str, pathlib.Path],
init_file: Optional[Union[str, pathlib.Path]] = None,
with_view: Optional[Union[str, pathlib.Path, bool]] = None,
keep_relative: bool = False,
@@ -318,35 +318,72 @@ def create_in_dir(
are considered manifest files.
Args:
- manifest_dir: directory where to create the environment.
+ root: directory where to create the environment.
init_file: either a lockfile, a manifest file, or None
with_view: whether a view should be maintained for the environment. If the value is a
string, it specifies the path to the view
keep_relative: if True, develop paths are copied verbatim into the new environment file,
otherwise they are made absolute
"""
- initialize_environment_dir(manifest_dir, envfile=init_file)
+ initialize_environment_dir(root, envfile=init_file)
if with_view is None and keep_relative:
- return Environment(manifest_dir)
+ return Environment(root)
try:
- manifest = EnvironmentManifestFile(manifest_dir)
+ manifest = EnvironmentManifestFile(root)
if with_view is not None:
manifest.set_default_view(with_view)
- if not keep_relative and init_file is not None and str(init_file).endswith(manifest_name):
- init_file = pathlib.Path(init_file)
- manifest.absolutify_dev_paths(init_file.parent)
-
manifest.flush()
except (spack.config.ConfigFormatError, SpackEnvironmentConfigError) as e:
- shutil.rmtree(manifest_dir)
+ shutil.rmtree(root)
raise e
- return Environment(manifest_dir)
+ env = Environment(root)
+
+ if init_file:
+ init_file_dir = os.path.abspath(os.path.dirname(init_file))
+
+ if not keep_relative:
+ if env.path != init_file_dir:
+ # If we are here, we are creating an environment based on an
+ # spack.yaml file in another directory, and moreover we want
+ # dev paths in this environment to refer to their original
+ # locations.
+ _rewrite_relative_dev_paths_on_relocation(env, init_file_dir)
+
+ return env
+
+
+def _rewrite_relative_dev_paths_on_relocation(env, init_file_dir):
+ """When initializing the environment from a manifest file and we plan
+ to store the environment in a different directory, we have to rewrite
+ relative paths to absolute ones."""
+ with env:
+ dev_specs = spack.config.get("develop", default={}, scope=env.env_file_config_scope_name())
+ if not dev_specs:
+ return
+ for name, entry in dev_specs.items():
+ dev_path = entry["path"]
+ expanded_path = os.path.normpath(os.path.join(init_file_dir, entry["path"]))
+
+ # Skip if the expanded path is the same (e.g. when absolute)
+ if dev_path == expanded_path:
+ continue
+
+ tty.debug("Expanding develop path for {0} to {1}".format(name, expanded_path))
+
+ dev_specs[name]["path"] = expanded_path
+
+ spack.config.set("develop", dev_specs, scope=env.env_file_config_scope_name())
+
+ env._dev_specs = None
+ # If we changed the environment's spack.yaml scope, that will not be reflected
+ # in the manifest that we read
+ env._re_read()
def environment_dir_from_name(name: str, exists_ok: bool = True) -> str:
@@ -753,8 +790,6 @@ class Environment:
#: Specs from "spack.yaml"
self.spec_lists: Dict[str, SpecList] = {user_speclist_name: SpecList()}
- #: Dev-build specs from "spack.yaml"
- self.dev_specs: Dict[str, Any] = {}
#: User specs from the last concretization
self.concretized_user_specs: List[Spec] = []
#: Roots associated with the last concretization, in order
@@ -765,6 +800,7 @@ class Environment:
self._repo = None
#: Previously active environment
self._previous_active = None
+ self._dev_specs = None
with lk.ReadTransaction(self.txlock):
self.manifest = EnvironmentManifestFile(manifest_dir)
@@ -858,19 +894,29 @@ class Environment:
else:
self.views = {}
- # Retrieve dev-build packages:
- self.dev_specs = copy.deepcopy(env_configuration.get("develop", {}))
- for name, entry in self.dev_specs.items():
- # spec must include a concrete version
- assert Spec(entry["spec"]).versions.concrete_range_as_version
- # default path is the spec name
- if "path" not in entry:
- self.dev_specs[name]["path"] = name
-
@property
def user_specs(self):
return self.spec_lists[user_speclist_name]
+ @property
+ def dev_specs(self):
+ if not self._dev_specs:
+ self._dev_specs = self._read_dev_specs()
+ return self._dev_specs
+
+ def _read_dev_specs(self):
+ dev_specs = {}
+ dev_config = spack.config.get("develop", {})
+ for name, entry in dev_config.items():
+ local_entry = {"spec": str(entry["spec"])}
+ # default path is the spec name
+ if "path" not in entry:
+ local_entry["path"] = name
+ else:
+ local_entry["path"] = entry["path"]
+ dev_specs[name] = local_entry
+ return dev_specs
+
def clear(self, re_read=False):
"""Clear the contents of the environment
@@ -883,7 +929,7 @@ class Environment:
self.spec_lists = collections.OrderedDict()
self.spec_lists[user_speclist_name] = SpecList()
- self.dev_specs = {} # dev-build specs from yaml
+ self._dev_specs = {}
self.concretized_user_specs = [] # user specs from last concretize
self.concretized_order = [] # roots of last concretize, in order
self.specs_by_hash = {} # concretized specs by hash
@@ -1251,82 +1297,6 @@ class Environment:
del self.concretized_order[i]
del self.specs_by_hash[dag_hash]
- def develop(self, spec: Spec, path: str, clone: bool = False) -> bool:
- """Add dev-build info for package
-
- Args:
- spec: Set constraints on development specs. Must include a
- concrete version.
- path: Path to find code for developer builds. Relative
- paths will be resolved relative to the environment.
- clone: Clone the package code to the path.
- If clone is False Spack will assume the code is already present
- at ``path``.
-
- Return:
- (bool): True iff the environment was changed.
- """
- spec = spec.copy() # defensive copy since we access cached attributes
-
- if not spec.versions.concrete:
- raise SpackEnvironmentError("Cannot develop spec %s without a concrete version" % spec)
-
- for name, entry in self.dev_specs.items():
- if name == spec.name:
- e_spec = Spec(entry["spec"])
- e_path = entry["path"]
-
- if e_spec == spec:
- if path == e_path:
- tty.msg("Spec %s already configured for development" % spec)
- return False
- else:
- tty.msg("Updating development path for spec %s" % spec)
- break
- else:
- msg = "Updating development spec for package "
- msg += "%s with path %s" % (spec.name, path)
- tty.msg(msg)
- break
- else:
- tty.msg("Configuring spec %s for development at path %s" % (spec, path))
-
- if clone:
- # "steal" the source code via staging API. We ask for a stage
- # to be created, then copy it afterwards somewhere else. It would be
- # better if we can create the `source_path` directly into its final
- # destination.
- abspath = spack.util.path.canonicalize_path(path, default_wd=self.path)
- pkg_cls = spack.repo.PATH.get_pkg_class(spec.name)
- # We construct a package class ourselves, rather than asking for
- # Spec.package, since Spec only allows this when it is concrete
- package = pkg_cls(spec)
- if isinstance(package.fetcher, spack.fetch_strategy.GitFetchStrategy):
- package.fetcher.get_full_repo = True
- # If we retrieved this version before and cached it, we may have
- # done so without cloning the full git repo; likewise, any
- # mirror might store an instance with truncated history.
- package.stage.disable_mirrors()
-
- package.stage.steal_source(abspath)
-
- # If it wasn't already in the list, append it
- entry = {"path": path, "spec": str(spec)}
- self.dev_specs[spec.name] = entry
- self.manifest.add_develop_spec(spec.name, entry=entry.copy())
- return True
-
- def undevelop(self, spec):
- """Remove develop info for abstract spec ``spec``.
-
- returns True on success, False if no entry existed."""
- spec = Spec(spec) # In case it's a spec object
- if spec.name in self.dev_specs:
- del self.dev_specs[spec.name]
- self.manifest.remove_develop_spec(spec.name)
- return True
- return False
-
def is_develop(self, spec):
"""Returns true when the spec is built from local sources"""
return spec.name in self.dev_specs
@@ -2901,57 +2871,6 @@ class EnvironmentManifestFile(collections.abc.Mapping):
self.set_default_view(view=False)
- def add_develop_spec(self, pkg_name: str, entry: Dict[str, str]) -> None:
- """Adds a develop spec to the manifest file
-
- Args:
- pkg_name: name of the package to be developed
- entry: spec and path of the developed package
- """
- # The environment sets the path to pkg_name is that is implicit
- if entry["path"] == pkg_name:
- entry.pop("path")
-
- self.pristine_configuration.setdefault("develop", {}).setdefault(pkg_name, {}).update(
- entry
- )
- self.configuration.setdefault("develop", {}).setdefault(pkg_name, {}).update(entry)
- self.changed = True
-
- def remove_develop_spec(self, pkg_name: str) -> None:
- """Removes a develop spec from the manifest file
-
- Args:
- pkg_name: package to be removed from development
-
- Raises:
- SpackEnvironmentError: if there is nothing to remove
- """
- try:
- del self.pristine_configuration["develop"][pkg_name]
- except KeyError as e:
- msg = f"cannot remove '{pkg_name}' from develop specs in {self}, entry does not exist"
- raise SpackEnvironmentError(msg) from e
- del self.configuration["develop"][pkg_name]
- self.changed = True
-
- def absolutify_dev_paths(self, init_file_dir: Union[str, pathlib.Path]) -> None:
- """Normalizes the dev paths in the environment with respect to the directory where the
- initialization file resides.
-
- Args:
- init_file_dir: directory with the "spack.yaml" used to initialize the environment.
- """
- init_file_dir = pathlib.Path(init_file_dir).absolute()
- for _, entry in self.pristine_configuration.get("develop", {}).items():
- expanded_path = os.path.normpath(str(init_file_dir / entry["path"]))
- entry["path"] = str(expanded_path)
-
- for _, entry in self.configuration.get("develop", {}).items():
- expanded_path = os.path.normpath(str(init_file_dir / entry["path"]))
- entry["path"] = str(expanded_path)
- self.changed = True
-
def flush(self) -> None:
"""Synchronizes the object with the manifest file on disk."""
if not self.changed:
diff --git a/lib/spack/spack/schema/develop.py b/lib/spack/spack/schema/develop.py
new file mode 100644
index 0000000000..e8c658de50
--- /dev/null
+++ b/lib/spack/spack/schema/develop.py
@@ -0,0 +1,34 @@
+# Copyright 2013-2023 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)
+
+
+properties = {
+ "develop": {
+ "type": "object",
+ "default": {},
+ "additionalProperties": False,
+ "patternProperties": {
+ r"\w[\w-]*": {
+ "type": "object",
+ "additionalProperties": False,
+ "properties": {"spec": {"type": "string"}, "path": {"type": "string"}},
+ }
+ },
+ }
+}
+
+
+def update(data):
+ return False
+
+
+#: Full schema with metadata
+schema = {
+ "$schema": "http://json-schema.org/draft-07/schema#",
+ "title": "Spack repository configuration file schema",
+ "type": "object",
+ "additionalProperties": False,
+ "properties": properties,
+}
diff --git a/lib/spack/spack/schema/env.py b/lib/spack/spack/schema/env.py
index 463c6680f0..fcb3a546bd 100644
--- a/lib/spack/spack/schema/env.py
+++ b/lib/spack/spack/schema/env.py
@@ -37,21 +37,6 @@ schema = {
# extra environment schema properties
{
"include": {"type": "array", "default": [], "items": {"type": "string"}},
- "develop": {
- "type": "object",
- "default": {},
- "additionalProperties": False,
- "patternProperties": {
- r"\w[\w-]*": {
- "type": "object",
- "additionalProperties": False,
- "properties": {
- "spec": {"type": "string"},
- "path": {"type": "string"},
- },
- }
- },
- },
"specs": spack.schema.spec_list_schema,
"view": {
"anyOf": [
diff --git a/lib/spack/spack/schema/merged.py b/lib/spack/spack/schema/merged.py
index 7ceb649410..fbd8474070 100644
--- a/lib/spack/spack/schema/merged.py
+++ b/lib/spack/spack/schema/merged.py
@@ -18,6 +18,7 @@ import spack.schema.concretizer
import spack.schema.config
import spack.schema.container
import spack.schema.definitions
+import spack.schema.develop
import spack.schema.mirrors
import spack.schema.modules
import spack.schema.packages
@@ -34,6 +35,7 @@ properties = union_dicts(
spack.schema.container.properties,
spack.schema.ci.properties,
spack.schema.definitions.properties,
+ spack.schema.develop.properties,
spack.schema.mirrors.properties,
spack.schema.modules.properties,
spack.schema.packages.properties,
diff --git a/lib/spack/spack/test/cmd/develop.py b/lib/spack/spack/test/cmd/develop.py
index f80038c0ed..2b1320c690 100644
--- a/lib/spack/spack/test/cmd/develop.py
+++ b/lib/spack/spack/test/cmd/develop.py
@@ -19,7 +19,7 @@ env = SpackCommand("env")
pytestmark = pytest.mark.not_on_windows("does not run on windows")
-@pytest.mark.usefixtures("mutable_mock_env_path", "mock_packages", "mock_fetch", "config")
+@pytest.mark.usefixtures("mutable_mock_env_path", "mock_packages", "mock_fetch", "mutable_config")
class TestDevelop:
def check_develop(self, env, spec, path=None):
path = path or spec.name
@@ -31,9 +31,9 @@ class TestDevelop:
assert dev_specs_entry["spec"] == str(spec)
# check yaml representation
- yaml = env.manifest[ev.TOP_LEVEL_KEY]
- assert spec.name in yaml["develop"]
- yaml_entry = yaml["develop"][spec.name]
+ dev_config = spack.config.get("develop", {})
+ assert spec.name in dev_config
+ yaml_entry = dev_config[spec.name]
assert yaml_entry["spec"] == str(spec)
if path == spec.name:
# default paths aren't written out
@@ -102,7 +102,7 @@ class TestDevelop:
self.check_develop(e, spack.spec.Spec("mpich@=2.0"))
assert len(e.dev_specs) == 1
- def test_develop_canonicalize_path(self, monkeypatch, config):
+ def test_develop_canonicalize_path(self, monkeypatch):
env("create", "test")
with ev.read("test") as e:
path = "../$user"
@@ -119,7 +119,7 @@ class TestDevelop:
# Check modifications actually worked
assert spack.spec.Spec("mpich@1.0").concretized().satisfies("dev_path=%s" % abspath)
- def test_develop_canonicalize_path_no_args(self, monkeypatch, config):
+ def test_develop_canonicalize_path_no_args(self, monkeypatch):
env("create", "test")
with ev.read("test") as e:
path = "$user"
diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py
index 4402dd0ba1..74752a737d 100644
--- a/lib/spack/spack/test/cmd/env.py
+++ b/lib/spack/spack/test/cmd/env.py
@@ -53,6 +53,7 @@ concretize = SpackCommand("concretize")
stage = SpackCommand("stage")
uninstall = SpackCommand("uninstall")
find = SpackCommand("find")
+develop = SpackCommand("develop")
module = SpackCommand("module")
sep = os.sep
@@ -719,10 +720,10 @@ spack:
assert any(x.intersects("mpileaks@2.2") for x in e._get_environment_specs())
-def test_with_config_bad_include(environment_from_manifest):
+def test_with_config_bad_include_create(environment_from_manifest):
"""Confirm missing include paths raise expected exception and error."""
with pytest.raises(spack.config.ConfigFileError, match="2 missing include path"):
- e = environment_from_manifest(
+ environment_from_manifest(
"""
spack:
include:
@@ -730,9 +731,42 @@ spack:
- no/such/file.yaml
"""
)
- with e:
- e.concretize()
+
+def test_with_config_bad_include_activate(environment_from_manifest, tmpdir):
+ env_root = pathlib.Path(tmpdir.ensure("env-root", dir=True))
+ include1 = env_root / "include1.yaml"
+ include1.touch()
+
+ abs_include_path = os.path.abspath(tmpdir.join("subdir").ensure("include2.yaml"))
+
+ spack_yaml = env_root / ev.manifest_name
+ spack_yaml.write_text(
+ f"""
+spack:
+ include:
+ - ./include1.yaml
+ - {abs_include_path}
+"""
+ )
+
+ e = ev.Environment(env_root)
+ with e:
+ e.concretize()
+
+ # we've created an environment with some included config files (which do
+ # in fact exist): now we remove them and check that we get a sensible
+ # error message
+
+ os.remove(abs_include_path)
+ os.remove(include1)
+ with pytest.raises(spack.config.ConfigFileError) as exc:
+ ev.activate(e)
+
+ err = exc.value.message
+ assert "missing include" in err
+ assert abs_include_path in err
+ assert "include1.yaml" in err
assert ev.active_environment() is None
@@ -1173,7 +1207,7 @@ def test_env_blocks_uninstall(mock_stage, mock_fetch, install_mockery):
add("mpileaks")
install("--fake")
- out = uninstall("mpileaks", fail_on_error=False)
+ out = uninstall("-y", "mpileaks", fail_on_error=False)
assert uninstall.returncode == 1
assert "The following environments still reference these specs" in out
@@ -2902,13 +2936,15 @@ def test_virtual_spec_concretize_together(tmpdir):
assert any(s.package.provides("mpi") for _, s in e.concretized_specs())
-def test_query_develop_specs():
+def test_query_develop_specs(tmpdir):
"""Test whether a spec is develop'ed or not"""
+ srcdir = tmpdir.ensure("here")
+
env("create", "test")
with ev.read("test") as e:
e.add("mpich")
e.add("mpileaks")
- e.develop(Spec("mpich@=1"), "here", clone=False)
+ develop("--no-clone", "-p", str(srcdir), "mpich@=1")
assert e.is_develop(Spec("mpich"))
assert not e.is_develop(Spec("mpileaks"))
diff --git a/lib/spack/spack/test/cmd/undevelop.py b/lib/spack/spack/test/cmd/undevelop.py
index 93b7c7ef90..7ba761aade 100644
--- a/lib/spack/spack/test/cmd/undevelop.py
+++ b/lib/spack/spack/test/cmd/undevelop.py
@@ -17,7 +17,7 @@ concretize = SpackCommand("concretize")
pytestmark = pytest.mark.not_on_windows("does not run on windows")
-def test_undevelop(tmpdir, config, mock_packages, mutable_mock_env_path):
+def test_undevelop(tmpdir, mutable_config, mock_packages, mutable_mock_env_path):
# setup environment
envdir = tmpdir.mkdir("env")
with envdir.as_cwd():
@@ -46,7 +46,7 @@ spack:
assert not after.satisfies("dev_path=*")
-def test_undevelop_nonexistent(tmpdir, config, mock_packages, mutable_mock_env_path):
+def test_undevelop_nonexistent(tmpdir, mutable_config, mock_packages, mutable_mock_env_path):
# setup environment
envdir = tmpdir.mkdir("env")
with envdir.as_cwd():
diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py
index 2453172bec..e3513df42b 100644
--- a/lib/spack/spack/test/config.py
+++ b/lib/spack/spack/test/config.py
@@ -502,6 +502,34 @@ def test_parse_install_tree(config_settings, expected, mutable_config):
assert projections == expected_proj
+def test_change_or_add(mutable_config, mock_packages):
+ spack.config.add("packages:a:version:['1.0']", scope="user")
+
+ spack.config.add("packages:b:version:['1.1']", scope="system")
+
+ class ChangeTest:
+ def __init__(self, pkg_name, new_version):
+ self.pkg_name = pkg_name
+ self.new_version = new_version
+
+ def find_fn(self, section):
+ return self.pkg_name in section
+
+ def change_fn(self, section):
+ pkg_section = section.get(self.pkg_name, {})
+ pkg_section["version"] = self.new_version
+ section[self.pkg_name] = pkg_section
+
+ change1 = ChangeTest("b", ["1.2"])
+ spack.config.change_or_add("packages", change1.find_fn, change1.change_fn)
+ assert "b" not in mutable_config.get("packages", scope="user")
+ assert mutable_config.get("packages")["b"]["version"] == ["1.2"]
+
+ change2 = ChangeTest("c", ["1.0"])
+ spack.config.change_or_add("packages", change2.find_fn, change2.change_fn)
+ assert "c" in mutable_config.get("packages", scope="user")
+
+
@pytest.mark.not_on_windows("Padding unsupported on Windows")
@pytest.mark.parametrize(
"config_settings,expected",
diff --git a/share/spack/spack-completion.fish b/share/spack/spack-completion.fish
index 090f725fa1..a05cb6c658 100755
--- a/share/spack/spack-completion.fish
+++ b/share/spack/spack-completion.fish
@@ -1173,19 +1173,19 @@ complete -c spack -n '__fish_spack_using_command config' -l scope -r -d 'configu
# spack config get
set -g __fish_spack_optspecs_spack_config_get h/help
-complete -c spack -n '__fish_spack_using_command_pos 0 config get' -f -a 'bootstrap cdash ci compilers concretizer config definitions mirrors modules packages repos upstreams'
+complete -c spack -n '__fish_spack_using_command_pos 0 config get' -f -a 'bootstrap cdash ci compilers concretizer config definitions develop mirrors modules packages repos upstreams'
complete -c spack -n '__fish_spack_using_command config get' -s h -l help -f -a help
complete -c spack -n '__fish_spack_using_command config get' -s h -l help -d 'show this help message and exit'
# spack config blame
set -g __fish_spack_optspecs_spack_config_blame h/help
-complete -c spack -n '__fish_spack_using_command_pos 0 config blame' -f -a 'bootstrap cdash ci compilers concretizer config definitions mirrors modules packages repos upstreams'
+complete -c spack -n '__fish_spack_using_command_pos 0 config blame' -f -a 'bootstrap cdash ci compilers concretizer config definitions develop mirrors modules packages repos upstreams'
complete -c spack -n '__fish_spack_using_command config blame' -s h -l help -f -a help
complete -c spack -n '__fish_spack_using_command config blame' -s h -l help -d 'show this help message and exit'
# spack config edit
set -g __fish_spack_optspecs_spack_config_edit h/help print-file
-complete -c spack -n '__fish_spack_using_command_pos 0 config edit' -f -a 'bootstrap cdash ci compilers concretizer config definitions mirrors modules packages repos upstreams'
+complete -c spack -n '__fish_spack_using_command_pos 0 config edit' -f -a 'bootstrap cdash ci compilers concretizer config definitions develop mirrors modules packages repos upstreams'
complete -c spack -n '__fish_spack_using_command config edit' -s h -l help -f -a help
complete -c spack -n '__fish_spack_using_command config edit' -s h -l help -d 'show this help message and exit'
complete -c spack -n '__fish_spack_using_command config edit' -l print-file -f -a print_file