From 25beeef865a5fca39435f399b52aa2a828249101 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Mon, 14 Apr 2025 11:22:13 +0200 Subject: [PATCH] Environment: separate parsing concerns from SpecList (#49973) Signed-off-by: Massimiliano Culpo --- lib/spack/spack/environment/environment.py | 101 +++----- .../{spec_list.py => environment/list.py} | 217 +++++++++------- lib/spack/spack/test/env.py | 11 +- lib/spack/spack/test/spec_list.py | 245 ++++++++---------- 4 files changed, 270 insertions(+), 304 deletions(-) rename lib/spack/spack/{spec_list.py => environment/list.py} (50%) diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index ca8e996bd6b..93dca484d5a 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -31,7 +31,6 @@ import spack.repo import spack.schema.env import spack.spec -import spack.spec_list import spack.store import spack.user_environment as uenv import spack.util.environment @@ -44,10 +43,10 @@ from spack.installer import PackageInstaller from spack.schema.env import TOP_LEVEL_KEY from spack.spec import Spec -from spack.spec_list import SpecList from spack.util.path import substitute_path_variables from ..enums import ConfigScopePriority +from .list import SpecList, SpecListError, SpecListParser SpecPair = spack.concretize.SpecPair @@ -932,8 +931,10 @@ def __init__(self, manifest_dir: Union[str, pathlib.Path]) -> None: self.new_specs: List[Spec] = [] self.views: Dict[str, ViewDescriptor] = {} + #: Parser for spec lists + self._spec_lists_parser = SpecListParser() #: Specs from "spack.yaml" - self.spec_lists: Dict[str, SpecList] = {user_speclist_name: SpecList()} + self.spec_lists: Dict[str, SpecList] = {} #: User specs from the last concretization self.concretized_user_specs: List[Spec] = [] #: Roots associated with the last concretization, in order @@ -1001,26 +1002,6 @@ def write_transaction(self): """Get a write lock context manager for use in a `with` block.""" return lk.WriteTransaction(self.txlock, acquire=self._re_read) - def _process_definition(self, entry): - """Process a single spec definition item.""" - when_string = entry.get("when") - if when_string is not None: - when = spack.spec.eval_conditional(when_string) - assert len([x for x in entry if x != "when"]) == 1 - else: - when = True - assert len(entry) == 1 - - if when: - for name, spec_list in entry.items(): - if name == "when": - continue - user_specs = SpecList(name, spec_list, self.spec_lists.copy()) - if name in self.spec_lists: - self.spec_lists[name].extend(user_specs) - else: - self.spec_lists[name] = user_specs - def _process_view(self, env_view: Optional[Union[bool, str, Dict]]): """Process view option(s), which can be boolean, string, or None. @@ -1082,21 +1063,24 @@ def _process_concrete_includes(self): def _construct_state_from_manifest(self): """Set up user specs and views from the manifest file.""" - self.spec_lists = collections.OrderedDict() self.views = {} + self._sync_speclists() + self._process_view(spack.config.get("view", True)) + self._process_concrete_includes() - for item in spack.config.get("definitions", []): - self._process_definition(item) + def _sync_speclists(self): + self.spec_lists = {} + self.spec_lists.update( + self._spec_lists_parser.parse_definitions( + data=spack.config.CONFIG.get("definitions", []) + ) + ) env_configuration = self.manifest[TOP_LEVEL_KEY] spec_list = env_configuration.get(user_speclist_name, []) - user_specs = SpecList( - user_speclist_name, [s for s in spec_list if s], self.spec_lists.copy() + self.spec_lists[user_speclist_name] = self._spec_lists_parser.parse_user_specs( + name=user_speclist_name, yaml_list=spec_list ) - self.spec_lists[user_speclist_name] = user_specs - - self._process_view(spack.config.get("view", True)) - self._process_concrete_includes() def all_concretized_user_specs(self) -> List[Spec]: """Returns all of the concretized user specs of the environment and @@ -1167,9 +1151,7 @@ def clear(self, re_read=False): re_read: If ``True``, do not clear ``new_specs``. This value cannot be read from yaml, and needs to be maintained when re-reading an existing environment. """ - self.spec_lists = collections.OrderedDict() - self.spec_lists[user_speclist_name] = SpecList() - + self.spec_lists = {} self._dev_specs = {} self.concretized_order = [] # roots of last concretize, in order self.concretized_user_specs = [] # user specs from last concretize @@ -1276,22 +1258,6 @@ def destroy(self): """Remove this environment from Spack entirely.""" shutil.rmtree(self.path) - def update_stale_references(self, from_list=None): - """Iterate over spec lists updating references.""" - if not from_list: - from_list = next(iter(self.spec_lists.keys())) - index = list(self.spec_lists.keys()).index(from_list) - - # spec_lists is an OrderedDict to ensure lists read from the manifest - # are maintainted in order, hence, all list entries after the modified - # list may refer to the modified list requiring stale references to be - # updated. - for i, (name, speclist) in enumerate( - list(self.spec_lists.items())[index + 1 :], index + 1 - ): - new_reference = dict((n, self.spec_lists[n]) for n in list(self.spec_lists.keys())[:i]) - speclist.update_reference(new_reference) - def add(self, user_spec, list_name=user_speclist_name): """Add a single user_spec (non-concretized) to the Environment @@ -1311,18 +1277,17 @@ def add(self, user_spec, list_name=user_speclist_name): elif not spack.repo.PATH.exists(spec.name) and not spec.abstract_hash: virtuals = spack.repo.PATH.provider_index.providers.keys() if spec.name not in virtuals: - msg = "no such package: %s" % spec.name - raise SpackEnvironmentError(msg) + raise SpackEnvironmentError(f"no such package: {spec.name}") list_to_change = self.spec_lists[list_name] existing = str(spec) in list_to_change.yaml_list if not existing: list_to_change.add(str(spec)) - self.update_stale_references(list_name) if list_name == user_speclist_name: self.manifest.add_user_spec(str(user_spec)) else: self.manifest.add_definition(str(user_spec), list_name=list_name) + self._sync_speclists() return bool(not existing) @@ -1366,18 +1331,17 @@ def change_existing_spec( "There are no specs named {0} in {1}".format(match_spec.name, list_name) ) elif len(matches) > 1 and not allow_changing_multiple_specs: - raise ValueError("{0} matches multiple specs".format(str(match_spec))) + raise ValueError(f"{str(match_spec)} matches multiple specs") for idx, spec in matches: override_spec = Spec.override(spec, change_spec) - self.spec_lists[list_name].replace(idx, str(override_spec)) if list_name == user_speclist_name: self.manifest.override_user_spec(str(override_spec), idx=idx) else: self.manifest.override_definition( str(spec), override=str(override_spec), list_name=list_name ) - self.update_stale_references(from_list=list_name) + self._sync_speclists() def remove(self, query_spec, list_name=user_speclist_name, force=False): """Remove specs from an environment that match a query_spec""" @@ -1405,22 +1369,17 @@ def remove(self, query_spec, list_name=user_speclist_name, force=False): raise SpackEnvironmentError(f"{err_msg_header}, no spec matches") old_specs = set(self.user_specs) - new_specs = set() + + # Remove specs from the appropriate spec list for spec in matches: if spec not in list_to_change: continue try: list_to_change.remove(spec) - self.update_stale_references(list_name) - new_specs = set(self.user_specs) - except spack.spec_list.SpecListError as e: - # define new specs list - new_specs = set(self.user_specs) + except SpecListError as e: msg = str(e) if force: msg += " It will be removed from the concrete specs." - # Mock new specs, so we can remove this spec from concrete spec lists - new_specs.remove(spec) tty.warn(msg) else: if list_name == user_speclist_name: @@ -1428,7 +1387,11 @@ def remove(self, query_spec, list_name=user_speclist_name, force=False): else: self.manifest.remove_definition(str(spec), list_name=list_name) - # If force, update stale concretized specs + # Recompute "definitions" and user specs + self._sync_speclists() + new_specs = set(self.user_specs) + + # If 'force', update stale concretized specs for spec in old_specs - new_specs: if force and spec in self.concretized_user_specs: i = self.concretized_user_specs.index(spec) @@ -2827,6 +2790,8 @@ def add_definition(self, user_spec: str, list_name: str) -> None: item[list_name].append(user_spec) break + # "definitions" can be remote, so we need to update the global config too + spack.config.CONFIG.set("definitions", defs, scope=self.scope_name) self.changed = True def remove_definition(self, user_spec: str, list_name: str) -> None: @@ -2853,6 +2818,8 @@ def remove_definition(self, user_spec: str, list_name: str) -> None: except ValueError: pass + # "definitions" can be remote, so we need to update the global config too + spack.config.CONFIG.set("definitions", defs, scope=self.scope_name) self.changed = True def override_definition(self, user_spec: str, *, override: str, list_name: str) -> None: @@ -2878,6 +2845,8 @@ def override_definition(self, user_spec: str, *, override: str, list_name: str) except ValueError: pass + # "definitions" can be remote, so we need to update the global config too + spack.config.CONFIG.set("definitions", defs, scope=self.scope_name) self.changed = True def _iterate_on_definitions(self, definitions, *, list_name, err_msg): diff --git a/lib/spack/spack/spec_list.py b/lib/spack/spack/environment/list.py similarity index 50% rename from lib/spack/spack/spec_list.py rename to lib/spack/spack/environment/list.py index eadc28fa8f8..9bb258fa777 100644 --- a/lib/spack/spack/spec_list.py +++ b/lib/spack/spack/environment/list.py @@ -2,36 +2,24 @@ # # SPDX-License-Identifier: (Apache-2.0 OR MIT) import itertools -from typing import List +from typing import Any, Dict, List, NamedTuple, Optional, Union import spack.spec +import spack.util.spack_yaml import spack.variant from spack.error import SpackError from spack.spec import Spec class SpecList: - def __init__(self, name="specs", yaml_list=None, reference=None): - # Normalize input arguments - yaml_list = yaml_list or [] - reference = reference or {} - + def __init__(self, *, name: str = "specs", yaml_list=None, expanded_list=None): self.name = name - self._reference = reference # TODO: Do we need defensive copy here? - - # Validate yaml_list before assigning - if not all(isinstance(s, str) or isinstance(s, (list, dict)) for s in yaml_list): - raise ValueError( - "yaml_list can contain only valid YAML types! Found:\n %s" - % [type(s) for s in yaml_list] - ) - self.yaml_list = yaml_list[:] - + self.yaml_list = yaml_list[:] if yaml_list is not None else [] # Expansions can be expensive to compute and difficult to keep updated # We cache results and invalidate when self.yaml_list changes - self._expanded_list = None + self.specs_as_yaml_list = expanded_list or [] self._constraints = None - self._specs = None + self._specs: Optional[List[Spec]] = None @property def is_matrix(self): @@ -40,12 +28,6 @@ def is_matrix(self): return True return False - @property - def specs_as_yaml_list(self): - if self._expanded_list is None: - self._expanded_list = self._expand_references(self.yaml_list) - return self._expanded_list - @property def specs_as_constraints(self): if self._constraints is None: @@ -62,7 +44,7 @@ def specs_as_constraints(self): @property def specs(self) -> List[Spec]: if self._specs is None: - specs = [] + specs: List[Spec] = [] # This could be slightly faster done directly from yaml_list, # but this way is easier to maintain. for constraint_list in self.specs_as_constraints: @@ -74,12 +56,13 @@ def specs(self) -> List[Spec]: return self._specs - def add(self, spec): - self.yaml_list.append(str(spec)) + def add(self, spec: Spec): + spec_str = str(spec) + self.yaml_list.append(spec_str) # expanded list can be updated without invalidation - if self._expanded_list is not None: - self._expanded_list.append(str(spec)) + if self.specs_as_yaml_list is not None: + self.specs_as_yaml_list.append(spec_str) # Invalidate cache variables when we change the list self._constraints = None @@ -101,83 +84,18 @@ def remove(self, spec): # Remove may contain more than one string representation of the same spec for item in remove: self.yaml_list.remove(item) + self.specs_as_yaml_list.remove(item) # invalidate cache variables when we change the list - self._expanded_list = None self._constraints = None self._specs = None - def replace(self, idx: int, spec: str): - """Replace the existing spec at the index with the new one. - - Args: - idx: index of the spec to replace in the speclist - spec: new spec - """ - self.yaml_list[idx] = spec - - # invalidate cache variables when we change the list - self._expanded_list = None - self._constraints = None - self._specs = None - - def extend(self, other, copy_reference=True): + def extend(self, other: "SpecList", copy_reference=True) -> None: self.yaml_list.extend(other.yaml_list) - self._expanded_list = None + self.specs_as_yaml_list.extend(other.specs_as_yaml_list) self._constraints = None self._specs = None - if copy_reference: - self._reference = other._reference - - def update_reference(self, reference): - self._reference = reference - self._expanded_list = None - self._constraints = None - self._specs = None - - def _parse_reference(self, name): - sigil = "" - name = name[1:] - - # Parse specs as constraints - if name.startswith("^") or name.startswith("%"): - sigil = name[0] - name = name[1:] - - # Make sure the reference is valid - if name not in self._reference: - msg = f"SpecList '{self.name}' refers to named list '{name}'" - msg += " which does not appear in its reference dict." - raise UndefinedReferenceError(msg) - - return (name, sigil) - - def _expand_references(self, yaml): - if isinstance(yaml, list): - ret = [] - - for item in yaml: - # if it's a reference, expand it - if isinstance(item, str) and item.startswith("$"): - # replace the reference and apply the sigil if needed - name, sigil = self._parse_reference(item) - - referent = [ - _sigilify(item, sigil) for item in self._reference[name].specs_as_yaml_list - ] - ret.extend(referent) - else: - # else just recurse - ret.append(self._expand_references(item)) - return ret - elif isinstance(yaml, dict): - # There can't be expansions in dicts - return dict((name, self._expand_references(val)) for (name, val) in yaml.items()) - else: - # Strings are just returned - return yaml - def __len__(self): return len(self.specs) @@ -251,6 +169,111 @@ def _sigilify(item, sigil): return sigil + item +class Definition(NamedTuple): + name: str + yaml_list: List[Union[str, Dict]] + when: Optional[str] + + +class SpecListParser: + """Parse definitions and user specs from data in environments""" + + def __init__(self): + self.definitions: Dict[str, SpecList] = {} + + def parse_definitions(self, *, data: List[Dict[str, Any]]) -> Dict[str, SpecList]: + definitions_from_yaml: Dict[str, List[Definition]] = {} + for item in data: + value = self._parse_yaml_definition(item) + definitions_from_yaml.setdefault(value.name, []).append(value) + + self.definitions = {} + self._build_definitions(definitions_from_yaml) + return self.definitions + + def parse_user_specs(self, *, name, yaml_list) -> SpecList: + definition = Definition(name=name, yaml_list=yaml_list, when=None) + return self._speclist_from_definitions(name, [definition]) + + def _parse_yaml_definition(self, yaml_entry) -> Definition: + when_string = yaml_entry.get("when") + + if (when_string and len(yaml_entry) > 2) or (not when_string and len(yaml_entry) > 1): + mark = spack.util.spack_yaml.get_mark_from_yaml_data(yaml_entry) + attributes = ", ".join(x for x in yaml_entry if x != "when") + error_msg = f"definition must have a single attribute, got many: {attributes}" + raise SpecListError(f"{mark.name}:{mark.line + 1}: {error_msg}") + + for name, yaml_list in yaml_entry.items(): + if name == "when": + continue + return Definition(name=name, yaml_list=yaml_list, when=when_string) + + # If we are here, it means only "when" is in the entry + mark = spack.util.spack_yaml.get_mark_from_yaml_data(yaml_entry) + error_msg = "definition must have a single attribute, got none" + raise SpecListError(f"{mark.name}:{mark.line + 1}: {error_msg}") + + def _build_definitions(self, definitions_from_yaml: Dict[str, List[Definition]]): + for name, definitions in definitions_from_yaml.items(): + self.definitions[name] = self._speclist_from_definitions(name, definitions) + + def _speclist_from_definitions(self, name, definitions) -> SpecList: + combined_yaml_list = [] + for def_part in definitions: + if def_part.when is not None and not spack.spec.eval_conditional(def_part.when): + continue + combined_yaml_list.extend(def_part.yaml_list) + expanded_list = self._expand_yaml_list(combined_yaml_list) + return SpecList(name=name, yaml_list=combined_yaml_list, expanded_list=expanded_list) + + def _expand_yaml_list(self, raw_yaml_list): + result = [] + for item in raw_yaml_list: + if isinstance(item, str) and item.startswith("$"): + result.extend(self._expand_reference(item)) + continue + + value = item + if isinstance(item, dict): + value = self._expand_yaml_matrix(item) + result.append(value) + return result + + def _expand_reference(self, item: str): + sigil, name = "", item[1:] + if name.startswith("^") or name.startswith("%"): + sigil, name = name[0], name[1:] + + if name not in self.definitions: + mark = spack.util.spack_yaml.get_mark_from_yaml_data(item) + error_msg = f"trying to expand the name '{name}', which is not defined yet" + raise UndefinedReferenceError(f"{mark.name}:{mark.line + 1}: {error_msg}") + + value = self.definitions[name].specs_as_yaml_list + if not sigil: + return value + return [_sigilify(x, sigil) for x in value] + + def _expand_yaml_matrix(self, matrix_yaml): + extra_attributes = set(matrix_yaml) - {"matrix", "exclude"} + if extra_attributes: + mark = spack.util.spack_yaml.get_mark_from_yaml_data(matrix_yaml) + error_msg = f"extra attributes in spec matrix: {','.join(sorted(extra_attributes))}" + raise SpecListError(f"{mark.name}:{mark.line + 1}: {error_msg}") + + if "matrix" not in matrix_yaml: + mark = spack.util.spack_yaml.get_mark_from_yaml_data(matrix_yaml) + error_msg = "matrix is missing the 'matrix' attribute" + raise SpecListError(f"{mark.name}:{mark.line + 1}: {error_msg}") + + # Assume data has been validated against the YAML schema + result = {"matrix": [self._expand_yaml_list(row) for row in matrix_yaml["matrix"]]} + if "exclude" in matrix_yaml: + result["exclude"] = matrix_yaml["exclude"] + return result + + class SpecListError(SpackError): """Error class for all errors related to SpecList objects.""" diff --git a/lib/spack/spack/test/env.py b/lib/spack/spack/test/env.py index a888c88d99b..64476624d0d 100644 --- a/lib/spack/spack/test/env.py +++ b/lib/spack/spack/test/env.py @@ -20,7 +20,7 @@ SpackEnvironmentViewError, _error_on_nonempty_view_dir, ) -from spack.spec_list import UndefinedReferenceError +from spack.environment.list import UndefinedReferenceError pytestmark = pytest.mark.not_on_windows("Envs are not supported on windows") @@ -107,7 +107,8 @@ def test_env_change_spec_in_definition(tmp_path, mock_packages, mutable_mock_env assert any(x.intersects("mpileaks@2.1%gcc") for x in e.user_specs) - e.change_existing_spec(spack.spec.Spec("mpileaks@2.2"), list_name="desired_specs") + with e: + e.change_existing_spec(spack.spec.Spec("mpileaks@2.2"), list_name="desired_specs") e.write() # Ensure changed specs are in memory @@ -776,10 +777,8 @@ def test_env_with_include_def_missing(mutable_mock_env_path, mock_packages): """ ) - e = ev.Environment(env_path) - with e: - with pytest.raises(UndefinedReferenceError, match=r"which does not appear"): - e.concretize() + with pytest.raises(UndefinedReferenceError, match=r"which is not defined"): + _ = ev.Environment(env_path) @pytest.mark.regression("41292") diff --git a/lib/spack/spack/test/spec_list.py b/lib/spack/spack/test/spec_list.py index 275d267fa69..fdafdbf05d7 100644 --- a/lib/spack/spack/test/spec_list.py +++ b/lib/spack/spack/test/spec_list.py @@ -6,53 +6,56 @@ import pytest import spack.concretize +from spack.environment.list import SpecListParser from spack.installer import PackageInstaller from spack.spec import Spec -from spack.spec_list import SpecList + +DEFAULT_EXPANSION = [ + "mpileaks", + "zmpi@1.0", + "mpich@3.0", + {"matrix": [["hypre"], ["%gcc@4.5.0", "%clang@3.3"]]}, + "libelf", +] + +DEFAULT_CONSTRAINTS = [ + [Spec("mpileaks")], + [Spec("zmpi@1.0")], + [Spec("mpich@3.0")], + [Spec("hypre"), Spec("%gcc@4.5.0")], + [Spec("hypre"), Spec("%clang@3.3")], + [Spec("libelf")], +] + +DEFAULT_SPECS = [ + Spec("mpileaks"), + Spec("zmpi@1.0"), + Spec("mpich@3.0"), + Spec("hypre%gcc@4.5.0"), + Spec("hypre%clang@3.3"), + Spec("libelf"), +] + + +@pytest.fixture() +def parser_and_speclist(): + """Default configuration of parser and user spec list for tests""" + parser = SpecListParser() + parser.parse_definitions( + data=[ + {"gccs": ["%gcc@4.5.0"]}, + {"clangs": ["%clang@3.3"]}, + {"mpis": ["zmpi@1.0", "mpich@3.0"]}, + ] + ) + result = parser.parse_user_specs( + name="specs", + yaml_list=["mpileaks", "$mpis", {"matrix": [["hypre"], ["$gccs", "$clangs"]]}, "libelf"], + ) + return parser, result class TestSpecList: - default_input = ["mpileaks", "$mpis", {"matrix": [["hypre"], ["$gccs", "$clangs"]]}, "libelf"] - - default_reference = { - "gccs": SpecList("gccs", ["%gcc@4.5.0"]), - "clangs": SpecList("clangs", ["%clang@3.3"]), - "mpis": SpecList("mpis", ["zmpi@1.0", "mpich@3.0"]), - } - - default_expansion = [ - "mpileaks", - "zmpi@1.0", - "mpich@3.0", - {"matrix": [["hypre"], ["%gcc@4.5.0", "%clang@3.3"]]}, - "libelf", - ] - - default_constraints = [ - [Spec("mpileaks")], - [Spec("zmpi@1.0")], - [Spec("mpich@3.0")], - [Spec("hypre"), Spec("%gcc@4.5.0")], - [Spec("hypre"), Spec("%clang@3.3")], - [Spec("libelf")], - ] - - default_specs = [ - Spec("mpileaks"), - Spec("zmpi@1.0"), - Spec("mpich@3.0"), - Spec("hypre%gcc@4.5.0"), - Spec("hypre%clang@3.3"), - Spec("libelf"), - ] - - def test_spec_list_expansions(self): - speclist = SpecList("specs", self.default_input, self.default_reference) - - assert speclist.specs_as_yaml_list == self.default_expansion - assert speclist.specs_as_constraints == self.default_constraints - assert speclist.specs == self.default_specs - @pytest.mark.regression("28749") @pytest.mark.parametrize( "specs,expected", @@ -86,116 +89,87 @@ def test_spec_list_expansions(self): ], ) def test_spec_list_constraint_ordering(self, specs, expected): - speclist = SpecList("specs", specs) - expected_specs = [Spec(x) for x in expected] - assert speclist.specs == expected_specs + result = SpecListParser().parse_user_specs(name="specs", yaml_list=specs) + assert result.specs == [Spec(x) for x in expected] - def test_spec_list_add(self): - speclist = SpecList("specs", self.default_input, self.default_reference) + def test_mock_spec_list(self, parser_and_speclist): + """Tests expected properties on the default mock spec list""" + parser, mock_list = parser_and_speclist + assert mock_list.specs_as_yaml_list == DEFAULT_EXPANSION + assert mock_list.specs_as_constraints == DEFAULT_CONSTRAINTS + assert mock_list.specs == DEFAULT_SPECS - assert speclist.specs_as_yaml_list == self.default_expansion - assert speclist.specs_as_constraints == self.default_constraints - assert speclist.specs == self.default_specs + def test_spec_list_add(self, parser_and_speclist): + parser, mock_list = parser_and_speclist + mock_list.add("libdwarf") - speclist.add("libdwarf") + assert mock_list.specs_as_yaml_list == DEFAULT_EXPANSION + ["libdwarf"] + assert mock_list.specs_as_constraints == DEFAULT_CONSTRAINTS + [[Spec("libdwarf")]] + assert mock_list.specs == DEFAULT_SPECS + [Spec("libdwarf")] - assert speclist.specs_as_yaml_list == self.default_expansion + ["libdwarf"] - assert speclist.specs_as_constraints == self.default_constraints + [[Spec("libdwarf")]] - assert speclist.specs == self.default_specs + [Spec("libdwarf")] + def test_spec_list_remove(self, parser_and_speclist): + parser, mock_list = parser_and_speclist + mock_list.remove("libelf") - def test_spec_list_remove(self): - speclist = SpecList("specs", self.default_input, self.default_reference) + assert mock_list.specs_as_yaml_list + ["libelf"] == DEFAULT_EXPANSION + assert mock_list.specs_as_constraints + [[Spec("libelf")]] == DEFAULT_CONSTRAINTS + assert mock_list.specs + [Spec("libelf")] == DEFAULT_SPECS - assert speclist.specs_as_yaml_list == self.default_expansion - assert speclist.specs_as_constraints == self.default_constraints - assert speclist.specs == self.default_specs - - speclist.remove("libelf") - - assert speclist.specs_as_yaml_list + ["libelf"] == self.default_expansion - - assert speclist.specs_as_constraints + [[Spec("libelf")]] == self.default_constraints - - assert speclist.specs + [Spec("libelf")] == self.default_specs - - def test_spec_list_update_reference(self): - speclist = SpecList("specs", self.default_input, self.default_reference) - - assert speclist.specs_as_yaml_list == self.default_expansion - assert speclist.specs_as_constraints == self.default_constraints - assert speclist.specs == self.default_specs - - new_mpis = SpecList("mpis", self.default_reference["mpis"].yaml_list) - new_mpis.add("mpich@3.3") - new_reference = self.default_reference.copy() - new_reference["mpis"] = new_mpis - - speclist.update_reference(new_reference) - - expansion = list(self.default_expansion) - expansion.insert(3, "mpich@3.3") - constraints = list(self.default_constraints) - constraints.insert(3, [Spec("mpich@3.3")]) - specs = list(self.default_specs) - specs.insert(3, Spec("mpich@3.3")) - - assert speclist.specs_as_yaml_list == expansion - assert speclist.specs_as_constraints == constraints - assert speclist.specs == specs - - def test_spec_list_extension(self): - speclist = SpecList("specs", self.default_input, self.default_reference) - - assert speclist.specs_as_yaml_list == self.default_expansion - assert speclist.specs_as_constraints == self.default_constraints - assert speclist.specs == self.default_specs - - new_ref = self.default_reference.copy() - otherlist = SpecList("specs", ["zlib", {"matrix": [["callpath"], ["%intel@18"]]}], new_ref) - - speclist.extend(otherlist) - - assert speclist.specs_as_yaml_list == ( - self.default_expansion + otherlist.specs_as_yaml_list + def test_spec_list_extension(self, parser_and_speclist): + parser, mock_list = parser_and_speclist + other_list = parser.parse_user_specs( + name="specs", yaml_list=[{"matrix": [["callpath"], ["%intel@18"]]}] ) - assert speclist.specs == self.default_specs + otherlist.specs - assert speclist._reference is new_ref + mock_list.extend(other_list) + + assert mock_list.specs_as_yaml_list == (DEFAULT_EXPANSION + other_list.specs_as_yaml_list) + assert mock_list.specs == DEFAULT_SPECS + other_list.specs + + def test_spec_list_nested_matrices(self, parser_and_speclist): + parser, _ = parser_and_speclist - def test_spec_list_nested_matrices(self): inner_matrix = [{"matrix": [["zlib", "libelf"], ["%gcc", "%intel"]]}] outer_addition = ["+shared", "~shared"] outer_matrix = [{"matrix": [inner_matrix, outer_addition]}] - speclist = SpecList("specs", outer_matrix) + result = parser.parse_user_specs(name="specs", yaml_list=outer_matrix) expected_components = itertools.product( ["zlib", "libelf"], ["%gcc", "%intel"], ["+shared", "~shared"] ) expected = [Spec(" ".join(combo)) for combo in expected_components] - assert set(speclist.specs) == set(expected) + assert set(result.specs) == set(expected) @pytest.mark.regression("16897") def test_spec_list_recursion_specs_as_constraints(self): input = ["mpileaks", "$mpis", {"matrix": [["hypre"], ["$%gccs", "$%clangs"]]}, "libelf"] - reference = { - "gccs": SpecList("gccs", ["gcc@4.5.0"]), - "clangs": SpecList("clangs", ["clang@3.3"]), - "mpis": SpecList("mpis", ["zmpi@1.0", "mpich@3.0"]), - } - - speclist = SpecList("specs", input, reference) - - assert speclist.specs_as_yaml_list == self.default_expansion - assert speclist.specs_as_constraints == self.default_constraints - assert speclist.specs == self.default_specs - - def test_spec_list_matrix_exclude(self, mock_packages): - # Test on non-boolean variants for regression for #16841 - matrix = [ - {"matrix": [["multivalue-variant"], ["foo=bar", "foo=baz"]], "exclude": ["foo=bar"]} + definitions = [ + {"gccs": ["gcc@4.5.0"]}, + {"clangs": ["clang@3.3"]}, + {"mpis": ["zmpi@1.0", "mpich@3.0"]}, ] - speclist = SpecList("specs", matrix) - assert len(speclist.specs) == 1 + + parser = SpecListParser() + parser.parse_definitions(data=definitions) + result = parser.parse_user_specs(name="specs", yaml_list=input) + + assert result.specs_as_yaml_list == DEFAULT_EXPANSION + assert result.specs_as_constraints == DEFAULT_CONSTRAINTS + assert result.specs == DEFAULT_SPECS + + @pytest.mark.regression("16841") + def test_spec_list_matrix_exclude(self, mock_packages): + parser = SpecListParser() + result = parser.parse_user_specs( + name="specs", + yaml_list=[ + { + "matrix": [["multivalue-variant"], ["foo=bar", "foo=baz"]], + "exclude": ["foo=bar"], + } + ], + ) + assert len(result.specs) == 1 def test_spec_list_exclude_with_abstract_hashes(self, mock_packages, install_mockery): # Put mpich in the database so it can be referred to by hash. @@ -205,9 +179,10 @@ def test_spec_list_exclude_with_abstract_hashes(self, mock_packages, install_moc # Create matrix and exclude +debug, which excludes the first mpich after its abstract hash # is resolved. - speclist = SpecList( - "specs", - [ + parser = SpecListParser() + result = parser.parse_user_specs( + name="specs", + yaml_list=[ { "matrix": [ ["mpileaks"], @@ -220,5 +195,5 @@ def test_spec_list_exclude_with_abstract_hashes(self, mock_packages, install_moc ) # Ensure that only mpich~debug is selected, and that the assembled spec remains abstract. - assert len(speclist.specs) == 1 - assert speclist.specs[0] == Spec(f"mpileaks ^callpath ^mpich/{mpich_2.dag_hash(5)}") + assert len(result.specs) == 1 + assert result.specs[0] == Spec(f"mpileaks ^callpath ^mpich/{mpich_2.dag_hash(5)}")