Merge pull request #4625 from xdelaruelle/old_tool_cleanup

Clarify references to old Environment Modules classes

easybuild/tools/config.py

@@ -735,7 +735,7 @@ def container_path():
 
 def get_modules_tool():
     """
-    Return modules tool (EnvironmentModulesC, Lmod, ...)
+    Return modules tool (EnvironmentModules, Lmod, ...)
     """
     # 'modules_tool' key will only be present if EasyBuild config is initialized
     return ConfigurationVariables().get('modules_tool', None)

easybuild/tools/module_generator.py

@@ -346,7 +346,7 @@ def modulerc(self, module_version=None, filepath=None, modulerc_txt=None):
 
                 module_version_statement = "module-version %(modname)s %(sym_version)s"
 
-                # for Environment Modules we need to guard the module-version statement,
+                # for EnvironmentModulesC we need to guard the module-version statement,
                 # to avoid "Duplicate version symbol" warning messages where EasyBuild trips over,
                 # which occur because the .modulerc is parsed twice
                 # "module-info version <arg>" returns its argument if that argument is not a symbolic version (yet),
@@ -1018,7 +1018,7 @@ def set_alias(self, key, value):
 
     def set_as_default(self, module_dir_path, module_version, mod_symlink_paths=None):
         """
-        Create a .version file inside the package module folder in order to set the default version for TMod
+        Create a .version file inside the package module folder in order to set the default version
 
         :param module_dir_path: module directory path, e.g. $HOME/easybuild/modules/all/Bison
         :param module_version: module version, e.g. 3.0.4

easybuild/tools/modules.py

@@ -590,7 +590,8 @@ def mod_exists_via_show(mod_name):
                     self.log.debug("Skipping warning line '%s'", line)
                     continue
 
-                # skip lines that start with 'module-' (like 'module-version'),
+                # skip lines that start with 'module-' (like 'module-version')
+                # that may appear with EnvironmentModulesC or EnvironmentModulesTcl,
                 # see https://github.com/easybuilders/easybuild-framework/issues/3376
                 if line.startswith('module-'):
                     self.log.debug("Skipping line '%s' since it starts with 'module-'", line)
@@ -745,7 +746,7 @@ def modulefile_path(self, mod_name, strip_ext=False):
         :param mod_name: module name
         :param strip_ext: strip (.lua) extension from module fileame (if present)"""
         # (possible relative) path is always followed by a ':', and may be prepended by whitespace
-        # this works for both environment modules and Lmod
+        # this works for both Environment Modules and Lmod
         modpath_re = re.compile(r'^\s*(?P<modpath>[^/\n]*/[^\s]+):$', re.M)
         modpath = self.get_value_from_modulefile(mod_name, modpath_re)
 
@@ -1180,7 +1181,7 @@ def update(self):
 
 
 class EnvironmentModulesC(ModulesTool):
-    """Interface to (C) environment modules (modulecmd)."""
+    """Interface to (C) Environment Modules (modulecmd)."""
     NAME = "Environment Modules"
     COMMAND = "modulecmd"
     REQ_VERSION = '3.2.10'
@@ -1195,7 +1196,7 @@ def run_module(self, *args, **kwargs):
         if isinstance(args[0], (list, tuple,)):
             args = args[0]
 
-        # some versions of Cray's environment modules tool (3.2.10.x) include a "source */init/bash" command
+        # some versions of Cray's Environment Modules tool (3.2.10.x) include a "source */init/bash" command
         # in the output of some "modulecmd python load" calls, which is not a valid Python command,
         # which must be stripped out to avoid "invalid syntax" errors when evaluating the output
         def tweak_stdout(txt):
@@ -1237,9 +1238,9 @@ def get_setenv_value_from_modulefile(self, mod_name, var_name):
 
 
 class EnvironmentModulesTcl(EnvironmentModulesC):
-    """Interface to (Tcl) environment modules (modulecmd.tcl)."""
+    """Interface to (ancient Tcl-only) Environment Modules (modulecmd.tcl)."""
     NAME = "ancient Tcl-only Environment Modules"
-    # Tcl environment modules have no --terse (yet),
+    # ancient Tcl-only Environment Modules have no --terse (yet),
     #   -t must be added after the command ('avail', 'list', etc.)
     TERSE_OPTION = (1, '-t')
     COMMAND = 'modulecmd.tcl'
@@ -1253,7 +1254,7 @@ class EnvironmentModulesTcl(EnvironmentModulesC):
     def set_path_env_var(self, key, paths):
         """Set environment variable with given name to the given list of paths."""
         super(EnvironmentModulesTcl, self).set_path_env_var(key, paths)
