lmod: allow core compiler to be specified with a version range (#37789)

Use CompilerSpec with satisfies instead of string equality tests

Co-authored-by: Harmen Stoppels <harmenstoppels@gmail.com>

lib/spack/spack/modules/lmod.py

@@ -7,7 +7,7 @@
 import itertools
 import os.path
 import posixpath
-from typing import Any, Dict
+from typing import Any, Dict, List
 
 import llnl.util.lang as lang
 
@@ -56,7 +56,7 @@ def make_context(spec, module_set_name, explicit):
     return LmodContext(conf)
 
 
-def guess_core_compilers(name, store=False):
+def guess_core_compilers(name, store=False) -> List[spack.spec.CompilerSpec]:
     """Guesses the list of core compilers installed in the system.
 
     Args:
@@ -64,21 +64,19 @@ def guess_core_compilers(name, store=False):
             modules.yaml configuration file
 
     Returns:
-        List of core compilers, if found, or None
+        List of found core compilers
     """
     core_compilers = []
-    for compiler_config in spack.compilers.all_compilers_config():
+    for compiler in spack.compilers.all_compilers():
         try:
-            compiler = compiler_config["compiler"]
             # A compiler is considered to be a core compiler if any of the
             # C, C++ or Fortran compilers reside in a system directory
             is_system_compiler = any(
-                os.path.dirname(x) in spack.util.environment.SYSTEM_DIRS
+                os.path.dirname(getattr(compiler, x, "")) in spack.util.environment.SYSTEM_DIRS
-                for x in compiler["paths"].values()
+                for x in ("cc", "cxx", "f77", "fc")
-                if x is not None
             )
             if is_system_compiler:
-                core_compilers.append(str(compiler["spec"]))
+                core_compilers.append(compiler.spec)
         except (KeyError, TypeError, AttributeError):
             continue
 
@@ -89,10 +87,10 @@ def guess_core_compilers(name, store=False):
         modules_cfg = spack.config.get(
             "modules:" + name, {}, scope=spack.config.default_modify_scope()
         )
-        modules_cfg.setdefault("lmod", {})["core_compilers"] = core_compilers
+        modules_cfg.setdefault("lmod", {})["core_compilers"] = [str(x) for x in core_compilers]
         spack.config.set("modules:" + name, modules_cfg, scope=spack.config.default_modify_scope())
 
-    return core_compilers or None
+    return core_compilers
 
 
 class LmodConfiguration(BaseConfiguration):
@@ -104,7 +102,7 @@ class LmodConfiguration(BaseConfiguration):
     default_projections = {"all": posixpath.join("{name}", "{version}")}
 
     @property
-    def core_compilers(self):
+    def core_compilers(self) -> List[spack.spec.CompilerSpec]:
         """Returns the list of "Core" compilers
 
         Raises:
@@ -112,14 +110,18 @@ def core_compilers(self):
                 specified in the configuration file or the sequence
                 is empty
         """
-        value = configuration(self.name).get("core_compilers") or guess_core_compilers(
+        compilers = [
-            self.name, store=True
+            spack.spec.CompilerSpec(c) for c in configuration(self.name).get("core_compilers", [])
-        )
+        ]
 
-        if not value:
+        if not compilers:
+            compilers = guess_core_compilers(self.name, store=True)
+
+        if not compilers:
             msg = 'the key "core_compilers" must be set in modules.yaml'
             raise CoreCompilersNotFoundError(msg)
-        return value
+
+        return compilers
 
     @property
     def core_specs(self):
@@ -283,16 +285,18 @@ def token_to_path(self, name, value):
 
         # If we are dealing with a core compiler, return 'Core'
         core_compilers = self.conf.core_compilers
-        if name == "compiler" and str(value) in core_compilers:
+        if name == "compiler" and any(
+            spack.spec.CompilerSpec(value).satisfies(c) for c in core_compilers
+        ):
             return "Core"
 
-        # CompilerSpec does not have an hash, as we are not allowed to
+        # CompilerSpec does not have a hash, as we are not allowed to
         # use different flavors of the same compiler
         if name == "compiler":
             return path_part_fmt.format(token=value)
 
         # In case the hierarchy token refers to a virtual provider
-        # we need to append an hash to the version to distinguish
+        # we need to append a hash to the version to distinguish
         # among flavors of the same library (e.g. openblas~openmp vs.
         # openblas+openmp)
         path = path_part_fmt.format(token=value)

lib/spack/spack/test/data/modules/lmod/complex_hierarchy.yaml

@@ -4,7 +4,7 @@ lmod:
   hash_length: 0
 
   core_compilers:
-    - 'clang@3.3'
+    - 'clang@12.0.0'
 
   core_specs:
     - 'mpich@3.0.1'

lib/spack/spack/test/data/modules/lmod/core_compilers.yaml

@@ -0,0 +1,5 @@
+enable:
+  - lmod
+lmod:
+  core_compilers:
+    - 'clang@12.0.0'

lib/spack/spack/test/data/modules/lmod/core_compilers_at_equal.yaml

@@ -0,0 +1,5 @@
+enable:
+  - lmod
+lmod:
+  core_compilers:
+    - 'clang@=12.0.0'

lib/spack/spack/test/modules/lmod.py

@@ -45,6 +45,18 @@ def provider(request):
 
 @pytest.mark.usefixtures("config", "mock_packages")
 class TestLmod(object):
+    @pytest.mark.regression("37788")
+    @pytest.mark.parametrize("modules_config", ["core_compilers", "core_compilers_at_equal"])
+    def test_layout_for_specs_compiled_with_core_compilers(
+        self, modules_config, module_configuration, factory
+    ):
+        """Tests that specs compiled with core compilers are in the 'Core' folder. Also tests that
+        we can use both ``compiler@version`` and ``compiler@=version`` to specify a core compiler.
+        """
+        module_configuration(modules_config)
+        module, spec = factory("libelf%clang@12.0.0")
+        assert "Core" in module.layout.available_path_parts
+
     def test_file_layout(self, compiler, provider, factory, module_configuration):
         """Tests the layout of files in the hierarchy is the one expected."""
         module_configuration("complex_hierarchy")
@@ -61,7 +73,7 @@ def test_file_layout(self, compiler, provider, factory, module_configuration):
         # is transformed to r"Core" if the compiler is listed among core
         # compilers
         # Check that specs listed as core_specs are transformed to "Core"
-        if compiler == "clang@=3.3" or spec_string == "mpich@3.0.1":
+        if compiler == "clang@=12.0.0" or spec_string == "mpich@3.0.1":
             assert "Core" in layout.available_path_parts
         else:
             assert compiler.replace("@=", "/") in layout.available_path_parts