`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>

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