From f16afdb71c2e9a20cd91186a4833274fa4186874 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 6 Apr 2021 17:43:08 +0200 Subject: [PATCH 01/11] Use mk_module_path --- easybuild/tools/modules.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index fc21a564b9..13bc12b881 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -1566,7 +1566,7 @@ def use(self, path, priority=None): new_mod_path = path else: new_mod_path = [path] + [p for p in cur_mod_path.split(':') if normalize_path(p) != path] - new_mod_path = ':'.join(new_mod_path) + new_mod_path = mk_module_path(new_mod_path) self.log.debug('Changing MODULEPATH from %s to %s' % ('' if cur_mod_path is None else cur_mod_path, new_mod_path)) os.environ['MODULEPATH'] = new_mod_path @@ -1582,7 +1582,7 @@ def unuse(self, path): del os.environ['MODULEPATH'] else: path = normalize_path(path) - new_mod_path = ':'.join(p for p in cur_mod_path.split(':') if normalize_path(p) != path) + new_mod_path = mk_module_path(p for p in cur_mod_path.split(':') if normalize_path(p) != path) if new_mod_path != cur_mod_path: self.log.debug('Changing MODULEPATH from %s to %s' % (cur_mod_path, new_mod_path)) os.environ['MODULEPATH'] = new_mod_path From 5bb2216ad72a001a9733a6ecbd5708850dc35ffa Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 6 Apr 2021 17:50:38 +0200 Subject: [PATCH 02/11] Add clean flag to curr_module_paths to not strip any paths Lmod ignores if a path exists so we should be able to do the same --- easybuild/tools/modules.py | 11 ++++++++--- test/framework/modules.py | 8 ++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index 13bc12b881..1bfa7ff3bc 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -1728,14 +1728,19 @@ def get_software_version(name): return version -def curr_module_paths(normalize=False): +def curr_module_paths(normalize=False, clean=True): """ Return a list of current module paths. :param normalize: Normalize the paths + :param clean: If True remove empty and non-existing paths """ - # avoid empty or nonexistent paths, which don't make any sense - module_paths = (p for p in os.environ.get('MODULEPATH', '').split(':') if p and os.path.exists(p)) + if clean: + # avoid empty or nonexistent paths, which don't make any sense + module_paths = (p for p in os.environ.get('MODULEPATH', '').split(':') if p and os.path.exists(p)) + else: + modulepath = os.environ.get('MODULEPATH') + module_paths = [] if modulepath is None else modulepath.split(':') if normalize: module_paths = (normalize_path(p) for p in module_paths) return list(module_paths) diff --git a/test/framework/modules.py b/test/framework/modules.py index 82d03f6440..27b2edc065 100644 --- a/test/framework/modules.py +++ b/test/framework/modules.py @@ -492,15 +492,23 @@ def test_curr_module_paths(self): test3 = os.path.join(self.test_prefix, 'test3') mkdir(test3) + del os.environ['MODULEPATH'] + self.assertEqual(curr_module_paths(), []) + self.assertEqual(curr_module_paths(clean=False), []) + os.environ['MODULEPATH'] = '' self.assertEqual(curr_module_paths(), []) + self.assertEqual(curr_module_paths(clean=False), ['']) os.environ['MODULEPATH'] = '%s:%s:%s' % (test1, test2, test3) self.assertEqual(curr_module_paths(), [test1, test2, test3]) + self.assertEqual(curr_module_paths(clean=False), [test1, test2, test3]) # empty entries and non-existing directories are filtered out os.environ['MODULEPATH'] = '/doesnotexist:%s::%s:' % (test2, test1) self.assertEqual(curr_module_paths(), [test2, test1]) + # Disabling the clean returns them + self.assertEqual(curr_module_paths(clean=False), ['/doesnotexist', test2, '', test1, '']) def test_check_module_path(self): """Test ModulesTool.check_module_path() method""" From b3077ef1e93b674b220cfaf95087fa535df379de Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 6 Apr 2021 18:17:34 +0200 Subject: [PATCH 03/11] Unify handling of setting the MODULEPATH in Lmod Introduce _set_module_path which correctly handles empty paths and empty lists with proper logging --- easybuild/tools/modules.py | 48 +++++++++++++++++++++----------------- test/framework/modules.py | 26 +++++++++++++++++++++ 2 files changed, 52 insertions(+), 22 deletions(-) diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index 1bfa7ff3bc..d2134635aa 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -50,7 +50,7 @@ from easybuild.tools.environment import ORIG_OS_ENVIRON, restore_env, setvar, unset_env_vars from easybuild.tools.filetools import convert_name, mkdir, normalize_path, path_matches, read_file, which, write_file from easybuild.tools.module_naming_scheme.mns import DEVEL_MODULE_SUFFIX -from easybuild.tools.py2vs3 import subprocess_popen_text +from easybuild.tools.py2vs3 import subprocess_popen_text, string_type from easybuild.tools.run import run_cmd from easybuild.tools.utilities import get_subclasses, nub @@ -1538,6 +1538,28 @@ def update(self): mkdir(cache_dir, parents=True) write_file(cache_fp, stdout) + def _set_module_path(self, new_mod_path): + """ + Set $MODULEPATH to the specified paths and logs the change. + new_mod_path can be None or an iterable of paths + """ + if new_mod_path is not None: + if not isinstance(new_mod_path, list): + assert not isinstance(new_mod_path, string_type) # Just to be sure + new_mod_path = list(new_mod_path) # Expand generators + new_mod_path = mk_module_path(new_mod_path) if new_mod_path else None + cur_mod_path = os.environ.get('MODULEPATH') + if new_mod_path != cur_mod_path: + self.log.debug( + 'Changing MODULEPATH from %s to %s', + '' if cur_mod_path is None else cur_mod_path, + '' if new_mod_path is None else new_mod_path, + ) + if new_mod_path is None: + del os.environ['MODULEPATH'] + else: + os.environ['MODULEPATH'] = new_mod_path + def use(self, path, priority=None): """ Add path to $MODULEPATH via 'module use'. @@ -1561,31 +1583,13 @@ def use(self, path, priority=None): self.run_module(['use', path]) else: path = normalize_path(path) - cur_mod_path = os.environ.get('MODULEPATH') - if cur_mod_path is None: - new_mod_path = path - else: - new_mod_path = [path] + [p for p in cur_mod_path.split(':') if normalize_path(p) != path] - new_mod_path = mk_module_path(new_mod_path) - self.log.debug('Changing MODULEPATH from %s to %s' % - ('' if cur_mod_path is None else cur_mod_path, new_mod_path)) - os.environ['MODULEPATH'] = new_mod_path + self._set_module_path([path] + [p for p in curr_module_paths(clean=False) if normalize_path(p) != path]) def unuse(self, path): """Remove a module path""" # We can simply remove the path from MODULEPATH to avoid the costly module call - cur_mod_path = os.environ.get('MODULEPATH') - if cur_mod_path is not None: - # Removing the last entry unsets the variable - if cur_mod_path == path: - self.log.debug('Changing MODULEPATH from %s to ' % cur_mod_path) - del os.environ['MODULEPATH'] - else: - path = normalize_path(path) - new_mod_path = mk_module_path(p for p in cur_mod_path.split(':') if normalize_path(p) != path) - if new_mod_path != cur_mod_path: - self.log.debug('Changing MODULEPATH from %s to %s' % (cur_mod_path, new_mod_path)) - os.environ['MODULEPATH'] = new_mod_path + path = normalize_path(path) + self._set_module_path(p for p in curr_module_paths(clean=False) if normalize_path(p) != path) def prepend_module_path(self, path, set_mod_paths=True, priority=None): """ diff --git a/test/framework/modules.py b/test/framework/modules.py index 27b2edc065..03fbc277cd 100644 --- a/test/framework/modules.py +++ b/test/framework/modules.py @@ -1213,6 +1213,29 @@ def test_module_use_unuse(self): # Tests for Lmod only if isinstance(self.modtool, Lmod): + # Check the helper function + old_module_path = os.environ['MODULEPATH'] + self.modtool._set_module_path(['/foo']) + self.assertEqual(os.environ['MODULEPATH'], '/foo') + self.modtool._set_module_path(['/foo', '/bar']) + self.assertEqual(os.environ['MODULEPATH'], '/foo:/bar') + self.modtool._set_module_path(['']) + self.assertEqual(os.environ['MODULEPATH'], '') + self.modtool._set_module_path([]) + self.assertFalse('MODULEPATH' in os.environ) + self.modtool._set_module_path(None) + self.assertFalse('MODULEPATH' in os.environ) + # Same for generators + self.modtool._set_module_path(i for i in ['/foo']) + self.assertEqual(os.environ['MODULEPATH'], '/foo') + self.modtool._set_module_path(i for i in ['/foo', '/bar']) + self.assertEqual(os.environ['MODULEPATH'], '/foo:/bar') + self.modtool._set_module_path(i for i in ['']) + self.assertEqual(os.environ['MODULEPATH'], '') + self.modtool._set_module_path(i for i in []) + self.assertFalse('MODULEPATH' in os.environ) + os.environ['MODULEPATH'] = old_module_path # Restore + # check whether prepend with priority actually works (priority is specific to Lmod) self.modtool.use(test_dir1, priority=100) self.modtool.use(test_dir3) @@ -1238,6 +1261,9 @@ def test_module_use_unuse(self): self.modtool.use(test_dir1) self.assertEqual(os.environ['MODULEPATH'], test_dir1) self.modtool.unuse(test_dir1) + self.assertFalse('MODULEPATH' in os.environ) + # Unuse when the MODULEPATH is already empty + self.modtool.unuse(test_dir1) self.assertNotIn('MODULEPATH', os.environ) os.environ['MODULEPATH'] = old_module_path # Restore From bd1e791e56258f679203fe4ba8671a4dd0ce7739 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Wed, 7 Apr 2021 12:27:52 +0200 Subject: [PATCH 04/11] Use dir not prefix for mkdtemp --- easybuild/toolchains/mpi/openmpi.py | 2 +- test/framework/easyconfig.py | 6 +++--- test/framework/modules.py | 16 ++++++++-------- test/framework/toolchain.py | 2 +- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/easybuild/toolchains/mpi/openmpi.py b/easybuild/toolchains/mpi/openmpi.py index 91d10923e7..a832b9c27d 100644 --- a/easybuild/toolchains/mpi/openmpi.py +++ b/easybuild/toolchains/mpi/openmpi.py @@ -83,7 +83,7 @@ def prepare(self, *args, **kwargs): ompi_ver = self.get_software_version(self.MPI_MODULE_NAME)[0] if LooseVersion(ompi_ver) >= LooseVersion('2.0') and LooseVersion(ompi_ver) < LooseVersion('3.0'): if len(self.orig_tmpdir) > 40: - tmpdir = tempfile.mkdtemp(prefix='/tmp/') + tmpdir = tempfile.mkdtemp(dir='/tmp', prefix='') env.setvar('TMPDIR', tmpdir) warn_msg = "Long $TMPDIR path may cause problems with OpenMPI 2.x, using shorter path: %s" % tmpdir self.log.warning(warn_msg) diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index 45e2852221..2ce37327ad 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -2528,7 +2528,7 @@ def test_dump_extra(self): '', ]) - handle, testec = tempfile.mkstemp(prefix=self.test_prefix, suffix='.eb') + handle, testec = tempfile.mkstemp(suffix='.eb') os.close(handle) ec = EasyConfig(None, rawtxt=rawtxt) @@ -2591,7 +2591,7 @@ def test_dump_template(self): 'moduleclass = "tools"', ]) - handle, testec = tempfile.mkstemp(prefix=self.test_prefix, suffix='.eb') + handle, testec = tempfile.mkstemp(suffix='.eb') os.close(handle) ec = EasyConfig(None, rawtxt=rawtxt) @@ -2667,7 +2667,7 @@ def test_dump_comments(self): "# trailing comment", ]) - handle, testec = tempfile.mkstemp(prefix=self.test_prefix, suffix='.eb') + handle, testec = tempfile.mkstemp(suffix='.eb') os.close(handle) ec = EasyConfig(None, rawtxt=rawtxt) diff --git a/test/framework/modules.py b/test/framework/modules.py index 03fbc277cd..180685bb19 100644 --- a/test/framework/modules.py +++ b/test/framework/modules.py @@ -592,7 +592,7 @@ def broken_prepend_module_path(*args, **kwargs): def test_prepend_module_path(self): """Test prepend_module_path method.""" - test_path = tempfile.mkdtemp(prefix=self.test_prefix) + test_path = tempfile.mkdtemp() self.modtool.prepend_module_path(test_path) self.assertTrue(os.path.samefile(curr_module_paths()[0], test_path)) @@ -615,17 +615,17 @@ def test_prepend_module_path(self): self.assertEqual(modulepath, curr_module_paths()) # test prepending with high priority - test_path_bis = tempfile.mkdtemp(prefix=self.test_prefix) - test_path_tris = tempfile.mkdtemp(prefix=self.test_prefix) - self.modtool.prepend_module_path(test_path_bis, priority=10000) - self.assertEqual(test_path_bis, curr_module_paths()[0]) + test_path_0 = tempfile.mkdtemp(suffix='path_0') + test_path_1 = tempfile.mkdtemp(suffix='path_1') + self.modtool.prepend_module_path(test_path_0, priority=1000) + self.assertEqual(test_path_0, curr_module_paths()[0]) # check whether prepend with priority actually works (only for Lmod) if isinstance(self.modtool, Lmod): - self.modtool.prepend_module_path(test_path_tris) + self.modtool.prepend_module_path(test_path_1) modulepath = curr_module_paths() - self.assertEqual(test_path_bis, modulepath[0]) - self.assertEqual(test_path_tris, modulepath[1]) + self.assertEqual(test_path_0, modulepath[0]) + self.assertEqual(test_path_1, modulepath[1]) def test_ld_library_path(self): """Make sure LD_LIBRARY_PATH is what it should be when loaded multiple modules.""" diff --git a/test/framework/toolchain.py b/test/framework/toolchain.py index 0e53639491..e2ff8ef2cc 100644 --- a/test/framework/toolchain.py +++ b/test/framework/toolchain.py @@ -2916,7 +2916,7 @@ def prep(): orig_tmpdir = os.environ.get('TMPDIR') if len(orig_tmpdir) > 40: # we need to make sure we have a short $TMPDIR for this test... - orig_tmpdir = tempfile.mkdtemp(prefix='/tmp/') + orig_tmpdir = tempfile.mkdtemp(dir='/tmp') mkdir(orig_tmpdir) os.environ['TMPDIR'] = orig_tmpdir From 318576e89773e881a4ab0a0ebdfb11a59caef259 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Wed, 7 Apr 2021 12:31:20 +0200 Subject: [PATCH 05/11] Avoid use of priority in prepend_module_path --- easybuild/framework/easyblock.py | 2 +- easybuild/tools/modules.py | 12 ++++++++++++ test/framework/modules.py | 8 +++++++- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index d2727d61ee..adebddb372 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1708,7 +1708,7 @@ def load_fake_module(self, purge=False, extra_modules=None, verbose=False): fake_mod_path = self.make_module_step(fake=True) # load fake module - self.modules_tool.prepend_module_path(os.path.join(fake_mod_path, self.mod_subdir), priority=10000) + self.modules_tool.prepend_module_path(os.path.join(fake_mod_path, self.mod_subdir)) self.load_module(purge=purge, extra_modules=extra_modules, verbose=verbose) return (fake_mod_path, env) diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index d2134635aa..d34c81bb2c 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -155,6 +155,8 @@ class ModulesTool(object): VERSION_REGEXP = None # modules tool user cache directory USER_CACHE_DIR = None + # Priority used to put module paths at the front when priorities are otherwise used + HIGH_PRIORITY = 10000 def __init__(self, mod_paths=None, testing=False): """ @@ -1602,7 +1604,17 @@ def prepend_module_path(self, path, set_mod_paths=True, priority=None): # Lmod pushes a path to the front on 'module use', no need for (costly) 'module unuse' modulepath = curr_module_paths() if not modulepath or os.path.realpath(modulepath[0]) != os.path.realpath(path): + # If no explicit priority is set, but priorities are already in use we need to use a high + # priority to make sure the path (very likely) ends up at the front + if priority is None and os.environ.get('__LMOD_Priority_MODULEPATH'): + priority = self.HIGH_PRIORITY self.use(path, priority=priority) + modulepath = curr_module_paths(clean=False) + path_idx = modulepath.index(path) + if path_idx != 0: + self.log.warn("Path '%s' could not be prepended to $MODULEPATH. The following paths have a higher " + "priority: %s'", + path, "; ".join(modulepath[:path_idx])) if set_mod_paths: self.set_mod_paths() diff --git a/test/framework/modules.py b/test/framework/modules.py index 180685bb19..3500e4b39a 100644 --- a/test/framework/modules.py +++ b/test/framework/modules.py @@ -624,8 +624,14 @@ def test_prepend_module_path(self): if isinstance(self.modtool, Lmod): self.modtool.prepend_module_path(test_path_1) modulepath = curr_module_paths() - self.assertEqual(test_path_0, modulepath[0]) + self.assertEqual(test_path_1, modulepath[0]) + self.assertEqual(test_path_0, modulepath[1]) + test_path_2 = tempfile.mkdtemp(suffix='path_2') + self.modtool.prepend_module_path(test_path_2) + modulepath = curr_module_paths() + self.assertEqual(test_path_2, modulepath[0]) self.assertEqual(test_path_1, modulepath[1]) + self.assertEqual(test_path_0, modulepath[2]) def test_ld_library_path(self): """Make sure LD_LIBRARY_PATH is what it should be when loaded multiple modules.""" From 75802a29b268307cbab95231763058957af1e8f6 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Wed, 7 Apr 2021 14:54:26 +0200 Subject: [PATCH 06/11] Factor out use of `os.environ.get('__LMOD_Priority_MODULEPATH')` --- easybuild/tools/modules.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index d34c81bb2c..4e2fea0323 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -1562,6 +1562,12 @@ def _set_module_path(self, new_mod_path): else: os.environ['MODULEPATH'] = new_mod_path + def _has_module_paths_with_priority(self): + """Return True if there are priorities attached to $MODULEPATH""" + # We simply check, if the Lmod variable is set and non-empty + # See https://github.com/TACC/Lmod/issues/509 + return bool(os.environ.get('__LMOD_Priority_MODULEPATH')) + def use(self, path, priority=None): """ Add path to $MODULEPATH via 'module use'. @@ -1581,7 +1587,7 @@ def use(self, path, priority=None): else: # LMod allows modifying MODULEPATH directly. So do that to avoid the costly module use # unless priorities are in use already - if os.environ.get('__LMOD_Priority_MODULEPATH'): + if self._has_module_paths_with_priority(): self.run_module(['use', path]) else: path = normalize_path(path) @@ -1606,7 +1612,7 @@ def prepend_module_path(self, path, set_mod_paths=True, priority=None): if not modulepath or os.path.realpath(modulepath[0]) != os.path.realpath(path): # If no explicit priority is set, but priorities are already in use we need to use a high # priority to make sure the path (very likely) ends up at the front - if priority is None and os.environ.get('__LMOD_Priority_MODULEPATH'): + if priority is None and self._has_module_paths_with_priority(): priority = self.HIGH_PRIORITY self.use(path, priority=priority) modulepath = curr_module_paths(clean=False) From 18f20cb18c8a6a34eaad1f4807ab8b8b2bc2cf3a Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Wed, 7 Apr 2021 14:54:43 +0200 Subject: [PATCH 07/11] Upgrade prepend failed warning to print_warning --- easybuild/tools/modules.py | 8 +++++--- test/framework/modules.py | 13 +++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index 4e2fea0323..3c6373009b 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -1618,9 +1618,11 @@ def prepend_module_path(self, path, set_mod_paths=True, priority=None): modulepath = curr_module_paths(clean=False) path_idx = modulepath.index(path) if path_idx != 0: - self.log.warn("Path '%s' could not be prepended to $MODULEPATH. The following paths have a higher " - "priority: %s'", - path, "; ".join(modulepath[:path_idx])) + print_warning("Path '%s' could not be prepended to $MODULEPATH. " + "The following paths are still in front of it: %s\n" + "This can happen if paths were added via `module use` with a priority higher than %s", + path, "; ".join(modulepath[:path_idx]), self.HIGH_PRIORITY, + log=self.log) if set_mod_paths: self.set_mod_paths() diff --git a/test/framework/modules.py b/test/framework/modules.py index 3500e4b39a..02589cef91 100644 --- a/test/framework/modules.py +++ b/test/framework/modules.py @@ -633,6 +633,19 @@ def test_prepend_module_path(self): self.assertEqual(test_path_1, modulepath[1]) self.assertEqual(test_path_0, modulepath[2]) + # When prepend fails due to a higher priority path, a warning is shown + self.modtool.unuse(test_path_2) + self.modtool.prepend_module_path(test_path_0, priority=999999) + self.mock_stderr(True) + self.modtool.prepend_module_path(test_path_2) + stderr = self.get_stderr() + self.mock_stderr(False) + self.assertTrue('could not be prepended' in stderr) + modulepath = curr_module_paths() + self.assertEqual(test_path_0, modulepath[0]) + self.assertEqual(test_path_2, modulepath[1]) + self.assertEqual(test_path_1, modulepath[2]) + def test_ld_library_path(self): """Make sure LD_LIBRARY_PATH is what it should be when loaded multiple modules.""" self.init_testmods() From f4bd8930511042b0767dde3c676a9121b396d1c2 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Wed, 7 Apr 2021 15:23:58 +0200 Subject: [PATCH 08/11] Add force_module_command parameter --- easybuild/tools/modules.py | 39 ++++++++++++++++++++++++++++---------- test/framework/modules.py | 13 +++++++++++++ 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index 3c6373009b..4b7086c0b0 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -361,12 +361,14 @@ def set_mod_paths(self, mod_paths=None): self.log.debug("$MODULEPATH after set_mod_paths: %s" % os.environ.get('MODULEPATH', '')) - def use(self, path, priority=None): + def use(self, path, priority=None, force_module_command=False): """ Add path to $MODULEPATH via 'module use'. :param path: path to add to $MODULEPATH :param priority: priority for this path in $MODULEPATH (Lmod-specific) + :param force_module_command: If False running the module command may be skipped which is faster + but does not reload modules """ if priority: self.log.info("Ignoring specified priority '%s' when running 'module use %s' (Lmod-specific)", @@ -380,8 +382,14 @@ def use(self, path, priority=None): mkdir(path, parents=True) self.run_module(['use', path]) - def unuse(self, path): - """Remove module path via 'module unuse'.""" + def unuse(self, path, force_module_command=False): + """ + Remove a module path (usually) via 'module unuse'. + + :param path: path to remove from $MODULEPATH + :param force_module_command: If False running the module command may be skipped which is faster + but does not reload modules + """ self.run_module(['unuse', path]) def add_module_path(self, path, set_mod_paths=True): @@ -1568,12 +1576,14 @@ def _has_module_paths_with_priority(self): # See https://github.com/TACC/Lmod/issues/509 return bool(os.environ.get('__LMOD_Priority_MODULEPATH')) - def use(self, path, priority=None): + def use(self, path, priority=None, force_module_command=False): """ Add path to $MODULEPATH via 'module use'. :param path: path to add to $MODULEPATH :param priority: priority for this path in $MODULEPATH (Lmod-specific) + :param force_module_command: If False running the module command may be skipped which is faster + but does not reload modules """ if not path: raise EasyBuildError("Cannot add empty path to $MODULEPATH") @@ -1587,17 +1597,26 @@ def use(self, path, priority=None): else: # LMod allows modifying MODULEPATH directly. So do that to avoid the costly module use # unless priorities are in use already - if self._has_module_paths_with_priority(): + if force_module_command or self._has_module_paths_with_priority(): self.run_module(['use', path]) else: path = normalize_path(path) self._set_module_path([path] + [p for p in curr_module_paths(clean=False) if normalize_path(p) != path]) - def unuse(self, path): - """Remove a module path""" - # We can simply remove the path from MODULEPATH to avoid the costly module call - path = normalize_path(path) - self._set_module_path(p for p in curr_module_paths(clean=False) if normalize_path(p) != path) + def unuse(self, path, force_module_command=False): + """ + Remove a module path + + :param path: path to remove from $MODULEPATH + :param force_module_command: If False running the module command may be skipped which is faster + but does not reload modules + """ + if force_module_command: + super(Lmod, self).unuse(path) + else: + # We can simply remove the path from MODULEPATH to avoid the costly module call + path = normalize_path(path) + self._set_module_path(p for p in curr_module_paths(clean=False) if normalize_path(p) != path) def prepend_module_path(self, path, set_mod_paths=True, priority=None): """ diff --git a/test/framework/modules.py b/test/framework/modules.py index 02589cef91..eabf63734e 100644 --- a/test/framework/modules.py +++ b/test/framework/modules.py @@ -1286,6 +1286,19 @@ def test_module_use_unuse(self): self.assertNotIn('MODULEPATH', os.environ) os.environ['MODULEPATH'] = old_module_path # Restore + # Forcing to use the module command reloads modules (Only Lmod does this) + old_module_path = os.environ['MODULEPATH'] + os.environ['MODULEPATH'] = test_dir1 + self.assertFalse('TEST123' in os.environ) + self.modtool.load(['test']) + self.assertEqual(os.getenv('TEST123'), 'one') + self.modtool.use(test_dir2, force_module_command=True) + self.assertEqual(os.getenv('TEST123'), 'two') # Module reloaded + self.modtool.unuse(test_dir2, force_module_command=True) + self.assertEqual(os.getenv('TEST123'), 'one') # Module reloaded + self.modtool.unload(['test']) + os.environ['MODULEPATH'] = old_module_path # Restore + def test_add_and_remove_module_path(self): """Test add_module_path and whether remove_module_path undoes changes of add_module_path""" test_dir1 = tempfile.mkdtemp(suffix="_dir1") From 097d3c7b610d171e90b6f7b2f72626439001aa8a Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 27 May 2021 10:58:26 +0200 Subject: [PATCH 09/11] Fix normalization when checking path in curr_module_paths --- easybuild/tools/modules.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index 4b7086c0b0..baaacdc935 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -400,7 +400,7 @@ def add_module_path(self, path, set_mod_paths=True): :param set_mod_paths: (re)set self.mod_paths """ path = normalize_path(path) - if path not in curr_module_paths(normalize=True): + if path not in curr_module_paths(clean=False, normalize=True): # add module path via 'module use' and make sure self.mod_paths is synced self.use(path) if set_mod_paths: @@ -1634,8 +1634,8 @@ def prepend_module_path(self, path, set_mod_paths=True, priority=None): if priority is None and self._has_module_paths_with_priority(): priority = self.HIGH_PRIORITY self.use(path, priority=priority) - modulepath = curr_module_paths(clean=False) - path_idx = modulepath.index(path) + modulepath = curr_module_paths(normalize=True, clean=False) + path_idx = modulepath.index(normalize_path(path)) if path_idx != 0: print_warning("Path '%s' could not be prepended to $MODULEPATH. " "The following paths are still in front of it: %s\n" From a503b87420eb57f7350c1d12a45ea249c141dfea Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 27 May 2021 14:27:55 +0200 Subject: [PATCH 10/11] Use os.pathsep consistently --- easybuild/tools/modules.py | 6 ++--- test/framework/modules.py | 46 ++++++++++++++++++++++---------------- 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index baaacdc935..9f4759efad 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -1780,10 +1780,10 @@ def curr_module_paths(normalize=False, clean=True): """ if clean: # avoid empty or nonexistent paths, which don't make any sense - module_paths = (p for p in os.environ.get('MODULEPATH', '').split(':') if p and os.path.exists(p)) + module_paths = (p for p in os.environ.get('MODULEPATH', '').split(os.pathsep) if p and os.path.exists(p)) else: modulepath = os.environ.get('MODULEPATH') - module_paths = [] if modulepath is None else modulepath.split(':') + module_paths = [] if modulepath is None else modulepath.split(os.pathsep) if normalize: module_paths = (normalize_path(p) for p in module_paths) return list(module_paths) @@ -1793,7 +1793,7 @@ def mk_module_path(paths): """ Create a string representing the list of module paths. """ - return ':'.join(paths) + return os.pathsep.join(paths) def avail_modules_tools(): diff --git a/test/framework/modules.py b/test/framework/modules.py index eabf63734e..9dd4e2127c 100644 --- a/test/framework/modules.py +++ b/test/framework/modules.py @@ -500,12 +500,12 @@ def test_curr_module_paths(self): self.assertEqual(curr_module_paths(), []) self.assertEqual(curr_module_paths(clean=False), ['']) - os.environ['MODULEPATH'] = '%s:%s:%s' % (test1, test2, test3) + os.environ['MODULEPATH'] = os.pathsep.join([test1, test2, test3]) self.assertEqual(curr_module_paths(), [test1, test2, test3]) self.assertEqual(curr_module_paths(clean=False), [test1, test2, test3]) # empty entries and non-existing directories are filtered out - os.environ['MODULEPATH'] = '/doesnotexist:%s::%s:' % (test2, test1) + os.environ['MODULEPATH'] = os.pathsep.join(['/doesnotexist', test2, '', test1, '']) self.assertEqual(curr_module_paths(), [test2, test1]) # Disabling the clean returns them self.assertEqual(curr_module_paths(clean=False), ['/doesnotexist', test2, '', test1, '']) @@ -545,7 +545,7 @@ def test_check_module_path(self): self.assertEqual(os.environ['MODULEPATH'], os.pathsep.join([mod_install_dir, test1, test2])) # check behaviour if non-existing directories are included in $MODULEPATH - os.environ['MODULEPATH'] = '%s:/does/not/exist:%s' % (test3, test2) + os.environ['MODULEPATH'] = os.pathsep.join([test3, '/does/not/exist', test2]) modtool.check_module_path() # non-existing dir is filtered from mod_paths, but stays in $MODULEPATH self.assertEqual(modtool.mod_paths, [mod_install_dir, test1, test3, test2]) @@ -570,11 +570,11 @@ def test_check_module_path_hmns(self): doesnotexist = os.path.join(self.test_prefix, 'doesnotexist') self.assertNotExists(doesnotexist) - os.environ['MODULEPATH'] = '%s:%s' % (core_mod_dir, doesnotexist) + os.environ['MODULEPATH'] = os.pathsep.join([core_mod_dir, doesnotexist]) modtool = modules_tool() self.assertEqual(modtool.mod_paths, [os.path.dirname(core_mod_dir), core_mod_dir]) - self.assertEqual(os.environ['MODULEPATH'], '%s:%s:%s' % (top_mod_dir, core_mod_dir, doesnotexist)) + self.assertEqual(os.environ['MODULEPATH'], os.pathsep.join([top_mod_dir, core_mod_dir, doesnotexist])) # hack prepend_module_path to make sure it's not called again if check_module_path is called again; # prepend_module_path is fairly expensive, so should be avoided, @@ -588,7 +588,7 @@ def broken_prepend_module_path(*args, **kwargs): modtool.check_module_path() self.assertEqual(modtool.mod_paths, [os.path.dirname(core_mod_dir), core_mod_dir]) - self.assertEqual(os.environ['MODULEPATH'], '%s:%s:%s' % (top_mod_dir, core_mod_dir, doesnotexist)) + self.assertEqual(os.environ['MODULEPATH'], os.pathsep.join([top_mod_dir, core_mod_dir, doesnotexist])) def test_prepend_module_path(self): """Test prepend_module_path method.""" @@ -744,7 +744,11 @@ def test_wrong_modulepath(self): """Test whether modules tool can deal with a broken $MODULEPATH.""" test_modules_path = os.path.realpath(os.path.join(os.path.dirname(os.path.abspath(__file__)), 'modules')) modules_test_installpath = os.path.join(self.test_installpath, 'modules', 'all') - os.environ['MODULEPATH'] = '/some/non-existing/path:/this/doesnt/exists/anywhere:%s' % test_modules_path + os.environ['MODULEPATH'] = os.pathsep.join([ + '/some/non-existing/path', + '/this/doesnt/exists/anywhere', + test_modules_path + ]) init_config() # purposely *not* using self.modtool here; # need to check whether creating new ModulesTool instance doesn't break when $MODULEPATH contains faulty paths @@ -1107,10 +1111,11 @@ def test_modules_tool_stateless(self): def test_mk_module_cache_key(self): """Test mk_module_cache_key method.""" - os.environ['MODULEPATH'] = '%s:/tmp/test' % self.test_prefix + module_path = os.pathsep.join([self.test_prefix, '/tmp/test']) + os.environ['MODULEPATH'] = module_path res = self.modtool.mk_module_cache_key('thisisapartialkey') self.assertIsInstance(res, tuple) - self.assertEqual(res, ('MODULEPATH=%s:/tmp/test' % self.test_prefix, self.modtool.COMMAND, 'thisisapartialkey')) + self.assertEqual(res, ('MODULEPATH=%s' % module_path, self.modtool.COMMAND, 'thisisapartialkey')) del os.environ['MODULEPATH'] res = self.modtool.mk_module_cache_key('thisisapartialkey') @@ -1189,11 +1194,11 @@ def test_module_use_unuse(self): self.assertNotIn(test_dir1, os.environ.get('MODULEPATH', '')) self.modtool.use(test_dir1) - self.assertTrue(os.environ['MODULEPATH'].startswith('%s:' % test_dir1)) + self.assertTrue(os.environ['MODULEPATH'].startswith(test_dir1 + os.pathsep)) self.modtool.use(test_dir2) - self.assertTrue(os.environ['MODULEPATH'].startswith('%s:' % test_dir2)) + self.assertTrue(os.environ['MODULEPATH'].startswith(test_dir2 + os.pathsep)) self.modtool.use(test_dir3) - self.assertTrue(os.environ['MODULEPATH'].startswith('%s:' % test_dir3)) + self.assertTrue(os.environ['MODULEPATH'].startswith(test_dir3 + os.pathsep)) # Adding an empty modulepath is not possible modulepath = os.environ.get('MODULEPATH', '') @@ -1224,7 +1229,7 @@ def test_module_use_unuse(self): # also test use with high priority self.modtool.use(test_dir2, priority=10000) - self.assertTrue(os.environ['MODULEPATH'].startswith('%s:' % test_dir2)) + self.assertTrue(os.environ['MODULEPATH'].startswith(test_dir2 + os.pathsep)) self.modtool.load(['test']) self.assertEqual(os.getenv('TEST123'), 'two') @@ -1236,8 +1241,9 @@ def test_module_use_unuse(self): old_module_path = os.environ['MODULEPATH'] self.modtool._set_module_path(['/foo']) self.assertEqual(os.environ['MODULEPATH'], '/foo') - self.modtool._set_module_path(['/foo', '/bar']) - self.assertEqual(os.environ['MODULEPATH'], '/foo:/bar') + foo_and_bar_paths = ['/foo', '/bar'] + self.modtool._set_module_path(foo_and_bar_paths) + self.assertEqual(os.environ['MODULEPATH'], os.pathsep.join(foo_and_bar_paths)) self.modtool._set_module_path(['']) self.assertEqual(os.environ['MODULEPATH'], '') self.modtool._set_module_path([]) @@ -1247,8 +1253,8 @@ def test_module_use_unuse(self): # Same for generators self.modtool._set_module_path(i for i in ['/foo']) self.assertEqual(os.environ['MODULEPATH'], '/foo') - self.modtool._set_module_path(i for i in ['/foo', '/bar']) - self.assertEqual(os.environ['MODULEPATH'], '/foo:/bar') + self.modtool._set_module_path(i for i in foo_and_bar_paths) + self.assertEqual(os.environ['MODULEPATH'], os.pathsep.join(foo_and_bar_paths)) self.modtool._set_module_path(i for i in ['']) self.assertEqual(os.environ['MODULEPATH'], '') self.modtool._set_module_path(i for i in []) @@ -1258,7 +1264,9 @@ def test_module_use_unuse(self): # check whether prepend with priority actually works (priority is specific to Lmod) self.modtool.use(test_dir1, priority=100) self.modtool.use(test_dir3) - self.assertTrue(os.environ['MODULEPATH'].startswith('%s:%s:%s:' % (test_dir2, test_dir1, test_dir3))) + self.assertTrue(os.environ['MODULEPATH'].startswith( + os.pathsep.join([test_dir2, test_dir1, test_dir3]) + )) self.modtool.load(['test']) self.assertEqual(os.getenv('TEST123'), 'two') self.modtool.unload(['test']) @@ -1587,7 +1595,7 @@ def test_modulecmd_strip_source(self): write_file(modulecmd, modulecmd_txt) adjust_permissions(modulecmd, stat.S_IXUSR, add=True) - os.environ['PATH'] = '%s:%s' % (self.test_prefix, os.getenv('PATH')) + os.environ['PATH'] = os.pathsep.join([self.test_prefix, os.getenv('PATH')]) modtool = EnvironmentModulesC() modtool.run_module('load', 'test123') From 6e11808103414ff55e2e2d25d37c9b104e2a874b Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 23 Oct 2025 12:55:02 +0200 Subject: [PATCH 11/11] Allow higher priority MODULEPATHs to be in front of paths added with prepend_module_path --- easybuild/tools/modules.py | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index f583f5d373..99b5b93532 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -592,8 +592,6 @@ class ModulesTool: VERSION_REGEXP = None # modules tool user cache directory USER_CACHE_DIR = None - # Priority used to put module paths at the front when priorities are otherwise used - HIGH_PRIORITY = 10000 def __init__(self, mod_paths=None, testing=False): """ @@ -2115,19 +2113,14 @@ def prepend_module_path(self, path, set_mod_paths=True, priority=None): # Lmod pushes a path to the front on 'module use', no need for (costly) 'module unuse' modulepath = curr_module_paths() if not modulepath or os.path.realpath(modulepath[0]) != os.path.realpath(path): - # If no explicit priority is set, but priorities are already in use we need to use a high - # priority to make sure the path (very likely) ends up at the front - if priority is None and self._has_module_paths_with_priority(): - priority = self.HIGH_PRIORITY self.use(path, priority=priority) - modulepath = curr_module_paths(normalize=True, clean=False) - path_idx = modulepath.index(normalize_path(path)) - if path_idx != 0: - print_warning("Path '%s' could not be prepended to $MODULEPATH. " - "The following paths are still in front of it: %s\n" - "This can happen if paths were added via `module use` with a priority higher than %s", - path, "; ".join(modulepath[:path_idx]), self.HIGH_PRIORITY, - log=self.log) + if priority is None and not self._has_module_paths_with_priority(): + modulepath = curr_module_paths(normalize=True, clean=False) + path_idx = modulepath.index(normalize_path(path)) + if path_idx != 0: + print_warning("Path '%s' could not be prepended to $MODULEPATH. " + "The following paths are still in front of it: %s", + path, "; ".join(modulepath[:path_idx]), log=self.log) if set_mod_paths: self.set_mod_paths()