-        # for Tcl environment modules, we need to make sure the _modshare env var is kept in sync
+        # for Tcl Environment Modules, we need to make sure the _modshare env var is kept in sync
         setvar('%s_modshare' % key, ':1:'.join(paths), verbose=False)
 
     def run_module(self, *args, **kwargs):
@@ -1316,14 +1317,14 @@ def remove_module_path(self, path, set_mod_paths=True):
             self.set_mod_paths()
 
 
-class EnvironmentModules(EnvironmentModulesTcl):
-    """Interface to environment modules 4.0+"""
+class EnvironmentModules(ModulesTool):
+    """Interface to Environment Modules 4.0+"""
     NAME = "Environment Modules"
     COMMAND = os.path.join(os.getenv('MODULESHOME', 'MODULESHOME_NOT_DEFINED'), 'libexec', 'modulecmd.tcl')
     COMMAND_ENVIRONMENT = 'MODULES_CMD'
     REQ_VERSION = '4.0.0'
     REQ_VERSION_TCL_GETENV = '4.2.0'
-    DEPR_VERSION = '4.0.0'  # needs to be set as EnvironmentModules inherits from EnvironmentModulesTcl
+    DEPR_VERSION = '4.0.0'
     MAX_VERSION = None
     REQ_VERSION_TCL_CHECK_GROUP = '4.6.0'
     REQ_VERSION_SAFE_AUTO_LOAD = '4.2.4'
@@ -1419,14 +1420,35 @@ def get_setenv_value_from_modulefile(self, mod_name, var_name):
         # - line starts with 'setenv'
         # - whitespace (spaces & tabs) around variable name
         # - curly braces around value if it contain spaces
-        value = super(EnvironmentModules, self).get_setenv_value_from_modulefile(mod_name=mod_name,
-                                                                                 var_name=var_name)
+        regex = re.compile(r'^setenv\s+%s\s+(?P<value>.+)' % var_name, re.M)
+        value = self.get_value_from_modulefile(mod_name, regex, strict=False)
 
         if value:
-            value = value.strip('{}')
+            value = value.strip(' {}')
 
         return value
 
+    def remove_module_path(self, path, set_mod_paths=True):
+        """
+        Remove specified module path (using 'module unuse').
+
+        :param path: path to remove from $MODULEPATH via 'unuse'
+        :param set_mod_paths: (re)set self.mod_paths
+        """
+        # remove module path via 'module use' and make sure self.mod_paths is synced
+        # Environment Modules <5.0 keeps track of how often a path was added via 'module use',
+        # so we need to check to make sure it's really removed
+        path = normalize_path(path)
+        while True:
+            try:
+                # Unuse the path that is actually present in the environment
+                module_path = next(p for p in curr_module_paths() if normalize_path(p) == path)
+            except StopIteration:
+                break
+            self.unuse(module_path)
+        if set_mod_paths:
+            self.set_mod_paths()
+
     def update(self):
         """Update after new modules were added."""
 
@@ -1786,7 +1808,7 @@ def avail_modules_tools():
 
 def modules_tool(mod_paths=None, testing=False):
     """
-    Return interface to modules tool (environment modules (C, Tcl), or Lmod)
+    Return interface to modules tool (EnvironmentModules, Lmod, ...)
     """
     # get_modules_tool might return none (e.g. if config was not initialized yet)
     modules_tool = get_modules_tool()

test/framework/module_generator.py

@@ -684,7 +684,7 @@ def test_swap(self):
 
         # create tiny test Tcl module to make sure that tested modules tools support single-argument swap
         # see https://github.com/easybuilders/easybuild-framework/issues/3396;
-        # this is known to fail with the ancient Tcl-only implementation of environment modules,
+        # this is known to fail with the ancient Tcl-only implementation of Environment Modules,
         # but that's considered to be a non-issue (since this is mostly relevant for Cray systems,
         # which are either using EnvironmentModulesC (3.2.10), EnvironmentModules (4.x) or Lmod...
         if self.MODULE_GENERATOR_CLASS == ModuleGeneratorTcl and self.modtool.__class__ != EnvironmentModulesTcl:

test/framework/modules.py

@@ -122,10 +122,10 @@ def test_run_module(self):
             error_pattern = "Module command '.*thisdoesnotmakesense' failed with exit code [1-9]"
             self.assertErrorRegex(EasyBuildError, error_pattern, self.modtool.run_module, 'thisdoesnotmakesense')
 
