From 9e508b0321357ddb9b01a6a7da42ecff385402b3 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Tue, 25 Feb 2025 11:33:41 +0100 Subject: [PATCH] Revert "Assign priorities to configuration scopes (#48420)" (#49185) All the build jobs in pipelines are apparently relying on the bug that was fixed. The issue was not caught in the PR because generation jobs were fine, and there was nothing to rebuild. Reverting to fix pipelines in a new PR. This reverts commit 3ad99d75f99fe926f434049f5d507bfe6a7f2331. --- lib/spack/docs/conf.py | 4 - lib/spack/llnl/util/lang.py | 87 +--------- lib/spack/spack/cmd/common/arguments.py | 1 + lib/spack/spack/config.py | 156 +++++++++--------- lib/spack/spack/enums.py | 10 -- lib/spack/spack/environment/environment.py | 6 +- lib/spack/spack/main.py | 17 +- lib/spack/spack/test/build_environment.py | 10 +- lib/spack/spack/test/config.py | 39 +---- lib/spack/spack/test/conftest.py | 32 ++-- lib/spack/spack/test/env.py | 13 +- lib/spack/spack/test/llnl/util/lang.py | 41 ----- .../cloud_pipelines/configs/concretizer.yaml | 1 + .../aws-pcluster-neoverse_v1/spack.yaml | 3 - .../stacks/aws-pcluster-x86_64_v4/spack.yaml | 3 - .../bootstrap-aarch64-darwin/spack.yaml | 3 - .../bootstrap-x86_64-linux-gnu/spack.yaml | 2 - .../stacks/build_systems/spack.yaml | 2 - .../stacks/ml-darwin-aarch64-mps/spack.yaml | 2 - .../stacks/ml-linux-aarch64-cpu/spack.yaml | 2 - .../stacks/ml-linux-aarch64-cuda/spack.yaml | 2 - .../stacks/ml-linux-x86_64-cpu/spack.yaml | 2 - .../stacks/ml-linux-x86_64-cuda/spack.yaml | 2 - .../stacks/ml-linux-x86_64-rocm/spack.yaml | 2 - .../stacks/radiuss-aws-aarch64/spack.yaml | 2 - .../stacks/radiuss-aws/spack.yaml | 2 - .../cloud_pipelines/stacks/radiuss/spack.yaml | 2 - .../stacks/tutorial/spack.yaml | 2 - .../stacks/windows-vis/spack.yaml | 2 - 29 files changed, 109 insertions(+), 343 deletions(-) diff --git a/lib/spack/docs/conf.py b/lib/spack/docs/conf.py index e3f0df9a4ba..87e6f6b227b 100644 --- a/lib/spack/docs/conf.py +++ b/lib/spack/docs/conf.py @@ -223,10 +223,6 @@ def setup(sphinx): ("py:class", "spack.compiler.CompilerCache"), # TypeVar that is not handled correctly ("py:class", "llnl.util.lang.T"), - ("py:class", "llnl.util.lang.KT"), - ("py:class", "llnl.util.lang.VT"), - ("py:obj", "llnl.util.lang.KT"), - ("py:obj", "llnl.util.lang.VT"), ] # The reST default role (used for this markup: `text`) to use for all documents. diff --git a/lib/spack/llnl/util/lang.py b/lib/spack/llnl/util/lang.py index 3e21e882e36..922d515c65c 100644 --- a/lib/spack/llnl/util/lang.py +++ b/lib/spack/llnl/util/lang.py @@ -14,7 +14,7 @@ import typing import warnings from datetime import datetime, timedelta -from typing import Callable, Dict, Iterable, List, Mapping, Optional, Tuple, TypeVar +from typing import Callable, Dict, Iterable, List, Tuple, TypeVar # Ignore emacs backups when listing modules ignore_modules = r"^\.#|~$" @@ -1080,88 +1080,3 @@ def __set__(self, instance, value): def factory(self, instance, owner): raise NotImplementedError("must be implemented by derived classes") - - -KT = TypeVar("KT") -VT = TypeVar("VT") - - -class PriorityOrderedMapping(Mapping[KT, VT]): - """Mapping that iterates over key according to an integer priority. If the priority is - the same for two keys, insertion order is what matters. - - The priority is set when the key/value pair is added. If not set, the highest current priority - is used. - """ - - _data: Dict[KT, VT] - _priorities: List[Tuple[int, KT]] - - def __init__(self) -> None: - self._data = {} - # Tuple of (priority, key) - self._priorities = [] - - def __getitem__(self, key: KT) -> VT: - return self._data[key] - - def __len__(self) -> int: - return len(self._data) - - def __iter__(self): - yield from (key for _, key in self._priorities) - - def __reversed__(self): - yield from (key for _, key in reversed(self._priorities)) - - def reversed_keys(self): - """Iterates over keys from the highest priority, to the lowest.""" - return reversed(self) - - def reversed_values(self): - """Iterates over values from the highest priority, to the lowest.""" - yield from (self._data[key] for _, key in reversed(self._priorities)) - - def _highest_priority(self) -> int: - if not self._priorities: - return 0 - result, _ = self._priorities[-1] - return result - - def add(self, key: KT, *, value: VT, priority: Optional[int] = None) -> None: - """Adds a key/value pair to the mapping, with a specific priority. - - If the priority is None, then it is assumed to be the highest priority value currently - in the container. - - Raises: - ValueError: when the same priority is already in the mapping - """ - if priority is None: - priority = self._highest_priority() - - if key in self._data: - self.remove(key) - - self._priorities.append((priority, key)) - # We rely on sort being stable - self._priorities.sort(key=lambda x: x[0]) - self._data[key] = value - assert len(self._data) == len(self._priorities) - - def remove(self, key: KT) -> VT: - """Removes a key from the mapping. - - Returns: - The value associated with the key being removed - - Raises: - KeyError: if the key is not in the mapping - """ - if key not in self._data: - raise KeyError(f"cannot find {key}") - - popped_item = self._data.pop(key) - self._priorities = [(p, k) for p, k in self._priorities if k != key] - assert len(self._data) == len(self._priorities) - return popped_item diff --git a/lib/spack/spack/cmd/common/arguments.py b/lib/spack/spack/cmd/common/arguments.py index bd6dfbbb467..41a7024d285 100644 --- a/lib/spack/spack/cmd/common/arguments.py +++ b/lib/spack/spack/cmd/common/arguments.py @@ -528,6 +528,7 @@ def __call__(self, parser, namespace, values, option_string): # the const from the constructor or a value from the CLI. # Note that this is only called if the argument is actually # specified on the command line. + spack.config.CONFIG.ensure_scope_ordering() spack.config.set(self.config_path, self.const, scope="command_line") diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index 79e7bd997c7..b23b7e4ae92 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -66,8 +66,6 @@ import spack.util.web as web_util from spack.util.cpus import cpus_available -from .enums import ConfigScopePriority - #: Dict from section names -> schema for that section SECTION_SCHEMAS: Dict[str, Any] = { "compilers": spack.schema.compilers.schema, @@ -410,18 +408,26 @@ def _method(self, *args, **kwargs): return _method -ScopeWithOptionalPriority = Union[ConfigScope, Tuple[int, ConfigScope]] -ScopeWithPriority = Tuple[int, ConfigScope] - - class Configuration: - """A hierarchical configuration, merging a number of scopes at different priorities.""" + """A full Spack configuration, from a hierarchy of config files. + + This class makes it easy to add a new scope on top of an existing one. + """ # convert to typing.OrderedDict when we drop 3.6, or OrderedDict when we reach 3.9 - scopes: lang.PriorityOrderedMapping[str, ConfigScope] + scopes: Dict[str, ConfigScope] - def __init__(self) -> None: - self.scopes = lang.PriorityOrderedMapping() + def __init__(self, *scopes: ConfigScope) -> None: + """Initialize a configuration with an initial list of scopes. + + Args: + scopes: list of scopes to add to this + Configuration, ordered from lowest to highest precedence + + """ + self.scopes = collections.OrderedDict() + for scope in scopes: + self.push_scope(scope) self.format_updates: Dict[str, List[ConfigScope]] = collections.defaultdict(list) def ensure_unwrapped(self) -> "Configuration": @@ -429,31 +435,36 @@ def ensure_unwrapped(self) -> "Configuration": return self def highest(self) -> ConfigScope: - """Scope with the highest precedence""" - return next(self.scopes.reversed_values()) # type: ignore + """Scope with highest precedence""" + return next(reversed(self.scopes.values())) # type: ignore @_config_mutator - def push_scope(self, scope: ConfigScope, priority: Optional[int] = None) -> None: - """Adds a scope to the Configuration, at a given priority. + def ensure_scope_ordering(self): + """Ensure that scope order matches documented precedent""" + # FIXME: We also need to consider that custom configurations and other orderings + # may not be preserved correctly + if "command_line" in self.scopes: + # TODO (when dropping python 3.6): self.scopes.move_to_end + self.scopes["command_line"] = self.remove_scope("command_line") - If a priority is not given, it is assumed to be the current highest priority. + @_config_mutator + def push_scope(self, scope: ConfigScope) -> None: + """Add a higher precedence scope to the Configuration.""" + tty.debug(f"[CONFIGURATION: PUSH SCOPE]: {str(scope)}", level=2) + self.scopes[scope.name] = scope - Args: - scope: scope to be added - priority: priority of the scope - """ - tty.debug(f"[CONFIGURATION: PUSH SCOPE]: {str(scope)}, priority={priority}", level=2) - self.scopes.add(scope.name, value=scope, priority=priority) + @_config_mutator + def pop_scope(self) -> ConfigScope: + """Remove the highest precedence scope and return it.""" + name, scope = self.scopes.popitem(last=True) # type: ignore[call-arg] + tty.debug(f"[CONFIGURATION: POP SCOPE]: {str(scope)}", level=2) + return scope @_config_mutator def remove_scope(self, scope_name: str) -> Optional[ConfigScope]: - """Removes a scope by name, and returns it. If the scope does not exist, returns None.""" - try: - scope = self.scopes.remove(scope_name) - tty.debug(f"[CONFIGURATION: POP SCOPE]: {str(scope)}", level=2) - except KeyError as e: - tty.debug(f"[CONFIGURATION: POP SCOPE]: {e}", level=2) - return None + """Remove scope by name; has no effect when ``scope_name`` does not exist""" + scope = self.scopes.pop(scope_name, None) + tty.debug(f"[CONFIGURATION: POP SCOPE]: {str(scope)}", level=2) return scope @property @@ -462,13 +473,15 @@ def writable_scopes(self) -> Generator[ConfigScope, None, None]: return (s for s in self.scopes.values() if s.writable) def highest_precedence_scope(self) -> ConfigScope: - """Writable scope with the highest precedence.""" - return next(s for s in self.scopes.reversed_values() if s.writable) + """Writable scope with highest precedence.""" + return next(s for s in reversed(self.scopes.values()) if s.writable) # type: ignore def highest_precedence_non_platform_scope(self) -> ConfigScope: - """Writable non-platform scope with the highest precedence""" + """Writable non-platform scope with highest precedence""" return next( - s for s in self.scopes.reversed_values() if s.writable and not s.is_platform_dependent + s + for s in reversed(self.scopes.values()) # type: ignore + if s.writable and not s.is_platform_dependent ) def matching_scopes(self, reg_expr) -> List[ConfigScope]: @@ -735,7 +748,7 @@ def override( """ if isinstance(path_or_scope, ConfigScope): overrides = path_or_scope - CONFIG.push_scope(path_or_scope, priority=None) + CONFIG.push_scope(path_or_scope) else: base_name = _OVERRIDES_BASE_NAME # Ensure the new override gets a unique scope name @@ -749,7 +762,7 @@ def override( break overrides = InternalConfigScope(scope_name) - CONFIG.push_scope(overrides, priority=None) + CONFIG.push_scope(overrides) CONFIG.set(path_or_scope, value, scope=scope_name) try: @@ -759,15 +772,13 @@ def override( assert scope is overrides -def _add_platform_scope( - cfg: Configuration, name: str, path: str, priority: ConfigScopePriority, writable: bool = True -) -> None: +def _add_platform_scope(cfg: Configuration, name: str, path: str, writable: bool = True) -> None: """Add a platform-specific subdirectory for the current platform.""" platform = spack.platforms.host().name scope = DirectoryConfigScope( f"{name}/{platform}", os.path.join(path, platform), writable=writable ) - cfg.push_scope(scope, priority=priority) + cfg.push_scope(scope) def config_paths_from_entry_points() -> List[Tuple[str, str]]: @@ -802,10 +813,11 @@ def create() -> Configuration: it. It is bundled inside a function so that configuration can be initialized lazily. """ + cfg = Configuration() + # first do the builtin, hardcoded defaults - cfg = create_from( - (ConfigScopePriority.BUILTIN, InternalConfigScope("_builtin", CONFIG_DEFAULTS)) - ) + builtin = InternalConfigScope("_builtin", CONFIG_DEFAULTS) + cfg.push_scope(builtin) # Builtin paths to configuration files in Spack configuration_paths = [ @@ -835,9 +847,10 @@ def create() -> Configuration: # add each scope and its platform-specific directory for name, path in configuration_paths: - cfg.push_scope(DirectoryConfigScope(name, path), priority=ConfigScopePriority.CONFIG_FILES) - # Each scope can have per-platform overrides in subdirectories - _add_platform_scope(cfg, name, path, priority=ConfigScopePriority.CONFIG_FILES) + cfg.push_scope(DirectoryConfigScope(name, path)) + + # Each scope can have per-platfom overrides in subdirectories + _add_platform_scope(cfg, name, path) return cfg @@ -942,7 +955,7 @@ def set(path: str, value: Any, scope: Optional[str] = None) -> None: return CONFIG.set(path, value, scope) -def scopes() -> lang.PriorityOrderedMapping[str, ConfigScope]: +def scopes() -> Dict[str, ConfigScope]: """Convenience function to get list of configuration scopes.""" return CONFIG.scopes @@ -1396,7 +1409,7 @@ def ensure_latest_format_fn(section: str) -> Callable[[YamlConfigDict], bool]: @contextlib.contextmanager def use_configuration( - *scopes_or_paths: Union[ScopeWithOptionalPriority, str] + *scopes_or_paths: Union[ConfigScope, str] ) -> Generator[Configuration, None, None]: """Use the configuration scopes passed as arguments within the context manager. @@ -1411,7 +1424,7 @@ def use_configuration( global CONFIG # Normalize input and construct a Configuration object - configuration = create_from(*scopes_or_paths) + configuration = _config_from(scopes_or_paths) CONFIG.clear_caches(), configuration.clear_caches() saved_config, CONFIG = CONFIG, configuration @@ -1422,44 +1435,23 @@ def use_configuration( CONFIG = saved_config -def _normalize_input(entry: Union[ScopeWithOptionalPriority, str]) -> ScopeWithPriority: - if isinstance(entry, tuple): - return entry - - default_priority = ConfigScopePriority.CONFIG_FILES - if isinstance(entry, ConfigScope): - return default_priority, entry - - # Otherwise we need to construct it - path = os.path.normpath(entry) - assert os.path.isdir(path), f'"{path}" must be a directory' - name = os.path.basename(path) - return default_priority, DirectoryConfigScope(name, path) - - @lang.memoized -def create_from(*scopes_or_paths: Union[ScopeWithOptionalPriority, str]) -> Configuration: - """Creates a configuration object from the scopes passed in input. +def _config_from(scopes_or_paths: List[Union[ConfigScope, str]]) -> Configuration: + scopes = [] + for scope_or_path in scopes_or_paths: + # If we have a config scope we are already done + if isinstance(scope_or_path, ConfigScope): + scopes.append(scope_or_path) + continue - Args: - *scopes_or_paths: either a tuple of (priority, ConfigScope), or a ConfigScope, or a string - If priority is not given, it is assumed to be ConfigScopePriority.CONFIG_FILES. If a - string is given, a DirectoryConfigScope is created from it. + # Otherwise we need to construct it + path = os.path.normpath(scope_or_path) + assert os.path.isdir(path), f'"{path}" must be a directory' + name = os.path.basename(path) + scopes.append(DirectoryConfigScope(name, path)) - Examples: - - >>> builtin_scope = InternalConfigScope("_builtin", {"config": {"build_jobs": 1}}) - >>> cl_scope = InternalConfigScope("command_line", {"config": {"build_jobs": 10}}) - >>> cfg = create_from( - ... (ConfigScopePriority.COMMAND_LINE, cl_scope), - ... (ConfigScopePriority.BUILTIN, builtin_scope) - ... ) - """ - scopes_with_priority = [_normalize_input(x) for x in scopes_or_paths] - result = Configuration() - for priority, scope in scopes_with_priority: - result.push_scope(scope, priority=priority) - return result + configuration = Configuration(*scopes) + return configuration def raw_github_gitlab_url(url: str) -> str: diff --git a/lib/spack/spack/enums.py b/lib/spack/spack/enums.py index 3a71d90fa88..47a48b06c33 100644 --- a/lib/spack/spack/enums.py +++ b/lib/spack/spack/enums.py @@ -12,13 +12,3 @@ class InstallRecordStatus(enum.Flag): DEPRECATED = enum.auto() MISSING = enum.auto() ANY = INSTALLED | DEPRECATED | MISSING - - -class ConfigScopePriority(enum.IntEnum): - """Priorities of the different kind of config scopes used by Spack""" - - BUILTIN = 0 - CONFIG_FILES = 1 - ENVIRONMENT = 2 - CUSTOM = 3 - COMMAND_LINE = 4 diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index bceae352371..92bdc6871fe 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -51,8 +51,6 @@ from spack.spec_list import SpecList from spack.util.path import substitute_path_variables -from ..enums import ConfigScopePriority - SpecPair = spack.concretize.SpecPair #: environment variable used to indicate the active environment @@ -3069,12 +3067,14 @@ def env_config_scopes(self) -> List[spack.config.ConfigScope]: def prepare_config_scope(self) -> None: """Add the manifest's scopes to the global configuration search path.""" for scope in self.env_config_scopes: - spack.config.CONFIG.push_scope(scope, priority=ConfigScopePriority.ENVIRONMENT) + spack.config.CONFIG.push_scope(scope) + spack.config.CONFIG.ensure_scope_ordering() def deactivate_config_scope(self) -> None: """Remove any of the manifest's scopes from the global config path.""" for scope in self.env_config_scopes: spack.config.CONFIG.remove_scope(scope.name) + spack.config.CONFIG.ensure_scope_ordering() @contextlib.contextmanager def use_config(self): diff --git a/lib/spack/spack/main.py b/lib/spack/spack/main.py index 414d467d7a7..8907022009c 100644 --- a/lib/spack/spack/main.py +++ b/lib/spack/spack/main.py @@ -47,8 +47,6 @@ import spack.util.environment import spack.util.lock -from .enums import ConfigScopePriority - #: names of profile statistics stat_names = pstats.Stats.sort_arg_dict_default @@ -874,19 +872,14 @@ def add_command_line_scopes( scopes = ev.environment_path_scopes(name, path) if scopes is None: if os.path.isdir(path): # directory with config files - cfg.push_scope( - spack.config.DirectoryConfigScope(name, path, writable=False), - priority=ConfigScopePriority.CUSTOM, - ) - spack.config._add_platform_scope( - cfg, name, path, priority=ConfigScopePriority.CUSTOM, writable=False - ) + cfg.push_scope(spack.config.DirectoryConfigScope(name, path, writable=False)) + spack.config._add_platform_scope(cfg, name, path, writable=False) continue else: raise spack.error.ConfigError(f"Invalid configuration scope: {path}") for scope in scopes: - cfg.push_scope(scope, priority=ConfigScopePriority.CUSTOM) + cfg.push_scope(scope) def _main(argv=None): @@ -959,9 +952,7 @@ def _main(argv=None): # Push scopes from the command line last if args.config_scopes: add_command_line_scopes(spack.config.CONFIG, args.config_scopes) - spack.config.CONFIG.push_scope( - spack.config.InternalConfigScope("command_line"), priority=ConfigScopePriority.COMMAND_LINE - ) + spack.config.CONFIG.push_scope(spack.config.InternalConfigScope("command_line")) setup_main_options(args) # ------------------------------------------------------------------------ diff --git a/lib/spack/spack/test/build_environment.py b/lib/spack/spack/test/build_environment.py index 86c6b5a120f..14a09e85dcf 100644 --- a/lib/spack/spack/test/build_environment.py +++ b/lib/spack/spack/test/build_environment.py @@ -542,7 +542,7 @@ def test_build_jobs_sequential_is_sequential(): spack.config.determine_number_of_jobs( parallel=False, max_cpus=8, - config=spack.config.create_from( + config=spack.config.Configuration( spack.config.InternalConfigScope("command_line", {"config": {"build_jobs": 8}}), spack.config.InternalConfigScope("defaults", {"config": {"build_jobs": 8}}), ), @@ -556,7 +556,7 @@ def test_build_jobs_command_line_overrides(): spack.config.determine_number_of_jobs( parallel=True, max_cpus=1, - config=spack.config.create_from( + config=spack.config.Configuration( spack.config.InternalConfigScope("command_line", {"config": {"build_jobs": 10}}), spack.config.InternalConfigScope("defaults", {"config": {"build_jobs": 1}}), ), @@ -567,7 +567,7 @@ def test_build_jobs_command_line_overrides(): spack.config.determine_number_of_jobs( parallel=True, max_cpus=100, - config=spack.config.create_from( + config=spack.config.Configuration( spack.config.InternalConfigScope("command_line", {"config": {"build_jobs": 10}}), spack.config.InternalConfigScope("defaults", {"config": {"build_jobs": 100}}), ), @@ -581,7 +581,7 @@ def test_build_jobs_defaults(): spack.config.determine_number_of_jobs( parallel=True, max_cpus=10, - config=spack.config.create_from( + config=spack.config.Configuration( spack.config.InternalConfigScope("defaults", {"config": {"build_jobs": 1}}) ), ) @@ -591,7 +591,7 @@ def test_build_jobs_defaults(): spack.config.determine_number_of_jobs( parallel=True, max_cpus=10, - config=spack.config.create_from( + config=spack.config.Configuration( spack.config.InternalConfigScope("defaults", {"config": {"build_jobs": 100}}) ), ) diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py index a8d79e13272..5682135ee26 100644 --- a/lib/spack/spack/test/config.py +++ b/lib/spack/spack/test/config.py @@ -33,8 +33,6 @@ import spack.util.path as spack_path import spack.util.spack_yaml as syaml -from ..enums import ConfigScopePriority - # sample config data config_low = { "config": { @@ -712,7 +710,10 @@ def assert_marked(obj): def test_internal_config_from_data(): - config = spack.config.create_from( + config = spack.config.Configuration() + + # add an internal config initialized from an inline dict + config.push_scope( spack.config.InternalConfigScope( "_builtin", {"config": {"verify_ssl": False, "build_jobs": 6}} ) @@ -1444,7 +1445,7 @@ def test_config_path_dsl(path, it_should_work, expected_parsed): @pytest.mark.regression("48254") -def test_env_activation_preserves_command_line_scope(mutable_mock_env_path): +def test_env_activation_preserves_config_scopes(mutable_mock_env_path): """Check that the "command_line" scope remains the highest priority scope, when we activate, or deactivate, environments. """ @@ -1468,33 +1469,3 @@ def test_env_activation_preserves_command_line_scope(mutable_mock_env_path): assert spack.config.CONFIG.highest() == expected_cl_scope assert spack.config.CONFIG.highest() == expected_cl_scope - - -@pytest.mark.regression("48414") -def test_env_activation_preserves_config_scopes(mutable_mock_env_path): - """Check that the priority of scopes is respected when merging configuration files.""" - custom_scope = spack.config.InternalConfigScope("custom_scope") - spack.config.CONFIG.push_scope(custom_scope, priority=ConfigScopePriority.CUSTOM) - expected_scopes = ["custom_scope", "command_line"] - - def highest_priority_scopes(config): - return list(config.scopes)[-2:] - - assert highest_priority_scopes(spack.config.CONFIG) == expected_scopes - # Creating an environment pushes a new scope - ev.create("test") - with ev.read("test"): - assert highest_priority_scopes(spack.config.CONFIG) == expected_scopes - - # No active environment pops the scope - with ev.no_active_environment(): - assert highest_priority_scopes(spack.config.CONFIG) == expected_scopes - assert highest_priority_scopes(spack.config.CONFIG) == expected_scopes - - # Switch the environment to another one - ev.create("test-2") - with ev.read("test-2"): - assert highest_priority_scopes(spack.config.CONFIG) == expected_scopes - assert highest_priority_scopes(spack.config.CONFIG) == expected_scopes - - assert highest_priority_scopes(spack.config.CONFIG) == expected_scopes diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py index 64ed92ae0b0..2193e84e70c 100644 --- a/lib/spack/spack/test/conftest.py +++ b/lib/spack/spack/test/conftest.py @@ -66,8 +66,6 @@ from spack.main import SpackCommand from spack.util.pattern import Bunch -from ..enums import ConfigScopePriority - mirror_cmd = SpackCommand("mirror") @@ -725,23 +723,11 @@ def configuration_dir(tmpdir_factory, linux_os): def _create_mock_configuration_scopes(configuration_dir): """Create the configuration scopes used in `config` and `mutable_config`.""" return [ - ( - ConfigScopePriority.BUILTIN, - spack.config.InternalConfigScope("_builtin", spack.config.CONFIG_DEFAULTS), - ), - ( - ConfigScopePriority.CONFIG_FILES, - spack.config.DirectoryConfigScope("site", str(configuration_dir.join("site"))), - ), - ( - ConfigScopePriority.CONFIG_FILES, - spack.config.DirectoryConfigScope("system", str(configuration_dir.join("system"))), - ), - ( - ConfigScopePriority.CONFIG_FILES, - spack.config.DirectoryConfigScope("user", str(configuration_dir.join("user"))), - ), - (ConfigScopePriority.COMMAND_LINE, spack.config.InternalConfigScope("command_line")), + spack.config.InternalConfigScope("_builtin", spack.config.CONFIG_DEFAULTS), + spack.config.DirectoryConfigScope("site", str(configuration_dir.join("site"))), + spack.config.DirectoryConfigScope("system", str(configuration_dir.join("system"))), + spack.config.DirectoryConfigScope("user", str(configuration_dir.join("user"))), + spack.config.InternalConfigScope("command_line"), ] @@ -808,11 +794,13 @@ def mock_wsdk_externals(monkeypatch_session): def concretize_scope(mutable_config, tmpdir): """Adds a scope for concretization preferences""" tmpdir.ensure_dir("concretize") - with spack.config.override( + mutable_config.push_scope( spack.config.DirectoryConfigScope("concretize", str(tmpdir.join("concretize"))) - ): - yield str(tmpdir.join("concretize")) + ) + yield str(tmpdir.join("concretize")) + + mutable_config.pop_scope() spack.repo.PATH._provider_index = None diff --git a/lib/spack/spack/test/env.py b/lib/spack/spack/test/env.py index 84c6046ae77..16c78bb5a9d 100644 --- a/lib/spack/spack/test/env.py +++ b/lib/spack/spack/test/env.py @@ -519,9 +519,7 @@ def test_error_message_when_using_too_new_lockfile(tmp_path): ("when_possible", True), ], ) -def test_environment_concretizer_scheme_used( - tmp_path, mutable_config, unify_in_lower_scope, unify_in_spack_yaml -): +def test_environment_concretizer_scheme_used(tmp_path, unify_in_lower_scope, unify_in_spack_yaml): """Tests that "unify" settings in spack.yaml always take precedence over settings in lower configuration scopes. """ @@ -535,11 +533,10 @@ def test_environment_concretizer_scheme_used( unify: {str(unify_in_spack_yaml).lower()} """ ) - mutable_config.set("concretizer:unify", unify_in_lower_scope) - assert mutable_config.get("concretizer:unify") == unify_in_lower_scope - with ev.Environment(manifest.parent) as e: - assert mutable_config.get("concretizer:unify") == unify_in_spack_yaml - assert e.unify == unify_in_spack_yaml + + with spack.config.override("concretizer:unify", unify_in_lower_scope): + with ev.Environment(manifest.parent) as e: + assert e.unify == unify_in_spack_yaml @pytest.mark.parametrize("unify_in_config", [True, False, "when_possible"]) diff --git a/lib/spack/spack/test/llnl/util/lang.py b/lib/spack/spack/test/llnl/util/lang.py index dbdcb1dea84..728119e31aa 100644 --- a/lib/spack/spack/test/llnl/util/lang.py +++ b/lib/spack/spack/test/llnl/util/lang.py @@ -364,44 +364,3 @@ def test_fnmatch_multiple(): assert not regex.match("libbar.so.1") assert not regex.match("libfoo.solibbar.so") assert not regex.match("libbaz.so") - - -class TestPriorityOrderedMapping: - @pytest.mark.parametrize( - "elements,expected", - [ - # Push out-of-order with explicit, and different, priorities - ([("b", 2), ("a", 1), ("d", 4), ("c", 3)], ["a", "b", "c", "d"]), - # Push in-order with priority=None - ([("a", None), ("b", None), ("c", None), ("d", None)], ["a", "b", "c", "d"]), - # Mix explicit and implicit priorities - ([("b", 2), ("c", None), ("a", 1), ("d", None)], ["a", "b", "c", "d"]), - ([("b", 10), ("c", None), ("a", -20), ("d", None)], ["a", "b", "c", "d"]), - ([("b", 10), ("c", None), ("a", 20), ("d", None)], ["b", "c", "a", "d"]), - # Adding the same key twice with different priorities - ([("b", 10), ("c", None), ("a", 20), ("d", None), ("a", -20)], ["a", "b", "c", "d"]), - # Adding the same key twice, no priorities - ([("b", None), ("a", None), ("b", None)], ["a", "b"]), - ], - ) - def test_iteration_order(self, elements, expected): - """Tests that the iteration order respects priorities, no matter the insertion order.""" - m = llnl.util.lang.PriorityOrderedMapping() - for key, priority in elements: - m.add(key, value=None, priority=priority) - assert list(m) == expected - - def test_reverse_iteration(self): - """Tests that we can conveniently use reverse iteration""" - m = llnl.util.lang.PriorityOrderedMapping() - for key, value in [("a", 1), ("b", 2), ("c", 3)]: - m.add(key, value=value) - - assert list(m) == ["a", "b", "c"] - assert list(reversed(m)) == ["c", "b", "a"] - - assert list(m.keys()) == ["a", "b", "c"] - assert list(m.reversed_keys()) == ["c", "b", "a"] - - assert list(m.values()) == [1, 2, 3] - assert list(m.reversed_values()) == [3, 2, 1] diff --git a/share/spack/gitlab/cloud_pipelines/configs/concretizer.yaml b/share/spack/gitlab/cloud_pipelines/configs/concretizer.yaml index 25b643327a5..9006e2ec56a 100644 --- a/share/spack/gitlab/cloud_pipelines/configs/concretizer.yaml +++ b/share/spack/gitlab/cloud_pipelines/configs/concretizer.yaml @@ -1,2 +1,3 @@ concretizer: reuse: false + unify: false diff --git a/share/spack/gitlab/cloud_pipelines/stacks/aws-pcluster-neoverse_v1/spack.yaml b/share/spack/gitlab/cloud_pipelines/stacks/aws-pcluster-neoverse_v1/spack.yaml index 64435384f04..e91665c398a 100644 --- a/share/spack/gitlab/cloud_pipelines/stacks/aws-pcluster-neoverse_v1/spack.yaml +++ b/share/spack/gitlab/cloud_pipelines/stacks/aws-pcluster-neoverse_v1/spack.yaml @@ -1,8 +1,5 @@ spack: view: false - concretizer: - unify: false - definitions: - apps: - gromacs diff --git a/share/spack/gitlab/cloud_pipelines/stacks/aws-pcluster-x86_64_v4/spack.yaml b/share/spack/gitlab/cloud_pipelines/stacks/aws-pcluster-x86_64_v4/spack.yaml index f9fdd4969b0..fd2d4f2138c 100644 --- a/share/spack/gitlab/cloud_pipelines/stacks/aws-pcluster-x86_64_v4/spack.yaml +++ b/share/spack/gitlab/cloud_pipelines/stacks/aws-pcluster-x86_64_v4/spack.yaml @@ -1,8 +1,5 @@ spack: view: false - concretizer: - unify: false - definitions: - apps: - gromacs %oneapi diff --git a/share/spack/gitlab/cloud_pipelines/stacks/bootstrap-aarch64-darwin/spack.yaml b/share/spack/gitlab/cloud_pipelines/stacks/bootstrap-aarch64-darwin/spack.yaml index 7888fe3f7a3..2c63d6fc4ce 100644 --- a/share/spack/gitlab/cloud_pipelines/stacks/bootstrap-aarch64-darwin/spack.yaml +++ b/share/spack/gitlab/cloud_pipelines/stacks/bootstrap-aarch64-darwin/spack.yaml @@ -1,9 +1,6 @@ spack: view: false - concretizer: - unify: false - packages: all: require: target=aarch64 diff --git a/share/spack/gitlab/cloud_pipelines/stacks/bootstrap-x86_64-linux-gnu/spack.yaml b/share/spack/gitlab/cloud_pipelines/stacks/bootstrap-x86_64-linux-gnu/spack.yaml index 4d8d316961c..67d5b20fa32 100644 --- a/share/spack/gitlab/cloud_pipelines/stacks/bootstrap-x86_64-linux-gnu/spack.yaml +++ b/share/spack/gitlab/cloud_pipelines/stacks/bootstrap-x86_64-linux-gnu/spack.yaml @@ -1,7 +1,5 @@ spack: view: false - concretizer: - unify: false packages: all: diff --git a/share/spack/gitlab/cloud_pipelines/stacks/build_systems/spack.yaml b/share/spack/gitlab/cloud_pipelines/stacks/build_systems/spack.yaml index 7f25097fbd1..3d0419e7318 100644 --- a/share/spack/gitlab/cloud_pipelines/stacks/build_systems/spack.yaml +++ b/share/spack/gitlab/cloud_pipelines/stacks/build_systems/spack.yaml @@ -1,7 +1,5 @@ spack: view: false - concretizer: - unify: false packages: all: require: target=x86_64_v3 diff --git a/share/spack/gitlab/cloud_pipelines/stacks/ml-darwin-aarch64-mps/spack.yaml b/share/spack/gitlab/cloud_pipelines/stacks/ml-darwin-aarch64-mps/spack.yaml index eff7543a190..1ccedc4b553 100644 --- a/share/spack/gitlab/cloud_pipelines/stacks/ml-darwin-aarch64-mps/spack.yaml +++ b/share/spack/gitlab/cloud_pipelines/stacks/ml-darwin-aarch64-mps/spack.yaml @@ -1,7 +1,5 @@ spack: view: false - concretizer: - unify: false packages: all: diff --git a/share/spack/gitlab/cloud_pipelines/stacks/ml-linux-aarch64-cpu/spack.yaml b/share/spack/gitlab/cloud_pipelines/stacks/ml-linux-aarch64-cpu/spack.yaml index c932c723404..23ed6aa665e 100644 --- a/share/spack/gitlab/cloud_pipelines/stacks/ml-linux-aarch64-cpu/spack.yaml +++ b/share/spack/gitlab/cloud_pipelines/stacks/ml-linux-aarch64-cpu/spack.yaml @@ -1,7 +1,5 @@ spack: view: false - concretizer: - unify: false packages: all: require: diff --git a/share/spack/gitlab/cloud_pipelines/stacks/ml-linux-aarch64-cuda/spack.yaml b/share/spack/gitlab/cloud_pipelines/stacks/ml-linux-aarch64-cuda/spack.yaml index 32070049a05..d37f8ec7d27 100644 --- a/share/spack/gitlab/cloud_pipelines/stacks/ml-linux-aarch64-cuda/spack.yaml +++ b/share/spack/gitlab/cloud_pipelines/stacks/ml-linux-aarch64-cuda/spack.yaml @@ -1,7 +1,5 @@ spack: view: false - concretizer: - unify: false packages: all: require: diff --git a/share/spack/gitlab/cloud_pipelines/stacks/ml-linux-x86_64-cpu/spack.yaml b/share/spack/gitlab/cloud_pipelines/stacks/ml-linux-x86_64-cpu/spack.yaml index e3e11577867..31ca52dd394 100644 --- a/share/spack/gitlab/cloud_pipelines/stacks/ml-linux-x86_64-cpu/spack.yaml +++ b/share/spack/gitlab/cloud_pipelines/stacks/ml-linux-x86_64-cpu/spack.yaml @@ -1,7 +1,5 @@ spack: view: false - concretizer: - unify: false packages: all: require: diff --git a/share/spack/gitlab/cloud_pipelines/stacks/ml-linux-x86_64-cuda/spack.yaml b/share/spack/gitlab/cloud_pipelines/stacks/ml-linux-x86_64-cuda/spack.yaml index cf10cbba4d6..7b508debd37 100644 --- a/share/spack/gitlab/cloud_pipelines/stacks/ml-linux-x86_64-cuda/spack.yaml +++ b/share/spack/gitlab/cloud_pipelines/stacks/ml-linux-x86_64-cuda/spack.yaml @@ -1,7 +1,5 @@ spack: view: false - concretizer: - unify: false packages: all: require: diff --git a/share/spack/gitlab/cloud_pipelines/stacks/ml-linux-x86_64-rocm/spack.yaml b/share/spack/gitlab/cloud_pipelines/stacks/ml-linux-x86_64-rocm/spack.yaml index 7dc7437133a..f3bdf578fa1 100644 --- a/share/spack/gitlab/cloud_pipelines/stacks/ml-linux-x86_64-rocm/spack.yaml +++ b/share/spack/gitlab/cloud_pipelines/stacks/ml-linux-x86_64-rocm/spack.yaml @@ -1,7 +1,5 @@ spack: view: false - concretizer: - unify: false packages: all: require: diff --git a/share/spack/gitlab/cloud_pipelines/stacks/radiuss-aws-aarch64/spack.yaml b/share/spack/gitlab/cloud_pipelines/stacks/radiuss-aws-aarch64/spack.yaml index dd8d74f5768..c5a1259ba62 100644 --- a/share/spack/gitlab/cloud_pipelines/stacks/radiuss-aws-aarch64/spack.yaml +++ b/share/spack/gitlab/cloud_pipelines/stacks/radiuss-aws-aarch64/spack.yaml @@ -1,7 +1,5 @@ spack: view: false - concretizer: - unify: false packages: all: providers: diff --git a/share/spack/gitlab/cloud_pipelines/stacks/radiuss-aws/spack.yaml b/share/spack/gitlab/cloud_pipelines/stacks/radiuss-aws/spack.yaml index e8ddea19869..ca7de563c44 100644 --- a/share/spack/gitlab/cloud_pipelines/stacks/radiuss-aws/spack.yaml +++ b/share/spack/gitlab/cloud_pipelines/stacks/radiuss-aws/spack.yaml @@ -1,7 +1,5 @@ spack: view: false - concretizer: - unify: false packages: all: providers: diff --git a/share/spack/gitlab/cloud_pipelines/stacks/radiuss/spack.yaml b/share/spack/gitlab/cloud_pipelines/stacks/radiuss/spack.yaml index 6fcbbcc7980..3aea6fc48a4 100644 --- a/share/spack/gitlab/cloud_pipelines/stacks/radiuss/spack.yaml +++ b/share/spack/gitlab/cloud_pipelines/stacks/radiuss/spack.yaml @@ -1,7 +1,5 @@ spack: view: false - concretizer: - unify: false packages: all: require: target=x86_64_v3 diff --git a/share/spack/gitlab/cloud_pipelines/stacks/tutorial/spack.yaml b/share/spack/gitlab/cloud_pipelines/stacks/tutorial/spack.yaml index a58c60376e1..58695a07175 100644 --- a/share/spack/gitlab/cloud_pipelines/stacks/tutorial/spack.yaml +++ b/share/spack/gitlab/cloud_pipelines/stacks/tutorial/spack.yaml @@ -1,7 +1,5 @@ spack: view: false - concretizer: - unify: false packages: all: require: target=x86_64_v3 diff --git a/share/spack/gitlab/cloud_pipelines/stacks/windows-vis/spack.yaml b/share/spack/gitlab/cloud_pipelines/stacks/windows-vis/spack.yaml index 38cd91cb9e7..1d2546b6980 100644 --- a/share/spack/gitlab/cloud_pipelines/stacks/windows-vis/spack.yaml +++ b/share/spack/gitlab/cloud_pipelines/stacks/windows-vis/spack.yaml @@ -5,8 +5,6 @@ spack: view: false - concretizer: - unify: false specs: - vtk~mpi