diff --git a/lib/spack/docs/environments.rst b/lib/spack/docs/environments.rst index aaf3e0c69cce81..f95961364c5bc9 100644 --- a/lib/spack/docs/environments.rst +++ b/lib/spack/docs/environments.rst @@ -282,6 +282,7 @@ To update the lockfile, the environment must be :ref:`re-concretized 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..b46db354ef33e9 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 @@ -61,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 @@ -76,6 +79,8 @@ 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, @@ -95,13 +100,8 @@ "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, + "specs": spack.schema.specs.schema, } #: Path to the main configuration scope @@ -152,27 +152,65 @@ 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 + + return None + @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 = [] + self._read_included_scopes() - includes = self.get_section("include") - if includes: - include_paths = [included_path(data) for data in includes["include"]] - included_scopes = chain(*[include.scopes(self) for include in include_paths]) + 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}", + level=3, + ) - # Do not include duplicate scopes - for included_scope in included_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 + return self._included_scopes - if included_scope not in self._included_scopes: - self._included_scopes.append(included_scope) + @property + def included_lockfiles(self) -> List["IncludedLockfile"]: + if self._included_lockfiles is None: + self._read_included_scopes() + return self._included_lockfiles - return self._included_scopes + 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: List[IncludedLockfile] = [] + + includes = self.get_includes() + if includes: + if "include" not in includes: + return + + 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: @@ -196,6 +234,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: @@ -496,6 +535,17 @@ 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 + self.name = self.path # for compatibility with ConfigScope in debug printing + + 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 @@ -548,7 +598,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 +898,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 +929,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 @@ -1047,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", "") @@ -1055,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]: @@ -1071,7 +1138,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,14 +1147,36 @@ 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 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 """ + # 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, 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 path. scope_dir = self._parent_scope_directory(parent_scope) - if not self.remote: + 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. @@ -1128,15 +1217,15 @@ def _subdir(): def _scope( self, path: str, config_path: str, parent_scope: ConfigScope - ) -> Optional[ConfigScope]: - """Instantiate a configuration scope for the configuration path. + ) -> Optional[ScopeLike]: + """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 IncludedLockfile placeholder Raises: ValueError: the required configuration path does not exist @@ -1149,10 +1238,10 @@ 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 + return IncludedLockfile(config_path) # Ensure the parent scope is valid self._validate_parent_scope(parent_scope) @@ -1201,10 +1290,17 @@ 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 + yaml_path = ["spack"] + else: + schema = spack.schema.merged.schema + yaml_path = None return SingleFileScope( config_name, config_path, - spack.schema.merged.schema, + schema, + yaml_path=yaml_path, prefer_modify=self.prefer_modify, included=True, ) @@ -1229,7 +1325,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``. """ @@ -1238,14 +1334,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 @@ -1258,6 +1355,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,25 +1383,35 @@ 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) + + 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}', path='{self.path}', sha256={self.sha256}, " + f"when='{self.when}', optional={self.optional}, " + 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 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. List may include + IncludedLockfile placeholders. 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,24 +1424,22 @@ 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) + # 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}" + ) - # Make sure to use a proper working directory when obtaining the local - # path for a local (or remote) file. 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 + if not self.destination: + self.destination = config_path - scope = self._scope(self.path, self.destination, parent_scope) + 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 @@ -1335,6 +1449,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 +1480,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 +1492,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 +1500,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]: @@ -1388,54 +1511,56 @@ def _clone(self, parent_scope: ConfigScope) -> Optional[str]: 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}") + self.destination = self.destination or self.base_directory(self.git, parent_scope) - with filesystem.working_dir(destination, create=True): + 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: + msg = f"Unable to initialize repository ({self.git}) under {self.destination}" + if self.optional: + tty.warn(msg + ". Ignoring optional include.") + else: + raise spack.error.ConfigError(msg + f": {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 initialize repository ({self.git}) under {destination}: {e}" + f"Unable to check out repository ({self}) in {self.destination}: {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}" - ) - - # only set the destination on successful clone/checkout - self.destination = destination return self.destination def fetched(self) -> bool: @@ -1443,14 +1568,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 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. List may include + IncludedLockfile placeholders Raises: ConfigFileError: unable to access remote configuration file(s) @@ -1469,7 +1595,7 @@ def scopes(self, parent_scope: ConfigScope) -> List[ConfigScope]: 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) @@ -1484,8 +1610,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 - return self._paths + if not self.fetched(): + return [] + + 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 +1660,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 @@ -1682,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: @@ -1836,7 +1975,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 +2302,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/environment.py b/lib/spack/spack/environment/environment.py index 9d53c34c0a438b..f0fcacf749beb8 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 @@ -1062,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 @@ -1094,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 @@ -1136,12 +1071,18 @@ 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): - self._construct_state_from_manifest() + """Set up user specs and views from the manifest file.""" + 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): with open(self.lock_path, encoding="utf-8") as f: read_lock_version = self._read_lockfile(f)["_meta"]["lockfile-version"] @@ -1213,69 +1154,46 @@ 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 + self.included_concrete_env_root_dirs = _old_includes - # Expand config and environment variables for concrete environments, - # indicated by the inclusion of lock files. - self.included_concrete_env_root_dirs = [] - - 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() - 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): - self._spec_lists_parser = SpecListParser( - toolchains=spack.config.CONFIG.get("toolchains", {}) - ) + toolchains = spack.config.CONFIG.get("toolchains", {}) + parser = SpecListParser(toolchains=toolchains) + self.spec_lists = {} self.spec_lists.update( - self._spec_lists_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 + # 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( + self.spec_lists[key] = parser.parse_user_specs( name=key, yaml_list=self.manifest.user_specs(group=group) ) @@ -1332,7 +1250,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 = {} @@ -1346,7 +1264,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): @@ -3098,17 +3016,37 @@ 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 + # 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, []) - paths = spack.config.paths_from_includes(includes) - for path in paths: + 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): + path = include.path + if os.path.isabs(path): continue @@ -3151,7 +3089,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: @@ -3167,6 +3104,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: @@ -3201,12 +3139,12 @@ 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): - specs_yaml = self.configuration.get(USER_SPECS_KEY, []) + def init_user_specs(self): + 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) @@ -3249,7 +3187,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) @@ -3302,23 +3240,28 @@ 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) -> 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: @@ -3333,7 +3276,7 @@ def _get_group(self, group: str) -> Dict: self._user_specs[group] = [] self._explicit[group] = True - return group_entry + 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 @@ -3345,17 +3288,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 @@ -3369,22 +3317,51 @@ def override_user_spec(self, user_spec: str, idx: int) -> None: Raises: SpackEnvironmentError: when the user spec cannot be overridden """ + try: - self.configuration["specs"][idx] = user_spec - 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 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) + + with self.use_config(): + spack.config.CONFIG.set("include", concrete_paths) + self.changed = True def add_definition(self, user_spec: str, list_name: str) -> None: @@ -3489,33 +3466,39 @@ 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") - if isinstance(view_data, collections.abc.Mapping): - self.configuration["view"].pop(default_view_name) - self.changed = True - return + with self.use_config(): + 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) + self.changed = True + return - self.set_default_view(view=False) + self.set_default_view(view=False) def flush(self) -> None: """Synchronizes the object with the manifest file on disk.""" 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 @@ -3555,7 +3538,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..e5d308b0835c1a 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, }, 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/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..aed76951b246ea 100644 --- a/lib/spack/spack/schema/spec_list.py +++ b/lib/spack/spack/schema/spec_list.py @@ -1,6 +1,9 @@ # Copyright Spack Project Developers. See COPYRIGHT file for details. # # SPDX-License-Identifier: (Apache-2.0 OR MIT) + +# from spack.schema.merged import ref_sections + matrix_schema = { "type": "array", "description": "List of spec constraint lists whose cross product generates multiple specs", @@ -11,6 +14,27 @@ }, } +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": True, # TODO: fix circular import + # "properties": {**ref_sections}, + }, +} + spec_list_properties = { "matrix": matrix_schema, "exclude": { @@ -36,6 +60,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..14883d351b232f --- /dev/null +++ b/lib/spack/spack/schema/specs.py @@ -0,0 +1,54 @@ +# 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 group_name_and_deps, spec_list_properties, spec_list_schema + +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/ci.py b/lib/spack/spack/test/cmd/ci.py index c111bbba4728e5..6e1ccb723ea711 100644 --- a/lib/spack/spack/test/cmd/ci.py +++ b/lib/spack/spack/test/cmd/ci.py @@ -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: diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index 327f2778703094..4cf89815684f45 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(): @@ -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,15 @@ 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, 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("./")) -# 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 +1340,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 +1505,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 +2065,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 +2101,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 +2113,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) @@ -2221,9 +2217,12 @@ 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 @@ -4741,6 +4740,145 @@ 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"] + 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)) + paths.append(str(include)) + + include_condition = 'platform == "test"' + + e = environment_from_manifest( + f"""\ +spack: + include: + - path: {paths[0]} + when: {include_condition} + - path: {paths[1]} + when: {include_condition} +""" + ) + + 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( + 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"] + 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() + 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..f57d6954edb883 100644 --- a/lib/spack/spack/test/config.py +++ b/lib/spack/spack/test/config.py @@ -6,6 +6,8 @@ import io import os import pathlib +import shutil +import stat import sys import tempfile import textwrap @@ -1707,6 +1709,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,10 +1721,10 @@ 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 + # Second pass uses the previously built scope assert include._scopes is not None scopes = include.scopes(parent_scope) captured = capfd.readouterr()[1] @@ -1734,12 +1738,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 +1772,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 +1782,43 @@ 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" + fs.mkdirp(str(destination)) + 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"] = "fdfde5b0a67544eeda35e2351e829de9310a5b407ce6fda160ddb7a25094f663" + else: + entry["sha256"] = "4672076f2085f5c266308ce4dfcb026dbb7bd146d5aa4d4e88f1590a1c72c335" + + 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 +1828,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 +1841,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 @@ -1943,7 +1964,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" @@ -1989,9 +2010,73 @@ def test_included_path_git_temp_dest(mock_low_high_config): assert dest_dir == temp_dir, pre + rest -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)) +@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.""" + + 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) + + 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) + + 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 + 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): paths = ["concretizer.yaml"] entry = { "git": "https://example.com/linux/configs.git", @@ -2028,8 +2113,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 +2237,69 @@ 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 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() + current_mode = os.stat(str(dest)).st_mode + dest.chmod(current_mode & ~(stat.S_IWUSR | stat.S_IWGRP | stat.S_IWOTH)) + + sha256 = ( + "4672076f2085f5c266308ce4dfcb026dbb7bd146d5aa4d4e88f1590a1c72c335" + if sys.platform == "win32" + else "fdfde5b0a67544eeda35e2351e829de9310a5b407ce6fda160ddb7a25094f663" + ) + entry = { + "path": "https://github.com/path/to/raw/config/config.yaml", + "destination": str(dest), + "sha256": sha256, + } + + try: + 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) + + +@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), + } + + parent_scope = spack.config.ConfigScope("fake") + try: + 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) + # Ensure the unwritable temp dir is removed to avoid failure on some + # platforms (e.g., rhel8). + 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 a25fffde003691..32398bbfc5e525 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 @@ -2138,3 +2146,130 @@ 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 prefers its 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 + + 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 + inc_pattern = rf"{str(includes_dir)}/({'|'.join(pkgs)})/spack\.yaml" + assert ( + len(mutable_config.matching_scopes(rf"{pattern}:{inc_pattern}$")) == 2 + ) # 1st level includes + leaf_pattern = rf"{str(includes_dir)}/({'|'.join(pkgs)})/config\.yaml" + assert ( + 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 + 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..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 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 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 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 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