From 8c7adbf8f3767a06014e014f0132e8f5607cce0d Mon Sep 17 00:00:00 2001 From: Xavier Delaruelle Date: Tue, 18 Jul 2023 10:24:46 +0200 Subject: 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 --- lib/spack/docs/module_file_support.rst | 46 +++++++++++----------- lib/spack/spack/cmd/modules/__init__.py | 15 +++++-- lib/spack/spack/modules/common.py | 36 +++++++++++++++++ lib/spack/spack/modules/tcl.py | 32 --------------- .../spack/test/data/modules/lmod/conflicts.yaml | 10 +++++ .../test/data/modules/lmod/wrong_conflicts.yaml | 11 ++++++ lib/spack/spack/test/modules/lmod.py | 20 ++++++++++ lib/spack/spack/test/modules/tcl.py | 5 ++- 8 files changed, 116 insertions(+), 59 deletions(-) create mode 100644 lib/spack/spack/test/data/modules/lmod/conflicts.yaml create mode 100644 lib/spack/spack/test/data/modules/lmod/wrong_conflicts.yaml (limited to 'lib') diff --git a/lib/spack/docs/module_file_support.rst b/lib/spack/docs/module_file_support.rst index 53f172accc..52d74a5669 100644 --- a/lib/spack/docs/module_file_support.rst +++ b/lib/spack/docs/module_file_support.rst @@ -400,28 +400,30 @@ that are already in the Lmod hierarchy. .. note:: - Tcl modules - Tcl modules also allow for explicit conflicts between modulefiles. - - .. code-block:: yaml - - modules: - default: - enable: - - tcl - tcl: - projections: - all: '{name}/{version}-{compiler.name}-{compiler.version}' - all: - conflict: - - '{name}' - - 'intel/14.0.1' - - 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 - the :meth:`~spack.spec.Spec.format` method. + Tcl and Lua modules also allow for explicit conflicts between modulefiles. + + .. code-block:: yaml + + modules: + default: + enable: + - tcl + tcl: + projections: + all: '{name}/{version}-{compiler.name}-{compiler.version}' + all: + conflict: + - '{name}' + - 'intel/14.0.1' + + 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 the + :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:: diff --git a/lib/spack/spack/cmd/modules/__init__.py b/lib/spack/spack/cmd/modules/__init__.py index ec54b0d0e7..1fb50f291d 100644 --- a/lib/spack/spack/cmd/modules/__init__.py +++ b/lib/spack/spack/cmd/modules/__init__.py @@ -11,6 +11,7 @@ import shutil 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 = "Could not write module file [{0}]" - tty.warn(msg.format(x.layout.filename)) - tty.warn("\t--> {0} <--".format(str(e))) + msg = f"{x.layout.filename}: {str(e)}" + errors.append(msg) + + 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. diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py index 151893c9b1..dac08fdd0d 100644 --- a/lib/spack/spack/modules/common.py +++ b/lib/spack/spack/modules/common.py @@ -35,6 +35,7 @@ import inspect import os.path import pathlib import re +import string import warnings from typing import Optional @@ -470,6 +471,11 @@ class BaseConfiguration: 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 @@ class BaseContext(tengine.Context): 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.""" diff --git a/lib/spack/spack/modules/tcl.py b/lib/spack/spack/modules/tcl.py index 634426c357..58b0753792 100644 --- a/lib/spack/spack/modules/tcl.py +++ b/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 @@ class TclContext(BaseContext): """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.""" diff --git a/lib/spack/spack/test/data/modules/lmod/conflicts.yaml b/lib/spack/spack/test/data/modules/lmod/conflicts.yaml new file mode 100644 index 0000000000..7336dc10a7 --- /dev/null +++ b/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' diff --git a/lib/spack/spack/test/data/modules/lmod/wrong_conflicts.yaml b/lib/spack/spack/test/data/modules/lmod/wrong_conflicts.yaml new file mode 100644 index 0000000000..f674ee631a --- /dev/null +++ b/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}' diff --git a/lib/spack/spack/test/modules/lmod.py b/lib/spack/spack/test/modules/lmod.py index 3ae1d38f1a..6e4222f7cd 100644 --- a/lib/spack/spack/test/modules/lmod.py +++ b/lib/spack/spack/test/modules/lmod.py @@ -290,6 +290,26 @@ class TestLmod: 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.""" diff --git a/lib/spack/spack/test/modules/tcl.py b/lib/spack/spack/test/modules/tcl.py index 3f55d92136..f255bc42b8 100644 --- a/lib/spack/spack/test/modules/tcl.py +++ b/lib/spack/spack/test/modules/tcl.py @@ -311,9 +311,12 @@ class TestTcl: 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): -- cgit v1.2.3-60-g2f50