diff --git a/lib/spack/spack/ci/common.py b/lib/spack/spack/ci/common.py index 3f61803a733..90ce3af8325 100644 --- a/lib/spack/spack/ci/common.py +++ b/lib/spack/spack/ci/common.py @@ -26,6 +26,7 @@ import spack.environment as ev import spack.error import spack.mirrors.mirror +import spack.schema import spack.spec import spack.util.spack_yaml as syaml import spack.util.url as url_util @@ -608,7 +609,7 @@ def __apply_submapping(self, dest, spec, section): if "build-job-remove" in match_attrs: spack.config.remove_yaml(dest, attrs["build-job-remove"]) if "build-job" in match_attrs: - spack.config.merge_yaml(dest, attrs["build-job"]) + spack.schema.merge_yaml(dest, attrs["build-job"]) break if matched and only_first: break @@ -679,7 +680,7 @@ def _apply_section(dest, src): if do_remove: dest = spack.config.remove_yaml(dest, src[remove_job_name]) if do_merge: - dest = copy.copy(spack.config.merge_yaml(dest, src[merge_job_name])) + dest = copy.copy(spack.schema.merge_yaml(dest, src[merge_job_name])) if name == "build": # Apply attributes to all build jobs @@ -808,7 +809,7 @@ def job_query(job): tty.warn(f"Response missing required keys: {missing_keys}") if clean_config: - job["attributes"] = spack.config.merge_yaml( + job["attributes"] = spack.schema.merge_yaml( job.get("attributes", {}), clean_config ) diff --git a/lib/spack/spack/ci/gitlab.py b/lib/spack/spack/ci/gitlab.py index 65cab9a887c..735b9a20fd6 100644 --- a/lib/spack/spack/ci/gitlab.py +++ b/lib/spack/spack/ci/gitlab.py @@ -15,6 +15,7 @@ import spack.binary_distribution as bindist import spack.config as cfg import spack.mirrors.mirror +import spack.schema import spack.spec import spack.util.spack_yaml as syaml @@ -248,7 +249,7 @@ def main_script_replacements(cmd): build_stamp = options.cdash_handler.build_stamp job_vars["SPACK_CDASH_BUILD_STAMP"] = build_stamp - job_object["artifacts"] = cfg.merge_yaml( + job_object["artifacts"] = spack.schema.merge_yaml( job_object.get("artifacts", {}), { "when": "always", diff --git a/lib/spack/spack/cmd/config.py b/lib/spack/spack/cmd/config.py index aef2d7a5412..e8f573e67dd 100644 --- a/lib/spack/spack/cmd/config.py +++ b/lib/spack/spack/cmd/config.py @@ -14,6 +14,7 @@ import spack.config import spack.environment as ev import spack.error +import spack.schema import spack.schema.env import spack.spec import spack.store @@ -566,7 +567,7 @@ def config_prefer_upstream(args): # Simply write the config to the specified file. existing = spack.config.get("packages", scope=scope) - new = spack.config.merge_yaml(existing, pkgs) + new = spack.schema.merge_yaml(existing, pkgs) spack.config.set("packages", new, scope) config_file = spack.config.CONFIG.get_config_filename(scope, section) diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index 1cafc1f738a..cd29af98806 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -619,7 +619,7 @@ def _get_config_memoized(self, section: str, scope: Optional[str]) -> YamlConfig if changed: self.format_updates[section].append(scope) - merged_section = merge_yaml(merged_section, data) + merged_section = spack.schema.merge_yaml(merged_section, data) # no config files -- empty config. if section not in merged_section: @@ -680,7 +680,7 @@ def set(self, path: str, value: Any, scope: Optional[str] = None) -> None: while len(parts) > 1: key = parts.pop(0) - if _override(key): + if spack.schema.override(key): new = type(data[key])() del data[key] else: @@ -693,7 +693,7 @@ def set(self, path: str, value: Any, scope: Optional[str] = None) -> None: data[key] = new data = new - if _override(parts[0]): + if spack.schema.override(parts[0]): data.pop(parts[0], None) # update new value @@ -894,7 +894,7 @@ def add_from_file(filename: str, scope: Optional[str] = None) -> None: value = data[section] existing = get(section, scope=scope) - new = merge_yaml(existing, value) + new = spack.schema.merge_yaml(existing, value) # We cannot call config.set directly (set is a type) CONFIG.set(section, new, scope) @@ -946,7 +946,7 @@ def add(fullpath: str, scope: Optional[str] = None) -> None: value: List[str] = [value] # type: ignore[no-redef] # merge value into existing - new = merge_yaml(existing, value) + new = spack.schema.merge_yaml(existing, value) CONFIG.set(path, new, scope) @@ -1120,44 +1120,6 @@ def read_config_file( raise ConfigFileError(str(e)) from e -def _override(string: str) -> bool: - """Test if a spack YAML string is an override. - - See ``spack_yaml`` for details. Keys in Spack YAML can end in `::`, - and if they do, their values completely replace lower-precedence - configs instead of merging into them. - - """ - return hasattr(string, "override") and string.override - - -def _append(string: str) -> bool: - """Test if a spack YAML string is an override. - - See ``spack_yaml`` for details. Keys in Spack YAML can end in `+:`, - and if they do, their values append lower-precedence - configs. - - str, str : concatenate strings. - [obj], [obj] : append lists. - - """ - return getattr(string, "append", False) - - -def _prepend(string: str) -> bool: - """Test if a spack YAML string is an override. - - See ``spack_yaml`` for details. Keys in Spack YAML can end in `+:`, - and if they do, their values prepend lower-precedence - configs. - - str, str : concatenate strings. - [obj], [obj] : prepend lists. (default behavior) - """ - return getattr(string, "prepend", False) - - def _mark_internal(data, name): """Add a simple name mark to raw YAML/JSON data. @@ -1260,7 +1222,7 @@ def they_are(t): unmerge = sk in dest old_dest_value = dest.pop(sk, None) - if unmerge and not _override(sk): + if unmerge and not spack.schema.override(sk): dest[sk] = remove_yaml(old_dest_value, sv) return dest @@ -1270,81 +1232,6 @@ def they_are(t): return dest -def merge_yaml(dest, source, prepend=False, append=False): - """Merges source into dest; entries in source take precedence over dest. - - This routine may modify dest and should be assigned to dest, in - case dest was None to begin with, e.g.: - - dest = merge_yaml(dest, source) - - In the result, elements from lists from ``source`` will appear before - elements of lists from ``dest``. Likewise, when iterating over keys - or items in merged ``OrderedDict`` objects, keys from ``source`` will - appear before keys from ``dest``. - - Config file authors can optionally end any attribute in a dict - with `::` instead of `:`, and the key will override that of the - parent instead of merging. - - `+:` will extend the default prepend merge strategy to include string concatenation - `-:` will change the merge strategy to append, it also includes string concatentation - """ - - def they_are(t): - return isinstance(dest, t) and isinstance(source, t) - - # If source is None, overwrite with source. - if source is None: - return None - - # Source list is prepended (for precedence) - if they_are(list): - if append: - # Make sure to copy ruamel comments - dest[:] = [x for x in dest if x not in source] + source - else: - # Make sure to copy ruamel comments - dest[:] = source + [x for x in dest if x not in source] - return dest - - # Source dict is merged into dest. - elif they_are(dict): - # save dest keys to reinsert later -- this ensures that source items - # come *before* dest in OrderdDicts - dest_keys = [dk for dk in dest.keys() if dk not in source] - - for sk, sv in source.items(): - # always remove the dest items. Python dicts do not overwrite - # keys on insert, so this ensures that source keys are copied - # into dest along with mark provenance (i.e., file/line info). - merge = sk in dest - old_dest_value = dest.pop(sk, None) - - if merge and not _override(sk): - dest[sk] = merge_yaml(old_dest_value, sv, _prepend(sk), _append(sk)) - else: - # if sk ended with ::, or if it's new, completely override - dest[sk] = copy.deepcopy(sv) - - # reinsert dest keys so they are last in the result - for dk in dest_keys: - dest[dk] = dest.pop(dk) - - return dest - - elif they_are(str): - # Concatenate strings in prepend mode - if prepend: - return source + dest - elif append: - return dest + source - - # If we reach here source and dest are either different types or are - # not both lists or dicts: replace with source. - return copy.copy(source) - - class ConfigPath: quoted_string = "(?:\"[^\"]+\")|(?:'[^']+')" unquoted_string = "[^:'\"]+" diff --git a/lib/spack/spack/detection/common.py b/lib/spack/spack/detection/common.py index 1b16aa6ccd4..3b78b1783f4 100644 --- a/lib/spack/spack/detection/common.py +++ b/lib/spack/spack/detection/common.py @@ -27,6 +27,7 @@ import spack.config import spack.error import spack.operating_systems.windows_os as winOs +import spack.schema import spack.spec import spack.util.environment import spack.util.spack_yaml @@ -226,7 +227,7 @@ def update_configuration( pkg_to_cfg[package_name] = pkg_config pkgs_cfg = spack.config.get("packages", scope=scope) - pkgs_cfg = spack.config.merge_yaml(pkgs_cfg, pkg_to_cfg) + pkgs_cfg = spack.schema.merge_yaml(pkgs_cfg, pkg_to_cfg) spack.config.set("packages", pkgs_cfg, scope=scope) return all_new_specs @@ -246,7 +247,7 @@ def set_virtuals_nonbuildable(virtuals: Set[str], scope: Optional[str] = None) - # Update the provided scope spack.config.set( "packages", - spack.config.merge_yaml(spack.config.get("packages", scope=scope), new_config), + spack.schema.merge_yaml(spack.config.get("packages", scope=scope), new_config), scope=scope, ) diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py index e62520a6c2e..8ec53013fe2 100644 --- a/lib/spack/spack/modules/common.py +++ b/lib/spack/spack/modules/common.py @@ -48,6 +48,7 @@ import spack.error import spack.paths import spack.projections as proj +import spack.schema import spack.schema.environment import spack.spec import spack.store @@ -216,7 +217,7 @@ def root_path(name, module_set_name): roots = spack.config.get(f"modules:{module_set_name}:roots", {}) # Merge config values into the defaults so we prefer configured values - roots = spack.config.merge_yaml(defaults, roots) + roots = spack.schema.merge_yaml(defaults, roots) path = roots.get(name, os.path.join(spack.paths.share_path, name)) return spack.util.path.canonicalize_path(path) @@ -624,10 +625,10 @@ def environment_modifications(self): """List of environment modifications to be processed.""" # Modifications guessed by inspecting the spec prefix prefix_inspections = syaml.syaml_dict() - spack.config.merge_yaml( + spack.schema.merge_yaml( prefix_inspections, spack.config.get("modules:prefix_inspections", {}) ) - spack.config.merge_yaml( + spack.schema.merge_yaml( prefix_inspections, spack.config.get(f"modules:{self.conf.name}:prefix_inspections", {}), ) diff --git a/lib/spack/spack/schema/__init__.py b/lib/spack/spack/schema/__init__.py index 8c04ebdaac4..29001c48ab7 100644 --- a/lib/spack/spack/schema/__init__.py +++ b/lib/spack/spack/schema/__init__.py @@ -3,6 +3,7 @@ # # SPDX-License-Identifier: (Apache-2.0 OR MIT) """This module contains jsonschema files for all of Spack's YAML formats.""" +import copy import typing import warnings @@ -73,3 +74,116 @@ def _deprecated_properties(validator, deprecated, instance, schema): Validator = llnl.util.lang.Singleton(_make_validator) + + +def _append(string: str) -> bool: + """Test if a spack YAML string is an append. + + See ``spack_yaml`` for details. Keys in Spack YAML can end in `+:`, + and if they do, their values append lower-precedence + configs. + + str, str : concatenate strings. + [obj], [obj] : append lists. + + """ + return getattr(string, "append", False) + + +def _prepend(string: str) -> bool: + """Test if a spack YAML string is an prepend. + + See ``spack_yaml`` for details. Keys in Spack YAML can end in `+:`, + and if they do, their values prepend lower-precedence + configs. + + str, str : concatenate strings. + [obj], [obj] : prepend lists. (default behavior) + """ + return getattr(string, "prepend", False) + + +def override(string: str) -> bool: + """Test if a spack YAML string is an override. + + See ``spack_yaml`` for details. Keys in Spack YAML can end in `::`, + and if they do, their values completely replace lower-precedence + configs instead of merging into them. + + """ + return hasattr(string, "override") and string.override + + +def merge_yaml(dest, source, prepend=False, append=False): + """Merges source into dest; entries in source take precedence over dest. + + This routine may modify dest and should be assigned to dest, in + case dest was None to begin with, e.g.: + + dest = merge_yaml(dest, source) + + In the result, elements from lists from ``source`` will appear before + elements of lists from ``dest``. Likewise, when iterating over keys + or items in merged ``OrderedDict`` objects, keys from ``source`` will + appear before keys from ``dest``. + + Config file authors can optionally end any attribute in a dict + with `::` instead of `:`, and the key will override that of the + parent instead of merging. + + `+:` will extend the default prepend merge strategy to include string concatenation + `-:` will change the merge strategy to append, it also includes string concatentation + """ + + def they_are(t): + return isinstance(dest, t) and isinstance(source, t) + + # If source is None, overwrite with source. + if source is None: + return None + + # Source list is prepended (for precedence) + if they_are(list): + if append: + # Make sure to copy ruamel comments + dest[:] = [x for x in dest if x not in source] + source + else: + # Make sure to copy ruamel comments + dest[:] = source + [x for x in dest if x not in source] + return dest + + # Source dict is merged into dest. + elif they_are(dict): + # save dest keys to reinsert later -- this ensures that source items + # come *before* dest in OrderdDicts + dest_keys = [dk for dk in dest.keys() if dk not in source] + + for sk, sv in source.items(): + # always remove the dest items. Python dicts do not overwrite + # keys on insert, so this ensures that source keys are copied + # into dest along with mark provenance (i.e., file/line info). + merge = sk in dest + old_dest_value = dest.pop(sk, None) + + if merge and not override(sk): + dest[sk] = merge_yaml(old_dest_value, sv, _prepend(sk), _append(sk)) + else: + # if sk ended with ::, or if it's new, completely override + dest[sk] = copy.deepcopy(sv) + + # reinsert dest keys so they are last in the result + for dk in dest_keys: + dest[dk] = dest.pop(dk) + + return dest + + elif they_are(str): + # Concatenate strings in prepend mode + if prepend: + return source + dest + elif append: + return dest + source + + # If we reach here source and dest are either different types or are + # not both lists or dicts: replace with source. + return copy.copy(source) diff --git a/lib/spack/spack/schema/config.py b/lib/spack/spack/schema/config.py index 9f380da92b7..b42c1e27ffa 100644 --- a/lib/spack/spack/schema/config.py +++ b/lib/spack/spack/schema/config.py @@ -11,7 +11,7 @@ from llnl.util.lang import union_dicts -import spack.config +import spack.schema import spack.schema.projections #: Properties for inclusion in other schemas @@ -157,7 +157,7 @@ def update(data): # whether install_tree was updated or not # we merge the yaml to ensure we don't invalidate other projections update_data = data.get("install_tree", {}) - update_data = spack.config.merge_yaml(update_data, projections_data) + update_data = spack.schema.merge_yaml(update_data, projections_data) data["install_tree"] = update_data changed = True diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py index 52e1cc9e04a..31d86d66fc4 100644 --- a/lib/spack/spack/test/config.py +++ b/lib/spack/spack/test/config.py @@ -639,31 +639,6 @@ def test_read_config_override_list(mock_low_high_config, write_config_file): } -def test_ordereddict_merge_order(): - """ "Test that source keys come before dest keys in merge_yaml results.""" - source = syaml.syaml_dict([("k1", "v1"), ("k2", "v2"), ("k3", "v3")]) - - dest = syaml.syaml_dict([("k4", "v4"), ("k3", "WRONG"), ("k5", "v5")]) - - result = spack.config.merge_yaml(dest, source) - assert "WRONG" not in result.values() - - expected_keys = ["k1", "k2", "k3", "k4", "k5"] - expected_items = [("k1", "v1"), ("k2", "v2"), ("k3", "v3"), ("k4", "v4"), ("k5", "v5")] - assert expected_keys == list(result.keys()) - assert expected_items == list(result.items()) - - -def test_list_merge_order(): - """ "Test that source lists are prepended to dest.""" - source = ["a", "b", "c"] - dest = ["d", "e", "f"] - - result = spack.config.merge_yaml(dest, source) - - assert ["a", "b", "c", "d", "e", "f"] == result - - def test_internal_config_update(mock_low_high_config, write_config_file): write_config_file("config", config_low, "low") diff --git a/lib/spack/spack/test/schema.py b/lib/spack/spack/test/schema.py index 509bfd331a6..12f2c57e019 100644 --- a/lib/spack/spack/test/schema.py +++ b/lib/spack/spack/test/schema.py @@ -11,6 +11,7 @@ import spack.paths import spack.schema +import spack.util.spack_yaml as syaml @pytest.fixture() @@ -124,3 +125,28 @@ def test_deprecated_properties(module_suffixes_schema): data = {"tcl": {"all": {"suffixes": {"^python": "py"}}}} # The next validation doesn't raise anymore v.validate(data) + + +def test_ordereddict_merge_order(): + """ "Test that source keys come before dest keys in merge_yaml results.""" + source = syaml.syaml_dict([("k1", "v1"), ("k2", "v2"), ("k3", "v3")]) + + dest = syaml.syaml_dict([("k4", "v4"), ("k3", "WRONG"), ("k5", "v5")]) + + result = spack.schema.merge_yaml(dest, source) + assert "WRONG" not in result.values() + + expected_keys = ["k1", "k2", "k3", "k4", "k5"] + expected_items = [("k1", "v1"), ("k2", "v2"), ("k3", "v3"), ("k4", "v4"), ("k5", "v5")] + assert expected_keys == list(result.keys()) + assert expected_items == list(result.items()) + + +def test_list_merge_order(): + """ "Test that source lists are prepended to dest.""" + source = ["a", "b", "c"] + dest = ["d", "e", "f"] + + result = spack.schema.merge_yaml(dest, source) + + assert ["a", "b", "c", "d", "e", "f"] == result