From 9d58d5e645bd5e01a229d230db14b27adb8869a7 Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Wed, 8 Nov 2023 22:56:55 +0100 Subject: [PATCH] modules: restore exclude_implicits (#40958) --- lib/spack/spack/modules/common.py | 30 ++++++---------- lib/spack/spack/schema/modules.py | 49 -------------------------- lib/spack/spack/test/modules/common.py | 21 ----------- lib/spack/spack/test/modules/tcl.py | 18 +++++----- 4 files changed, 19 insertions(+), 99 deletions(-) diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py index 465fed0324..d1afdd22fd 100644 --- a/lib/spack/spack/modules/common.py +++ b/lib/spack/spack/modules/common.py @@ -486,43 +486,35 @@ def excluded(self): spec = self.spec conf = self.module.configuration(self.name) - # Compute the list of include rules that match - include_rules = conf.get("include", []) - include_matches = [x for x in include_rules if spec.satisfies(x)] - - # Compute the list of exclude rules that match - exclude_rules = conf.get("exclude", []) - exclude_matches = [x for x in exclude_rules if spec.satisfies(x)] + # Compute the list of matching include / exclude rules, and whether excluded as implicit + include_matches = [x for x in conf.get("include", []) if spec.satisfies(x)] + exclude_matches = [x for x in conf.get("exclude", []) if spec.satisfies(x)] + excluded_as_implicit = not self.explicit and conf.get("exclude_implicits", False) def debug_info(line_header, match_list): if match_list: - msg = "\t{0} : {1}".format(line_header, spec.cshort_spec) - tty.debug(msg) + tty.debug(f"\t{line_header} : {spec.cshort_spec}") for rule in match_list: - tty.debug("\t\tmatches rule: {0}".format(rule)) + tty.debug(f"\t\tmatches rule: {rule}") debug_info("INCLUDE", include_matches) debug_info("EXCLUDE", exclude_matches) - if not include_matches and exclude_matches: - return True + if excluded_as_implicit: + tty.debug(f"\tEXCLUDED_AS_IMPLICIT : {spec.cshort_spec}") - return False + return not include_matches and (exclude_matches or excluded_as_implicit) @property def hidden(self): """Returns True if the module has been hidden, False otherwise.""" - # A few variables for convenience of writing the method - spec = self.spec conf = self.module.configuration(self.name) - hidden_as_implicit = not self.explicit and conf.get( - "hide_implicits", conf.get("exclude_implicits", False) - ) + hidden_as_implicit = not self.explicit and conf.get("hide_implicits", False) if hidden_as_implicit: - tty.debug(f"\tHIDDEN_AS_IMPLICIT : {spec.cshort_spec}") + tty.debug(f"\tHIDDEN_AS_IMPLICIT : {self.spec.cshort_spec}") return hidden_as_implicit diff --git a/lib/spack/spack/schema/modules.py b/lib/spack/spack/schema/modules.py index adf1a93586..3814c6810c 100644 --- a/lib/spack/spack/schema/modules.py +++ b/lib/spack/spack/schema/modules.py @@ -188,52 +188,3 @@ "additionalProperties": False, "properties": properties, } - - -# deprecated keys and their replacements -old_to_new_key = {"exclude_implicits": "hide_implicits"} - - -def update_keys(data, key_translations): - """Change blacklist/whitelist to exclude/include. - - Arguments: - data (dict): data from a valid modules configuration. - key_translations (dict): A dictionary of keys to translate to - their respective values. - - Return: - (bool) whether anything was changed in data - """ - changed = False - - if isinstance(data, dict): - keys = list(data.keys()) - for key in keys: - value = data[key] - - translation = key_translations.get(key) - if translation: - data[translation] = data.pop(key) - changed = True - - changed |= update_keys(value, key_translations) - - elif isinstance(data, list): - for elt in data: - changed |= update_keys(elt, key_translations) - - return changed - - -def update(data): - """Update the data in place to remove deprecated properties. - - Args: - data (dict): dictionary to be updated - - Returns: - True if data was changed, False otherwise - """ - # translate blacklist/whitelist to exclude/include - return update_keys(data, old_to_new_key) diff --git a/lib/spack/spack/test/modules/common.py b/lib/spack/spack/test/modules/common.py index 15656dff25..11b4305b48 100644 --- a/lib/spack/spack/test/modules/common.py +++ b/lib/spack/spack/test/modules/common.py @@ -14,7 +14,6 @@ import spack.package_base import spack.schema.modules import spack.spec -import spack.util.spack_yaml as syaml from spack.modules.common import UpstreamModuleIndex from spack.spec import Spec @@ -191,26 +190,6 @@ def find_nothing(*args): spack.package_base.PackageBase.uninstall_by_spec(spec) -@pytest.mark.parametrize( - "module_type, old_config,new_config", - [("tcl", "exclude_implicits.yaml", "hide_implicits.yaml")], -) -def test_exclude_include_update(module_type, old_config, new_config): - module_test_data_root = os.path.join(spack.paths.test_path, "data", "modules", module_type) - with open(os.path.join(module_test_data_root, old_config)) as f: - old_yaml = syaml.load(f) - with open(os.path.join(module_test_data_root, new_config)) as f: - new_yaml = syaml.load(f) - - # ensure file that needs updating is translated to the right thing. - assert spack.schema.modules.update_keys(old_yaml, spack.schema.modules.old_to_new_key) - assert new_yaml == old_yaml - # ensure a file that doesn't need updates doesn't get updated - original_new_yaml = new_yaml.copy() - assert not spack.schema.modules.update_keys(new_yaml, spack.schema.modules.old_to_new_key) - assert original_new_yaml == new_yaml - - @pytest.mark.regression("37649") def test_check_module_set_name(mutable_config): """Tests that modules set name are validated correctly and an error is reported if the diff --git a/lib/spack/spack/test/modules/tcl.py b/lib/spack/spack/test/modules/tcl.py index 00460b6796..f43f3d041e 100644 --- a/lib/spack/spack/test/modules/tcl.py +++ b/lib/spack/spack/test/modules/tcl.py @@ -425,40 +425,38 @@ def test_extend_context(self, modulefile_content, module_configuration): @pytest.mark.regression("4400") @pytest.mark.db - @pytest.mark.parametrize("config_name", ["hide_implicits", "exclude_implicits"]) - def test_hide_implicits_no_arg(self, module_configuration, database, config_name): - module_configuration(config_name) + def test_hide_implicits_no_arg(self, module_configuration, database): + module_configuration("exclude_implicits") # mpileaks has been installed explicitly when setting up # the tests database mpileaks_specs = database.query("mpileaks") for item in mpileaks_specs: writer = writer_cls(item, "default") - assert not writer.conf.hidden + assert not writer.conf.excluded # callpath is a dependency of mpileaks, and has been pulled # in implicitly callpath_specs = database.query("callpath") for item in callpath_specs: writer = writer_cls(item, "default") - assert writer.conf.hidden + assert writer.conf.excluded @pytest.mark.regression("12105") - @pytest.mark.parametrize("config_name", ["hide_implicits", "exclude_implicits"]) - def test_hide_implicits_with_arg(self, module_configuration, config_name): - module_configuration(config_name) + def test_hide_implicits_with_arg(self, module_configuration): + module_configuration("exclude_implicits") # mpileaks is defined as explicit with explicit argument set on writer mpileaks_spec = spack.spec.Spec("mpileaks") mpileaks_spec.concretize() writer = writer_cls(mpileaks_spec, "default", True) - assert not writer.conf.hidden + assert not writer.conf.excluded # callpath is defined as implicit with explicit argument set on writer callpath_spec = spack.spec.Spec("callpath") callpath_spec.concretize() writer = writer_cls(callpath_spec, "default", False) - assert writer.conf.hidden + assert writer.conf.excluded @pytest.mark.regression("9624") @pytest.mark.db