From c9dfb9b0fd68869f86cab7ce714035ed499f95dd Mon Sep 17 00:00:00 2001
From: Tamara Dahlgren <35777542+tldahlgren@users.noreply.github.com>
Date: Sun, 5 Nov 2023 00:47:06 -0700
Subject: Environments: Add support for including definitions files (#33960)

This PR adds support for including separate definitions from `spack.yaml`.

Supporting the inclusion of files with definitions enables user to make
curated/standardized collections of packages that can re-used by others.
---
 lib/spack/spack/config.py                  |   2 +
 lib/spack/spack/environment/environment.py |  69 ++++++----
 lib/spack/spack/schema/__init__.py         |  22 ++++
 lib/spack/spack/schema/definitions.py      |  34 +++++
 lib/spack/spack/schema/env.py              |  34 +----
 lib/spack/spack/schema/merged.py           |   2 +
 lib/spack/spack/spec_list.py               |   9 +-
 lib/spack/spack/test/cmd/env.py            | 199 ++++++++++++++++-------------
 lib/spack/spack/test/env.py                |  62 +++++++++
 lib/spack/spack/test/schema.py             |  12 +-
 share/spack/spack-completion.fish          |   6 +-
 11 files changed, 299 insertions(+), 152 deletions(-)
 create mode 100644 lib/spack/spack/schema/definitions.py

diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index 86e8981a18..cd1be71c9d 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -69,6 +69,7 @@ from spack.util.cpus import cpus_available
 SECTION_SCHEMAS = {
     "compilers": spack.schema.compilers.schema,
     "concretizer": spack.schema.concretizer.schema,
+    "definitions": spack.schema.definitions.schema,
     "mirrors": spack.schema.mirrors.schema,
     "repos": spack.schema.repos.schema,
     "packages": spack.schema.packages.schema,
@@ -994,6 +995,7 @@ def read_config_file(filename, schema=None):
                 key = next(iter(data))
                 schema = _ALL_SCHEMAS[key]
             validate(data, schema)
+
         return data
 
     except StopIteration:
diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py
index 85c10e366b..ab6fef6fc0 100644
--- a/lib/spack/spack/environment/environment.py
+++ b/lib/spack/spack/environment/environment.py
@@ -781,10 +781,18 @@ class Environment:
         """Reinitialize the environment object."""
         self.clear(re_read=True)
         self.manifest = EnvironmentManifestFile(self.path)
-        self._read()
+        self._read(re_read=True)
 
-    def _read(self):
-        self._construct_state_from_manifest()
+    def _read(self, re_read=False):
+        # If the manifest has included files, then some of the information
+        # (e.g., definitions) MAY be in those files. So we need to ensure
+        # the config is populated with any associated spec lists in order
+        # to fully construct the manifest state.
+        includes = self.manifest[TOP_LEVEL_KEY].get("include", [])
+        if includes and not re_read:
+            prepare_config_scope(self)
+
+        self._construct_state_from_manifest(re_read)
 
         if os.path.exists(self.lock_path):
             with open(self.lock_path) as f:
@@ -798,21 +806,30 @@ class Environment:
         """Get a write lock context manager for use in a `with` block."""
         return lk.WriteTransaction(self.txlock, acquire=self._re_read)
 
-    def _construct_state_from_manifest(self):
+    def _process_definition(self, item):
+        """Process a single spec definition item."""
+        entry = copy.deepcopy(item)
+        when = _eval_conditional(entry.pop("when", "True"))
+        assert len(entry) == 1
+        if when:
+            name, spec_list = next(iter(entry.items()))
+            user_specs = SpecList(name, spec_list, self.spec_lists.copy())
+            if name in self.spec_lists:
+                self.spec_lists[name].extend(user_specs)
+            else:
+                self.spec_lists[name] = user_specs
+
+    def _construct_state_from_manifest(self, re_read=False):
         """Read manifest file and set up user specs."""
         self.spec_lists = collections.OrderedDict()
+
+        if not re_read:
+            for item in spack.config.get("definitions", []):
+                self._process_definition(item)
+
         env_configuration = self.manifest[TOP_LEVEL_KEY]
         for item in env_configuration.get("definitions", []):
-            entry = copy.deepcopy(item)
-            when = _eval_conditional(entry.pop("when", "True"))
-            assert len(entry) == 1
-            if when:
-                name, spec_list = next(iter(entry.items()))
-                user_specs = SpecList(name, spec_list, self.spec_lists.copy())
-                if name in self.spec_lists:
-                    self.spec_lists[name].extend(user_specs)
-                else:
-                    self.spec_lists[name] = user_specs
+            self._process_definition(item)
 
         spec_list = env_configuration.get(user_speclist_name, [])
         user_specs = SpecList(
@@ -857,7 +874,9 @@ class Environment:
                 yaml, and need to be maintained when re-reading an existing
                 environment.
         """
-        self.spec_lists = {user_speclist_name: SpecList()}  # specs from yaml
+        self.spec_lists = collections.OrderedDict()
+        self.spec_lists[user_speclist_name] = SpecList()
+
         self.dev_specs = {}  # dev-build specs from yaml
         self.concretized_user_specs = []  # user specs from last concretize
         self.concretized_order = []  # roots of last concretize, in order
@@ -1006,7 +1025,8 @@ class Environment:
 
             elif include_url.scheme:
                 raise ValueError(
-                    "Unsupported URL scheme for environment include: {}".format(config_path)
+                    f"Unsupported URL scheme ({include_url.scheme}) for "
+                    f"environment include: {config_path}"
                 )
 
             # treat relative paths as relative to the environment
@@ -1068,8 +1088,10 @@ class Environment:
             from_list = next(iter(self.spec_lists.keys()))
         index = list(self.spec_lists.keys()).index(from_list)
 
-        # spec_lists is an OrderedDict, all list entries after the modified
-        # list may refer to the modified list. Update stale references
+        # spec_lists is an OrderedDict to ensure lists read from the manifest
+        # are maintainted in order, hence, all list entries after the modified
+        # list may refer to the modified list requiring stale references to be
+        # updated.
         for i, (name, speclist) in enumerate(
             list(self.spec_lists.items())[index + 1 :], index + 1
         ):
@@ -1167,7 +1189,7 @@ class Environment:
     def remove(self, query_spec, list_name=user_speclist_name, force=False):
         """Remove specs from an environment that match a query_spec"""
         err_msg_header = (
-            f"cannot remove {query_spec} from '{list_name}' definition "
+            f"Cannot remove '{query_spec}' from '{list_name}' definition "
             f"in {self.manifest.manifest_file}"
         )
         query_spec = Spec(query_spec)
@@ -1198,11 +1220,10 @@ class Environment:
                 list_to_change.remove(spec)
                 self.update_stale_references(list_name)
                 new_specs = set(self.user_specs)
-            except spack.spec_list.SpecListError:
+            except spack.spec_list.SpecListError as e:
                 # define new specs list
                 new_specs = set(self.user_specs)
-                msg = f"Spec '{spec}' is part of a spec matrix and "
-                msg += f"cannot be removed from list '{list_to_change}'."
+                msg = str(e)
                 if force:
                     msg += " It will be removed from the concrete specs."
                     # Mock new specs, so we can remove this spec from concrete spec lists
@@ -2067,7 +2088,7 @@ class Environment:
 
     def removed_specs(self):
         """Tuples of (user spec, concrete spec) for all specs that will be
-        removed on nexg concretize."""
+        removed on next concretize."""
         needed = set()
         for s, c in self.concretized_specs():
             if s in self.user_specs:
@@ -2726,7 +2747,7 @@ class EnvironmentManifestFile(collections.abc.Mapping):
         self.changed = True
 
     def add_definition(self, user_spec: str, list_name: str) -> None:
-        """Appends a user spec to the first active definition mathing the name passed as argument.
+        """Appends a user spec to the first active definition matching the name passed as argument.
 
         Args:
             user_spec: user spec to be appended
diff --git a/lib/spack/spack/schema/__init__.py b/lib/spack/spack/schema/__init__.py
index f99f47a455..bdb1a272d0 100644
--- a/lib/spack/spack/schema/__init__.py
+++ b/lib/spack/spack/schema/__init__.py
@@ -62,3 +62,25 @@ def _make_validator():
 
 
 Validator = llnl.util.lang.Singleton(_make_validator)
+
+spec_list_schema = {
+    "type": "array",
+    "default": [],
+    "items": {
+        "anyOf": [
+            {
+                "type": "object",
+                "additionalProperties": False,
+                "properties": {
+                    "matrix": {
+                        "type": "array",
+                        "items": {"type": "array", "items": {"type": "string"}},
+                    },
+                    "exclude": {"type": "array", "items": {"type": "string"}},
+                },
+            },
+            {"type": "string"},
+            {"type": "null"},
+        ]
+    },
+}
diff --git a/lib/spack/spack/schema/definitions.py b/lib/spack/spack/schema/definitions.py
new file mode 100644
index 0000000000..470eb7e898
--- /dev/null
+++ b/lib/spack/spack/schema/definitions.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)
+
+"""Schema for definitions
+
+.. literalinclude:: _spack_root/lib/spack/spack/schema/definitions.py
+   :lines: 13-
+"""
+
+import spack.schema
+
+#: Properties for inclusion in other schemas
+properties = {
+    "definitions": {
+        "type": "array",
+        "default": [],
+        "items": {
+            "type": "object",
+            "properties": {"when": {"type": "string"}},
+            "patternProperties": {r"^(?!when$)\w*": spack.schema.spec_list_schema},
+        },
+    }
+}
+
+#: Full schema with metadata
+schema = {
+    "$schema": "http://json-schema.org/draft-07/schema#",
+    "title": "Spack definitions 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 6548ca4b2b..463c6680f0 100644
--- a/lib/spack/spack/schema/env.py
+++ b/lib/spack/spack/schema/env.py
@@ -12,34 +12,11 @@ from llnl.util.lang import union_dicts
 
 import spack.schema.gitlab_ci  # DEPRECATED
 import spack.schema.merged
-import spack.schema.packages
 import spack.schema.projections
 
 #: Top level key in a manifest file
 TOP_LEVEL_KEY = "spack"
 
-spec_list_schema = {
-    "type": "array",
-    "default": [],
-    "items": {
-        "anyOf": [
-            {
-                "type": "object",
-                "additionalProperties": False,
-                "properties": {
-                    "matrix": {
-                        "type": "array",
-                        "items": {"type": "array", "items": {"type": "string"}},
-                    },
-                    "exclude": {"type": "array", "items": {"type": "string"}},
-                },
-            },
-            {"type": "string"},
-            {"type": "null"},
-        ]
-    },
-}
-
 projections_scheme = spack.schema.projections.properties["projections"]
 
 schema = {
@@ -75,16 +52,7 @@ schema = {
                             }
                         },
                     },
-                    "definitions": {
-                        "type": "array",
-                        "default": [],
-                        "items": {
-                            "type": "object",
-                            "properties": {"when": {"type": "string"}},
-                            "patternProperties": {r"^(?!when$)\w*": spec_list_schema},
-                        },
-                    },
-                    "specs": spec_list_schema,
+                    "specs": spack.schema.spec_list_schema,
                     "view": {
                         "anyOf": [
                             {"type": "boolean"},
diff --git a/lib/spack/spack/schema/merged.py b/lib/spack/spack/schema/merged.py
index b20700a03c..7ceb649410 100644
--- a/lib/spack/spack/schema/merged.py
+++ b/lib/spack/spack/schema/merged.py
@@ -17,6 +17,7 @@ import spack.schema.compilers
 import spack.schema.concretizer
 import spack.schema.config
 import spack.schema.container
+import spack.schema.definitions
 import spack.schema.mirrors
 import spack.schema.modules
 import spack.schema.packages
@@ -32,6 +33,7 @@ properties = union_dicts(
     spack.schema.config.properties,
     spack.schema.container.properties,
     spack.schema.ci.properties,
+    spack.schema.definitions.properties,
     spack.schema.mirrors.properties,
     spack.schema.modules.properties,
     spack.schema.packages.properties,
diff --git a/lib/spack/spack/spec_list.py b/lib/spack/spack/spec_list.py
index 3f60d57249..6bb1ba8d04 100644
--- a/lib/spack/spack/spec_list.py
+++ b/lib/spack/spack/spec_list.py
@@ -93,8 +93,8 @@ class SpecList:
             if (isinstance(s, str) and not s.startswith("$")) and Spec(s) == Spec(spec)
         ]
         if not remove:
-            msg = "Cannot remove %s from SpecList %s\n" % (spec, self.name)
-            msg += "Either %s is not in %s or %s is " % (spec, self.name, spec)
+            msg = f"Cannot remove {spec} from SpecList {self.name}.\n"
+            msg += f"Either {spec} is not in {self.name} or {spec} is "
             msg += "expanded from a matrix and cannot be removed directly."
             raise SpecListError(msg)
 
@@ -133,9 +133,8 @@ class SpecList:
 
         # Make sure the reference is valid
         if name not in self._reference:
-            msg = "SpecList %s refers to " % self.name
-            msg += "named list %s " % name
-            msg += "which does not appear in its reference dict"
+            msg = f"SpecList '{self.name}' refers to named list '{name}'"
+            msg += " which does not appear in its reference dict."
             raise UndefinedReferenceError(msg)
 
         return (name, sigil)
diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py
index 3b843be72a..308e0b0e90 100644
--- a/lib/spack/spack/test/cmd/env.py
+++ b/lib/spack/spack/test/cmd/env.py
@@ -632,7 +632,7 @@ def test_env_view_external_prefix(tmp_path, mutable_database, mock_packages):
     manifest_dir.mkdir(parents=True, exist_ok=False)
     manifest_file = manifest_dir / ev.manifest_name
     manifest_file.write_text(
-        """
+        """\
 spack:
   specs:
   - a
@@ -720,38 +720,25 @@ spack:
 
 def test_with_config_bad_include(environment_from_manifest):
     """Confirm missing include paths raise expected exception and error."""
-    e = environment_from_manifest(
-        """
+    with pytest.raises(spack.config.ConfigFileError, match="2 missing include path"):
+        e = environment_from_manifest(
+            """
 spack:
   include:
   - /no/such/directory
   - no/such/file.yaml
 """
-    )
-    with pytest.raises(spack.config.ConfigFileError, match="2 missing include path"):
+        )
         with e:
             e.concretize()
 
     assert ev.active_environment() is None
 
 
-def test_env_with_include_config_files_same_basename(environment_from_manifest):
-    e = environment_from_manifest(
-        """
-spack:
-  include:
-  - ./path/to/included-config.yaml
-  - ./second/path/to/include-config.yaml
-  specs:
-  - libelf
-  - mpileaks
-"""
-    )
-
-    e = ev.read("test")
-
-    fs.mkdirp(os.path.join(e.path, "path", "to"))
-    with open(os.path.join(e.path, "./path/to/included-config.yaml"), "w") as f:
+def test_env_with_include_config_files_same_basename(tmp_path, environment_from_manifest):
+    file1 = fs.join_path(tmp_path, "path", "to", "included-config.yaml")
+    fs.mkdirp(os.path.dirname(file1))
+    with open(file1, "w") as f:
         f.write(
             """\
         packages:
@@ -760,8 +747,9 @@ spack:
         """
         )
 
-    fs.mkdirp(os.path.join(e.path, "second", "path", "to"))
-    with open(os.path.join(e.path, "./second/path/to/include-config.yaml"), "w") as f:
+    file2 = fs.join_path(tmp_path, "second", "path", "included-config.yaml")
+    fs.mkdirp(os.path.dirname(file2))
+    with open(file2, "w") as f:
         f.write(
             """\
         packages:
@@ -770,6 +758,18 @@ spack:
         """
         )
 
+    e = environment_from_manifest(
+        f"""
+spack:
+  include:
+  - {file1}
+  - {file2}
+  specs:
+  - libelf
+  - mpileaks
+"""
+    )
+
     with e:
         e.concretize()
 
@@ -806,12 +806,18 @@ spack:
     )
 
 
-def test_env_with_included_config_file(environment_from_manifest, packages_file):
+def test_env_with_included_config_file(mutable_mock_env_path, packages_file):
     """Test inclusion of a relative packages configuration file added to an
     existing environment.
     """
+    env_root = mutable_mock_env_path
+    fs.mkdirp(env_root)
     include_filename = "included-config.yaml"
-    e = environment_from_manifest(
+    included_path = env_root / include_filename
+    shutil.move(packages_file.strpath, included_path)
+
+    spack_yaml = env_root / ev.manifest_name
+    spack_yaml.write_text(
         f"""\
 spack:
   include:
@@ -821,9 +827,7 @@ spack:
 """
     )
 
-    included_path = os.path.join(e.path, include_filename)
-    shutil.move(packages_file.strpath, included_path)
-
+    e = ev.Environment(env_root)
     with e:
         e.concretize()
 
@@ -856,68 +860,67 @@ def test_env_with_included_config_missing_file(tmpdir, mutable_empty_config):
     with spack_yaml.open("w") as f:
         f.write("spack:\n  include:\n    - {0}\n".format(missing_file.strpath))
 
-    env = ev.Environment(tmpdir.strpath)
     with pytest.raises(spack.config.ConfigError, match="missing include path"):
-        ev.activate(env)
+        ev.Environment(tmpdir.strpath)
 
 
-def test_env_with_included_config_scope(environment_from_manifest, packages_file):
+def test_env_with_included_config_scope(mutable_mock_env_path, packages_file):
     """Test inclusion of a package file from the environment's configuration
     stage directory. This test is intended to represent a case where a remote
     file has already been staged."""
-    config_scope_path = os.path.join(ev.root("test"), "config")
-
-    # Configure the environment to include file(s) from the environment's
-    # remote configuration stage directory.
-    e = environment_from_manifest(mpileaks_env_config(config_scope_path))
+    env_root = mutable_mock_env_path
+    config_scope_path = env_root / "config"
 
     # Copy the packages.yaml file to the environment configuration
     # directory, so it is picked up during concretization. (Using
     # copy instead of rename in case the fixture scope changes.)
     fs.mkdirp(config_scope_path)
     include_filename = os.path.basename(packages_file.strpath)
-    included_path = os.path.join(config_scope_path, include_filename)
+    included_path = config_scope_path / include_filename
     fs.copy(packages_file.strpath, included_path)
 
+    # Configure the environment to include file(s) from the environment's
+    # remote configuration stage directory.
+    spack_yaml = env_root / ev.manifest_name
+    spack_yaml.write_text(mpileaks_env_config(config_scope_path))
+
     # Ensure the concretized environment reflects contents of the
     # packages.yaml file.
+    e = ev.Environment(env_root)
     with e:
         e.concretize()
 
     assert any(x.satisfies("mpileaks@2.2") for x in e._get_environment_specs())
 
 
-def test_env_with_included_config_var_path(environment_from_manifest, packages_file):
+def test_env_with_included_config_var_path(tmpdir, packages_file):
     """Test inclusion of a package configuration file with path variables
     "staged" in the environment's configuration stage directory."""
-    config_var_path = os.path.join("$tempdir", "included-config.yaml")
-    e = environment_from_manifest(mpileaks_env_config(config_var_path))
+    included_file = packages_file.strpath
+    env_path = pathlib.PosixPath(tmpdir)
+    config_var_path = os.path.join("$tempdir", "included-packages.yaml")
+
+    spack_yaml = env_path / ev.manifest_name
+    spack_yaml.write_text(mpileaks_env_config(config_var_path))
 
     config_real_path = substitute_path_variables(config_var_path)
-    fs.mkdirp(os.path.dirname(config_real_path))
-    shutil.move(packages_file.strpath, config_real_path)
+    shutil.move(included_file, config_real_path)
     assert os.path.exists(config_real_path)
 
+    e = ev.Environment(env_path)
     with e:
         e.concretize()
 
     assert any(x.satisfies("mpileaks@2.2") for x in e._get_environment_specs())
 
 
-def test_env_config_precedence(environment_from_manifest):
-    e = environment_from_manifest(
-        """
-spack:
-  packages:
-    libelf:
-      version: ["0.8.12"]
-  include:
-  - ./included-config.yaml
-  specs:
-  - mpileaks
-"""
-    )
-    with open(os.path.join(e.path, "included-config.yaml"), "w") as f:
+def test_env_with_included_config_precedence(tmp_path):
+    """Test included scope and manifest precedence when including a package
+    configuration file."""
+
+    included_file = "included-packages.yaml"
+    included_path = tmp_path / included_file
+    with open(included_path, "w") as f:
         f.write(
             """\
 packages:
@@ -928,29 +931,50 @@ packages:
 """
         )
 
+    spack_yaml = tmp_path / ev.manifest_name
+    spack_yaml.write_text(
+        f"""\
+spack:
+  packages:
+    libelf:
+      version: ["0.8.12"]
+  include:
+  - {os.path.join(".", included_file)}
+  specs:
+  - mpileaks
+"""
+    )
+
+    e = ev.Environment(tmp_path)
     with e:
         e.concretize()
+    specs = e._get_environment_specs()
 
     # ensure included scope took effect
-    assert any(x.satisfies("mpileaks@2.2") for x in e._get_environment_specs())
+    assert any(x.satisfies("mpileaks@2.2") for x in specs)
 
     # ensure env file takes precedence
-    assert any(x.satisfies("libelf@0.8.12") for x in e._get_environment_specs())
+    assert any(x.satisfies("libelf@0.8.12") for x in specs)
 
 
-def test_included_config_precedence(environment_from_manifest):
-    e = environment_from_manifest(
-        """
+def test_env_with_included_configs_precedence(tmp_path):
+    """Test precendence of multiple included configuration files."""
+    file1 = "high-config.yaml"
+    file2 = "low-config.yaml"
+
+    spack_yaml = tmp_path / ev.manifest_name
+    spack_yaml.write_text(
+        f"""\
 spack:
   include:
-  - ./high-config.yaml  # this one should take precedence
-  - ./low-config.yaml
+  - {os.path.join(".", file1)} # this one should take precedence
+  - {os.path.join(".", file2)}
   specs:
   - mpileaks
 """
     )
 
-    with open(os.path.join(e.path, "high-config.yaml"), "w") as f:
+    with open(tmp_path / file1, "w") as f:
         f.write(
             """\
 packages:
@@ -959,7 +983,7 @@ packages:
 """
         )
 
-    with open(os.path.join(e.path, "low-config.yaml"), "w") as f:
+    with open(tmp_path / file2, "w") as f:
         f.write(
             """\
 packages:
@@ -970,12 +994,16 @@ packages:
 """
         )
 
+    e = ev.Environment(tmp_path)
     with e:
         e.concretize()
+    specs = e._get_environment_specs()
 
-    assert any(x.satisfies("mpileaks@2.2") for x in e._get_environment_specs())
+    # ensure included package spec took precedence over manifest spec
+    assert any(x.satisfies("mpileaks@2.2") for x in specs)
 
-    assert any([x.satisfies("libelf@0.8.10") for x in e._get_environment_specs()])
+    # ensure first included package spec took precedence over one from second
+    assert any(x.satisfies("libelf@0.8.10") for x in specs)
 
 
 def test_bad_env_yaml_format(environment_from_manifest):
@@ -1578,11 +1606,10 @@ spack:
         assert Spec("callpath") in test.user_specs
 
 
-def test_stack_yaml_remove_from_list_force(tmpdir):
-    filename = str(tmpdir.join("spack.yaml"))
-    with open(filename, "w") as f:
-        f.write(
-            """\
+def test_stack_yaml_remove_from_list_force(tmp_path):
+    spack_yaml = tmp_path / ev.manifest_name
+    spack_yaml.write_text(
+        """\
 spack:
   definitions:
     - packages: [mpileaks, callpath]
@@ -1591,20 +1618,20 @@ spack:
         - [$packages]
         - [^mpich, ^zmpi]
 """
-        )
-    with tmpdir.as_cwd():
-        env("create", "test", "./spack.yaml")
-        with ev.read("test"):
-            concretize()
-            remove("-f", "-l", "packages", "mpileaks")
-            find_output = find("-c")
+    )
 
-        assert "mpileaks" not in find_output
+    env("create", "test", str(spack_yaml))
+    with ev.read("test"):
+        concretize()
+        remove("-f", "-l", "packages", "mpileaks")
+        find_output = find("-c")
 
-        test = ev.read("test")
-        assert len(test.user_specs) == 2
-        assert Spec("callpath ^zmpi") in test.user_specs
-        assert Spec("callpath ^mpich") in test.user_specs
+    assert "mpileaks" not in find_output
+
+    test = ev.read("test")
+    assert len(test.user_specs) == 2
+    assert Spec("callpath ^zmpi") in test.user_specs
+    assert Spec("callpath ^mpich") in test.user_specs
 
 
 def test_stack_yaml_remove_from_matrix_no_effect(tmpdir):
@@ -1650,7 +1677,7 @@ spack:
     with tmpdir.as_cwd():
         env("create", "test", "./spack.yaml")
         with ev.read("test") as e:
-            concretize()
+            e.concretize()
 
             before_user = e.user_specs.specs
             before_conc = e.concretized_user_specs
diff --git a/lib/spack/spack/test/env.py b/lib/spack/spack/test/env.py
index f6b89e2108..7490a6e0b2 100644
--- a/lib/spack/spack/test/env.py
+++ b/lib/spack/spack/test/env.py
@@ -18,6 +18,7 @@ from spack.environment.environment import (
     SpackEnvironmentViewError,
     _error_on_nonempty_view_dir,
 )
+from spack.spec_list import UndefinedReferenceError
 
 pytestmark = pytest.mark.not_on_windows("Envs are not supported on windows")
 
@@ -716,3 +717,64 @@ def test_variant_propagation_with_unify_false(tmp_path, mock_packages):
     root = env.matching_spec("parent-foo")
     for node in root.traverse():
         assert node.satisfies("+foo")
+
+
+def test_env_with_include_defs(mutable_mock_env_path, mock_packages):
+    """Test environment with included definitions file."""
+    env_path = mutable_mock_env_path
+    env_path.mkdir()
+    defs_file = env_path / "definitions.yaml"
+    defs_file.write_text(
+        """definitions:
+- core_specs: [libdwarf, libelf]
+- compilers: ['%gcc']
+"""
+    )
+
+    spack_yaml = env_path / ev.manifest_name
+    spack_yaml.write_text(
+        f"""spack:
+  include:
+  - file://{defs_file}
+
+  definitions:
+  - my_packages: [zlib]
+
+  specs:
+  - matrix:
+    - [$core_specs]
+    - [$compilers]
+  - $my_packages
+"""
+    )
+
+    e = ev.Environment(env_path)
+    with e:
+        e.concretize()
+
+
+def test_env_with_include_def_missing(mutable_mock_env_path, mock_packages):
+    """Test environment with included definitions file that is missing a definition."""
+    env_path = mutable_mock_env_path
+    env_path.mkdir()
+    filename = "missing-def.yaml"
+    defs_file = env_path / filename
+    defs_file.write_text("definitions:\n- my_compilers: ['%gcc']\n")
+
+    spack_yaml = env_path / ev.manifest_name
+    spack_yaml.write_text(
+        f"""spack:
+  include:
+  - file://{defs_file}
+
+  specs:
+  - matrix:
+    - [$core_specs]
+    - [$my_compilers]
+"""
+    )
+
+    e = ev.Environment(env_path)
+    with e:
+        with pytest.raises(UndefinedReferenceError, match=r"which does not appear"):
+            e.concretize()
diff --git a/lib/spack/spack/test/schema.py b/lib/spack/spack/test/schema.py
index d7f4e524ff..916e61cf26 100644
--- a/lib/spack/spack/test/schema.py
+++ b/lib/spack/spack/test/schema.py
@@ -80,7 +80,17 @@ def test_module_suffixes(module_suffixes_schema):
 @pytest.mark.regression("10246")
 @pytest.mark.parametrize(
     "config_name",
-    ["compilers", "config", "env", "merged", "mirrors", "modules", "packages", "repos"],
+    [
+        "compilers",
+        "config",
+        "definitions",
+        "env",
+        "merged",
+        "mirrors",
+        "modules",
+        "packages",
+        "repos",
+    ],
 )
 def test_schema_validation(meta_schema, config_name):
     import importlib
diff --git a/share/spack/spack-completion.fish b/share/spack/spack-completion.fish
index ee9011e11c..7ea1d18484 100755
--- a/share/spack/spack-completion.fish
+++ b/share/spack/spack-completion.fish
@@ -1159,19 +1159,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 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 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 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 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 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 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
-- 
cgit v1.2.3-70-g09d2