From 6da3023a9448072fdacf88b218ac396783db9d61 Mon Sep 17 00:00:00 2001 From: Tamara Dahlgren <35777542+tldahlgren@users.noreply.github.com> Date: Thu, 24 Apr 2025 17:10:35 -0700 Subject: [PATCH 01/33] Environments: include other environments * config.py: use the proper schema for an included environment file * Fix order types * Add nested environments schema * remove types from _env_create arguments docstrings * Tighten include-concrete processing * WIP: test_env_include_env tweaks * Snapshot: include_concrete->include * Use the include concrete variable for the key * Remove preliminary handling of include concretes as part of configuration * Remove IncludePath's is_config * Snapshot: mainly distinguish include and include-concrete (lockfile) keys * Snapshot: cleanup and leverage CONFIG (but broken) * Cleaned up the include-concrete handling and leveraged spack.yaml merged in CONFIG. Unfortunately, fixing the simplified new include spack.yaml test ended up breaking 28 other test/cmd/env.py tests that were working before the last iteration. * Comment out environment definitions change that broke 28 test/cmd/env.py tests * Fix style issues * Update fish command completion * Include environment definitions in config; add test_env_include_env_pkgs_def * Drop unnecessary check * Restore check to get_env_section * Snapshot of include-concrete-related docs changes * included_concrete_envs->included_concrete_env_roots; another pass over user specs * Remove config:_ALL_SCHEMAS since SECTION_SCHEMAS now contains environment * test/config.py: add test_config_scope_empty_write * Snapshot test/cmd/env.py: added docstrings to several new tests; drafted test_env_include_env_includes * config support for spack:include; use hash of included paths in environment names * Remove unnecessary fixtures from test_env_include_env* tests * Docs: refined and expanded environment file inclusion * Comment: added reminder included file copies offline feedback * Add test_env_include_env_highest_config and switch to BFS nested include processing * Ugh! test_env_include_env_includes SHOULD use mutable_config * test_include_recurse_diamond: Fix to use the required section name for top-most include * Add missing ref link; fix style * Restore DFS nested include handling Also restore the docs warning and change new BFS unit test to XFAIL. * support include path rewrites * Remove commented out copy operations * test_gitlab_config_scopes: recognize includes of new environment are absolute * test_gitlab_config_scopes: confirm ignoring duplicate included scopes output * test_env_include_envs: switch to conditional path syntax to confirm path substitution retains information * Restore use of string variable for unit tests Appears some pythons handle embedding a join but others don't. * test_env_include_envs: use python@:3.8 removeprefix solution * Make sure cached included concrete path is canonicalized * Post-rebase cleanup * config.py: Fix docstring typos * environment.py: add debug for adding lockfile to concrete roots * test/config.py: add test_config_optional_include_failures * test_config_optional_include_failures: fix style * Filter out lockfiles from include scopes * OptionalIncludes: Leverage simplified property checks * config.push_scope: restore simplified included scope processing * test/cmd/env.py: add test_env_include_concrete_only * Support default paths with git includes (per offline feedback) * Initial rebase fix * Fish completion update: adds v/verbose * config.get_config: added debug messages to indicate source * Bugfix: don't try to update environment data scope; more debug statements * git includes: clone environments under the environment hidden directory * Add missing space to debug message * Initial docs updates from PR feedback * Cache env path_sha256 includes under the environment. * config.py: revise the manifest precedence comment * Rebase cleanup (e.g.,remove unused _ALL_SCHEMAS) * Address remaining documentation feedback * Support remote include destination: * Optional remote include path destination * Fix a couple of remote include unit tests' sha256s * test_included_path_git_default_paths: Add optional explicit remote include destination * Ensure variable substitution remote include destination * Docs: update include.yaml with 'destination' option * 'spack env update' convert include_concrete to include * Docs: Add section on 'spack env update' * Remove schema/env.py -> spack.environment circular import * Added test_include_concrete_deprecation_warning and test_env_schema_update_wrong_type * config: try canonicalize through rfc_util * Check if include destination is writable; add unit tests * Disable unwritable directories for windows * Flag new 'destination:' option as optional * test/cmd/emv/py: Rebase imports cleanup * (WIP) _sync_speclists: comment out the rest of the inclusion * Post-rebase style fixes * Post-rebase schema fix * test/cmd/env.py: e.all_concretized_user_specs -> e.user_specs * Post-rebase cleanup * Add included toolchain (needs tests) Signed-off-by: tldahlgren Signed-off-by: Gregory Becker --- lib/spack/docs/environments.rst | 104 ++++++- lib/spack/docs/include_yaml.rst | 61 ++-- lib/spack/spack/config.py | 315 ++++++++++++++------- lib/spack/spack/environment/__init__.py | 2 + lib/spack/spack/environment/environment.py | 211 +++++++------- lib/spack/spack/schema/env.py | 2 + lib/spack/spack/schema/include.py | 12 +- lib/spack/spack/test/cmd/ci.py | 26 +- lib/spack/spack/test/cmd/env.py | 188 ++++++++++-- lib/spack/spack/test/config.py | 189 +++++++++++-- lib/spack/spack/test/env.py | 126 +++++++++ lib/spack/spack/test/main.py | 9 +- share/spack/spack-completion.fish | 8 +- 13 files changed, 960 insertions(+), 293 deletions(-) diff --git a/lib/spack/docs/environments.rst b/lib/spack/docs/environments.rst index aaf3e0c69cce81..9d4e3947aa1614 100644 --- a/lib/spack/docs/environments.rst +++ b/lib/spack/docs/environments.rst @@ -263,7 +263,6 @@ Note that when we installed the abstract spec ``zlib@1.2.8``, it was presented a All explicitly installed packages will be listed as roots of the environment. All of the Spack commands that act on the list of installed specs are environment-aware in this way, including ``install``, ``uninstall``, ``find``, ``extensions``, etc. -In the :ref:`environment-configuration` section we will discuss environment-aware commands further. .. _cmd-spack-add: @@ -282,6 +281,7 @@ To update the lockfile, the environment must be :ref:`re-concretized In environment myenv + ==> Root specs + python + + ==> 0 installed packages + + $ spack -e included_env find + ==> In environment included_env + ==> No root specs + ==> Included specs + python + + ==> 0 installed packages + +Here we see that ``included_env`` has access to the python package through the ``myenv`` environment. +But if we were to add another spec to ``myenv``, ``included_env`` will not be able to access the new information. + +.. code-block:: spec + + $ spack -e myenv add perl + $ spack -e myenv concretize + $ spack -e myenv find + ==> In environment myenv + ==> Root specs + perl python + + ==> 0 installed packages + $ spack -e included_env find + ==> In environment included_env + ==> No root specs + ==> Included specs + python + + ==> 0 installed packages + +It isn't until you run the ``spack concretize`` command that the combined environment will get the updated information from the re-concretized base environment. + +.. code-block:: console + + $ spack -e included_env concretize + $ spack -e included_env find + ==> In environment included_env + ==> No root specs + ==> Included specs + perl python + + ==> 0 installed packages + +.. _environment-configuration: + Configuring Environments ------------------------ diff --git a/lib/spack/docs/include_yaml.rst b/lib/spack/docs/include_yaml.rst index 65b8078c68a381..62d2a510e29081 100644 --- a/lib/spack/docs/include_yaml.rst +++ b/lib/spack/docs/include_yaml.rst @@ -62,6 +62,20 @@ The contents of the downloaded file are read and included in Spack's configurati You can check the destination of the downloaded file by running: ``spack config scopes -p``. +You can override the default download directory by including a ``destination``. +For example:: + + include: + - path: https://github.com/path/to/raw/config/config.yaml + sha256: 26e871804a92cd07bb3d611b31b4156ae93d35b6a6d6e0ef3a67871fcb1d258b + destination: $HOME/my/custom/remote/cache/directory # optional + +will download the file under ``$HOME/my/custom/remote/cache/directory``. + +.. versionadded:: 1.2 + optional ``destination:`` attribute. + + .. warning:: Remote file URLs must link to the **raw** form of the file's contents (e.g., `GitHub `_ or `GitLab `_). @@ -75,52 +89,63 @@ The contents of the downloaded file are read and included in Spack's configurati You can also include configuration files from a ``git`` repository. The ``branch``, ``commit``, or ``tag`` to be checked out is required. A list of relative paths in which to find the configuration files is also required. + +We recommend a ``commit`` be specified when using a ``tag``. Inclusion of the repository (and its paths) can be optional or conditional. +You can also add a ``destination`` to specify the local root directory. If you want to control the :ref:`name of the configuration scope `, you can provide a ``name``. For example, suppose we only want to include the ``config.yaml`` and ``packages.yaml`` files from the `spack/spack-configs `_ repository's ``USC/config`` directory when using the ``centos7`` operating system. -And we want the configuration scope name to start ``common``. -We could then configure the include in, for example, the user scope include file (i.e., ``$HOME/.spack/include.yaml`` by default), as follows:: +And we want the configuration scope name to start ``common`` and the files to be cached under ``$HOME/my_usc_includes``. +We would then configure the ``include.yaml`` file as follows:: include: - - name: common + - name: USC # optional git: https://github.com/spack/spack-configs.git branch: main when: os == "centos7" paths: - USC/config/config.yaml - USC/config/packages.yaml + destination: $HOME/my_usc_includes # optional .. note:: - The git URL could be specified through an environment variable (e.g., ``$MY_USC_CONFIG_URL``). + The git URL could have been specified through an environment variable (e.g., ``$MY_USC_CONFIG_URL``). -If the condition is satisfied, then the ``main`` branch of the repository will be cloned -- under ``$HOME/.spack/includes`` -- when configuration scopes are initially created. -Once cloned, the settings for the two files under the ``USC/config`` directory will be integrated into Spack's configuration. -In this example, the new scopes and their paths can be seen by running:: +If the condition is satisfied, then the ``main`` branch of the repository will be cloned when the configuration scopes are initially created. +The resulting repository can be found under ``$HOME/my_usc_includes``. + +Once cloned, the settings for the two files under the ``$HOME/my_usc_includes/USC/config`` subdirectory will be integrated into Spack's configuration. + +In this example, the new scopes can be seen by running:: $ spack config scopes -p - Scope Path + Scope Path command_line - spack /Users/username/spack/etc/spack/ - user /Users/username/.spack/ - common:USC/config/config.yaml /Users/username/.spack/includes/common/USC/config/config.yaml - common:USC/config/packages.yaml /Users/username/.spack/includes/common/USC/config/packages.yaml - site /Users/username/spack/etc/spack/site/ - system /etc/spack/ - defaults /Users/username/spack/etc/spack/defaults/ - defaults:darwin /Users/username/spack/etc/spack/defaults/darwin/ - defaults:base /Users/username/spack/etc/spack/defaults/base/ + spack /Users/username/spack/etc/spack/ + user /Users/username/.spack/ + common:USC/config/config.yaml /Users/username/my_usc_includes/USC/config/config.yaml + common:USC/config/packages.yaml /Users/username/my_usc_includes/USC/config/packages.yaml + site /Users/username/spack/etc/spack/site/ + system /etc/spack/ + defaults /Users/username/spack/etc/spack/defaults/ + defaults:darwin /Users/username/spack/etc/spack/defaults/darwin/ + defaults:base /Users/username/spack/etc/spack/defaults/base/ _builtin Since there are two unique paths, each results in a separate configuration scope. If only the ``USC/config`` directory was listed under ``paths``, then there would be only one configuration scope, named ``USC``, and the configuration settings from all of the configuration files within that directory would be integrated. +Had a list of relative paths to the configuration files not been provided, the root directory of the repository is checked for suitable files. +The default behavior is to select the environment manifest (``spack.yaml``) file, if present. +Otherwise, all known ``.yaml`` files (see :ref:`configuration`) at the root directory will be used. + .. versionadded:: 1.1 ``git:``, ``branch:``, ``commit:``, and ``tag:`` attributes. .. versionadded:: 1.2 - ``name:`` attribute and git environment variable support. + ``name:`` and ``destination:`` attributes, git environment variable support, and support for including ``spack.yaml`` files. Precedence ~~~~~~~~~~ diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index 7485d5d0e63fa6..55543c3dbea8b0 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -30,12 +30,14 @@ import contextlib import copy import functools +import glob import os import os.path import pathlib import re import sys import tempfile +import urllib.parse from collections import defaultdict from itertools import chain from typing import Any, Callable, Dict, Generator, List, Optional, Set, Tuple, Union, cast @@ -95,13 +97,7 @@ "ci": spack.schema.ci.schema, "cdash": spack.schema.cdash.schema, "toolchains": spack.schema.toolchains.schema, -} - -# Same as above, but including keys for environments -# this allows us to unify config reading between configs and environments -_ALL_SCHEMAS: Dict[str, Any] = { - **SECTION_SCHEMAS, - spack.schema.env.TOP_LEVEL_KEY: spack.schema.env.schema, + "spack": spack.schema.env.schema, } #: Path to the main configuration scope @@ -152,19 +148,39 @@ def __init__(self, name: str, included: bool = False) -> None: #: included configuration scopes self._included_scopes: Optional[List["ConfigScope"]] = None + def get_includes(self) -> Optional[YamlConfigDict]: + """Retrieve the 'include' section, allowing for the fact that includes + nested in a manifest file fall under spack:include.""" + includes = self.get_section("include") + if includes: + tty.debug(f"Retrieved configuration includes for {self.name}: {includes}", level=3) + return includes + + spack = self.get_section("spack") + if spack: + includes = spack["spack"] + tty.debug(f"Retrieved environment includes for {self.name}: {includes}", level=3) + return includes + @property def included_scopes(self) -> List["ConfigScope"]: """Memoized list of included scopes, in the order they appear in this scope.""" if self._included_scopes is None: self._included_scopes = [] - includes = self.get_section("include") + includes = self.get_includes() if includes: + if "include" not in includes: + return self._included_scopes + include_paths = [included_path(data) for data in includes["include"]] + tty.debug( + f"Processing included paths: {[str(path) for path in include_paths]}", level=3 + ) included_scopes = chain(*[include.scopes(self) for include in include_paths]) - # Do not include duplicate scopes for included_scope in included_scopes: + # Do not include duplicate scopes if any([included_scope.name == scope.name for scope in self._included_scopes]): tty.warn(f"Ignoring duplicate included scope: {included_scope.name}") continue @@ -172,6 +188,11 @@ def included_scopes(self) -> List["ConfigScope"]: if included_scope not in self._included_scopes: self._included_scopes.append(included_scope) + tty.debug( + f"{self.name} has {'' if len(self._included_scopes) else 'no '}" + f"included scopes: {self._included_scopes}", + level=3, + ) return self._included_scopes @property @@ -196,6 +217,7 @@ def transitive_includes(self, _names: Optional[Set[str]] = None) -> Set[str]: _names.add(self.name) for scope in self.included_scopes: _names |= scope.transitive_includes(_names=_names) + tty.debug(f"Names of scope and transitively included scopes: {_names}") return _names def get_section_filename(self, section: str) -> str: @@ -548,7 +570,6 @@ def push_scope_incremental( Args: scope: scope to be added priority: priority of the scope - """ # TODO: As a follow on to #48784, change this to create a graph of the # TODO: includes AND ensure properly sorted such that the order included @@ -849,8 +870,20 @@ def _get_config_memoized( # filter any scopes overridden by `include::` scopes = self._filter_overridden(scopes) + def get_env_section(config_scope, section): + data = config_scope.get_section(spack.schema.env.TOP_LEVEL_KEY) + if data is None: + return None + + # Grab the actual data + if spack.schema.env.TOP_LEVEL_KEY in data: + data = data[spack.schema.env.TOP_LEVEL_KEY] + + return data + merged_section: Dict[str, Any] = syaml.syaml_dict() updated_scopes = [] + # process from lowest to highest precedence since newer take precedence for config_scope in scopes: if section == "include" and config_scope not in self.active_include_section_scopes: continue @@ -868,15 +901,21 @@ def _get_config_memoized( data["include"] = data.pop("include") # strip override # Skip empty configs + env_data = None if not isinstance(data, dict) or section not in data: - continue + data = None + env_data = get_env_section(config_scope, section) + if not isinstance(env_data, dict): + continue + + tty.debug(f"Retrieved {config_scope.name}'s '{section}' data", level=3) # If configuration is in an old format, transform it and keep track of the scope that # may need to be written out to disk. - if _update_in_memory(data, section): + if data and _update_in_memory(data, section): updated_scopes.append(config_scope) - merged_section = spack.schema.merge_yaml(merged_section, data) + merged_section = spack.schema.merge_yaml(merged_section, data or env_data) self.updated_scopes_by_section[section] = updated_scopes @@ -1071,7 +1110,7 @@ def _parent_scope_directory(parent_scope: Optional[ConfigScope]) -> Optional[str def base_directory( self, path_or_url: str, parent_scope: Optional[ConfigScope] = None ) -> Optional[str]: - """Return the local directory to use for this include. + """Return the local directory to use for remote or relative includes. For remote includes this is the cache destination directory. For local relative includes this is the working directory from which to resolve the path. @@ -1080,12 +1119,26 @@ def base_directory( path_or_url: path or URL of the include parent_scope: including scope - Returns: ``None`` for a local include without an enclosing parent scope; - an appropriate subdirectory of the enclosing (parent) scope's writable - directory (when available); otherwise a stable temporary directory. + Returns: ``None`` for an absolute path, the explicit destination (if writable) + or an appropriate subdirectory of the enclosing (parent) scope's writable directory + (when available); otherwise a stable temporary directory. + + Raises: + AssertionError: The explicit destination is not writable """ + # An absolute path does not need a local base directory. + if os.path.isabs(path_or_url): + tty.debug(f"The included path ({self}) is absolute so needs no base directory") + return None + + # Use the explicit destination but require it to be writable. + if self.destination: + assert filesystem.can_write_to_dir(self.destination) + return self.destination + + # Prefer the (writable) parent scope directory for a local relative file. scope_dir = self._parent_scope_directory(parent_scope) - if not self.remote: + if not self.remote and filesystem.can_write_to_dir(scope_dir): return scope_dir def _subdir(): @@ -1129,14 +1182,14 @@ def _subdir(): def _scope( self, path: str, config_path: str, parent_scope: ConfigScope ) -> Optional[ConfigScope]: - """Instantiate a configuration scope for the configuration path. + """Instantiate a configuration scope for a configuration path. Args: path: raw include path config_path: configuration path parent_scope: including scope - Returns: configuration scopes + Returns: configuration scope or ``None`` Raises: ValueError: the required configuration path does not exist @@ -1149,8 +1202,8 @@ def _scope( # processing is handled when the environment is processed. if path and os.path.basename(path) == "spack.lock": tty.debug( - f"Ignoring inclusion of '{path}' since environment lock files " - "are processed elsewhere" + f"Ignoring inclusion of lock file '{path}' since it is processed " + "in the environment." ) return None @@ -1201,12 +1254,12 @@ def _scope( ) elif ext == ".yaml" or ext == ".yml": tty.debug(f"Creating SingleFileScope {config_name} for '{config_path}'") + if os.path.basename(config_path) == "spack.yaml": + schema = spack.schema.env.schema + else: + schema = spack.schema.merged.schema return SingleFileScope( - config_name, - config_path, - spack.schema.merged.schema, - prefer_modify=self.prefer_modify, - included=True, + config_name, config_path, schema, prefer_modify=self.prefer_modify, included=True ) elif exists: raise ValueError( @@ -1229,7 +1282,7 @@ def _validate_parent_scope(self, parent_scope: ConfigScope): assert parent_scope.name.strip(), "Parent scope of an include must have a name" def evaluate_condition(self) -> bool: - """Evaluate the include condition: + """Evaluate the include condition. Returns: ``True`` if the include condition is satisfied; else ``False``. """ @@ -1258,6 +1311,15 @@ def paths(self) -> List[str]: raise NotImplementedError("must be implemented in derived classes") + def to_dict(self) -> Dict[str, Any]: + """Return the minimal configuration information in dictionary form.""" + result = {} + for name in ["when", "optional"]: + value = getattr(self, name) + if value: + result[name] = value + return result + class IncludePath(OptionalInclude): path: str @@ -1277,13 +1339,24 @@ def __init__(self, entry: dict): self.path = spack.util.path.substitute_path_variables(path) self.sha256 = entry.get("sha256", "") - self.remote = "sha256" in entry - self.destination = None + self.destination = entry.get("destination", "") + if self.destination: + # Possible work-around for adding spack.util.path circular import + # since already a circular import for spack.util.remote_file_cache. + self.destination = rfc_util.canonicalize_path(self.destination) + + # Ensure the explicit destination is writable. + assert filesystem.can_write_to_dir(self.destination) + + self.remote = urllib.parse.urlparse(self.path).scheme in ("http", "https", "ftp") + if self.remote and not self.sha256: + raise spack.error.ConfigError(f"Remote include {self.path} requires a 'sha256'") def __repr__(self): return ( - f"IncludePath({self.path}, sha256={self.sha256}, " - f"when='{self.when}', optional={self.optional})" + f"IncludePath('{self.name}', '{self.path}', sha256={self.sha256}, " + f"when='{self.when}', optional={self.optional}, " + f"destination={self.destination})" ) def scopes(self, parent_scope: ConfigScope) -> List[ConfigScope]: @@ -1292,10 +1365,11 @@ def scopes(self, parent_scope: ConfigScope) -> List[ConfigScope]: Args: parent_scope: including scope - Returns: configuration scopes IF the when condition is satisfied; - otherwise, an empty list. + Returns: configuration scopes for configuration files IF the when + condition is satisfied; otherwise, an empty list. Raises: + AssertionError: unable to write to the directory ConfigFileError: unable to access remote configuration file ValueError: included path has an unsupported URL scheme, is required but does not exist; configuration stage directory argument is missing @@ -1308,22 +1382,25 @@ def scopes(self, parent_scope: ConfigScope) -> List[ConfigScope]: tty.debug(f"Using existing scopes: {[s.name for s in self._scopes]}") return self._scopes - # An absolute path does not need a local base directory. - if os.path.isabs(self.path): - tty.debug(f"The included path ({self}) is absolute so needs no base directory") - base = None - else: - base = self.base_directory(self.path, parent_scope) + def _destination(): + if self.destination: + return self.destination + + return self.base_directory(self.path, parent_scope) # Make sure to use a proper working directory when obtaining the local # path for a local (or remote) file. + base = _destination() tty.debug(f"Local base directory for {self.path} is {base}") config_path = rfc_util.local_path(self.path, self.sha256, base) assert config_path - self.destination = config_path - scope = self._scope(self.path, self.destination, parent_scope) + # TODO/TLD/TBD: Is this the right thing to do here? + if not self.destination: + self.destination = config_path + + scope = self._scope(self.path, config_path, parent_scope) if scope is not None: self._scopes = [scope] @@ -1335,6 +1412,15 @@ def paths(self) -> List[str]: return [self.path] + def to_dict(self) -> Dict[str, Any]: + """Return minimal the configuration information in dictionary form.""" + result = super().to_dict() + for name in ["path", "sha256"]: + value = getattr(self, name) + if value: + result[name] = value + return result + class GitIncludePaths(OptionalInclude): git: str @@ -1357,7 +1443,11 @@ def __init__(self, entry: dict): self._paths = [ spack.util.path.substitute_path_variables(path) for path in entry.get("paths", []) ] - self.destination = None + self.destination = entry.get("destination", "") + if self.destination: + # Possible work-around for adding spack.util.path circular import + # since already a circular import for spack.util.remote_file_cache. + self.destination = rfc_util.canonicalize_path(self.destination) self.remote = True if not self.branch and not self.commit and not self.tag: @@ -1365,11 +1455,6 @@ def __init__(self, entry: dict): "Git include paths ({self}) must specify one or more of: branch, commit, tag" ) - if not self._paths: - raise spack.error.ConfigError( - "Git include paths ({self}) must include one or more relative paths" - ) - def __repr__(self): if self.branch: identifier = f"branch={self.branch}" @@ -1378,7 +1463,8 @@ def __repr__(self): return ( f"GitIncludePaths('{self.name}', {self.git}, paths={self._paths}, " - f"{identifier}, when='{self.when}', optional={self.optional})" + f"{identifier}, when='{self.when}', optional={self.optional}, " + f"destination={self.destination})" ) def _clone(self, parent_scope: ConfigScope) -> Optional[str]: @@ -1387,55 +1473,60 @@ def _clone(self, parent_scope: ConfigScope) -> Optional[str]: Args: parent_scope: enclosing scope + Returns: destination path if cloned or ``None`` + + Raises: + AssertionError: unable to write to the directory """ if self.fetched(): tty.debug(f"Repository ({self.git}) already cloned to {self.destination}") return self.destination - # environment includes should be located under the environment - destination = self.base_directory(self.git, parent_scope) - assert destination, f"{self} requires a local cache directory" - tty.debug(f"Cloning {self.git} into {destination}") - - with filesystem.working_dir(destination, create=True): + # TODO/TLD/TBD: Was base_directory called for a git repo before this? + tty.debug(f"Cloning {self.git} into {self.destination}") + with filesystem.working_dir(self.destination, create=True): if not os.path.exists(".git"): try: tty.debug("Initializing the git repository") spack.util.git.init_git_repo(self.git) except spack.util.executable.ProcessError as e: - raise spack.error.ConfigError( - f"Unable to initialize repository ({self.git}) under {destination}: {e}" + msg = ( + f"Unable to initialize repository ({self.git}) under " + f"{self.destination}" ) + if self.optional: + tty.warning(msg + ". Ignoring optional include.") + else: + raise spack.error.ConfigError(msg + f": {e}") - try: - if self.commit: - tty.debug(f"Pulling commit {self.commit}") - spack.util.git.pull_checkout_commit(self.commit) - elif self.tag: - tty.debug(f"Pulling tag {self.tag}") - spack.util.git.pull_checkout_tag(self.tag) - elif self.branch: - # if the branch already exists we should use the - # previously configured remote - tty.debug(f"Pulling branch {self.branch}") - try: - git = spack.util.git.git(required=True) - output = git("config", f"branch.{self.branch}.remote", output=str) - remote = output.strip() - except spack.util.executable.ProcessError: - remote = "origin" - spack.util.git.pull_checkout_branch(self.branch, remote=remote) - else: - raise spack.error.ConfigError(f"Missing or unsupported options in {self}") - - except spack.util.executable.ProcessError as e: - raise spack.error.ConfigError( - f"Unable to check out repository ({self}) in {destination}: {e}" - ) + if os.path.exists(".git"): + try: + if self.commit: + tty.debug(f"Pulling commit {self.commit}") + spack.util.git.pull_checkout_commit(self.commit) + elif self.tag: + tty.debug(f"Pulling tag {self.tag}") + spack.util.git.pull_checkout_tag(self.tag) + elif self.branch: + # if the branch already exists we should use the + # previously configured remote + tty.debug(f"Pulling branch {self.branch}") + try: + git = spack.util.git.git(required=True) + output = git("config", f"branch.{self.branch}.remote", output=str) + remote = output.strip() + except spack.util.executable.ProcessError: + remote = "origin" + spack.util.git.pull_checkout_branch(self.branch, remote=remote) + else: + raise spack.error.ConfigError(f"Missing or unsupported options in {self}") + + except spack.util.executable.ProcessError as e: + raise spack.error.ConfigError( + f"Unable to check out repository ({self}) in {self.destination}: {e}" + ) - # only set the destination on successful clone/checkout - self.destination = destination return self.destination def fetched(self) -> bool: @@ -1449,8 +1540,8 @@ def scopes(self, parent_scope: ConfigScope) -> List[ConfigScope]: Args: parent_scope: including scope - Returns: configuration scopes IF the when condition is satisfied; - otherwise, an empty list. + Returns: configuration scopes for configuration files IF the when + condition is satisfied; otherwise, an empty list. Raises: ConfigFileError: unable to access remote configuration file(s) @@ -1484,8 +1575,37 @@ def scopes(self, parent_scope: ConfigScope) -> List[ConfigScope]: @property def paths(self) -> List[str]: """Path(s) associated with the include.""" + if self._paths: + return self._paths + + if not self.fetched(): + return [] - return self._paths + config_sections = SECTION_SCHEMAS.keys() + + def config_file(path: str) -> bool: + root, _ = os.path.splitext(path) + return root in config_sections + + manifest_filename = "spack.yaml" + with filesystem.working_dir(self.destination, create=False): + yaml_files = glob.glob("*.yaml") + # An environment manifest file takes precedence over any other + # configuration files in a directory. + if manifest_filename in yaml_files: + return [manifest_filename] + + # Otherwise, only return supported configuraton files. + return [path for path in yaml_files if config_file(path)] + + def to_dict(self) -> Dict[str, Any]: + """Return the minimal configuration information in dictionary form.""" + result = super().to_dict() + for name in ["git", "branch", "tag", "commit", "paths"]: + value = getattr(self, name) + if value: + result[name] = value + return result def included_path(entry: Union[str, pathlib.Path, dict]) -> Union[IncludePath, GitIncludePaths]: @@ -1505,22 +1625,6 @@ def included_path(entry: Union[str, pathlib.Path, dict]) -> Union[IncludePath, G return GitIncludePaths(entry) -def paths_from_includes(includes: List[Union[str, dict]]) -> List[str]: - """The path(s) from the configured includes. - - Args: - includes: include configuration information - - Returns: list of path or an empty list if there are none - """ - - paths = [] - for entry in includes: - include = included_path(entry) - paths.extend(include.paths) - return paths - - def config_paths_from_entry_points() -> List[Tuple[str, str]]: """Load configuration paths from entry points @@ -1836,7 +1940,7 @@ def read_config_file( if data: if schema is None: key = next(iter(data)) - schema = _ALL_SCHEMAS[key] + schema = SECTION_SCHEMAS[key] validate(data, schema) return data @@ -2163,7 +2267,8 @@ def ensure_latest_format_fn(section: str) -> Callable[[YamlConfigDict], bool]: section: section of the configuration e.g. "packages", "config", etc. """ # Every module we need is already imported at the top level, so getattr should not raise - return getattr(getattr(spack.schema, section), "update", lambda _: False) + attr = "env" if section == "spack" else section # handle special env case + return getattr(getattr(spack.schema, attr), "update", lambda _: False) @contextlib.contextmanager diff --git a/lib/spack/spack/environment/__init__.py b/lib/spack/spack/environment/__init__.py index fe3c425065ff2d..16cd1bc0afa273 100644 --- a/lib/spack/spack/environment/__init__.py +++ b/lib/spack/spack/environment/__init__.py @@ -618,6 +618,7 @@ environment_from_name_or_dir, environment_path_scope, exists, + included_env_config, initialize_environment_dir, installed_specs, is_env_dir, @@ -658,6 +659,7 @@ "environment_from_name_or_dir", "environment_path_scope", "exists", + "included_env_config", "initialize_environment_dir", "installed_specs", "is_env_dir", diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index 9d53c34c0a438b..a8e08833dc0e59 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -182,8 +182,8 @@ def default_manifest_yaml(): # Default behavior to link all packages into views (vs. only root packages) default_view_link = "all" -# (DEPRECATED) Use as the heading/name in the manifest is deprecated. # The key for any concrete specs included in a lockfile. +# (DEPRECATED) Use as the heading/name in the manifest is deprecated. lockfile_include_key = "include_concrete" # The name/heading for include paths in the manifest file. @@ -410,10 +410,7 @@ def create_in_dir( manifest.set_default_view(with_view) if include_concrete is not None: - set_included_envs_to_env_paths(include_concrete) - validate_included_envs_exists(include_concrete) - validate_included_envs_concrete(include_concrete) - manifest.set_include_concrete(include_concrete) + manifest.add_include_concrete(include_concrete) manifest.flush() @@ -535,67 +532,6 @@ def ensure_env_root_path_exists(): fs.mkdirp(env_root_path()) -def set_included_envs_to_env_paths(include_concrete: List[str]) -> None: - """If the included environment(s) is the environment name - it is replaced by the path to the environment - - Args: - include_concrete: list of env name or path to env""" - - for i, env_name in enumerate(include_concrete): - if is_env_dir(env_name): - include_concrete[i] = env_name - elif exists(env_name): - include_concrete[i] = root(env_name) - - -def validate_included_envs_exists(include_concrete: List[str]) -> None: - """Checks that all of the included environments exist - - Args: - include_concrete: list of already existing concrete environments to include - - Raises: - SpackEnvironmentError: if any of the included environments do not exist - """ - - missing_envs = set() - - for i, env_name in enumerate(include_concrete): - if not is_env_dir(env_name): - missing_envs.add(env_name) - - if missing_envs: - msg = "The following environment(s) are missing: {0}".format(", ".join(missing_envs)) - raise SpackEnvironmentError(msg) - - -def validate_included_envs_concrete(include_concrete: List[str]) -> None: - """Checks that all of the included environments are concrete - - Args: - include_concrete: list of already existing concrete environments to include - - Raises: - SpackEnvironmentError: if any of the included environments are not concrete - """ - - non_concrete_envs = set() - - for env_path in include_concrete: - if not os.path.exists(os.path.join(env_path, lockfile_name)): - non_concrete_envs.add(environment_name(env_path)) - - if non_concrete_envs: - msg = "The following environment(s) are not concrete: {0}\nPlease run:".format( - ", ".join(non_concrete_envs) - ) - for env in non_concrete_envs: - msg += f"\n\t`spack -e {env} concretize`" - - raise SpackEnvironmentError(msg) - - def all_environment_names(): """List the names of environments that currently exist.""" # just return empty if the env path does not exist. A read-only @@ -1043,6 +979,17 @@ def from_info_dict(info_dict: Dict[str, str]) -> "ConcretizedRootInfo": ) +def included_env_config(key: str, default: Optional[Any] = None) -> Any: + """Extract the included environment configuration for the key. + + Args: + key: environment manifest configuration key + default: optional default value if key is not detected under ``spack:`` + """ + data = spack.config.CONFIG.get(TOP_LEVEL_KEY, {}) + return data.get(key, default) + + class Environment: """A Spack environment, which bundles together configuration and a list of specs.""" @@ -1262,22 +1209,34 @@ def _construct_state_from_manifest(self): self._process_view(spack.config.get("view", True)) self._process_included_lockfiles() + # TODO/TLD/TBD: Properly resolve the conflicts related to adding included + # TODO/TLD/TBD: user specs def _sync_speclists(self): - self._spec_lists_parser = SpecListParser( - toolchains=spack.config.CONFIG.get("toolchains", {}) - ) + # Combined toolchains from global config and included environments. + toolchains = spack.config.CONFIG.get("toolchains", {}) + toolchains.update(included_env_config("toolchains", {})) + self._spec_lists_parser = SpecListParser(toolchains=toolchains) + + # Combined definitions from the global config and included environments. self.spec_lists = {} - self.spec_lists.update( - self._spec_lists_parser.parse_definitions( - data=spack.config.CONFIG.get("definitions", []) - ) - ) + combined_yaml = [] + combined_yaml.extend(spack.config.CONFIG.get("definitions", [])) + combined_yaml.extend(included_env_config("definitions", [])) + self.spec_lists.update(self._spec_lists_parser.parse_definitions(data=combined_yaml)) + + # TODO/TLD/TBD: Make sure process groups from included manifests for group in self.manifest.groups(): tty.debug(f"[{__name__}]: Synchronizing user specs from the '{group}' group", level=2) key = self._user_specs_key(group=group) self.spec_lists[key] = self._spec_lists_parser.parse_user_specs( name=key, yaml_list=self.manifest.user_specs(group=group) ) + # TODO/TLD/TBD: Resolve this correctly + # self.spec_lists[key].extend( + # self._spec_lists_parser.parse_user_specs( + # name=key, yaml_list=included_env_config.user_specs(group=group) + # ) + # ) def _user_specs_key(self, *, group: Optional[str] = None) -> str: if group is None or group == DEFAULT_USER_SPEC_GROUP: @@ -3098,41 +3057,58 @@ def _ensure_env_dir(): shutil.copy(envfile, target_manifest) - # Copy relative path includes that live inside the environment dir + # Rewrite the relative path in the include path data + def new_include(included_path: spack.config.IncludePath, new_path: str): + info = included_path.to_dict() + if len(info) == 1: + return new_path + + info["path"] = new_path + return info + try: manifest = EnvironmentManifestFile(environment_dir) except Exception: # error handling for bad manifests is handled on other code paths return - # TODO: make this recursive + # Override relative paths for IncludePath entries. + # GitIncludePaths should NOT be overriden. includes = manifest[TOP_LEVEL_KEY].get(manifest_include_name, []) - paths = spack.config.paths_from_includes(includes) - for path in paths: - if os.path.isabs(path): + included_paths = [spack.config.included_path(entry) for entry in includes] + included_paths = [ + include for include in included_paths if isinstance(include, spack.config.IncludePath) + ] + for idx, include in enumerate(included_paths): + if os.path.isabs(include.path): continue - abspath = pathlib.Path(os.path.normpath(environment_dir / path)) + abspath = pathlib.Path(os.path.normpath(environment_dir / include.path)) common_path = pathlib.Path(os.path.commonpath([environment_dir, abspath])) if common_path != environment_dir: - tty.debug(f"Will not copy relative include file from outside environment: {path}") - continue - - orig_abspath = os.path.normpath(envfile.parent / path) - if os.path.isfile(orig_abspath): - fs.touchp(abspath) - shutil.copy(orig_abspath, abspath) + tty.warn( + f"Relative include path ({include.path}) is outside environment " + "so will retain as is" + ) continue + orig_abspath = os.path.normpath(envfile.parent / include.path) if not os.path.exists(orig_abspath): - tty.warn(f"Skipping copy of non-existent include path: '{path}'") + tty.warn(f"Include '{include.path}' does not exist so will retain as is") continue - if os.path.exists(abspath): - tty.warn(f"Skipping copy of directory over existing path: {path}") - continue + if os.path.isfile(orig_abspath): + manifest.override_include(new_include(include, orig_abspath), idx) + else: + if os.path.exists(abspath): + tty.warn( + f"Skipping replacement of duplicate directory ({include.path}) " + f"since {abspath} exists" + ) + continue + manifest.override_include(new_include(include, orig_abspath), idx) - shutil.copytree(orig_abspath, abspath, symlinks=True) + manifest.flush() class EnvironmentManifestFile(collections.abc.Mapping): @@ -3151,7 +3127,6 @@ def from_lockfile(manifest_dir: Union[pathlib.Path, str]) -> "EnvironmentManifes Args: manifest_dir: directory containing the manifest and lockfile """ - # TBD: Should this be the abspath? manifest_dir = pathlib.Path(manifest_dir) lockfile = manifest_dir / lockfile_name with lockfile.open("r", encoding="utf-8") as f: @@ -3378,13 +3353,53 @@ def override_user_spec(self, user_spec: str, idx: int) -> None: raise SpackEnvironmentError(msg) from e self.changed = True - def set_include_concrete(self, include_concrete: List[str]) -> None: - """Sets the included concrete environments in the manifest to the value(s) passed as input. + def add_include_concrete(self, include_concrete: List[str]) -> None: + """Adds the valid included concrete environment lockfiles to the manifest. Args: - include_concrete: list of already existing concrete environments to include + include_concrete: concrete environment root paths to include + + Raises: + SpackEnvironmentError: if any included concrete environments do not exist + ValueError: if any included managed environments have invalid names """ - self.configuration[lockfile_include_key] = list(include_concrete) + abstract = set() + concrete_paths = [] + for include in include_concrete: + env_root = as_env_dir(include) + lockfile = os.path.join(env_root, lockfile_name) + if os.path.exists(lockfile): + concrete_paths.append(lockfile) + else: + abstract.add(include) + + if abstract: + msg = ( + f"The following environment(s) are not concrete: {', '.join(abstract)}\n" + "Please run:" + ) + for env in abstract: + msg += f"\n\t`spack -e {env} concretize`" + raise SpackEnvironmentError(msg) + + self.configuration[manifest_include_name] = concrete_paths + self.changed = True + + def override_include(self, include: Union[str, dict], idx: int) -> None: + """Overrides the included path data at index idx with the one passed as input. + Args: + include: new include data + idx: index of the include to be overridden + + Raises: + SpackEnvironmentError: when the include cannot be overridden + """ + try: + current_include = self.configuration["include"][idx] + self.configuration["include"][idx] = include + except ValueError as e: + msg = f"cannot override '{current_include}' with '{include}'" + raise SpackEnvironmentError(msg) from e self.changed = True def add_definition(self, user_spec: str, list_name: str) -> None: @@ -3555,7 +3570,7 @@ def prepare_config_scope(self) -> None: ) def deactivate_config_scope(self) -> None: - """Remove the manifest's scope from the global config path.""" + """Remove the manifest's scope(s) from the global config path.""" spack.config.CONFIG.remove_scope(self.env_config_scope.name) @contextlib.contextmanager diff --git a/lib/spack/spack/schema/env.py b/lib/spack/spack/schema/env.py index 267d32399a8e7d..0a0c59043ec841 100644 --- a/lib/spack/spack/schema/env.py +++ b/lib/spack/spack/schema/env.py @@ -94,6 +94,8 @@ }, # (DEPRECATED) include concrete to be merged under the include key "include_concrete": include_concrete, + # nested environments + "spack": {"$ref": "#/spack/properties"}, }, } } diff --git a/lib/spack/spack/schema/include.py b/lib/spack/spack/schema/include.py index 6938dee47df929..aef6c3c70ccdf2 100644 --- a/lib/spack/spack/schema/include.py +++ b/lib/spack/spack/schema/include.py @@ -50,6 +50,11 @@ "description": "Required SHA256 hash for remote URLs to verify " "file integrity", }, + "destination": { + "type": "string", + "description": "Optional destination for a remote (ftp, http, " + "https) path.", + }, "optional": { "type": "boolean", "description": "If true, include only if path exists; if false " @@ -94,6 +99,11 @@ "configuration files are located", }, "name": {"type": "string"}, + "destination": { + "type": "string", + "description": "Optional destination for a remote (ftp, http, " + "https) path.", + }, "when": { "type": "string", "description": "Include this config only when the condition (as " @@ -105,7 +115,7 @@ "if false (default), inaccessible repository causes errors", }, }, - "required": ["git", "paths"], + "required": ["git"], "additionalProperties": False, }, ] diff --git a/lib/spack/spack/test/cmd/ci.py b/lib/spack/spack/test/cmd/ci.py index c111bbba4728e5..a233c8adfee645 100644 --- a/lib/spack/spack/test/cmd/ci.py +++ b/lib/spack/spack/test/cmd/ci.py @@ -1556,7 +1556,7 @@ def example_function(): ) -def test_gitlab_config_scopes(install_mockery, ci_generate_test, tmp_path: pathlib.Path): +def test_gitlab_config_scopes(install_mockery, ci_generate_test, tmp_path: pathlib.Path, capfd): """Test pipeline generation with included configs""" # Create an included config scope configs_path = tmp_path / "gitlab" / "configs" @@ -1587,6 +1587,7 @@ def test_gitlab_config_scopes(install_mockery, ci_generate_test, tmp_path: pathl - path: {rel_configs_path} - {configs_path} - when: 'False' + sha256: "theactualvaluedoesnotmatterhere" path: https://dummy.io view: false specs: @@ -1600,6 +1601,8 @@ def test_gitlab_config_scopes(install_mockery, ci_generate_test, tmp_path: pathl tags: ["some_tag"] """ ) + _, err = capfd.readouterr() + assert err.count("Ignoring duplicate included scope") > 2 yaml_contents = syaml.load(outputfile.read_text()) @@ -1622,25 +1625,12 @@ def test_gitlab_config_scopes(install_mockery, ci_generate_test, tmp_path: pathl env_manifest = syaml.load(conc_env_manifest.read_text()) assert "include" in env_manifest["spack"] - # Ensure relative path include correctly updated - # Ensure the relocated concrete env includes point to the same location - rel_conc_path = env_manifest["spack"]["include"][0] - abs_conc_path = (conc_env_path / rel_conc_path).absolute().resolve() - assert str(abs_conc_path) == os.path.join(ev.as_env_dir("test"), "gitlab", "configs") - - # Ensure relative path include with "path" correctly updated - # Ensure the relocated concrete env includes point to the same location - rel_conc_path = env_manifest["spack"]["include"][1]["path"] - abs_conc_path = (conc_env_path / rel_conc_path).absolute().resolve() - assert str(abs_conc_path) == os.path.join(ev.as_env_dir("test"), "gitlab", "configs") - - # Ensure absolute path is unchanged - # Ensure the relocated concrete env includes point to the same location - abs_config_path = env_manifest["spack"]["include"][2] - assert str(abs_config_path) == str(configs_path) + # Ensure all but last concrete env includes point to the same location + conc_env_includes = env_manifest["spack"]["include"] + assert all([path == str(configs_path) for path in conc_env_includes[:-1]]) # Ensure URL path is unchanged - url_config_path = env_manifest["spack"]["include"][3]["path"] + url_config_path = conc_env_includes[-1]["path"] assert str(url_config_path) == "https://dummy.io" diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index 327f2778703094..6fc2e8d53ce4ac 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -1083,12 +1083,7 @@ def test_init_from_env_no_spackfile(tmp_path): def test_init_from_yaml_relative_includes(tmp_path: pathlib.Path): - files = [ - "relative_copied/packages.yaml", - "./relative_copied/compilers.yaml", - "repos.yaml", - "./config.yaml", - ] + files = ["relative/packages.yaml", "./relative/compilers.yaml", "repos.yaml", "./config.yaml"] manifest = f""" spack: @@ -1106,14 +1101,19 @@ def test_init_from_yaml_relative_includes(tmp_path: pathlib.Path): (e1_path / f).parent.mkdir(parents=True, exist_ok=True) (e1_path / f).touch() - e2 = _env_create("test2", init_file=str(e1_manifest)) + e2 = ev.create("test2", init_file=str(e1_manifest)) - for f in files: - assert os.path.exists(os.path.join(e2.path, f)) + # Check new environment's (manifest) includes + new_env_includes = e2.manifest.configuration.get("include", []) + for orig, new in zip(files, new_env_includes): + assert new.endswith(orig.strip("./")) + + # Check configured environment's includes + config_includes = ev.included_env_config("include", []) + for orig, new in zip(files, config_includes): + assert new.endswith(orig.strip("./")) -# TODO: Should we be supporting relative path rewrites when creating new env from existing? -# TODO: If so, then this should confirm that the absolute include paths in the new env exist. def test_init_from_yaml_relative_includes_outside_env(tmp_path: pathlib.Path): """Ensure relative includes to files outside the environment fail.""" files = ["../outside_env/repos.yaml"] @@ -1344,13 +1344,13 @@ def packages_file(tmp_path: pathlib.Path): def mpileaks_env_config(include_path): """Return the contents of an environment that includes the provided path and lists mpileaks as the sole spec.""" - return """\ + return f"""\ spack: include: - - {0} + - {include_path} specs: - mpileaks -""".format(include_path) +""" def test_env_with_included_config_file(mutable_mock_env_path, packages_file): @@ -1509,7 +1509,7 @@ def test_env_with_included_config_file_url( spack_yaml = tmp_path / "spack.yaml" with spack_yaml.open("w") as f: - f.write("spack:\n include:\n - {0}\n".format(packages_file.as_uri())) + f.write(f"spack:\n include:\n - {packages_file.as_uri()}\n") env = ev.Environment(str(tmp_path)) ev.activate(env) @@ -2069,8 +2069,8 @@ def test_env_include_concrete_env_yaml(env_name): combined = ev.read("combined_env") combined_yaml = combined.manifest["spack"] - assert ev.lockfile_include_key in combined_yaml - assert test.path in combined_yaml[ev.lockfile_include_key] + assert ev.manifest_include_name in combined_yaml + assert test.lock_path in combined_yaml[ev.manifest_include_name] @pytest.mark.regression("45766") @@ -2105,8 +2105,8 @@ def test_env_multiple_include_concrete_envs(): combined_yaml = combined.manifest["spack"] - assert test1.path in combined_yaml[ev.lockfile_include_key][0] - assert test2.path in combined_yaml[ev.lockfile_include_key][1] + assert test1.path in combined_yaml[ev.manifest_include_name][0] + assert test2.path in combined_yaml[ev.manifest_include_name][1] # No local specs in the combined env assert not combined_yaml["specs"] @@ -2117,8 +2117,8 @@ def test_env_include_concrete_envs_lockfile(): combined_yaml = combined.manifest["spack"] - assert ev.lockfile_include_key in combined_yaml - assert test1.path in combined_yaml[ev.lockfile_include_key] + assert ev.manifest_include_name in combined_yaml + assert test1.lock_path in combined_yaml[ev.manifest_include_name] with open(combined.lock_path, encoding="utf-8") as f: lockfile_as_dict = combined._read_lockfile(f) @@ -4741,6 +4741,152 @@ def test_view_can_select_group_of_specs_using_string( assert not bin_file.exists() if item.group == "apps2" else bin_file.exists() +def test_env_include_envs(tmp_path, mock_packages, mutable_config, environment_from_manifest): + """Confirm that an environment that includes two other environments + picks up the specs from both.""" + specs_template = """\ +spack: + specs: + - {0} +""" + + specs = ["libdwarf", "mpileaks"] + includes = [] + includes_dir = tmp_path / "includes" + for pkg in specs: + include = includes_dir / pkg / ev.manifest_name + fs.mkdirp(include.parent) + include.write_text(specs_template.format(pkg)) + includes.append(str(include)) + + include_condition = 'platform == "test"' + + # TODO: Remove this once minimum python is python@3.9 + def remove_prefix(text, prefix): + if text.startswith(prefix): + return text[len(prefix) :] + return text + + def include_entry(path): + return f"- path: {path}\n when: {include_condition}" + + prefix = str(tmp_path) + os.sep + # TODO: Once minimum python is python@3.9 + # rel_paths = [include.removeprefix(prefix) for include in includes] + rel_paths = [remove_prefix(include, prefix) for include in includes] + paths_str = "\n ".join([include_entry(path) for path in rel_paths]) + e = environment_from_manifest( + f"""\ +spack: + include: + {paths_str} +""" + ) + + for spec in specs: + assert Spec(spec) in e.user_specs + + # Confirm the relative included paths were replaced with absolute paths + # and the include condition is unchanged. + env_includes = e.manifest[ev.TOP_LEVEL_KEY].get("include", []) + for orig, include_path in zip(rel_paths, env_includes): + assert not os.path.isabs(orig) + + full_path = include_path["path"] + assert os.path.isabs(full_path) + # TODO: Once minimum python is python@3.9 + # assert full_path.removeprefix(prefix) == orig + assert remove_prefix(full_path, prefix) + + assert include_path["when"] == include_condition + + +def test_env_include_env_pkgs_def( + tmp_path, mock_packages, mutable_config, environment_from_manifest +): + """Confirm that an environment that lists specs and includes an environment + with a definition for an additional spec includes that extra spec.""" + spack_yaml = """\ +spack: + definitions: + - packages: ["libdwarf"] + specs: + - $packages +""" + + includes_dir = tmp_path / "includes" + fs.mkdirp(includes_dir) + include = includes_dir / ev.manifest_name + include.write_text(spack_yaml) + + env_yaml = f"""\ +spack: + specs: + - libelf@0.8.10 + - mpileaks + include: + - {include} +""" + e = environment_from_manifest(env_yaml) + + specs = ["libelf@0.8.10", "mpileaks", "libdwarf"] + user_specs = e.user_specs + for spec in specs: + assert Spec(spec) in user_specs + + # Confirm manifest contents include explicit and not included specs + e.write() + with open(e.manifest.manifest_file, "r", encoding="utf-8") as f: + contents = f.read() + + assert "libelf@0.8.10" in contents + assert "mpileaks" in contents + assert "libdwarf" not in contents + + +def test_env_include_env_pkgs_dup_defs( + tmp_path, mock_packages, mutable_config, environment_from_manifest +): + """Confirm that an environment that uses a definition with spec(s) and + includes another environment with the same definition but different spec(s), + results in user specs from both files.""" + spack_yaml = """\ +spack: + definitions: + - packages: ["libelf"] + specs: + - $packages +""" + + includes_dir = tmp_path / "includes" + fs.mkdirp(includes_dir) + include = includes_dir / ev.manifest_name + include.write_text(spack_yaml) + + env_yaml = f"""\ +spack: + definitions: + - packages: ["libdwarf"] + specs: + - $packages + include: + - {include} +""" + e = environment_from_manifest(env_yaml) + + user_specs = e.user_specs + for spec in ["libelf", "libdwarf"]: + assert Spec(spec) in user_specs + + # Confirm manifest only includes the expected (not included) spec + e.write() + with open(e.manifest.manifest_file, "r", encoding="utf-8") as f: + contents = f.read() + + assert "libdwarf" in contents + assert "libelf" not in contents + + def test_env_include_concrete_only(tmp_path, mock_packages, mutable_config): """Confirm that an environment that only includes a concrete environment actually loads it.""" specs = ["libdwarf", "libelf"] diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py index 30ec284cc75a16..f9dd66d406f965 100644 --- a/lib/spack/spack/test/config.py +++ b/lib/spack/spack/test/config.py @@ -6,6 +6,7 @@ import io import os import pathlib +import stat import sys import tempfile import textwrap @@ -1672,8 +1673,7 @@ def test_config_include_similar_name(tmp_path: pathlib.Path): # Ensure all of the scopes are found assert len(config.matching_scopes("^test$")) == 1 - assert len(config.matching_scopes("^test:a/config$")) == 1 - assert len(config.matching_scopes("^test:b/config$")) == 1 + assert len(config.matching_scopes("^test:")) == 2 def test_deepcopy_as_builtin(env_yaml): @@ -1707,6 +1707,8 @@ def test_included_path_string( assert include.path == str(path) assert not include.optional assert include.evaluate_condition() + out = include.to_dict() + assert len(out) == 1 and out["path"] == str(path) parent_scope = mock_low_high_config.scopes["low"] @@ -1717,7 +1719,7 @@ def test_included_path_string( # First successful pass builds the scope path.touch() scopes = include.scopes(parent_scope) - assert scopes and len(scopes) == 1 + assert len(scopes) == 1 assert isinstance(scopes[0], spack.config.SingleFileScope) # Second pass uses the scopes previously built @@ -1734,12 +1736,14 @@ def test_included_path_string_no_parent_path( will be rooted in the current working directory (usually SPACK_ROOT).""" entry = {"path": "config.yaml", "optional": True} include = spack.config.included_path(entry) + parent_scope = spack.config.InternalConfigScope("parent-scope") included_scopes = include.scopes(parent_scope) # ensure scope is returned even if there is no parent path assert len(included_scopes) == 1 # ensure scope for include is singlefile as it ends in .yaml assert isinstance(included_scopes[0], spack.config.SingleFileScope) + destination = include.destination curr_dir = os.getcwd() assert curr_dir == os.path.commonprefix([curr_dir, destination]) # type: ignore[list-item] @@ -1766,11 +1770,9 @@ def test_included_path_conditional_bad_when( path.mkdir() entry = {"path": str(path), "when": 'platform == "nosuchplatform"', "optional": True} include = spack.config.included_path(entry) - assert isinstance(include, spack.config.IncludePath) - assert include.path == entry["path"] - assert include.when == entry["when"] - assert include.optional assert not include.evaluate_condition() + out = include.to_dict() + assert out == entry scopes = include.scopes(mock_low_high_config.scopes["low"]) captured = capfd.readouterr()[1] @@ -1778,19 +1780,42 @@ def test_included_path_conditional_bad_when( assert not scopes +def test_included_path_destination( + tmp_path: pathlib.Path, mock_fetch_url_text, mock_low_high_config, ensure_debug +): + destination = tmp_path / "remotes" + entry = { + "path": "https://github.com/path/to/raw/config/config.yaml", + "destination": str(destination), + } + + # confirm missing sha256 is flagged + with pytest.raises(spack.error.ConfigError, match="requires a 'sha256'"): + spack.config.included_path(entry) + + # confirm proper remote include with destination works + if sys.platform != "win32": + entry["sha256"] = "805ab8e677f83311a141e948674e5a3ad57d94b54fc126d410a853937ca8255f" + else: + entry["sha256"] = "1e44af11d28bb1ddce335ff90129d6b081faf067930f1bb9b745990c6958d8b6" + + include = spack.config.included_path(entry) + assert include.scopes(mock_low_high_config.scopes["low"]) + + def test_included_path_conditional_success(tmp_path: pathlib.Path, mock_low_high_config): path = tmp_path / "local" path.mkdir() entry = {"path": str(path), "when": 'platform == "test"', "optional": True} include = spack.config.included_path(entry) assert isinstance(include, spack.config.IncludePath) - assert include.path == entry["path"] - assert include.when == entry["when"] assert include.optional assert include.evaluate_condition() + out = include.to_dict() + assert out == entry scopes = include.scopes(mock_low_high_config.scopes["low"]) - assert scopes and len(scopes) == 1 + assert len(scopes) == 1 assert isinstance(scopes[0], spack.config.DirectoryConfigScope) @@ -1800,12 +1825,6 @@ def test_included_path_git_missing_args(): with pytest.raises(spack.error.ConfigError, match="specify one or more"): spack.config.included_path(entry) - # must have one or more paths - entry["tag"] = "v1.0" - entry["paths"] = [] - with pytest.raises(spack.error.ConfigError, match="must include one or more"): - spack.config.included_path(entry) - def test_included_path_git_unsat( tmp_path: pathlib.Path, mock_low_high_config, ensure_debug, monkeypatch, capfd @@ -1819,12 +1838,11 @@ def test_included_path_git_unsat( } include = spack.config.included_path(entry) assert isinstance(include, spack.config.GitIncludePaths) - assert include.git == entry["git"] - assert include.tag == entry["tag"] - assert include.paths == entry["paths"] - assert include.when == entry["when"] assert not include.optional and not include.evaluate_condition() + out = include.to_dict() + assert out == entry + scopes = include.scopes(mock_low_high_config.scopes["low"]) captured = capfd.readouterr()[1] assert "condition is not satisfied" in captured @@ -1881,6 +1899,9 @@ def test_included_path_git( assert isinstance(include, spack.config.GitIncludePaths) assert not include.optional and include.evaluate_condition() + destination = include.base_directory(include.git) + assert destination and not os.path.exists(destination) + # set up minimal git and repository operations class MockIncludeGit(spack.util.executable.Executable): def __init__(self, required: bool): @@ -1989,6 +2010,69 @@ def test_included_path_git_temp_dest(mock_low_high_config): assert dest_dir == temp_dir, pre + rest +@pytest.mark.parametrize( + "paths,dest", [(["spack.yaml"], True), (["config.yaml", "packages.yaml"], False), ([], False)] +) +def test_included_path_git_default_paths( + tmp_path: pathlib.Path, mock_low_high_config, ensure_debug, monkeypatch, paths, dest +): + """Confirm the default selection of paths.""" + monkeypatch.setattr(spack.paths, "user_cache_path", str(tmp_path)) + + class MockIncludeGit(spack.util.executable.Executable): + def __init__(self, required: bool): + pass + + def __call__(self, *args, **kwargs) -> str: # type: ignore + action = args[0] + + if action == "config": + return "origin" + + return "" + + entry = {"git": "https://example.com/linux/configs.git", "branch": "main"} + if dest: + path = tmp_path / "remotes" + path.mkdir() + path = str(path) + entry["destination"] = path + else: + path = "" + + include = spack.config.included_path(entry) + assert isinstance(include, spack.config.GitIncludePaths) + + destination = path if dest else include._destination_directory(include.git) + + # set up minimal git and repository operations + monkeypatch.setattr(spack.util.git, "git", MockIncludeGit) + + def _init_repo(*args, **kwargs): + fs.mkdirp(fs.join_path(destination, ".git")) + + def _checkout(*args, **kwargs): + # Make sure the files exist at the clone destination + with fs.working_dir(destination): + for p in paths: + fs.touch(p) + + # Throw in a couple of files that should NOT get picked up + fs.touch("spack.lock") + fs.touch("unsupported.yaml") + + monkeypatch.setattr(spack.util.git, "init_git_repo", _init_repo) + monkeypatch.setattr(spack.util.git, "pull_checkout_branch", _checkout) + + # Ensure the repository is cloned and the paths, if any, are chosen + parent_scope = mock_low_high_config.scopes["low"] + scopes = include.scopes(parent_scope) + assert len(scopes) == len(paths) + for scope in scopes: + assert isinstance(scope, spack.config.SingleFileScope) + assert os.path.basename(scope.path) in paths # type: ignore[union-attr] + + def test_included_path_git_errs(tmp_path: pathlib.Path, mock_low_high_config, monkeypatch): monkeypatch.setattr(spack.paths, "user_cache_path", str(tmp_path)) @@ -2028,8 +2112,10 @@ def __call__(self, *args, **kwargs) -> str: # type: ignore with pytest.raises(spack.error.ConfigError, match="Unable to check out"): include.scopes(parent_scope) - # set up invalid option failure + # set up invalid option failure, making sure to first clear the default + # destination from the previous call include.branch = "" # type: ignore[union-attr] + include.destination = "" # type: ignore[union-attr] with pytest.raises(spack.error.ConfigError, match="Missing or unsupported options"): include.scopes(parent_scope) @@ -2150,3 +2236,64 @@ def test_config_invalid_scope(mock_low_high_config): err = "Must be one of \\['low', 'high'\\]" # noqa: W605 with pytest.raises(ValueError, match=err): spack.config.CONFIG.get_config_filename("noscope", "nosection") + + +def test_config_optional_include_failures(tmp_path: pathlib.Path): + include = spack.config.OptionalInclude({}) + + with pytest.raises(NotImplementedError): + include.paths + + config_scope = spack.config.DirectoryConfigScope("test", str(tmp_path)) + with pytest.raises(NotImplementedError): + include.scopes(config_scope) + + +@pytest.mark.not_on_windows("chmod behaves differently") +def test_included_path_unwritable_dest(tmp_path: pathlib.Path, mock_fetch_url_text): + dest = tmp_path / "unwritable" + dest.mkdir() + current_mode = os.stat(str(dest)).st_mode + dest.chmod(current_mode & ~(stat.S_IWUSR | stat.S_IWGRP | stat.S_IWOTH)) + + sha256 = ( + "1e44af11d28bb1ddce335ff90129d6b081faf067930f1bb9b745990c6958d8b6" + if sys.platform == "win32" + else "805ab8e677f83311a141e948674e5a3ad57d94b54fc126d410a853937ca8255f" + ) + entry = { + "path": "https://github.com/path/to/raw/config/config.yaml", + "destination": str(dest), + "sha256": sha256, + } + include = spack.config.included_path(entry) + + parent_scope = spack.config.ConfigScope("FakeScope") + try: + with pytest.raises(AssertionError, match="to unwritable"): + include.scopes(parent_scope) + finally: + dest.chmod(current_mode) + + +@pytest.mark.not_on_windows("chmod behaves differently") +def test_included_path_git_unwritable_dest(tmp_path: pathlib.Path): + dest = tmp_path / "unwritable" + dest.mkdir() + current_mode = os.stat(str(dest)).st_mode + dest.chmod(current_mode & ~(stat.S_IWUSR | stat.S_IWGRP | stat.S_IWOTH)) + + entry = { + "git": "https://example.com/windows/configs.git", + "branch": "main", + "paths": ["config.yaml"], + "destination": str(dest), + } + include = spack.config.included_path(entry) + + parent_scope = spack.config.ConfigScope("fake") + try: + with pytest.raises(AssertionError, match="to unwritable"): + include.scopes(parent_scope) + finally: + dest.chmod(current_mode) diff --git a/lib/spack/spack/test/env.py b/lib/spack/spack/test/env.py index a25fffde003691..50d4c0590bfe09 100644 --- a/lib/spack/spack/test/env.py +++ b/lib/spack/spack/test/env.py @@ -2138,3 +2138,129 @@ def test_unified_environment_with_mixed_compilers_and_fortran(tmp_path, config): assert mpich.satisfies("%fortran=gcc") assert openblas.satisfies("%c,fortran=gcc") assert mpich["fortran"].dag_hash() == openblas["fortran"].dag_hash() + + +def test_env_include_env_includes(tmp_path, mutable_config): + """Confirm that an environment that includes two other environments, + each including their own configuration, picks up the specs from both + but its own configuration setting.""" + includes_dir = tmp_path / "includes" + fs.mkdirp(includes_dir) + + config_template = """\ +config: + install_tree: + root: {0} +""" + specs_template = """\ +spack: + include: + - {0} + specs: + - {1} +""" + + includes = [] + pkgs = ["libdwarf", "mpileaks"] + for pkg in pkgs: + install_root = tmp_path / "store" / pkg + fs.mkdirp(install_root) + + env_path = includes_dir / pkg + fs.mkdirp(env_path) + config_path = env_path / "config.yaml" + config_path.write_text(config_template.format(install_root)) + + include = env_path / ev.manifest_name + include.write_text(specs_template.format(config_path, pkg)) + includes.append(str(include)) + + install_root = tmp_path / "store" + + spack_yaml = f"""\ +spack: + config: + install_tree: + root: {str(install_root)} + include: {includes} +""" + manifest = tmp_path / "spack.yaml" + manifest.write_text(spack_yaml) + + e = ev.Environment(tmp_path) + ev.activate(e) + + user_specs = e.user_specs + for pkg in pkgs: + assert spack.spec.Spec(pkg) in user_specs + + # ensure the included environment scopes are found + pattern = rf"^env:{str(tmp_path)}" + assert len(mutable_config.matching_scopes(rf"{pattern}$")) == 1 # environment's scope + hash_pattern = "[0-9a-fA-F]{7}" + assert ( + len(mutable_config.matching_scopes(rf"{pattern}:{hash_pattern}$")) == 2 + ) # 1st level includes + hash_pattern = rf"{hash_pattern}:{hash_pattern}" + assert ( + len(mutable_config.matching_scopes(rf"{pattern}:{hash_pattern}$")) == 2 + ) # 2nd level includes + + # ensure the install root matches the top-most (i.e., environment's) configured path + config_install_root = mutable_config.get("config:install_tree:root", None) + assert config_install_root == str(install_root) + + +def test_env_include_env_highest_config(tmp_path, config): + """Confirm that an environment that includes two other environments + picks up the configuration setting from the highest include.""" + + config_template = """\ +config: + build_jobs: {0} +""" + expected_build_jobs = 32 + + config_file = tmp_path / "config.yaml" + config_file.write_text(config_template.format(expected_build_jobs)) + + includes_dir = tmp_path / "includes" + fs.mkdirp(includes_dir) + env2_config_file = includes_dir / "config.yaml" + env2_config_file.write_text(config_template.format(8)) + + env2_file = includes_dir / ev.manifest_name + pkgs = ["libdwarf", "mpileaks"] + env2_file.write_text( + f"""\ +spack: + include: + - {env2_config_file} + specs: {pkgs} +""" + ) + + env_file = tmp_path / ev.manifest_name + env_file.write_text( + f"""\ +spack: + include: + - {env2_file} + - {config_file} +""" + ) + + e = ev.Environment(tmp_path) + ev.activate(e) + + user_specs = e.user_specs + for pkg in pkgs: + assert spack.spec.Spec(pkg) in user_specs + + # ensure the build job configuration matches the highest config.yaml setting + # i.e., that from config_file + build_jobs = spack.config.CONFIG.get("config:build_jobs", None) + try: + assert build_jobs is not None and build_jobs == expected_build_jobs + except AssertionError: + pytest.xfail("nested includes are not processed in BFS order") diff --git a/lib/spack/spack/test/main.py b/lib/spack/spack/test/main.py index ba18643bca7f8b..fbf0119c051804 100644 --- a/lib/spack/spack/test/main.py +++ b/lib/spack/spack/test/main.py @@ -292,7 +292,6 @@ def test_include_recurse_limit(tmp_path: pathlib.Path, mutable_config): spack.main.add_command_line_scopes(mutable_config, [os.path.dirname(include_path)]) -# TODO: Fix this once recursive includes are processed in the expected order. @pytest.mark.parametrize("child,expected", [("b", True), ("c", False)]) def test_include_recurse_diamond(tmp_path: pathlib.Path, mutable_config, child, expected): """Demonstrate include parent's value overrides that of child in diamond include. @@ -314,7 +313,8 @@ def include_contents(paths): values = indent.join([str(p) for p in paths]) return f"include:{indent}{values}" - a_yaml = tmp_path / "a.yaml" + # DirectoryConfigScopes are required to contain file(s) named after the section + a_yaml = tmp_path / "include.yaml" b_yaml = configs_root / "b.yaml" c_yaml = configs_root / "c.yaml" d_yaml = configs_root / "d.yaml" @@ -335,7 +335,4 @@ def include_contents(paths): spack.main.add_command_line_scopes(mutable_config, [str(tmp_path)]) - try: - assert mutable_config.get("config:debug") is expected - except AssertionError: - pytest.xfail("recursive includes are not processed in the expected order") + assert mutable_config.get("config:debug") is expected diff --git a/share/spack/spack-completion.fish b/share/spack/spack-completion.fish index 5a0a496923cdf7..b6fc5fd330677b 100644 --- a/share/spack/spack-completion.fish +++ b/share/spack/spack-completion.fish @@ -1270,7 +1270,7 @@ 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 json group= -complete -c spack -n '__fish_spack_using_command_pos 0 config get' -f -a 'bootstrap cdash ci compilers concretizer config definitions develop env_vars include mirrors modules packages repos toolchains upstreams view' +complete -c spack -n '__fish_spack_using_command_pos 0 config get' -f -a 'bootstrap cdash ci compilers concretizer config definitions develop env_vars include mirrors modules packages repos spack toolchains upstreams view' 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' complete -c spack -n '__fish_spack_using_command config get' -l json -f -a json @@ -1280,7 +1280,7 @@ complete -c spack -n '__fish_spack_using_command config get' -l group -r -d 'sho # spack config blame set -g __fish_spack_optspecs_spack_config_blame h/help group= -complete -c spack -n '__fish_spack_using_command_pos 0 config blame' -f -a 'bootstrap cdash ci compilers concretizer config definitions develop env_vars include mirrors modules packages repos toolchains upstreams view' +complete -c spack -n '__fish_spack_using_command_pos 0 config blame' -f -a 'bootstrap cdash ci compilers concretizer config definitions develop env_vars include mirrors modules packages repos spack toolchains upstreams view' 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' complete -c spack -n '__fish_spack_using_command config blame' -l group -r -f -a group @@ -1288,7 +1288,7 @@ complete -c spack -n '__fish_spack_using_command config blame' -l group -r -d 's # 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 develop env_vars include mirrors modules packages repos toolchains upstreams view' +complete -c spack -n '__fish_spack_using_command_pos 0 config edit' -f -a 'bootstrap cdash ci compilers concretizer config definitions develop env_vars include mirrors modules packages repos spack toolchains upstreams view' 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 @@ -1301,7 +1301,7 @@ complete -c spack -n '__fish_spack_using_command config list' -s h -l help -d 's # spack config scopes set -g __fish_spack_optspecs_spack_config_scopes h/help p/paths t/type= v/verbose -complete -c spack -n '__fish_spack_using_command_pos 0 config scopes' -f -a 'bootstrap cdash ci compilers concretizer config definitions develop env_vars include mirrors modules packages repos toolchains upstreams view' +complete -c spack -n '__fish_spack_using_command_pos 0 config scopes' -f -a 'bootstrap cdash ci compilers concretizer config definitions develop env_vars include mirrors modules packages repos spack toolchains upstreams view' complete -c spack -n '__fish_spack_using_command config scopes' -s h -l help -f -a help complete -c spack -n '__fish_spack_using_command config scopes' -s h -l help -d 'show this help message and exit' complete -c spack -n '__fish_spack_using_command config scopes' -s p -l paths -f -a paths From 15a400b8cce58630600c11d041b3fe3ceae64770 Mon Sep 17 00:00:00 2001 From: tldahlgren Date: Thu, 19 Mar 2026 14:09:42 -0700 Subject: [PATCH 02/33] test_included_path_unwritable_dest: skip if root Signed-off-by: tldahlgren Signed-off-by: Gregory Becker --- lib/spack/spack/test/config.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py index f9dd66d406f965..9ddb811781e948 100644 --- a/lib/spack/spack/test/config.py +++ b/lib/spack/spack/test/config.py @@ -2249,7 +2249,8 @@ def test_config_optional_include_failures(tmp_path: pathlib.Path): include.scopes(config_scope) -@pytest.mark.not_on_windows("chmod behaves differently") +@pytest.mark.not_on_windows("chmod behaves differently on Windows") +@pytest.mark.skipif(getuid() == 0, reason="user is root") def test_included_path_unwritable_dest(tmp_path: pathlib.Path, mock_fetch_url_text): dest = tmp_path / "unwritable" dest.mkdir() From f63e357a7355c4e329672c8c7a4423dbe9a18908 Mon Sep 17 00:00:00 2001 From: tldahlgren Date: Mon, 30 Mar 2026 18:08:59 -0700 Subject: [PATCH 03/33] Post-rebase cleanup of config.py Signed-off-by: tldahlgren Signed-off-by: Gregory Becker --- lib/spack/spack/config.py | 52 +++++++++++++++++----------------- lib/spack/spack/test/config.py | 44 ++++++++++++++-------------- 2 files changed, 48 insertions(+), 48 deletions(-) diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index 55543c3dbea8b0..c15175b4195e74 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -1119,9 +1119,10 @@ def base_directory( path_or_url: path or URL of the include parent_scope: including scope - Returns: ``None`` for an absolute path, the explicit destination (if writable) - or an appropriate subdirectory of the enclosing (parent) scope's writable directory - (when available); otherwise a stable temporary directory. + Returns: ``None`` for an absolute or relative local path, the explicit destination (if + writable), an appropriate subdirectory of the enclosing (parent) scope's writable + directory (when available); otherwise + a stable temporary directory. Raises: AssertionError: The explicit destination is not writable @@ -1131,16 +1132,23 @@ def base_directory( tty.debug(f"The included path ({self}) is absolute so needs no base directory") return None - # Use the explicit destination but require it to be writable. - if self.destination: - assert filesystem.can_write_to_dir(self.destination) - return self.destination + # Use the explicit destination, which is required to be writable, for a remote + # include. + destination = getattr(self, "destination", "") + if self.remote and destination: + assert filesystem.can_write_to_dir(destination) + return destination - # Prefer the (writable) parent scope directory for a local relative file. + # Prefer the (writable) parent scope directory for a local relative path. scope_dir = self._parent_scope_directory(parent_scope) - if not self.remote and filesystem.can_write_to_dir(scope_dir): + if not self.remote and scope_dir and filesystem.can_write_to_dir(scope_dir): return scope_dir + # Local relative paths do not have a destination since they default to the working + # directory. + if not self.remote: + return None + def _subdir(): # Prefer the provided include name over the git repository name. # If neither, use a hash of the url or path for uniqueness. @@ -1345,9 +1353,6 @@ def __init__(self, entry: dict): # since already a circular import for spack.util.remote_file_cache. self.destination = rfc_util.canonicalize_path(self.destination) - # Ensure the explicit destination is writable. - assert filesystem.can_write_to_dir(self.destination) - self.remote = urllib.parse.urlparse(self.path).scheme in ("http", "https", "ftp") if self.remote and not self.sha256: raise spack.error.ConfigError(f"Remote include {self.path} requires a 'sha256'") @@ -1382,21 +1387,16 @@ def scopes(self, parent_scope: ConfigScope) -> List[ConfigScope]: tty.debug(f"Using existing scopes: {[s.name for s in self._scopes]}") return self._scopes - def _destination(): - if self.destination: - return self.destination + # Ensure the explicit destination for a remote file is writable. + base = self.destination or self.base_directory(self.path, parent_scope) + if self.remote and base and os.path.isdir(base): + assert filesystem.can_write_to_dir( + self.destination + ), f"Cannot include {self.path}. Unable to write to {base}" - return self.base_directory(self.path, parent_scope) - - # Make sure to use a proper working directory when obtaining the local - # path for a local (or remote) file. - base = _destination() tty.debug(f"Local base directory for {self.path} is {base}") - config_path = rfc_util.local_path(self.path, self.sha256, base) assert config_path - - # TODO/TLD/TBD: Is this the right thing to do here? if not self.destination: self.destination = config_path @@ -1473,7 +1473,6 @@ def _clone(self, parent_scope: ConfigScope) -> Optional[str]: Args: parent_scope: enclosing scope - Returns: destination path if cloned or ``None`` Raises: @@ -1483,7 +1482,8 @@ def _clone(self, parent_scope: ConfigScope) -> Optional[str]: tty.debug(f"Repository ({self.git}) already cloned to {self.destination}") return self.destination - # TODO/TLD/TBD: Was base_directory called for a git repo before this? + self.destination = self.destination or self.base_directory(self.git, parent_scope) + tty.debug(f"Cloning {self.git} into {self.destination}") with filesystem.working_dir(self.destination, create=True): if not os.path.exists(".git"): @@ -1496,7 +1496,7 @@ def _clone(self, parent_scope: ConfigScope) -> Optional[str]: f"{self.destination}" ) if self.optional: - tty.warning(msg + ". Ignoring optional include.") + tty.warn(msg + ". Ignoring optional include.") else: raise spack.error.ConfigError(msg + f": {e}") diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py index 9ddb811781e948..6b00caa45ae136 100644 --- a/lib/spack/spack/test/config.py +++ b/lib/spack/spack/test/config.py @@ -6,6 +6,7 @@ import io import os import pathlib +import shutil import stat import sys import tempfile @@ -1722,7 +1723,7 @@ def test_included_path_string( assert len(scopes) == 1 assert isinstance(scopes[0], spack.config.SingleFileScope) - # Second pass uses the scopes previously built + # Second pass uses the previously built scope assert include._scopes is not None scopes = include.scopes(parent_scope) captured = capfd.readouterr()[1] @@ -1784,6 +1785,7 @@ def test_included_path_destination( tmp_path: pathlib.Path, mock_fetch_url_text, mock_low_high_config, ensure_debug ): destination = tmp_path / "remotes" + fs.mkdirp(str(destination)) entry = { "path": "https://github.com/path/to/raw/config/config.yaml", "destination": str(destination), @@ -1795,7 +1797,7 @@ def test_included_path_destination( # confirm proper remote include with destination works if sys.platform != "win32": - entry["sha256"] = "805ab8e677f83311a141e948674e5a3ad57d94b54fc126d410a853937ca8255f" + entry["sha256"] = "fdfde5b0a67544eeda35e2351e829de9310a5b407ce6fda160ddb7a25094f663" else: entry["sha256"] = "1e44af11d28bb1ddce335ff90129d6b081faf067930f1bb9b745990c6958d8b6" @@ -1899,9 +1901,6 @@ def test_included_path_git( assert isinstance(include, spack.config.GitIncludePaths) assert not include.optional and include.evaluate_condition() - destination = include.base_directory(include.git) - assert destination and not os.path.exists(destination) - # set up minimal git and repository operations class MockIncludeGit(spack.util.executable.Executable): def __init__(self, required: bool): @@ -1964,7 +1963,7 @@ def test_included_path_local_no_dest(path): """Confirm that local paths have no cache destination.""" entry = {"path": path} include = spack.config.included_path(entry) - destination = include.base_directory(entry["path"]) + destination = include.base_directory(path) assert not destination, f"Expected local include ({include}) to NOT have a cache destination" @@ -2017,7 +2016,6 @@ def test_included_path_git_default_paths( tmp_path: pathlib.Path, mock_low_high_config, ensure_debug, monkeypatch, paths, dest ): """Confirm the default selection of paths.""" - monkeypatch.setattr(spack.paths, "user_cache_path", str(tmp_path)) class MockIncludeGit(spack.util.executable.Executable): def __init__(self, required: bool): @@ -2043,7 +2041,8 @@ def __call__(self, *args, **kwargs) -> str: # type: ignore include = spack.config.included_path(entry) assert isinstance(include, spack.config.GitIncludePaths) - destination = path if dest else include._destination_directory(include.git) + parent_scope = mock_low_high_config.scopes["low"] + destination = path if path else include.base_directory(include.git, parent_scope) # set up minimal git and repository operations monkeypatch.setattr(spack.util.git, "git", MockIncludeGit) @@ -2065,17 +2064,18 @@ def _checkout(*args, **kwargs): monkeypatch.setattr(spack.util.git, "pull_checkout_branch", _checkout) # Ensure the repository is cloned and the paths, if any, are chosen - parent_scope = mock_low_high_config.scopes["low"] - scopes = include.scopes(parent_scope) - assert len(scopes) == len(paths) - for scope in scopes: - assert isinstance(scope, spack.config.SingleFileScope) - assert os.path.basename(scope.path) in paths # type: ignore[union-attr] + try: + scopes = include.scopes(parent_scope) + assert len(scopes) == len(paths) + for scope in scopes: + assert isinstance(scope, spack.config.SingleFileScope) + assert os.path.basename(scope.path) in paths # type: ignore[union-attr] + finally: + # Clean up between parameterized tests. + shutil.rmtree(str(include.destination)) def test_included_path_git_errs(tmp_path: pathlib.Path, mock_low_high_config, monkeypatch): - monkeypatch.setattr(spack.paths, "user_cache_path", str(tmp_path)) - paths = ["concretizer.yaml"] entry = { "git": "https://example.com/linux/configs.git", @@ -2260,18 +2260,18 @@ def test_included_path_unwritable_dest(tmp_path: pathlib.Path, mock_fetch_url_te sha256 = ( "1e44af11d28bb1ddce335ff90129d6b081faf067930f1bb9b745990c6958d8b6" if sys.platform == "win32" - else "805ab8e677f83311a141e948674e5a3ad57d94b54fc126d410a853937ca8255f" + else "fdfde5b0a67544eeda35e2351e829de9310a5b407ce6fda160ddb7a25094f663" ) entry = { "path": "https://github.com/path/to/raw/config/config.yaml", "destination": str(dest), "sha256": sha256, } - include = spack.config.included_path(entry) - parent_scope = spack.config.ConfigScope("FakeScope") try: - with pytest.raises(AssertionError, match="to unwritable"): + include = spack.config.included_path(entry) + parent_scope = spack.config.ConfigScope("fake") + with pytest.raises(AssertionError, match="Unable to write to"): include.scopes(parent_scope) finally: dest.chmod(current_mode) @@ -2290,11 +2290,11 @@ def test_included_path_git_unwritable_dest(tmp_path: pathlib.Path): "paths": ["config.yaml"], "destination": str(dest), } - include = spack.config.included_path(entry) parent_scope = spack.config.ConfigScope("fake") try: - with pytest.raises(AssertionError, match="to unwritable"): + include = spack.config.included_path(entry) + with pytest.raises(spack.error.ConfigError, match="Unable to initialize"): include.scopes(parent_scope) finally: dest.chmod(current_mode) From fa7a1f7c588adeda1a07bf87b911bceaac472b8c Mon Sep 17 00:00:00 2001 From: tldahlgren Date: Tue, 31 Mar 2026 10:47:45 -0700 Subject: [PATCH 04/33] Update windows include tests sha256s Signed-off-by: tldahlgren Signed-off-by: Gregory Becker --- lib/spack/spack/test/config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py index 6b00caa45ae136..7da565e1c75a89 100644 --- a/lib/spack/spack/test/config.py +++ b/lib/spack/spack/test/config.py @@ -1799,7 +1799,7 @@ def test_included_path_destination( if sys.platform != "win32": entry["sha256"] = "fdfde5b0a67544eeda35e2351e829de9310a5b407ce6fda160ddb7a25094f663" else: - entry["sha256"] = "1e44af11d28bb1ddce335ff90129d6b081faf067930f1bb9b745990c6958d8b6" + entry["sha256"] = "4672076f2085f5c266308ce4dfcb026dbb7bd146d5aa4d4e88f1590a1c72c335" include = spack.config.included_path(entry) assert include.scopes(mock_low_high_config.scopes["low"]) @@ -2258,7 +2258,7 @@ def test_included_path_unwritable_dest(tmp_path: pathlib.Path, mock_fetch_url_te dest.chmod(current_mode & ~(stat.S_IWUSR | stat.S_IWGRP | stat.S_IWOTH)) sha256 = ( - "1e44af11d28bb1ddce335ff90129d6b081faf067930f1bb9b745990c6958d8b6" + "4672076f2085f5c266308ce4dfcb026dbb7bd146d5aa4d4e88f1590a1c72c335" if sys.platform == "win32" else "fdfde5b0a67544eeda35e2351e829de9310a5b407ce6fda160ddb7a25094f663" ) From 6a7d512fb1ab620323c6661f0156ccdfefa590ab Mon Sep 17 00:00:00 2001 From: tldahlgren Date: Tue, 31 Mar 2026 10:59:30 -0700 Subject: [PATCH 05/33] Fix unwritable tempdir failure on rhel8 Signed-off-by: tldahlgren Signed-off-by: Gregory Becker --- lib/spack/spack/test/config.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py index 7da565e1c75a89..4c3af195472369 100644 --- a/lib/spack/spack/test/config.py +++ b/lib/spack/spack/test/config.py @@ -2298,3 +2298,6 @@ def test_included_path_git_unwritable_dest(tmp_path: pathlib.Path): include.scopes(parent_scope) finally: dest.chmod(current_mode) + # Ensure the unwritable temp dir is removed to avoid failure on some + # platforms (e.g., rhel8). + shutil.rmtree(include.destination) From 3c54b997e0d8efc20c3347beffd2e7018fe62fe7 Mon Sep 17 00:00:00 2001 From: tldahlgren Date: Tue, 31 Mar 2026 15:56:10 -0700 Subject: [PATCH 06/33] environments.rst: more rebase cleanup Signed-off-by: tldahlgren Signed-off-by: Gregory Becker --- lib/spack/docs/environments.rst | 106 +------------------------------- 1 file changed, 1 insertion(+), 105 deletions(-) diff --git a/lib/spack/docs/environments.rst b/lib/spack/docs/environments.rst index 9d4e3947aa1614..f95961364c5bc9 100644 --- a/lib/spack/docs/environments.rst +++ b/lib/spack/docs/environments.rst @@ -263,6 +263,7 @@ Note that when we installed the abstract spec ``zlib@1.2.8``, it was presented a All explicitly installed packages will be listed as roots of the environment. All of the Spack commands that act on the list of installed specs are environment-aware in this way, including ``install``, ``uninstall``, ``find``, ``extensions``, etc. +In the :ref:`environment-configuration` section we will discuss environment-aware commands further. .. _cmd-spack-add: @@ -294,10 +295,6 @@ or $ spack -e myenv add python -.. note:: - - All environment-aware commands can also be called using the ``spack -e`` flag to specify the environment. - .. _cmd-spack-concretize: Concretizing @@ -597,107 +594,6 @@ It isn't until you run the ``spack concretize`` command that the combined enviro ==> 0 installed packages -.. _environment-configuration: - -Included Concrete Environments ------------------------------- - -Spack environments can create an environment based off of information in already established environments. -You can think of it as a combination of existing environments. -It will gather information from the existing environment's ``spack.lock`` and use that during the creation of this included concrete environment. -When an included concrete environment is created it will generate a ``spack.lock`` file for the newly created environment. - - -Creating included environments -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -To create a combined concrete environment, you must have at least one existing concrete environment. -You will use the command ``spack env create`` with the argument ``--include-concrete`` followed by the name or path of the environment you'd like to include. -Here is an example of how to create a combined environment from the command line. - -.. code-block:: spec - - $ spack env create myenv - $ spack -e myenv add python - $ spack -e myenv concretize - $ spack env create --include-concrete myenv included_env - -You can also include an environment directly in the ``spack.yaml`` file. -It involves adding the ``include_concrete`` heading in the yaml followed by the absolute path to the independent environments. -Note that you may use Spack config variables such as ``$spack`` or environment variables as long as the expression expands to an absolute path. - -.. code-block:: yaml - - spack: - include: - - /absolute/path/to/environment1/spack.lock - - $spack/../path/to/environment2/spack.lock - specs: [] - concretizer: - unify: true - -Once the ``spack.yaml`` has been updated you must concretize the environment to get the concrete specs from the included environments. - -Updating an included environment -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -If changes were made to the base environment and you want that reflected in the included environment you will need to re-concretize both the base environment and the included environment for the change to be implemented. -For example: - -.. code-block:: spec - - $ spack env create myenv - $ spack -e myenv add python - $ spack -e myenv concretize - $ spack env create --include-concrete myenv included_env - - - $ spack -e myenv find - ==> In environment myenv - ==> Root specs - python - - ==> 0 installed packages - - $ spack -e included_env find - ==> In environment included_env - ==> No root specs - ==> Included specs - python - - ==> 0 installed packages - -Here we see that ``included_env`` has access to the python package through the ``myenv`` environment. -But if we were to add another spec to ``myenv``, ``included_env`` will not be able to access the new information. - -.. code-block:: spec - - $ spack -e myenv add perl - $ spack -e myenv concretize - $ spack -e myenv find - ==> In environment myenv - ==> Root specs - perl python - - ==> 0 installed packages - $ spack -e included_env find - ==> In environment included_env - ==> No root specs - ==> Included specs - python - - ==> 0 installed packages - -It isn't until you run the ``spack concretize`` command that the combined environment will get the updated information from the re-concretized base environment. - -.. code-block:: console - - $ spack -e included_env concretize - $ spack -e included_env find - ==> In environment included_env - ==> No root specs - ==> Included specs - perl python - - ==> 0 installed packages .. _environment-configuration: From 92c79e78883290544894c73f454d2e64d22af322 Mon Sep 17 00:00:00 2001 From: tldahlgren Date: Wed, 1 Apr 2026 18:50:13 -0700 Subject: [PATCH 07/33] Remove included_env_config and address selected style feedback Signed-off-by: tldahlgren Signed-off-by: Gregory Becker --- lib/spack/spack/config.py | 17 ++++--- lib/spack/spack/environment/__init__.py | 2 - lib/spack/spack/environment/environment.py | 19 -------- lib/spack/spack/test/cmd/env.py | 52 +++++----------------- lib/spack/spack/test/config.py | 3 +- lib/spack/spack/test/env.py | 2 +- 6 files changed, 23 insertions(+), 72 deletions(-) diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index c15175b4195e74..e959074273134d 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -156,11 +156,7 @@ def get_includes(self) -> Optional[YamlConfigDict]: tty.debug(f"Retrieved configuration includes for {self.name}: {includes}", level=3) return includes - spack = self.get_section("spack") - if spack: - includes = spack["spack"] - tty.debug(f"Retrieved environment includes for {self.name}: {includes}", level=3) - return includes + return None @property def included_scopes(self) -> List["ConfigScope"]: @@ -1264,10 +1260,17 @@ def _scope( tty.debug(f"Creating SingleFileScope {config_name} for '{config_path}'") if os.path.basename(config_path) == "spack.yaml": schema = spack.schema.env.schema + yaml_path = ["spack"] else: schema = spack.schema.merged.schema + yaml_path = None return SingleFileScope( - config_name, config_path, schema, prefer_modify=self.prefer_modify, included=True + config_name, + config_path, + schema, + yaml_path=yaml_path, + prefer_modify=self.prefer_modify, + included=True, ) elif exists: raise ValueError( @@ -1359,7 +1362,7 @@ def __init__(self, entry: dict): def __repr__(self): return ( - f"IncludePath('{self.name}', '{self.path}', sha256={self.sha256}, " + f"IncludePath('{self.name}', path='{self.path}', sha256={self.sha256}, " f"when='{self.when}', optional={self.optional}, " f"destination={self.destination})" ) diff --git a/lib/spack/spack/environment/__init__.py b/lib/spack/spack/environment/__init__.py index 16cd1bc0afa273..fe3c425065ff2d 100644 --- a/lib/spack/spack/environment/__init__.py +++ b/lib/spack/spack/environment/__init__.py @@ -618,7 +618,6 @@ environment_from_name_or_dir, environment_path_scope, exists, - included_env_config, initialize_environment_dir, installed_specs, is_env_dir, @@ -659,7 +658,6 @@ "environment_from_name_or_dir", "environment_path_scope", "exists", - "included_env_config", "initialize_environment_dir", "installed_specs", "is_env_dir", diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index a8e08833dc0e59..e892031eb9b9d8 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -979,17 +979,6 @@ def from_info_dict(info_dict: Dict[str, str]) -> "ConcretizedRootInfo": ) -def included_env_config(key: str, default: Optional[Any] = None) -> Any: - """Extract the included environment configuration for the key. - - Args: - key: environment manifest configuration key - default: optional default value if key is not detected under ``spack:`` - """ - data = spack.config.CONFIG.get(TOP_LEVEL_KEY, {}) - return data.get(key, default) - - class Environment: """A Spack environment, which bundles together configuration and a list of specs.""" @@ -1214,14 +1203,12 @@ def _construct_state_from_manifest(self): def _sync_speclists(self): # Combined toolchains from global config and included environments. toolchains = spack.config.CONFIG.get("toolchains", {}) - toolchains.update(included_env_config("toolchains", {})) self._spec_lists_parser = SpecListParser(toolchains=toolchains) # Combined definitions from the global config and included environments. self.spec_lists = {} combined_yaml = [] combined_yaml.extend(spack.config.CONFIG.get("definitions", [])) - combined_yaml.extend(included_env_config("definitions", [])) self.spec_lists.update(self._spec_lists_parser.parse_definitions(data=combined_yaml)) # TODO/TLD/TBD: Make sure process groups from included manifests @@ -1231,12 +1218,6 @@ def _sync_speclists(self): self.spec_lists[key] = self._spec_lists_parser.parse_user_specs( name=key, yaml_list=self.manifest.user_specs(group=group) ) - # TODO/TLD/TBD: Resolve this correctly - # self.spec_lists[key].extend( - # self._spec_lists_parser.parse_user_specs( - # name=key, yaml_list=included_env_config.user_specs(group=group) - # ) - # ) def _user_specs_key(self, *, group: Optional[str] = None) -> str: if group is None or group == DEFAULT_USER_SPEC_GROUP: diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index 6fc2e8d53ce4ac..78697f357c452c 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -1103,16 +1103,12 @@ def test_init_from_yaml_relative_includes(tmp_path: pathlib.Path): e2 = ev.create("test2", init_file=str(e1_manifest)) - # Check new environment's (manifest) includes + # Check new environment's (manifest) includes, all of which are simple + # file includes. new_env_includes = e2.manifest.configuration.get("include", []) for orig, new in zip(files, new_env_includes): assert new.endswith(orig.strip("./")) - # Check configured environment's includes - config_includes = ev.included_env_config("include", []) - for orig, new in zip(files, config_includes): - assert new.endswith(orig.strip("./")) - def test_init_from_yaml_relative_includes_outside_env(tmp_path: pathlib.Path): """Ensure relative includes to files outside the environment fail.""" @@ -4751,54 +4747,28 @@ def test_env_include_envs(tmp_path, mock_packages, mutable_config, environment_f """ specs = ["libdwarf", "mpileaks"] - includes = [] + paths = [] includes_dir = tmp_path / "includes" for pkg in specs: include = includes_dir / pkg / ev.manifest_name fs.mkdirp(include.parent) include.write_text(specs_template.format(pkg)) - includes.append(str(include)) + paths.append(str(include)) include_condition = 'platform == "test"' - # TODO: Remove this once minimum python is python@3.9 - def remove_prefix(text, prefix): - if text.startswith(prefix): - return text[len(prefix) :] - return text - - def include_entry(path): - return f"- path: {path}\n when: {include_condition}" - - prefix = str(tmp_path) + os.sep - # TODO: Once minimum python is python@3.9 - # rel_paths = [include.removeprefix(prefix) for include in includes] - rel_paths = [remove_prefix(include, prefix) for include in includes] - paths_str = "\n ".join([include_entry(path) for path in rel_paths]) e = environment_from_manifest( f"""\ spack: include: - {paths_str} + - path: {paths[0]} + when: {include_condition} + - path: {paths[1]} + when: {include_condition} """ ) - for spec in specs: - assert Spec(spec) in e.user_specs - - # Confirm the relative included paths were replaced with absolute paths - # and the include condition is unchanged. - env_includes = e.manifest[ev.TOP_LEVEL_KEY].get("include", []) - for orig, include_path in zip(rel_paths, env_includes): - assert not os.path.isabs(orig) - - full_path = include_path["path"] - assert os.path.isabs(full_path) - # TODO: Once minimum python is python@3.9 - # assert full_path.removeprefix(prefix) == orig - assert remove_prefix(full_path, prefix) - - assert include_path["when"] == include_condition + assert [Spec(s) for s in specs] == e.user_specs.specs def test_env_include_env_pkgs_def( @@ -4830,9 +4800,7 @@ def test_env_include_env_pkgs_def( e = environment_from_manifest(env_yaml) specs = ["libelf@0.8.10", "mpileaks", "libdwarf"] - user_specs = e.user_specs - for spec in specs: - assert Spec(spec) in user_specs + assert [Spec(s) for s in specs] == e.user_specs.specs # Confirm manifest contents include explicit and not included specs e.write() diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py index 4c3af195472369..7ed8e804da531b 100644 --- a/lib/spack/spack/test/config.py +++ b/lib/spack/spack/test/config.py @@ -2300,4 +2300,5 @@ def test_included_path_git_unwritable_dest(tmp_path: pathlib.Path): dest.chmod(current_mode) # Ensure the unwritable temp dir is removed to avoid failure on some # platforms (e.g., rhel8). - shutil.rmtree(include.destination) + if include.destination: + shutil.rmtree(include.destination) # type: ignore[arg-type] diff --git a/lib/spack/spack/test/env.py b/lib/spack/spack/test/env.py index 50d4c0590bfe09..5ccf2aaf712b19 100644 --- a/lib/spack/spack/test/env.py +++ b/lib/spack/spack/test/env.py @@ -2143,7 +2143,7 @@ def test_unified_environment_with_mixed_compilers_and_fortran(tmp_path, config): def test_env_include_env_includes(tmp_path, mutable_config): """Confirm that an environment that includes two other environments, each including their own configuration, picks up the specs from both - but its own configuration setting.""" + but prefers its configuration setting.""" includes_dir = tmp_path / "includes" fs.mkdirp(includes_dir) From 2ed364f95a8a4873fb70a988ce73ad33a8fd2cad Mon Sep 17 00:00:00 2001 From: tldahlgren Date: Thu, 2 Apr 2026 11:26:58 -0700 Subject: [PATCH 08/33] Restore test_config_include_similar_name checks Signed-off-by: tldahlgren Signed-off-by: Gregory Becker --- lib/spack/spack/test/config.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py index 7ed8e804da531b..f57d6954edb883 100644 --- a/lib/spack/spack/test/config.py +++ b/lib/spack/spack/test/config.py @@ -1674,7 +1674,8 @@ def test_config_include_similar_name(tmp_path: pathlib.Path): # Ensure all of the scopes are found assert len(config.matching_scopes("^test$")) == 1 - assert len(config.matching_scopes("^test:")) == 2 + assert len(config.matching_scopes("^test:a/config$")) == 1 + assert len(config.matching_scopes("^test:b/config$")) == 1 def test_deepcopy_as_builtin(env_yaml): From 1e74429ea84f5458463d575e079086f2a3cc400b Mon Sep 17 00:00:00 2001 From: tldahlgren Date: Thu, 2 Apr 2026 17:53:23 -0700 Subject: [PATCH 09/33] specs.yaml: added schema support but not use Signed-off-by: tldahlgren Signed-off-by: Gregory Becker --- lib/spack/spack/config.py | 4 +- lib/spack/spack/environment/environment.py | 20 ++++--- lib/spack/spack/schema/env.py | 54 ----------------- lib/spack/spack/schema/merged.py | 2 + lib/spack/spack/schema/spec_list.py | 35 +++++++++++ lib/spack/spack/schema/specs.py | 67 ++++++++++++++++++++++ lib/spack/spack/test/cmd/env.py | 25 +++++++- share/spack/spack-completion.fish | 8 +-- 8 files changed, 146 insertions(+), 69 deletions(-) create mode 100644 lib/spack/spack/schema/specs.py diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index e959074273134d..314a1102f271d6 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -63,6 +63,7 @@ import spack.schema.modules import spack.schema.packages import spack.schema.repos +import spack.schema.specs import spack.schema.toolchains import spack.schema.upstreams import spack.schema.view @@ -98,6 +99,7 @@ "cdash": spack.schema.cdash.schema, "toolchains": spack.schema.toolchains.schema, "spack": spack.schema.env.schema, + "specs": spack.schema.specs.schema, } #: Path to the main configuration scope @@ -156,7 +158,7 @@ def get_includes(self) -> Optional[YamlConfigDict]: tty.debug(f"Retrieved configuration includes for {self.name}: {includes}", level=3) return includes - return None + return None @property def included_scopes(self) -> List["ConfigScope"]: diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index e892031eb9b9d8..b6fc9801f14c35 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -1198,22 +1198,26 @@ def _construct_state_from_manifest(self): self._process_view(spack.config.get("view", True)) self._process_included_lockfiles() - # TODO/TLD/TBD: Properly resolve the conflicts related to adding included - # TODO/TLD/TBD: user specs + # TODO/50207: Properly resolve the conflicts related to adding included + # TODO/50207: user specs. def _sync_speclists(self): - # Combined toolchains from global config and included environments. toolchains = spack.config.CONFIG.get("toolchains", {}) self._spec_lists_parser = SpecListParser(toolchains=toolchains) - # Combined definitions from the global config and included environments. self.spec_lists = {} - combined_yaml = [] - combined_yaml.extend(spack.config.CONFIG.get("definitions", [])) - self.spec_lists.update(self._spec_lists_parser.parse_definitions(data=combined_yaml)) + self.spec_lists.update( + self._spec_lists_parser.parse_definitions( + data=spack.config.CONFIG.get("definitions", []) + ) + ) - # TODO/TLD/TBD: Make sure process groups from included manifests + # TODO/50207: Fix this to handle grabbing specs though not clear yet + # TODO/50207: how useful grabbing them from spack.config.CONFIG is + # TODO/50207: since it requires appropriate substitutions (e.g., + # TODO/50207: definitions) and group handling. for group in self.manifest.groups(): tty.debug(f"[{__name__}]: Synchronizing user specs from the '{group}' group", level=2) + key = self._user_specs_key(group=group) self.spec_lists[key] = self._spec_lists_parser.parse_user_specs( name=key, yaml_list=self.manifest.user_specs(group=group) diff --git a/lib/spack/spack/schema/env.py b/lib/spack/spack/schema/env.py index 0a0c59043ec841..0949e308996c19 100644 --- a/lib/spack/spack/schema/env.py +++ b/lib/spack/spack/schema/env.py @@ -13,8 +13,6 @@ import spack.schema.merged -from .spec_list import spec_list_properties, spec_list_schema - #: Top level key in a manifest file TOP_LEVEL_KEY = "spack" @@ -28,27 +26,6 @@ "items": {"type": "string"}, } -group_name_and_deps = { - "group": {"type": "string", "description": "Name for this group of specs"}, - "explicit": { - "type": "boolean", - "default": True, - "description": "When false, specs in this group are installed as implicit " - "dependencies and are eligible for garbage collection.", - }, - "needs": { - "type": "array", - "description": "Groups of specs that are needed by this group", - "items": {"type": "string"}, - }, - "override": { - "type": "object", - "description": "Top-most configuration scope for this group of specs", - "additionalProperties": False, - "properties": {**spack.schema.merged.ref_sections}, - }, -} - properties: Dict[str, Any] = { "spack": { @@ -61,37 +38,6 @@ # merged configuration scope schemas **spack.schema.merged.ref_sections, # extra environment schema properties - "specs": { - "type": "array", - "description": "List of specs to include in the environment, " - "supporting both simple specs and matrix configurations", - "default": [], - "items": { - "anyOf": [ - { - "type": "object", - "description": "Matrix configuration for generating multiple specs" - " from combinations of constraints", - "additionalProperties": False, - "properties": {**spec_list_properties}, - }, - {"type": "string", "description": "Simple spec string"}, - {"type": "null"}, - { - "type": "object", - "description": "User spec group with a single matrix", - "additionalProperties": False, - "properties": {**spec_list_properties, **group_name_and_deps}, - }, - { - "type": "object", - "description": "User spec group with multiple matrices", - "additionalProperties": False, - "properties": {**group_name_and_deps, "specs": spec_list_schema}, - }, - ] - }, - }, # (DEPRECATED) include concrete to be merged under the include key "include_concrete": include_concrete, # nested environments diff --git a/lib/spack/spack/schema/merged.py b/lib/spack/spack/schema/merged.py index 16c6144a554aed..9c5f9d0bcc23e9 100644 --- a/lib/spack/spack/schema/merged.py +++ b/lib/spack/spack/schema/merged.py @@ -27,6 +27,7 @@ import spack.schema.packages import spack.schema.projections import spack.schema.repos +import spack.schema.specs import spack.schema.toolchains import spack.schema.upstreams import spack.schema.view @@ -48,6 +49,7 @@ **spack.schema.modules.properties, **spack.schema.packages.properties, **spack.schema.repos.properties, + **spack.schema.specs.properties, **spack.schema.toolchains.properties, **spack.schema.upstreams.properties, **spack.schema.view.properties, diff --git a/lib/spack/spack/schema/spec_list.py b/lib/spack/spack/schema/spec_list.py index 179b5597010fd0..7dba135397b616 100644 --- a/lib/spack/spack/schema/spec_list.py +++ b/lib/spack/spack/schema/spec_list.py @@ -11,6 +11,29 @@ }, } + +group_name_and_deps = { + "group": {"type": "string", "description": "Name for this group of specs"}, + "explicit": { + "type": "boolean", + "default": True, + "description": "When false, specs in this group are installed as implicit " + "dependencies and are eligible for garbage collection.", + }, + "needs": { + "type": "array", + "description": "Groups of specs that are needed by this group", + "items": {"type": "string"}, + }, + "override": { + "type": "object", + "description": "Top-most configuration scope for this group of specs", + "additionalProperties": False, + "properties": {**spack.schema.merged.ref_sections}, + }, +} + + spec_list_properties = { "matrix": matrix_schema, "exclude": { @@ -36,6 +59,18 @@ }, {"type": "string", "description": "Simple spec string"}, {"type": "null"}, + { + "type": "object", + "description": "User spec group with a single matrix", + "additionalProperties": False, + "properties": {**spec_list_properties, **group_name_and_deps}, + }, + { + "type": "object", + "description": "User spec group with multiple matrices", + "additionalProperties": False, + "properties": {**group_name_and_deps, "specs": spec_list_schema}, + }, ] }, } diff --git a/lib/spack/spack/schema/specs.py b/lib/spack/spack/schema/specs.py new file mode 100644 index 00000000000000..3a084160594625 --- /dev/null +++ b/lib/spack/spack/schema/specs.py @@ -0,0 +1,67 @@ +# Copyright Spack Project Developers. See COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) +"""Schema for specs.yaml configuration file. + +.. literalinclude:: _spack_root/lib/spack/spack/schema/specs.py + :lines: 14- +""" +from typing import Any, Dict + +from .spec_list import spec_list_properties, spec_list_schema + +#: Properties for inclusion in other schemas +group_name_and_deps = { + "group": {"type": "string", "description": "Name for this group of specs"}, + "needs": { + "type": "array", + "description": "Groups of specs that are needed by this group", + "items": {"type": "string"}, + }, + "override": { + "description": "Top-most configuration scope for this group of specs", + "$ref": "#/properties", + }, +} + +properties: Dict[str, Any] = { + "specs": { + "type": "array", + "description": "List of specs, supporting both simple specs and matrix configurations", + "default": [], + "items": { + "anyOf": [ + { + "type": "object", + "description": "Matrix configuration for generating multiple specs" + " from combinations of constraints", + "additionalProperties": False, + "properties": {**spec_list_properties}, + }, + {"type": "string", "description": "Simple spec string"}, + {"type": "null"}, + { + "type": "object", + "description": "User spec group with a single matrix", + "additionalProperties": False, + "properties": {**spec_list_properties, **group_name_and_deps}, + }, + { + "type": "object", + "description": "User spec group with multiple matrices", + "additionalProperties": False, + "properties": {**group_name_and_deps, "specs": spec_list_schema}, + }, + ] + }, + } +} + +#: Full schema with metadata +schema = { + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "Spack specs configuration file schema", + "type": "object", + "additionalProperties": False, + "properties": properties, +} diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index 78697f357c452c..b45d28718aeadf 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -4768,7 +4768,17 @@ def test_env_include_envs(tmp_path, mock_packages, mutable_config, environment_f """ ) - assert [Spec(s) for s in specs] == e.user_specs.specs + expected = [Spec(s) for s in specs] + with e: + # TODO/50207: REMOVE global "specs" extraction + # TODO/50207: The following check fails because "$packages" + # TODO/50207: shows up in the raw "specs" list instead of "libdwarf". + # config_specs = spack.config.CONFIG.get("specs", []) + # assert config_specs == expected + + # TODO/50207: The environment needs to be changed s.t. user specs + # TODO/50207: from included environments are properly included. + assert e.user_specs.specs == expected def test_env_include_env_pkgs_def( @@ -4800,7 +4810,18 @@ def test_env_include_env_pkgs_def( e = environment_from_manifest(env_yaml) specs = ["libelf@0.8.10", "mpileaks", "libdwarf"] - assert [Spec(s) for s in specs] == e.user_specs.specs + expected = [Spec(s) for s in specs] + + with e: + # TODO/50207: REMOVE global "specs" extraction + # TODO/50207: The following check fails because "$packages" + # TODO/50207: shows up in the raw "specs" list instead of "libdwarf". + # config_specs = spack.config.CONFIG.get("specs", []) + # assert config_specs == expected + + # TODO/50207: The environment needs to be changed s.t. user specs + # TODO/50207: from included environments are properly included. + assert e.user_specs.specs == expected # Confirm manifest contents include explicit and not included specs e.write() diff --git a/share/spack/spack-completion.fish b/share/spack/spack-completion.fish index b6fc5fd330677b..bbde7c8264e43d 100644 --- a/share/spack/spack-completion.fish +++ b/share/spack/spack-completion.fish @@ -1270,7 +1270,7 @@ 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 json group= -complete -c spack -n '__fish_spack_using_command_pos 0 config get' -f -a 'bootstrap cdash ci compilers concretizer config definitions develop env_vars include mirrors modules packages repos spack toolchains upstreams view' +complete -c spack -n '__fish_spack_using_command_pos 0 config get' -f -a 'bootstrap cdash ci compilers concretizer config definitions develop env_vars include mirrors modules packages repos spack specs toolchains upstreams view' 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' complete -c spack -n '__fish_spack_using_command config get' -l json -f -a json @@ -1280,7 +1280,7 @@ complete -c spack -n '__fish_spack_using_command config get' -l group -r -d 'sho # spack config blame set -g __fish_spack_optspecs_spack_config_blame h/help group= -complete -c spack -n '__fish_spack_using_command_pos 0 config blame' -f -a 'bootstrap cdash ci compilers concretizer config definitions develop env_vars include mirrors modules packages repos spack toolchains upstreams view' +complete -c spack -n '__fish_spack_using_command_pos 0 config blame' -f -a 'bootstrap cdash ci compilers concretizer config definitions develop env_vars include mirrors modules packages repos spack specs toolchains upstreams view' 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' complete -c spack -n '__fish_spack_using_command config blame' -l group -r -f -a group @@ -1288,7 +1288,7 @@ complete -c spack -n '__fish_spack_using_command config blame' -l group -r -d 's # 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 develop env_vars include mirrors modules packages repos spack toolchains upstreams view' +complete -c spack -n '__fish_spack_using_command_pos 0 config edit' -f -a 'bootstrap cdash ci compilers concretizer config definitions develop env_vars include mirrors modules packages repos spack specs toolchains upstreams view' 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 @@ -1301,7 +1301,7 @@ complete -c spack -n '__fish_spack_using_command config list' -s h -l help -d 's # spack config scopes set -g __fish_spack_optspecs_spack_config_scopes h/help p/paths t/type= v/verbose -complete -c spack -n '__fish_spack_using_command_pos 0 config scopes' -f -a 'bootstrap cdash ci compilers concretizer config definitions develop env_vars include mirrors modules packages repos spack toolchains upstreams view' +complete -c spack -n '__fish_spack_using_command_pos 0 config scopes' -f -a 'bootstrap cdash ci compilers concretizer config definitions develop env_vars include mirrors modules packages repos spack specs toolchains upstreams view' complete -c spack -n '__fish_spack_using_command config scopes' -s h -l help -f -a help complete -c spack -n '__fish_spack_using_command config scopes' -s h -l help -d 'show this help message and exit' complete -c spack -n '__fish_spack_using_command config scopes' -s p -l paths -f -a paths From 7ce0b85db9828f57edd2fecae9f35f89d03ab640 Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Thu, 16 Apr 2026 08:52:20 -0700 Subject: [PATCH 10/33] cleanup schema Signed-off-by: Gregory Becker Signed-off-by: Gregory Becker --- lib/spack/spack/schema/env.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/spack/spack/schema/env.py b/lib/spack/spack/schema/env.py index 0949e308996c19..e5d308b0835c1a 100644 --- a/lib/spack/spack/schema/env.py +++ b/lib/spack/spack/schema/env.py @@ -40,8 +40,6 @@ # extra environment schema properties # (DEPRECATED) include concrete to be merged under the include key "include_concrete": include_concrete, - # nested environments - "spack": {"$ref": "#/spack/properties"}, }, } } From 0c0e78f2642b33f000e89219f849c948a6d82783 Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Fri, 17 Apr 2026 13:52:41 -0500 Subject: [PATCH 11/33] wip Signed-off-by: Gregory Becker Signed-off-by: Gregory Becker --- lib/spack/spack/environment/environment.py | 103 ++++++++++++++------- lib/spack/spack/test/cmd/env.py | 2 +- 2 files changed, 69 insertions(+), 36 deletions(-) diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index b6fc9801f14c35..be7d1da5397330 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -1072,7 +1072,7 @@ def __setstate__(self, state): def _re_read(self): """Reinitialize the environment object.""" - self.clear() + self.clear(to_disk=False) self._load_manifest_file() def _read(self): @@ -1276,7 +1276,7 @@ def add_root_specs(included_concrete_specs): add_root_specs(self.included_concrete_spec_data) return spec_list - def clear(self): + def clear(self, to_disk=True): """Clear the contents of the environment""" self.spec_lists = {} self._dev_specs = {} @@ -1290,7 +1290,7 @@ def clear(self): self.invalidate_repository_cache() self._previous_active = None # previously active environment - self.manifest.clear() + self.manifest.clear(to_disk=to_disk) @property def active(self): @@ -3057,6 +3057,12 @@ def new_include(included_path: spack.config.IncludePath, new_path: str): # error handling for bad manifests is handled on other code paths return + # TODO GBB: Use the config system for this + # This probably requires overriding the include config + # but still reading from the environment scope + # (so that we can construct the scope without the bad includes) + # then we can do the modifications, pop the scope, and add it back with includes + # Override relative paths for IncludePath entries. # GitIncludePaths should NOT be overriden. includes = manifest[TOP_LEVEL_KEY].get(manifest_include_name, []) @@ -3166,7 +3172,8 @@ def __init__(self, manifest_dir: Union[pathlib.Path, str], name: Optional[str] = self.changed = False def _init_user_specs(self): - specs_yaml = self.configuration.get(USER_SPECS_KEY, []) + with self.use_config(): + specs_yaml = spack.config.CONFIG.get("specs", []) for item in specs_yaml: if isinstance(item, str): self._user_specs[DEFAULT_USER_SPEC_GROUP].append(item) @@ -3209,7 +3216,7 @@ def _all_matches(self, user_spec: str) -> List[str]: ValueError: if no equivalent match is found """ result = [] - for yaml_spec_str in self.configuration["specs"]: + for yaml_spec_str in spack.config.CONFIG.get("specs", [], scope=self.scope_name): if Spec(yaml_spec_str) == Spec(user_spec): result.append(yaml_spec_str) @@ -3262,23 +3269,26 @@ def add_user_spec(self, user_spec: str, *, group: Optional[str] = None) -> None: user_spec: user spec to be appended group: group where the spec should be added. If None, the default group is used. """ - group = group or DEFAULT_USER_SPEC_GROUP + with self.use_config(): + group = group or DEFAULT_USER_SPEC_GROUP + specs_yaml = spack.config.CONFIG.get("specs", [], scope=self.scope_name) - if group == DEFAULT_USER_SPEC_GROUP: - # Append to top-most specs: attribute - specs_yaml = self.configuration.setdefault("specs", []) - specs_yaml.append(user_spec) - else: - # Append to specs: attribute within a group - group_in_yaml = self._get_group(group) - group_in_yaml.setdefault("specs", []).append(user_spec) + if group == DEFAULT_USER_SPEC_GROUP: + # Append to top-most specs: attribute + specs_yaml.append(user_spec) + else: + # Append to specs: attribute within a group + self._add_to_group(group, user_spec, specs_yaml) + spack.config.CONFIG.set("specs", specs_yaml, scope=self.scope_name) self._user_specs[group].append(user_spec) self.changed = True - def _get_group(self, group: str) -> Dict: - """Find or create the group entry in the manifest""" - specs_yaml = self.configuration.setdefault("specs", []) + def _add_to_group(self, group: str, addition: str, specs_yaml: Dict) -> Dict: + """Find or create a new group in specs_yaml and add to it + + Does the yaml addition, not the in-memory addition""" + specs_yaml = spack.config.CONFIG.get("specs", [], scope=self.scope_name) group_entry = None for item in specs_yaml: if isinstance(item, dict) and item.get("group") == group: @@ -3293,7 +3303,7 @@ def _get_group(self, group: str) -> Dict: self._user_specs[group] = [] self._explicit[group] = True - return group_entry + group_entry["specs"].setdefault([]).append(addition) def remove_user_spec(self, user_spec: str) -> None: """Removes the user spec passed as input from the default list of root specs @@ -3305,17 +3315,22 @@ def remove_user_spec(self, user_spec: str) -> None: SpackEnvironmentError: when the user spec is not in the list """ try: - for key in self._all_matches(user_spec): - self.configuration["specs"].remove(key) - self._user_specs[DEFAULT_USER_SPEC_GROUP].remove(key) + with self.use_config(): + for key in self._all_matches(user_spec): + specs_yaml = spack.config.CONFIG.get("specs", [], scope=self.scope_name) + specs_yaml.remove(key) + spack.config.CONFIG.set("specs", specs_yaml, scope=self.scope_name) + self._user_specs[DEFAULT_USER_SPEC_GROUP].remove(key) except ValueError as e: msg = f"cannot remove {user_spec} from {self}, no such spec exists" raise SpackEnvironmentError(msg) from e self.changed = True - def clear(self) -> None: + def clear(self, to_disk=True) -> None: """Clear all user specs from the list of root specs""" - self.configuration["specs"] = [] + if to_disk: + with self.use_config(): + spack.config.CONFIG.set("specs", [], scope=self.scope_name) self._clear_user_specs() self.changed = True @@ -3330,7 +3345,9 @@ def override_user_spec(self, user_spec: str, idx: int) -> None: SpackEnvironmentError: when the user spec cannot be overridden """ try: - self.configuration["specs"][idx] = user_spec + specs_yaml = spack.config.CONFIG.get("specs", [], scope=self.scope_name) + specs_yaml[idx] = user_spec + spack.config.CONFIG.set("specs", specs_yaml, scope=self.scope_name) self._clear_user_specs() self._init_user_specs() except ValueError as e: @@ -3380,8 +3397,9 @@ def override_include(self, include: Union[str, dict], idx: int) -> None: SpackEnvironmentError: when the include cannot be overridden """ try: - current_include = self.configuration["include"][idx] - self.configuration["include"][idx] = include + current_include = spack.config.CONFIG.get("include", [], scope=self.scope_name) + current_include[idx] = include + spack.config.CONFIG.set("include", current_include, scope=self.scope_name) except ValueError as e: msg = f"cannot override '{current_include}' with '{include}'" raise SpackEnvironmentError(msg) from e @@ -3489,21 +3507,26 @@ def set_default_view(self, view: Union[bool, str, pathlib.Path, Dict[str, str]]) True the default view is used for the environment, if False there's no view. """ if isinstance(view, dict): - self.configuration["view"][default_view_name].update(view) + with self.use_config(): + views = spack.config.CONFIG.get("view", {}, scope=self.scope_name) + views[default_view_name].update(view) + spack.config.CONFIG.set("view", views, scope=self.scope_name) self.changed = True return if not isinstance(view, bool): view = str(view) - self.configuration["view"] = view + with self.use_config(): + spack.config.CONFIG.set("view", view, scope=self.scope_name) self.changed = True def remove_default_view(self) -> None: """Removes the default view from the manifest file""" - view_data = self.configuration.get("view") + view_data = spack.config.CONFIG.get("view", {}, scope=self.scope_name) if isinstance(view_data, collections.abc.Mapping): - self.configuration["view"].pop(default_view_name) + view_data.pop(default_view_name) + spack.config.CONFIG.set("view", view_data) self.changed = True return @@ -3514,8 +3537,8 @@ def flush(self) -> None: if not self.changed: return - with fs.write_tmp_and_move(os.path.realpath(self.manifest_file)) as f: - _write_yaml(self.yaml_content, f) + # with fs.write_tmp_and_move(os.path.realpath(self.manifest_file)) as f: + # _write_yaml(self.yaml_content, f) self.changed = False @property @@ -3548,8 +3571,18 @@ def env_config_scope(self) -> spack.config.ConfigScope: ensure_no_disallowed_env_config_mods(self._env_config_scope) return self._env_config_scope - def prepare_config_scope(self) -> None: + def prepare_config_scope(self, includes=True) -> None: """Add the manifest's scope to the global configuration search path.""" + if includes: + scope = self.env_config_scope + else: + scope = spack.config.SingleFileScope( + self.scope_name, + str(self.manifest_file), + spack.schema.env.schema, + yaml_path=[TOP_LEVEL_KEY], + ) + scope._included_scopes = [] # TODO make this less hacky spack.config.CONFIG.push_scope( self.env_config_scope, priority=ConfigScopePriority.ENVIRONMENT ) @@ -3559,10 +3592,10 @@ def deactivate_config_scope(self) -> None: spack.config.CONFIG.remove_scope(self.env_config_scope.name) @contextlib.contextmanager - def use_config(self): + def use_config(self, includes=True): """Ensure only the manifest's configuration scopes are global.""" with no_active_environment(): - self.prepare_config_scope() + self.prepare_config_scope(includes=includes) yield self.deactivate_config_scope() diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index b45d28718aeadf..e29643d1c39c29 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -814,7 +814,7 @@ def test_bad_remove_included_env(): env("create", "--include-concrete", "test", "combined_env") with pytest.raises(SpackCommandError): - env("remove", "test") + env("remove", "-y", "test") def test_force_remove_included_env(): From c8e1e5a68fa18061de31c3187013fc2b79b7bc81 Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Mon, 11 May 2026 14:57:27 -0700 Subject: [PATCH 12/33] typo fix Signed-off-by: Gregory Becker --- lib/spack/spack/environment/environment.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index be7d1da5397330..cccba3fe5fb48b 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -3303,7 +3303,7 @@ def _add_to_group(self, group: str, addition: str, specs_yaml: Dict) -> Dict: self._user_specs[group] = [] self._explicit[group] = True - group_entry["specs"].setdefault([]).append(addition) + group_entry["specs"].append(addition) def remove_user_spec(self, user_spec: str) -> None: """Removes the user spec passed as input from the default list of root specs From eac6b9be7507a07fd74c94bd4478231ed13664ee Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Mon, 11 May 2026 16:24:05 -0700 Subject: [PATCH 13/33] fixup Signed-off-by: Gregory Becker --- lib/spack/spack/schema/spec_list.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/spack/spack/schema/spec_list.py b/lib/spack/spack/schema/spec_list.py index 7dba135397b616..c061a78782c61d 100644 --- a/lib/spack/spack/schema/spec_list.py +++ b/lib/spack/spack/schema/spec_list.py @@ -1,6 +1,8 @@ # Copyright Spack Project Developers. See COPYRIGHT file for details. # # SPDX-License-Identifier: (Apache-2.0 OR MIT) +import spack.schema.merged + matrix_schema = { "type": "array", "description": "List of spec constraint lists whose cross product generates multiple specs", @@ -11,7 +13,6 @@ }, } - group_name_and_deps = { "group": {"type": "string", "description": "Name for this group of specs"}, "explicit": { @@ -33,7 +34,6 @@ }, } - spec_list_properties = { "matrix": matrix_schema, "exclude": { From 02bfa85a101570734ce9b4c3a2f6e693d6ca23e8 Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Tue, 12 May 2026 14:38:37 -0700 Subject: [PATCH 14/33] refactor config included scopes to be easier to use in environments Signed-off-by: Gregory Becker --- lib/spack/spack/config.py | 93 ++++++++++++++++++++++++++------------- 1 file changed, 62 insertions(+), 31 deletions(-) diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index 314a1102f271d6..888823d6fc1626 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -79,6 +79,9 @@ from .enums import ConfigScopePriority + +ScopeLike = Union["ConfigScope", "IncludedLockfile"] + #: Dict from section names -> schema for that section SECTION_SCHEMAS: Dict[str, Any] = { "compilers": spack.schema.compilers.schema, @@ -162,37 +165,53 @@ def get_includes(self) -> Optional[YamlConfigDict]: @property def included_scopes(self) -> List["ConfigScope"]: - """Memoized list of included scopes, in the order they appear in this scope.""" if self._included_scopes is None: - self._included_scopes = [] - - includes = self.get_includes() - if includes: - if "include" not in includes: - return self._included_scopes - - include_paths = [included_path(data) for data in includes["include"]] - tty.debug( - f"Processing included paths: {[str(path) for path in include_paths]}", level=3 - ) - included_scopes = chain(*[include.scopes(self) for include in include_paths]) - - for included_scope in included_scopes: - # Do not include duplicate scopes - if any([included_scope.name == scope.name for scope in self._included_scopes]): - tty.warn(f"Ignoring duplicate included scope: {included_scope.name}") - continue - - if included_scope not in self._included_scopes: - self._included_scopes.append(included_scope) + self._read_included_scopes() tty.debug( f"{self.name} has {'' if len(self._included_scopes) else 'no '}" f"included scopes: {self._included_scopes}", level=3, ) + return self._included_scopes + @property + def included_lockfiles(self) -> List[IncludedLockfile]: + if self._included_lockfiles is None: + self._read_included_scopes() + return self._included_lockfiles + + def _read_included_scopes(self) -> List["ConfigScope"]: + """Memoized list of included scopes, in the order they appear in this scope.""" + self._included_scopes = [] + self._included_lockfiles = [] + + includes = self.get_includes() + if includes: + if "include" not in includes: + return self._included_scopes + + include_paths = [included_path(data) for data in includes["include"]] + tty.debug( + f"Processing included paths: {[str(path) for path in include_paths]}", level=3 + ) + included_scopes = chain(*[include.scopes(self) for include in include_paths]) + + for included_scope in included_scopes: + # If it's a lockfile, sort it accordingly + if isinstance(included_scope, IncludedLockfile): + if included_scope not in self._included_lockfiles: + self._included_lockfiles.append(included_scope) + continue + + # Do not include duplicate scopes + if any([included_scope.name == scope.name for scope in self._included_scopes]): + tty.warn(f"Ignoring duplicate included scope: {included_scope.name}") + continue + + self._included_scopes.append(included_scope) + @property def exists(self) -> bool: """Whether the config object indicated by the scope can be read""" @@ -516,6 +535,15 @@ def _process_dict_keyname_overrides(data: YamlConfigDict) -> YamlConfigDict: return result +class IncludedLockfile: + """This is a placeholder scope-like class for handling includes of lockfiles.""" + def __init__(self, path: str): + self.path = path + + def __eq__(self, other): + return self.path == other.path + + def _config_mutator(method): """Decorator to mark all the methods in the Configuration class that mutate the underlying configuration. Used to clear the @@ -1187,7 +1215,7 @@ def _subdir(): def _scope( self, path: str, config_path: str, parent_scope: ConfigScope - ) -> Optional[ConfigScope]: + ) -> ScopeLike: """Instantiate a configuration scope for a configuration path. Args: @@ -1195,7 +1223,7 @@ def _scope( config_path: configuration path parent_scope: including scope - Returns: configuration scope or ``None`` + Returns: configuration scope or IncludedLockfile placeholder Raises: ValueError: the required configuration path does not exist @@ -1211,7 +1239,7 @@ def _scope( f"Ignoring inclusion of lock file '{path}' since it is processed " "in the environment." ) - return None + return IncludedLockfile(config_path) # Ensure the parent scope is valid self._validate_parent_scope(parent_scope) @@ -1304,14 +1332,15 @@ def evaluate_condition(self) -> bool: return (not self.when) or spack.spec.eval_conditional(self.when) - def scopes(self, parent_scope: ConfigScope) -> List[ConfigScope]: + def scopes(self, parent_scope: ConfigScope) -> List[ScopeLike]: """Instantiate configuration scopes. Args: parent_scope: including scope Returns: configuration scopes for configuration files IF the when - condition is satisfied; otherwise, an empty list. + condition is satisfied; otherwise, an empty list. Scopes may include + IncludedLockfile placeholders. Raises: ValueError: the required configuration path does not exist @@ -1369,14 +1398,15 @@ def __repr__(self): f"destination={self.destination})" ) - def scopes(self, parent_scope: ConfigScope) -> List[ConfigScope]: + def scopes(self, parent_scope: ConfigScope) -> List[ScopeLike]: """Instantiate a configuration scope for the included path. Args: parent_scope: including scope Returns: configuration scopes for configuration files IF the when - condition is satisfied; otherwise, an empty list. + condition is satisfied; otherwise, an empty list. List may include + IncludedLockfile placeholders. Raises: AssertionError: unable to write to the directory @@ -1539,14 +1569,15 @@ def fetched(self) -> bool: os.path.join(self.destination, ".git") # type: ignore[arg-type] ) - def scopes(self, parent_scope: ConfigScope) -> List[ConfigScope]: + def scopes(self, parent_scope: ConfigScope) -> List[ScopeLike]: """Instantiate configuration scopes for the included paths. Args: parent_scope: including scope Returns: configuration scopes for configuration files IF the when - condition is satisfied; otherwise, an empty list. + condition is satisfied; otherwise, an empty list. List may include + IncludedLockfile placeholders Raises: ConfigFileError: unable to access remote configuration file(s) From 706e53c5756aa43e26f80bf94f4cf966407e6c45 Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Tue, 12 May 2026 14:39:29 -0700 Subject: [PATCH 15/33] hotfix for schema while working on other code Signed-off-by: Gregory Becker --- lib/spack/spack/schema/spec_list.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/spack/spack/schema/spec_list.py b/lib/spack/spack/schema/spec_list.py index c061a78782c61d..2ea334bf822861 100644 --- a/lib/spack/spack/schema/spec_list.py +++ b/lib/spack/spack/schema/spec_list.py @@ -1,7 +1,8 @@ # Copyright Spack Project Developers. See COPYRIGHT file for details. # # SPDX-License-Identifier: (Apache-2.0 OR MIT) -import spack.schema.merged + +# from spack.schema.merged import ref_sections matrix_schema = { "type": "array", @@ -29,8 +30,8 @@ "override": { "type": "object", "description": "Top-most configuration scope for this group of specs", - "additionalProperties": False, - "properties": {**spack.schema.merged.ref_sections}, + "additionalProperties": True, # TODO: fix circular import +# "properties": {**ref_sections}, }, } @@ -69,7 +70,7 @@ "type": "object", "description": "User spec group with multiple matrices", "additionalProperties": False, - "properties": {**group_name_and_deps, "specs": spec_list_schema}, + "properties": {**group_name_and_deps, "specs": {}}, # spec_list_schema}, }, ] }, From e5746e09b78d4484ec81d7fe99386a3e9730d4e6 Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Tue, 12 May 2026 14:42:20 -0700 Subject: [PATCH 16/33] refactor usage of SpecListParser Signed-off-by: Gregory Becker --- lib/spack/spack/environment/environment.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index cccba3fe5fb48b..7df601d1360360 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -998,8 +998,6 @@ def __init__(self, manifest_dir: Union[str, pathlib.Path]) -> None: self._unify = None self.views: Dict[str, ViewDescriptor] = {} - #: Parser for spec lists - self._spec_lists_parser = SpecListParser() #: Specs from "spack.yaml" self.spec_lists: Dict[str, SpecList] = {} #: Information on concretized roots @@ -1202,11 +1200,11 @@ def _construct_state_from_manifest(self): # TODO/50207: user specs. def _sync_speclists(self): toolchains = spack.config.CONFIG.get("toolchains", {}) - self._spec_lists_parser = SpecListParser(toolchains=toolchains) + parser = SpecListParser(toolchains=toolchains) self.spec_lists = {} self.spec_lists.update( - self._spec_lists_parser.parse_definitions( + parser.parse_definitions( data=spack.config.CONFIG.get("definitions", []) ) ) @@ -1219,7 +1217,7 @@ def _sync_speclists(self): tty.debug(f"[{__name__}]: Synchronizing user specs from the '{group}' group", level=2) key = self._user_specs_key(group=group) - self.spec_lists[key] = self._spec_lists_parser.parse_user_specs( + self.spec_lists[key] = parser.parse_user_specs( name=key, yaml_list=self.manifest.user_specs(group=group) ) From 6df83aa18d21f1defdf99c599f9af11ac84c4a0c Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Tue, 12 May 2026 14:51:28 -0700 Subject: [PATCH 17/33] Environment._read: simplify codepaths Signed-off-by: Gregory Becker --- lib/spack/spack/environment/environment.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index 7df601d1360360..7d35a0775929be 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -1074,8 +1074,13 @@ def _re_read(self): self._load_manifest_file() def _read(self): - self._construct_state_from_manifest() + """Set up user specs and views from the manifest file.""" + self.views = {} + self._sync_speclists() + self._process_view(spack.config.get("view", True)) + self._process_included_lockfiles() + # self._read_lockfile() is needed for correctness even without the version check if os.path.exists(self.lock_path): with open(self.lock_path, encoding="utf-8") as f: read_lock_version = self._read_lockfile(f)["_meta"]["lockfile-version"] @@ -1189,13 +1194,6 @@ def _process_included_lockfiles(self): # Cache concrete environments for required lock files. self._load_concrete_include_data() - def _construct_state_from_manifest(self): - """Set up user specs and views from the manifest file.""" - self.views = {} - self._sync_speclists() - self._process_view(spack.config.get("view", True)) - self._process_included_lockfiles() - # TODO/50207: Properly resolve the conflicts related to adding included # TODO/50207: user specs. def _sync_speclists(self): From 1330f41dc1ff76b391993a7de66c939b33c0c3ff Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Tue, 12 May 2026 15:25:12 -0700 Subject: [PATCH 18/33] environment concrete includes: refactor to pull from config system Signed-off-by: Gregory Becker --- lib/spack/spack/environment/environment.py | 40 +++++----------------- 1 file changed, 9 insertions(+), 31 deletions(-) diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index 7d35a0775929be..fa24dff01f1123 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -1152,44 +1152,22 @@ def _load_concrete_include_data(self): def _process_included_lockfiles(self): """Extract and load into memory included lock file data.""" - includes = self.manifest[TOP_LEVEL_KEY].get(lockfile_include_key, []) - if includes: + self.included_concrete_env_root_dirs = [] + + _old_includes = self.manifest[TOP_LEVEL_KEY].get(lockfile_include_key, []) + if _old_includes: tty.warn( f"Use of '{lockfile_include_key}' in manifest files " f"is deprecated. The key should be '{manifest_include_name}' " f"and the path should end with '{lockfile_name}'. Run " f"'spack env update {self.name}' to update the manifest." ) - includes = [os.path.join(inc, lockfile_name) for inc in includes] - includes += self.manifest[TOP_LEVEL_KEY].get(manifest_include_name, []) - if not includes: - return - - # Expand config and environment variables for concrete environments, - # indicated by the inclusion of lock files. - self.included_concrete_env_root_dirs = [] + self.included_concrete_env_root_dirs = _old_includes - for entry in includes: - include = spack.config.included_path(entry) - if isinstance(include, spack.config.GitIncludePaths): - # Git includes must be cloned first; paths are relative to the - # clone destination, not to the manifest directory. - destination = include._clone(self.manifest.env_config_scope) - if destination is None: - continue - resolved = [os.path.join(destination, p) for p in include.paths] - else: - resolved = [ - spack.util.path.canonicalize_path(p, default_wd=self.path) - for p in include.paths - ] - - for path in resolved: - if os.path.basename(path) != lockfile_name: - continue - - tty.debug(f"Adding {path} to the concrete environment root directories") - self.included_concrete_env_root_dirs.append(os.path.dirname(path)) + # Implicitly clones remote scopes and resolves all paths + self.included_concrete_env_root_dirs += [ + os.path.dirname(f.path) for f in self.manifest.env_config_scope.included_lockfiles + ] # Cache concrete environments for required lock files. self._load_concrete_include_data() From 095b8a79d2f955917f5668eae12dcad9174ad405 Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Tue, 12 May 2026 21:38:30 -0700 Subject: [PATCH 19/33] EnvironmentManifestFile: refactor to separate __init__ from reading speclists Signed-off-by: Gregory Becker --- lib/spack/spack/environment/environment.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index fa24dff01f1123..bb0dcc50b29c95 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -1028,6 +1028,7 @@ def _load_manifest_file(self): with lk.ReadTransaction(self.txlock): self.manifest = EnvironmentManifestFile(self.path, self.name) with self.manifest.use_config(): + self.manifest.init_user_specs() self._read() @contextlib.contextmanager @@ -3107,6 +3108,7 @@ def from_lockfile(manifest_dir: Union[pathlib.Path, str]) -> "EnvironmentManifes default_content = manifest_dir / manifest_name default_content.write_text(default_manifest_yaml()) manifest = EnvironmentManifestFile(manifest_dir) + manifest.init_user_specs() for group, specs in user_specs_by_group.items(): for spec in specs: @@ -3141,11 +3143,10 @@ def __init__(self, manifest_dir: Union[pathlib.Path, str], name: Optional[str] = self._config_override: Dict[str, Any] = {DEFAULT_USER_SPEC_GROUP: None} # Whether specs in each group are marked explicit self._explicit: Dict[str, bool] = {DEFAULT_USER_SPEC_GROUP: True} - self._init_user_specs() self.changed = False - def _init_user_specs(self): + def init_user_specs(self): with self.use_config(): specs_yaml = spack.config.CONFIG.get("specs", []) for item in specs_yaml: @@ -3323,7 +3324,7 @@ def override_user_spec(self, user_spec: str, idx: int) -> None: specs_yaml[idx] = user_spec spack.config.CONFIG.set("specs", specs_yaml, scope=self.scope_name) self._clear_user_specs() - self._init_user_specs() + self.init_user_specs() except ValueError as e: msg = f"cannot override {user_spec} from {self}" raise SpackEnvironmentError(msg) from e From 687992955c9d8437f8fa135cf80a76cbc32acd23 Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Tue, 12 May 2026 21:39:26 -0700 Subject: [PATCH 20/33] wrap calls to config in manifest.use_config() Signed-off-by: Gregory Becker --- lib/spack/spack/environment/environment.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index bb0dcc50b29c95..0a78194d0efca0 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -1076,10 +1076,11 @@ def _re_read(self): def _read(self): """Set up user specs and views from the manifest file.""" - self.views = {} - self._sync_speclists() - self._process_view(spack.config.get("view", True)) - self._process_included_lockfiles() + with self.manifest.use_config(): + self.views = {} + self._sync_speclists() + self._process_view(spack.config.get("view", True)) + self._process_included_lockfiles() # self._read_lockfile() is needed for correctness even without the version check if os.path.exists(self.lock_path): @@ -3359,7 +3360,9 @@ def add_include_concrete(self, include_concrete: List[str]) -> None: msg += f"\n\t`spack -e {env} concretize`" raise SpackEnvironmentError(msg) - self.configuration[manifest_include_name] = concrete_paths + with self.use_config(): + spack.config.CONFIG.set("include", concrete_paths) + self.changed = True def override_include(self, include: Union[str, dict], idx: int) -> None: From b5b52b459cc4ae0371a8248b3e60472a3968fea6 Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Tue, 12 May 2026 21:40:34 -0700 Subject: [PATCH 21/33] revert changes to relative include rewriting on env create Signed-off-by: Gregory Becker --- lib/spack/spack/environment/environment.py | 35 ++++++++++------------ 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index 0a78194d0efca0..6228ea826d09d5 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -3047,35 +3047,32 @@ def new_include(included_path: spack.config.IncludePath, new_path: str): include for include in included_paths if isinstance(include, spack.config.IncludePath) ] for idx, include in enumerate(included_paths): - if os.path.isabs(include.path): + path = include.path + + if os.path.isabs(path): continue - abspath = pathlib.Path(os.path.normpath(environment_dir / include.path)) + abspath = pathlib.Path(os.path.normpath(environment_dir / path)) common_path = pathlib.Path(os.path.commonpath([environment_dir, abspath])) if common_path != environment_dir: - tty.warn( - f"Relative include path ({include.path}) is outside environment " - "so will retain as is" - ) + tty.debug(f"Will not copy relative include file from outside environment: {path}") + continue + + orig_abspath = os.path.normpath(envfile.parent / path) + if os.path.isfile(orig_abspath): + fs.touchp(abspath) + shutil.copy(orig_abspath, abspath) continue - orig_abspath = os.path.normpath(envfile.parent / include.path) if not os.path.exists(orig_abspath): - tty.warn(f"Include '{include.path}' does not exist so will retain as is") + tty.warn(f"Skipping copy of non-existent include path: '{path}'") continue - if os.path.isfile(orig_abspath): - manifest.override_include(new_include(include, orig_abspath), idx) - else: - if os.path.exists(abspath): - tty.warn( - f"Skipping replacement of duplicate directory ({include.path}) " - f"since {abspath} exists" - ) - continue - manifest.override_include(new_include(include, orig_abspath), idx) + if os.path.exists(abspath): + tty.warn(f"Skipping copy of directory over existing path: {path}") + continue - manifest.flush() + shutil.copytree(orig_abspath, abspath, symlinks=True) class EnvironmentManifestFile(collections.abc.Mapping): From 5be77e656617be2ff9b9fa0e8ea4b448581eaa5a Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Tue, 12 May 2026 21:45:43 -0700 Subject: [PATCH 22/33] fix reference-before-set bug in error handling code Signed-off-by: Gregory Becker --- lib/spack/spack/environment/environment.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index 6228ea826d09d5..15660479d8ce39 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -3371,14 +3371,14 @@ def override_include(self, include: Union[str, dict], idx: int) -> None: Raises: SpackEnvironmentError: when the include cannot be overridden """ + current_include = spack.config.CONFIG.get("include", [], scope=self.scope_name) try: - current_include = spack.config.CONFIG.get("include", [], scope=self.scope_name) current_include[idx] = include spack.config.CONFIG.set("include", current_include, scope=self.scope_name) + self.changed = True except ValueError as e: msg = f"cannot override '{current_include}' with '{include}'" raise SpackEnvironmentError(msg) from e - self.changed = True def add_definition(self, user_spec: str, list_name: str) -> None: """Appends a user spec to the first active definition matching the name passed as argument. From 766dc43040cd9e8256cca26fc5295361a675e7eb Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Tue, 12 May 2026 21:53:25 -0700 Subject: [PATCH 23/33] remove vestigial code from a previous attempt Signed-off-by: Gregory Becker --- lib/spack/spack/environment/environment.py | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index 15660479d8ce39..5b827173ebab82 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -3546,18 +3546,9 @@ def env_config_scope(self) -> spack.config.ConfigScope: ensure_no_disallowed_env_config_mods(self._env_config_scope) return self._env_config_scope - def prepare_config_scope(self, includes=True) -> None: + def prepare_config_scope(self) -> None: """Add the manifest's scope to the global configuration search path.""" - if includes: - scope = self.env_config_scope - else: - scope = spack.config.SingleFileScope( - self.scope_name, - str(self.manifest_file), - spack.schema.env.schema, - yaml_path=[TOP_LEVEL_KEY], - ) - scope._included_scopes = [] # TODO make this less hacky + scope = self.env_config_scope spack.config.CONFIG.push_scope( self.env_config_scope, priority=ConfigScopePriority.ENVIRONMENT ) @@ -3567,10 +3558,10 @@ def deactivate_config_scope(self) -> None: spack.config.CONFIG.remove_scope(self.env_config_scope.name) @contextlib.contextmanager - def use_config(self, includes=True): + def use_config(self): """Ensure only the manifest's configuration scopes are global.""" with no_active_environment(): - self.prepare_config_scope(includes=includes) + self.prepare_config_scope() yield self.deactivate_config_scope() From c7fec361dbeaa9b792ffe8ee08de86f1565e952c Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Tue, 12 May 2026 22:14:46 -0700 Subject: [PATCH 24/33] fix test for new config behavior Signed-off-by: Gregory Becker --- lib/spack/spack/test/cmd/env.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index e29643d1c39c29..23cf4256d2d56f 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -2217,9 +2217,8 @@ def configure_reuse(reuse_mode, combined_env) -> Optional[ev.Environment]: # include concrete spec vs reuse due to the reuse configuration _config["concretizer"].update({"unify": False}) - combined_env.manifest.configuration.update(_config) - combined_env.manifest.changed = True - combined_env.write() + with combined_env.manifest.use_config(): + spack.config.CONFIG.set("concretizer", _config["concretizer"], scope=combined_env.manifest.env_config_scope.name) return override_env From 6e30e82238b977de33ef808bad586b015bbf133b Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Wed, 13 May 2026 09:18:02 -0700 Subject: [PATCH 25/33] fix type hint Signed-off-by: Gregory Becker --- lib/spack/spack/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index 888823d6fc1626..58eed580e38637 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -177,7 +177,7 @@ def included_scopes(self) -> List["ConfigScope"]: return self._included_scopes @property - def included_lockfiles(self) -> List[IncludedLockfile]: + def included_lockfiles(self) -> List["IncludedLockfile"]: if self._included_lockfiles is None: self._read_included_scopes() return self._included_lockfiles From 4dd6522062ce2d5fba2e407a1dad9664665b2ba4 Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Wed, 13 May 2026 09:44:11 -0700 Subject: [PATCH 26/33] remove vestigial method Signed-off-by: Gregory Becker --- lib/spack/spack/environment/environment.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index 5b827173ebab82..328b20cf387245 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -3362,24 +3362,6 @@ def add_include_concrete(self, include_concrete: List[str]) -> None: self.changed = True - def override_include(self, include: Union[str, dict], idx: int) -> None: - """Overrides the included path data at index idx with the one passed as input. - Args: - include: new include data - idx: index of the include to be overridden - - Raises: - SpackEnvironmentError: when the include cannot be overridden - """ - current_include = spack.config.CONFIG.get("include", [], scope=self.scope_name) - try: - current_include[idx] = include - spack.config.CONFIG.set("include", current_include, scope=self.scope_name) - self.changed = True - except ValueError as e: - msg = f"cannot override '{current_include}' with '{include}'" - raise SpackEnvironmentError(msg) from e - def add_definition(self, user_spec: str, list_name: str) -> None: """Appends a user spec to the first active definition matching the name passed as argument. From ff52246b6febbbbcab412e1fe6ef7863c993891d Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Wed, 13 May 2026 09:44:53 -0700 Subject: [PATCH 27/33] modify specs inside config Signed-off-by: Gregory Becker --- lib/spack/spack/environment/environment.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index 328b20cf387245..a947f4ac5a4025 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -3317,12 +3317,14 @@ def override_user_spec(self, user_spec: str, idx: int) -> None: Raises: SpackEnvironmentError: when the user spec cannot be overridden """ + try: - specs_yaml = spack.config.CONFIG.get("specs", [], scope=self.scope_name) - specs_yaml[idx] = user_spec - spack.config.CONFIG.set("specs", specs_yaml, scope=self.scope_name) - self._clear_user_specs() - self.init_user_specs() + with self.use_config(): + specs_yaml = spack.config.CONFIG.get("specs", [], scope=self.scope_name) + specs_yaml[idx] = user_spec + spack.config.CONFIG.set("specs", specs_yaml, scope=self.scope_name) + self._clear_user_specs() + self.init_user_specs() except ValueError as e: msg = f"cannot override {user_spec} from {self}" raise SpackEnvironmentError(msg) from e From 30bf2c73d75fc474a2c3e02f8af9b60696c9338e Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Wed, 13 May 2026 14:26:33 -0700 Subject: [PATCH 28/33] spec schema: deduplicate group_name_and_deps Signed-off-by: Gregory Becker --- lib/spack/spack/schema/specs.py | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/lib/spack/spack/schema/specs.py b/lib/spack/spack/schema/specs.py index 3a084160594625..90a9d32ef613da 100644 --- a/lib/spack/spack/schema/specs.py +++ b/lib/spack/spack/schema/specs.py @@ -8,21 +8,8 @@ """ from typing import Any, Dict -from .spec_list import spec_list_properties, spec_list_schema +from .spec_list import spec_list_properties, spec_list_schema, group_name_and_deps -#: Properties for inclusion in other schemas -group_name_and_deps = { - "group": {"type": "string", "description": "Name for this group of specs"}, - "needs": { - "type": "array", - "description": "Groups of specs that are needed by this group", - "items": {"type": "string"}, - }, - "override": { - "description": "Top-most configuration scope for this group of specs", - "$ref": "#/properties", - }, -} properties: Dict[str, Any] = { "specs": { From 223c28e39b905285df1867cf37bfc6cc9b2d4e9e Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Wed, 13 May 2026 14:27:24 -0700 Subject: [PATCH 29/33] EnvironmentManifestFile.remove_default_view: inside of self.use_config(): Signed-off-by: Gregory Becker --- lib/spack/spack/environment/environment.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index a947f4ac5a4025..1bb0a82398bcb4 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -3482,14 +3482,15 @@ def set_default_view(self, view: Union[bool, str, pathlib.Path, Dict[str, str]]) def remove_default_view(self) -> None: """Removes the default view from the manifest file""" - view_data = spack.config.CONFIG.get("view", {}, scope=self.scope_name) - if isinstance(view_data, collections.abc.Mapping): - view_data.pop(default_view_name) - spack.config.CONFIG.set("view", view_data) - self.changed = True - return - - self.set_default_view(view=False) + with self.use_config(): + view_data = spack.config.CONFIG.get("view", {}, scope=self.scope_name) + if isinstance(view_data, collections.abc.Mapping): + view_data.pop(default_view_name) + spack.config.CONFIG.set("view", view_data) + self.changed = True + return + + self.set_default_view(view=False) def flush(self) -> None: """Synchronizes the object with the manifest file on disk.""" From 973c26145821e501167d49b5c841368f48fbd236 Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Wed, 13 May 2026 14:27:52 -0700 Subject: [PATCH 30/33] test/env.py: update tests for new calling conventions Signed-off-by: Gregory Becker --- lib/spack/spack/test/env.py | 45 ++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/lib/spack/spack/test/env.py b/lib/spack/spack/test/env.py index 5ccf2aaf712b19..5979bc6059fcd7 100644 --- a/lib/spack/spack/test/env.py +++ b/lib/spack/spack/test/env.py @@ -273,7 +273,9 @@ def test_update_default_view(init_view, update_value, tmp_path: pathlib.Path, co if isinstance(init_view, str) and update_value is True: expected_value = init_view - assert env.manifest.yaml_content["spack"]["view"] == expected_value + with env.manifest.use_config(): + current = spack.config.CONFIG.get("view", scope=env.manifest.scope_name) + assert current == expected_value @pytest.mark.parametrize( @@ -409,7 +411,9 @@ def test_can_add_specs_to_environment_without_specs_attribute(tmp_path: pathlib. env.add("pkg-a") assert len(env.user_specs) == 1 - assert env.manifest.yaml_content["spack"]["specs"] == ["pkg-a"] + with env.manifest.use_config(): + specs = spack.config.CONFIG.get("specs") + assert specs == ["pkg-a"] @pytest.mark.parametrize( @@ -675,6 +679,8 @@ def test_manifest_file_removal_works_if_spec_is_not_normalized(tmp_path: pathlib ) s = spack.spec.Spec(spec_str) spack_yaml = EnvironmentManifestFile(tmp_path) + spack_yaml.init_user_specs() + # Doing a round trip str -> Spec -> str normalizes the representation spack_yaml.remove_user_spec(str(s)) spack_yaml.flush() @@ -1668,7 +1674,9 @@ def create_temporary_manifest(tmp_path): def _create(spack_yaml: str): manifest_path.write_text(spack_yaml) - return EnvironmentManifestFile(tmp_path) + manifest = EnvironmentManifestFile(tmp_path) + manifest.init_user_specs() + return manifest return _create @@ -2194,21 +2202,22 @@ def test_env_include_env_includes(tmp_path, mutable_config): for pkg in pkgs: assert spack.spec.Spec(pkg) in user_specs - # ensure the included environment scopes are found - pattern = rf"^env:{str(tmp_path)}" - assert len(mutable_config.matching_scopes(rf"{pattern}$")) == 1 # environment's scope - hash_pattern = "[0-9a-fA-F]{7}" - assert ( - len(mutable_config.matching_scopes(rf"{pattern}:{hash_pattern}$")) == 2 - ) # 1st level includes - hash_pattern = rf"{hash_pattern}:{hash_pattern}" - assert ( - len(mutable_config.matching_scopes(rf"{pattern}:{hash_pattern}$")) == 2 - ) # 2nd level includes - - # ensure the install root matches the top-most (i.e., environment's) configured path - config_install_root = mutable_config.get("config:install_tree:root", None) - assert config_install_root == str(install_root) + with e.manifest.use_config(): + # ensure the included environment scopes are found + pattern = rf"^env:{str(tmp_path)}" + assert len(mutable_config.matching_scopes(rf"{pattern}$")) == 1 # environment's scope + hash_pattern = "[0-9a-fA-F]{7}" + assert ( + len(mutable_config.matching_scopes(rf"{pattern}:{hash_pattern}$")) == 2 + ) # 1st level includes + hash_pattern = rf"{hash_pattern}:{hash_pattern}" + assert ( + len(mutable_config.matching_scopes(rf"{pattern}:{hash_pattern}$")) == 2 + ) # 2nd level includes + + # ensure the install root matches the top-most (i.e., environment's) configured path + config_install_root = mutable_config.get("config:install_tree:root", None) + assert config_install_root == str(install_root) def test_env_include_env_highest_config(tmp_path, config): From fe532bce443708367ad6e08094f6e6b2280b5e6c Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Wed, 13 May 2026 15:05:15 -0700 Subject: [PATCH 31/33] fixup env_include test for rebase since it was written Signed-off-by: Gregory Becker --- lib/spack/spack/test/env.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/spack/spack/test/env.py b/lib/spack/spack/test/env.py index 5979bc6059fcd7..32398bbfc5e525 100644 --- a/lib/spack/spack/test/env.py +++ b/lib/spack/spack/test/env.py @@ -2206,13 +2206,13 @@ def test_env_include_env_includes(tmp_path, mutable_config): # ensure the included environment scopes are found pattern = rf"^env:{str(tmp_path)}" assert len(mutable_config.matching_scopes(rf"{pattern}$")) == 1 # environment's scope - hash_pattern = "[0-9a-fA-F]{7}" + inc_pattern = rf"{str(includes_dir)}/({'|'.join(pkgs)})/spack\.yaml" assert ( - len(mutable_config.matching_scopes(rf"{pattern}:{hash_pattern}$")) == 2 + len(mutable_config.matching_scopes(rf"{pattern}:{inc_pattern}$")) == 2 ) # 1st level includes - hash_pattern = rf"{hash_pattern}:{hash_pattern}" + leaf_pattern = rf"{str(includes_dir)}/({'|'.join(pkgs)})/config\.yaml" assert ( - len(mutable_config.matching_scopes(rf"{pattern}:{hash_pattern}$")) == 2 + len(mutable_config.matching_scopes(rf"{pattern}:{inc_pattern}:{leaf_pattern}$")) == 2 ) # 2nd level includes # ensure the install root matches the top-most (i.e., environment's) configured path From 96a3c5955486dafc20f32a0394d3df3ade8f856a Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Wed, 20 May 2026 10:06:02 -0700 Subject: [PATCH 32/33] revert bad test change Signed-off-by: Gregory Becker --- lib/spack/spack/test/cmd/ci.py | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/lib/spack/spack/test/cmd/ci.py b/lib/spack/spack/test/cmd/ci.py index a233c8adfee645..6e1ccb723ea711 100644 --- a/lib/spack/spack/test/cmd/ci.py +++ b/lib/spack/spack/test/cmd/ci.py @@ -1556,7 +1556,7 @@ def example_function(): ) -def test_gitlab_config_scopes(install_mockery, ci_generate_test, tmp_path: pathlib.Path, capfd): +def test_gitlab_config_scopes(install_mockery, ci_generate_test, tmp_path: pathlib.Path): """Test pipeline generation with included configs""" # Create an included config scope configs_path = tmp_path / "gitlab" / "configs" @@ -1601,8 +1601,6 @@ def test_gitlab_config_scopes(install_mockery, ci_generate_test, tmp_path: pathl tags: ["some_tag"] """ ) - _, err = capfd.readouterr() - assert err.count("Ignoring duplicate included scope") > 2 yaml_contents = syaml.load(outputfile.read_text()) @@ -1625,12 +1623,25 @@ def test_gitlab_config_scopes(install_mockery, ci_generate_test, tmp_path: pathl env_manifest = syaml.load(conc_env_manifest.read_text()) assert "include" in env_manifest["spack"] - # Ensure all but last concrete env includes point to the same location - conc_env_includes = env_manifest["spack"]["include"] - assert all([path == str(configs_path) for path in conc_env_includes[:-1]]) + # Ensure relative path include correctly updated + # Ensure the relocated concrete env includes point to the same location + rel_conc_path = env_manifest["spack"]["include"][0] + abs_conc_path = (conc_env_path / rel_conc_path).absolute().resolve() + assert str(abs_conc_path) == os.path.join(ev.as_env_dir("test"), "gitlab", "configs") + + # Ensure relative path include with "path" correctly updated + # Ensure the relocated concrete env includes point to the same location + rel_conc_path = env_manifest["spack"]["include"][1]["path"] + abs_conc_path = (conc_env_path / rel_conc_path).absolute().resolve() + assert str(abs_conc_path) == os.path.join(ev.as_env_dir("test"), "gitlab", "configs") + + # Ensure absolute path is unchanged + # Ensure the relocated concrete env includes point to the same location + abs_config_path = env_manifest["spack"]["include"][2] + assert str(abs_config_path) == str(configs_path) # Ensure URL path is unchanged - url_config_path = conc_env_includes[-1]["path"] + url_config_path = env_manifest["spack"]["include"][3]["path"] assert str(url_config_path) == "https://dummy.io" From 3d944a3534988ebfafdd50707c4517a3ee8001a8 Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Thu, 21 May 2026 17:31:07 -0700 Subject: [PATCH 33/33] style Signed-off-by: Gregory Becker --- lib/spack/spack/config.py | 33 +++++++++++----------- lib/spack/spack/environment/environment.py | 11 ++++---- lib/spack/spack/schema/spec_list.py | 2 +- lib/spack/spack/schema/specs.py | 4 +-- lib/spack/spack/test/cmd/env.py | 6 +++- 5 files changed, 29 insertions(+), 27 deletions(-) diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index 58eed580e38637..b46db354ef33e9 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -79,7 +79,6 @@ from .enums import ConfigScopePriority - ScopeLike = Union["ConfigScope", "IncludedLockfile"] #: Dict from section names -> schema for that section @@ -168,6 +167,7 @@ def included_scopes(self) -> List["ConfigScope"]: if self._included_scopes is None: self._read_included_scopes() + assert isinstance(self._included_scopes, list) tty.debug( f"{self.name} has {'' if len(self._included_scopes) else 'no '}" f"included scopes: {self._included_scopes}", @@ -182,15 +182,15 @@ def included_lockfiles(self) -> List["IncludedLockfile"]: self._read_included_scopes() return self._included_lockfiles - def _read_included_scopes(self) -> List["ConfigScope"]: + def _read_included_scopes(self) -> None: """Memoized list of included scopes, in the order they appear in this scope.""" self._included_scopes = [] - self._included_lockfiles = [] + self._included_lockfiles: List[IncludedLockfile] = [] includes = self.get_includes() if includes: if "include" not in includes: - return self._included_scopes + return include_paths = [included_path(data) for data in includes["include"]] tty.debug( @@ -537,8 +537,10 @@ def _process_dict_keyname_overrides(data: YamlConfigDict) -> YamlConfigDict: class IncludedLockfile: """This is a placeholder scope-like class for handling includes of lockfiles.""" + def __init__(self, path: str): self.path = path + self.name = self.path # for compatibility with ConfigScope in debug printing def __eq__(self, other): return self.path == other.path @@ -1112,7 +1114,7 @@ class OptionalInclude: optional: bool prefer_modify: bool remote: bool - _scopes: List[ConfigScope] + _scopes: List[ScopeLike] def __init__(self, entry: dict): self.name = entry.get("name", "") @@ -1120,7 +1122,7 @@ def __init__(self, entry: dict): self.optional = entry.get("optional", False) self.prefer_modify = entry.get("prefer_modify", False) self.remote = False - self._scopes = [] + self._scopes: List[ScopeLike] = [] @staticmethod def _parent_scope_directory(parent_scope: Optional[ConfigScope]) -> Optional[str]: @@ -1215,7 +1217,7 @@ def _subdir(): def _scope( self, path: str, config_path: str, parent_scope: ConfigScope - ) -> ScopeLike: + ) -> Optional[ScopeLike]: """Instantiate a configuration scope for a configuration path. Args: @@ -1425,9 +1427,9 @@ def scopes(self, parent_scope: ConfigScope) -> List[ScopeLike]: # Ensure the explicit destination for a remote file is writable. base = self.destination or self.base_directory(self.path, parent_scope) if self.remote and base and os.path.isdir(base): - assert filesystem.can_write_to_dir( - self.destination - ), f"Cannot include {self.path}. Unable to write to {base}" + assert filesystem.can_write_to_dir(self.destination), ( + f"Cannot include {self.path}. Unable to write to {base}" + ) tty.debug(f"Local base directory for {self.path} is {base}") config_path = rfc_util.local_path(self.path, self.sha256, base) @@ -1437,7 +1439,7 @@ def scopes(self, parent_scope: ConfigScope) -> List[ScopeLike]: scope = self._scope(self.path, config_path, parent_scope) if scope is not None: - self._scopes = [scope] + self._scopes: List[ScopeLike] = [scope] return self._scopes @@ -1526,10 +1528,7 @@ def _clone(self, parent_scope: ConfigScope) -> Optional[str]: tty.debug("Initializing the git repository") spack.util.git.init_git_repo(self.git) except spack.util.executable.ProcessError as e: - msg = ( - f"Unable to initialize repository ({self.git}) under " - f"{self.destination}" - ) + msg = f"Unable to initialize repository ({self.git}) under {self.destination}" if self.optional: tty.warn(msg + ". Ignoring optional include.") else: @@ -1596,7 +1595,7 @@ def scopes(self, parent_scope: ConfigScope) -> List[ScopeLike]: if not destination: raise spack.error.ConfigError(f"Unable to cache the include: {self}") - scopes: List[ConfigScope] = [] + scopes: List[ScopeLike] = [] for path in self.paths: config_path = str(pathlib.Path(destination) / path) scope = self._scope(path, config_path, parent_scope) @@ -1822,7 +1821,7 @@ def get(path: str, default: Any = default_sigil, scope: Optional[str] = None) -> return CONFIG.get(path, default, scope) -_set = set #: save this before defining set -- maybe config.set was ill-advised :) +_set: Callable = set #: save this before defining set -- maybe config.set was ill-advised :) def set(path: str, value: Any, scope: Optional[str] = None) -> None: diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index 1bb0a82398bcb4..f0fcacf749beb8 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -1182,9 +1182,7 @@ def _sync_speclists(self): self.spec_lists = {} self.spec_lists.update( - parser.parse_definitions( - data=spack.config.CONFIG.get("definitions", []) - ) + parser.parse_definitions(data=spack.config.CONFIG.get("definitions", [])) ) # TODO/50207: Fix this to handle grabbing specs though not clear yet @@ -3257,11 +3255,13 @@ def add_user_spec(self, user_spec: str, *, group: Optional[str] = None) -> None: self._user_specs[group].append(user_spec) self.changed = True - def _add_to_group(self, group: str, addition: str, specs_yaml: Dict) -> Dict: + def _add_to_group(self, group: str, addition: str, specs_yaml: Dict) -> None: """Find or create a new group in specs_yaml and add to it Does the yaml addition, not the in-memory addition""" specs_yaml = spack.config.CONFIG.get("specs", [], scope=self.scope_name) + assert isinstance(specs_yaml, list) + group_entry = None for item in specs_yaml: if isinstance(item, dict) and item.get("group") == group: @@ -3483,7 +3483,7 @@ def set_default_view(self, view: Union[bool, str, pathlib.Path, Dict[str, str]]) def remove_default_view(self) -> None: """Removes the default view from the manifest file""" with self.use_config(): - view_data = spack.config.CONFIG.get("view", {}, scope=self.scope_name) + view_data: dict = spack.config.CONFIG.get("view", {}, scope=self.scope_name) if isinstance(view_data, collections.abc.Mapping): view_data.pop(default_view_name) spack.config.CONFIG.set("view", view_data) @@ -3533,7 +3533,6 @@ def env_config_scope(self) -> spack.config.ConfigScope: def prepare_config_scope(self) -> None: """Add the manifest's scope to the global configuration search path.""" - scope = self.env_config_scope spack.config.CONFIG.push_scope( self.env_config_scope, priority=ConfigScopePriority.ENVIRONMENT ) diff --git a/lib/spack/spack/schema/spec_list.py b/lib/spack/spack/schema/spec_list.py index 2ea334bf822861..aed76951b246ea 100644 --- a/lib/spack/spack/schema/spec_list.py +++ b/lib/spack/spack/schema/spec_list.py @@ -31,7 +31,7 @@ "type": "object", "description": "Top-most configuration scope for this group of specs", "additionalProperties": True, # TODO: fix circular import -# "properties": {**ref_sections}, + # "properties": {**ref_sections}, }, } diff --git a/lib/spack/spack/schema/specs.py b/lib/spack/spack/schema/specs.py index 90a9d32ef613da..14883d351b232f 100644 --- a/lib/spack/spack/schema/specs.py +++ b/lib/spack/spack/schema/specs.py @@ -6,10 +6,10 @@ .. literalinclude:: _spack_root/lib/spack/spack/schema/specs.py :lines: 14- """ -from typing import Any, Dict -from .spec_list import spec_list_properties, spec_list_schema, group_name_and_deps +from typing import Any, Dict +from .spec_list import group_name_and_deps, spec_list_properties, spec_list_schema properties: Dict[str, Any] = { "specs": { diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index 23cf4256d2d56f..4cf89815684f45 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -2218,7 +2218,11 @@ def configure_reuse(reuse_mode, combined_env) -> Optional[ev.Environment]: _config["concretizer"].update({"unify": False}) with combined_env.manifest.use_config(): - spack.config.CONFIG.set("concretizer", _config["concretizer"], scope=combined_env.manifest.env_config_scope.name) + spack.config.CONFIG.set( + "concretizer", + _config["concretizer"], + scope=combined_env.manifest.env_config_scope.name, + ) return override_env