-            # we need to use a different error pattern here with EnvironmentModulesC,
+            # we need to use a different error pattern here with Environment Modules,
             # because a load of a non-existing module doesnt' trigger a non-zero exit code...
             # it will still fail though, just differently
-            if isinstance(self.modtool, EnvironmentModulesC):
+            if isinstance(self.modtool, EnvironmentModulesC) or isinstance(self.modtool, EnvironmentModules):
                 error_pattern = "Unable to locate a modulefile for 'nosuchmodule/1.2.3'"
             else:
                 error_pattern = "Module command '.*load nosuchmodule/1.2.3' failed with exit code [1-9]"
@@ -213,10 +213,8 @@ def test_avail(self):
         # test modules include 3 GCC modules and one GCCcore module
         ms = self.modtool.available('GCC')
         expected = ['GCC/12.3.0', 'GCC/4.6.3', 'GCC/4.6.4', 'GCC/6.4.0-2.28', 'GCC/7.3.0-2.30']
-        # Tcl-only modules tool does an exact match on module name, Lmod & Tcl/C do prefix matching
-        # EnvironmentModules is a subclass of EnvironmentModulesTcl, but Modules 4+ behaves similarly to Tcl/C impl.,
-        # so also append GCCcore/6.2.0 if we are an instance of EnvironmentModules
-        if not isinstance(self.modtool, EnvironmentModulesTcl) or isinstance(self.modtool, EnvironmentModules):
+        # ancient Tcl-only Environment Modules tool does an exact match on module name, others do prefix matching
+        if not isinstance(self.modtool, EnvironmentModulesTcl):
             expected.extend(['GCCcore/12.3.0', 'GCCcore/6.2.0'])
         self.assertEqual(ms, expected)
 
@@ -343,12 +341,12 @@ def test_exist(self):
         easybuild.tools.modules.MODULE_SHOW_CACHE.clear()
         self.assertEqual(self.modtool.exist(['Java/1.8', 'Java/1.8.0_181']), [True, True])
 
