modules: add support for conflict in lua modulefile (#36701)

Add support for conflict directives in Lua modulefile like done for Tcl
modulefile.

Note that conflicts are correctly honored on Lmod and Environment
Modules <4.2 only if mutually expressed on both modulefiles that
conflict with each other.

Migrate conflict code from Tcl-specific classes to the common part. Add
tests for Lmod and split the conflict test case in two.

Co-authored-by: Massimiliano Culpo <massimiliano.culpo@gmail.com>

lib/spack/docs/module_file_support.rst

@@ -400,8 +400,7 @@ that are already in the Lmod hierarchy.
 
 
 .. note::
-   Tcl modules
+   Tcl and Lua modules also allow for explicit conflicts between modulefiles.
-     Tcl modules also allow for explicit conflicts between modulefiles.
 
    .. code-block:: yaml
 
@@ -420,8 +419,11 @@ that are already in the Lmod hierarchy.
    will create module files that will conflict with ``intel/14.0.1`` and with the
    base directory of the same module, effectively preventing the possibility to
    load two or more versions of the same software at the same time. The tokens
-     that are available for use in this directive are the same understood by
+   that are available for use in this directive are the same understood by the
-     the :meth:`~spack.spec.Spec.format` method.
+   :meth:`~spack.spec.Spec.format` method.
+
+   For Lmod and Environment Modules versions prior 4.2, it is important to
+   express the conflict on both modulefiles conflicting with each other.
 
 
 .. note::

lib/spack/spack/cmd/modules/__init__.py

@@ -11,6 +11,7 @@
 import sys
 
 from llnl.util import filesystem, tty
+from llnl.util.tty import color
 
 import spack.cmd
 import spack.cmd.common.arguments as arguments
@@ -347,14 +348,20 @@ def refresh(module_type, specs, args):
     spack.modules.common.generate_module_index(
         module_type_root, writers, overwrite=args.delete_tree
     )
+    errors = []
     for x in writers:
         try:
             x.write(overwrite=True)
+        except spack.error.SpackError as e:
+            msg = f"{x.layout.filename}: {e.message}"
+            errors.append(msg)
         except Exception as e:
-            tty.debug(e)
+            msg = f"{x.layout.filename}: {str(e)}"
-            msg = "Could not write module file [{0}]"
+            errors.append(msg)
-            tty.warn(msg.format(x.layout.filename))
+
-            tty.warn("\t--> {0} <--".format(str(e)))
+    if errors:
+        errors.insert(0, color.colorize("@*{some module files could not be written}"))
+        tty.warn("\n".join(errors))
 
 
 #: Dictionary populated with the list of sub-commands.

lib/spack/spack/modules/common.py

@@ -35,6 +35,7 @@
 import os.path
 import pathlib
 import re
+import string
 import warnings
 from typing import Optional
 
@@ -470,6 +471,11 @@ def hash(self):
             return self.spec.dag_hash(length=hash_length)
         return None
 
+    @property
+    def conflicts(self):
+        """Conflicts for this module file"""
+        return self.conf.get("conflict", [])
+
     @property
     def excluded(self):
         """Returns True if the module has been excluded, False otherwise."""
@@ -763,6 +769,36 @@ def has_manpath_modifications(self):
         else:
             return False
 
+    @tengine.context_property
+    def conflicts(self):
+        """List of conflicts for the module file."""
+        fmts = []
+        projection = proj.get_projection(self.conf.projections, self.spec)
+        for item in self.conf.conflicts:
+            self._verify_conflict_naming_consistency_or_raise(item, projection)
+            item = self.spec.format(item)
+            fmts.append(item)
+        return fmts
+
+    def _verify_conflict_naming_consistency_or_raise(self, item, projection):
+        f = string.Formatter()
+        errors = []
+        if len([x for x in f.parse(item)]) > 1:
+            for naming_dir, conflict_dir in zip(projection.split("/"), item.split("/")):
+                if naming_dir != conflict_dir:
+                    errors.extend(
+                        [
+                            f"spec={self.spec.cshort_spec}",
+                            f"conflict_scheme={item}",
+                            f"naming_scheme={projection}",
+                        ]
+                    )
+        if errors:
+            raise ModulesError(
+                message="conflict scheme does not match naming scheme",
+                long_message="\n    ".join(errors),
+            )
+
     @tengine.context_property
     def autoload(self):
         """List of modules that needs to be loaded automatically."""

lib/spack/spack/modules/tcl.py

@@ -7,13 +7,9 @@
 non-hierarchical modules.
 """
 import posixpath
-import string
 from typing import Any, Dict
 
-import llnl.util.tty as tty
-
 import spack.config
-import spack.projections as proj
 import spack.tengine as tengine
 
 from .common import BaseConfiguration, BaseContext, BaseFileLayout, BaseModuleFileWriter
@@ -56,11 +52,6 @@ def make_context(spec, module_set_name, explicit):
 class TclConfiguration(BaseConfiguration):
     """Configuration class for tcl module files."""
 
-    @property
-    def conflicts(self):
-        """Conflicts for this module file"""
-        return self.conf.get("conflict", [])
-
 
 class TclFileLayout(BaseFileLayout):
     """File layout for tcl module files."""
@@ -74,29 +65,6 @@ def prerequisites(self):
         """List of modules that needs to be loaded automatically."""
         return self._create_module_list_of("specs_to_prereq")
 
-    @tengine.context_property
-    def conflicts(self):
-        """List of conflicts for the tcl module file."""
-        fmts = []
-        projection = proj.get_projection(self.conf.projections, self.spec)
-        f = string.Formatter()
-        for item in self.conf.conflicts:
-            if len([x for x in f.parse(item)]) > 1:
-                for naming_dir, conflict_dir in zip(projection.split("/"), item.split("/")):
-                    if naming_dir != conflict_dir:
-                        message = "conflict scheme does not match naming "
-                        message += "scheme [{spec}]\n\n"
-                        message += 'naming scheme   : "{nformat}"\n'
-                        message += 'conflict scheme : "{cformat}"\n\n'
-                        message += "** You may want to check your "
-                        message += "`modules.yaml` configuration file **\n"
-                        tty.error(message.format(spec=self.spec, nformat=projection, cformat=item))
-                        raise SystemExit("Module generation aborted.")
-                item = self.spec.format(item)
-            fmts.append(item)
-        # Substitute spec tokens if present
-        return [self.spec.format(x) for x in fmts]
-
 
 class TclModulefileWriter(BaseModuleFileWriter):
     """Writer class for tcl module files."""

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

@@ -0,0 +1,10 @@
+enable:
+  - lmod
+lmod:
+  core_compilers:
+    - 'clang@3.3'
+  all:
+    autoload: none
+    conflict:
+      - '{name}'
+      - 'intel/14.0.1'

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

@@ -0,0 +1,11 @@
+enable:
+  - lmod
+lmod:
+  core_compilers:
+    - 'clang@3.3'
+  projections:
+    all: '{name}/{version}-{compiler.name}'
+  all:
+    autoload: none
+    conflict:
+      - '{name}/{compiler.name}'

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

@@ -290,6 +290,26 @@ def test_non_virtual_in_hierarchy(self, factory, module_configuration):
         with pytest.raises(spack.modules.lmod.NonVirtualInHierarchyError):
             module.write()
 
+    def test_conflicts(self, modulefile_content, module_configuration):
+        """Tests adding conflicts to the module."""
+
+        # This configuration has no error, so check the conflicts directives
+        # are there
+        module_configuration("conflicts")
+        content = modulefile_content("mpileaks")
+
+        assert len([x for x in content if x.startswith("conflict")]) == 2
+        assert len([x for x in content if x == 'conflict("mpileaks")']) == 1
+        assert len([x for x in content if x == 'conflict("intel/14.0.1")']) == 1
+
+    def test_inconsistent_conflict_in_modules_yaml(self, modulefile_content, module_configuration):
+        """Tests inconsistent conflict definition in `modules.yaml`."""
+
+        # This configuration is inconsistent, check an error is raised
+        module_configuration("wrong_conflicts")
+        with pytest.raises(spack.modules.common.ModulesError):
+            modulefile_content("mpileaks")
+
     def test_override_template_in_package(self, modulefile_content, module_configuration):
         """Tests overriding a template from and attribute in the package."""
 

lib/spack/spack/test/modules/tcl.py

@@ -311,9 +311,12 @@ def test_conflicts(self, modulefile_content, module_configuration):
         assert len([x for x in content if x == "conflict mpileaks"]) == 1
         assert len([x for x in content if x == "conflict intel/14.0.1"]) == 1
 
+    def test_inconsistent_conflict_in_modules_yaml(self, modulefile_content, module_configuration):
+        """Tests inconsistent conflict definition in `modules.yaml`."""
+
         # This configuration is inconsistent, check an error is raised
         module_configuration("wrong_conflicts")
-        with pytest.raises(SystemExit):
+        with pytest.raises(spack.modules.common.ModulesError):
             modulefile_content("mpileaks")
 
     def test_module_index(self, module_configuration, factory, tmpdir_factory):

share/spack/templates/modules/modulefile.lua

@@ -69,6 +69,12 @@ setenv("LMOD_{{ name|upper() }}_VERSION", "{{ version_part }}")
 depends_on("{{ module }}")
 {% endfor %}
 {% endblock %}
+{#  #}
+{% block conflict %}
+{% for name in conflicts %}
+conflict("{{ name }}")
+{% endfor %}
+{% endblock %}
 
 {% block environment %}
 {% for command_name, cmd in environment_modifications %}