modules: restore exclude_implicits (#40958)

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
+        # Compute the list of matching include / exclude rules, and whether excluded as implicit
-        include_rules = conf.get("include", [])
+        include_matches = [x for x in conf.get("include", []) if spec.satisfies(x)]
-        include_matches = [x for x in include_rules 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)
-        # 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)]
 
         def debug_info(line_header, match_list):
             if match_list:
-                msg = "\t{0} : {1}".format(line_header, spec.cshort_spec)
+                tty.debug(f"\t{line_header} : {spec.cshort_spec}")
-                tty.debug(msg)
                 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:
+        if excluded_as_implicit:
-            return True
+            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(
+        hidden_as_implicit = not self.explicit and conf.get("hide_implicits", False)
-            "hide_implicits", conf.get("exclude_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
 

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)

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

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):
-    def test_hide_implicits_no_arg(self, module_configuration, database, config_name):
+        module_configuration("exclude_implicits")
-        module_configuration(config_name)
 
         # 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):
-    def test_hide_implicits_with_arg(self, module_configuration, config_name):
+        module_configuration("exclude_implicits")
-        module_configuration(config_name)
 
         # 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