-        # mimic more verbose stderr output produced by old Tmod version,
-        # including a warning produced when multiple .modulerc files are being picked up
+        # mimic "module-*" output produced by EnvironmentModulesC or EnvironmentModulesTcl
+        # mimic warning produced by Environment Modules when a symbol is defined multiple times
         # see https://github.com/easybuilders/easybuild-framework/issues/3376
         ml_show_java18_stderr = '\n'.join([
             "module-version    Java/1.8.0_181 1.8",
-            "WARNING: Duplicate version symbol '1.8' found",
+            "WARNING: Symbolic version 'Java/1.8' already defined",
             "module-version  Java/1.8.0_181 1.8",
             "-------------------------------------------------------------------",
             "/modulefiles/lang/Java/1.8.0_181:",
@@ -460,7 +458,7 @@ def test_load(self):
         # if GCC is loaded again, $EBROOTGCC should be set again, and GCC should be listed last
         self.modtool.load(['GCC/6.4.0-2.28'])
 
-        # environment modules v4+ does not reload already loaded modules
+        # Environment Modules v4+ does not reload already loaded modules
         if not isinstance(self.modtool, EnvironmentModules):
             self.assertTrue(os.environ.get('EBROOTGCC'))
 
@@ -1413,7 +1411,7 @@ def test_exit_code_check(self):
         if isinstance(self.modtool, Lmod):
             error_pattern = "Module command '.*load nosuchmoduleavailableanywhere' failed with exit code"
         else:
-            # Tcl implementations exit with 0 even when a non-existing module is loaded...
+            # Environment Modules exits with 0 even when a non-existing module is loaded...
             error_pattern = "Unable to locate a modulefile for 'nosuchmoduleavailableanywhere'"
         self.assertErrorRegex(EasyBuildError, error_pattern, self.modtool.load, ['nosuchmoduleavailableanywhere'])
 

test/framework/modulestool.py

@@ -236,8 +236,9 @@ def test_environment_modules_specific(self):
             # pass (fake) full path to 'modulecmd.tcl' via $MODULES_CMD
             fake_path = os.path.join(self.test_installpath, 'libexec', 'modulecmd.tcl')
             fake_modulecmd_txt = '\n'.join([
-                'puts stderr {Modules Release 5.3.1+unload-188-g14b6b59b (2023-10-21)}',
-                "puts {os.environ['FOO'] = 'foo'}",
+                '#!/bin/bash',
+                'echo "Modules Release 5.3.1+unload-188-g14b6b59b (2023-10-21)" >&2',
+                'echo "os.environ[\'FOO\'] = \'foo\'"',
             ])
             write_file(fake_path, fake_modulecmd_txt)
             os.chmod(fake_path, stat.S_IRUSR | stat.S_IXUSR)

test/framework/options.py

@@ -864,7 +864,7 @@ def test_avail_lists(self):
         os.close(fd)
 
         name_items = {
-            'modules-tools': ['EnvironmentModulesC', 'Lmod'],
+            'modules-tools': ['EnvironmentModules', 'Lmod'],
             'module-naming-schemes': ['EasyBuildMNS', 'HierarchicalMNS', 'CategorizedHMNS'],
         }
         for (name, items) in name_items.items():
@@ -880,7 +880,7 @@ def test_avail_lists(self):
             info_msg = r"INFO List of supported %s:" % words
             self.assertTrue(re.search(info_msg, logtxt), "Info message with list of available %s" % words)
             for item in items:
-                res = re.findall(r"^\s*%s" % item, logtxt, re.M)
+                res = re.findall(r"^\s*%s\n" % item, logtxt, re.M)
                 self.assertTrue(res, "%s is included in list of available %s" % (item, words))
                 # every item should only be mentioned once
                 n = len(res)
@@ -5315,19 +5315,19 @@ def test_show_config_cfg_levels(self):
 
         # configuring --modules-tool and --module-syntax on different levels should NOT cause problems
         # cfr. bug report https://github.com/easybuilders/easybuild-framework/issues/2564
-        os.environ['EASYBUILD_MODULES_TOOL'] = 'EnvironmentModulesC'
+        os.environ['EASYBUILD_MODULES_TOOL'] = 'EnvironmentModules'
         args = [
             '--module-syntax=Tcl',
             '--show-config',
         ]
         # set init_config to False to avoid that eb_main (called by _run_mock_eb) re-initialises configuration
-        # this fails because $EASYBUILD_MODULES_TOOL=EnvironmentModulesC conflicts with default module syntax (Lua)
+        # this fails because $EASYBUILD_MODULES_TOOL=EnvironmentModules conflicts with default module syntax (Lua)
         stdout, _ = self._run_mock_eb(args, raise_error=True, redo_init_config=False)
 
         patterns = [
             r"^# Current EasyBuild configuration",
             r"^module-syntax\s*\(C\) = Tcl",
-            r"^modules-tool\s*\(E\) = EnvironmentModulesC",
+            r"^modules-tool\s*\(E\) = EnvironmentModules",
         ]
         for pattern in patterns:
             regex = re.compile(pattern, re.M)
@@ -5339,19 +5339,19 @@ def test_modules_tool_vs_syntax_check(self):
         # make sure default module syntax is used
         os.environ.pop('EASYBUILD_MODULE_SYNTAX', None)
 
-        # using EnvironmentModulesC modules tool with default module syntax (Lua) is a problem
-        os.environ['EASYBUILD_MODULES_TOOL'] = 'EnvironmentModulesC'
+        # using EnvironmentModules modules tool with default module syntax (Lua) is a problem
+        os.environ['EASYBUILD_MODULES_TOOL'] = 'EnvironmentModules'
         args = ['--show-full-config']
         error_pattern = "Generating Lua module files requires Lmod as modules tool"
         self.assertErrorRegex(EasyBuildError, error_pattern, self._run_mock_eb, args, raise_error=True)
 
         patterns = [
             r"^# Current EasyBuild configuration",
             r"^module-syntax\s*\(C\) = Tcl",
-            r"^modules-tool\s*\(E\) = EnvironmentModulesC",
+            r"^modules-tool\s*\(E\) = EnvironmentModules",
         ]
 
-        # EnvironmentModulesC modules tool + Tcl module syntax is fine
+        # EnvironmentModules modules tool + Tcl module syntax is fine
         args.append('--module-syntax=Tcl')
         stdout, _ = self._run_mock_eb(args, do_build=True, raise_error=True, testing=False, redo_init_config=False)
         for pattern in patterns:

test/framework/utilities.py

@@ -399,7 +399,7 @@ def setup_categorized_hmns_modules(self):
             src_mod_path = os.path.join(os.path.dirname(os.path.abspath(__file__)),
                                         'modules', 'CategorizedHMNS', mod_subdir)
             copy_dir(src_mod_path, os.path.join(mod_prefix, mod_subdir))
-        # create empty module file directory to make C/Tcl modules happy
+        # create empty module file directory to make Environment Modules <5.0 happy
         mpi_pref = os.path.join(mod_prefix, 'MPI', 'GCC', '6.4.0-2.28', 'OpenMPI', '2.1.2')
         mkdir(os.path.join(mpi_pref, 'base'))