diff --git a/easybuild/toolchains/mpi/openmpi.py b/easybuild/toolchains/mpi/openmpi.py index 436944fb71..57df781499 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/easybuild/tools/modules.py b/easybuild/tools/modules.py index 78825bde6c..8df5488ba0 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -790,12 +790,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)", @@ -809,8 +811,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): @@ -821,7 +829,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: @@ -2024,12 +2032,42 @@ def update(self): mkdir(cache_dir, parents=True) write_file(cache_fp, stdout) - def use(self, path, priority=None): + 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, str) # 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 _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, 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") @@ -2043,35 +2081,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 os.environ.get('__LMOD_Priority_MODULEPATH'): + if force_module_command or self._has_module_paths_with_priority(): 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 = ':'.join(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 = ':'.join(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 + 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): """ @@ -2081,10 +2110,23 @@ def prepend_module_path(self, path, set_mod_paths=True, priority=None): :param set_mod_paths: (re)set self.mod_paths :param priority: priority for this path in $MODULEPATH (Lmod-specific) """ - # Lmod pushes a path to the front on 'module use', no need for (costly) 'module unuse' - modulepath = curr_module_paths() + # If the path is already first in MODULEPATH, do nothing if we don't want to add it with a priority + modulepath = None if priority is not None else curr_module_paths() if not modulepath or os.path.realpath(modulepath[0]) != os.path.realpath(path): self.use(path, priority=priority) + if (priority is None) == (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: + prio_msg = "This can happen if paths were added via `module use` with a priority" + if priority is not None: + prio_msg += f" higher than {priority}." + else: + prio_msg += "." + print_warning("Path '%s' could not be prepended to $MODULEPATH. " + "The following paths are still in front of it: %s\n" + "%s", + path, "; ".join(modulepath[:path_idx]), prio_msg, log=self.log) if set_mod_paths: self.set_mod_paths() @@ -2227,14 +2269,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(os.pathsep) if p and os.path.exists(p)) + else: + modulepath = os.environ.get('MODULEPATH') + 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) @@ -2244,7 +2291,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/easyconfig.py b/test/framework/easyconfig.py index 7a07eab449..31dce0e3e6 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -2781,7 +2781,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) @@ -2844,7 +2844,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) @@ -2920,7 +2920,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 bfd21778ba..a7509030f3 100644 --- a/test/framework/modules.py +++ b/test/framework/modules.py @@ -525,15 +525,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) + 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, '']) def test_check_module_path(self): """Test ModulesTool.check_module_path() method""" @@ -570,7 +578,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]) @@ -595,11 +603,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, @@ -613,11 +621,11 @@ 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.""" - 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)) @@ -640,17 +648,35 @@ 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_0, modulepath[0]) + self.assertEqual(test_path_1, 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_bis, modulepath[0]) - self.assertEqual(test_path_tris, modulepath[1]) + self.assertEqual(test_path_0, modulepath[0]) + self.assertEqual(test_path_2, modulepath[1]) + self.assertEqual(test_path_1, 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) + with self.mocked_stdout_stderr(): + self.modtool.prepend_module_path(test_path_2, priority=1000) + stderr = self.get_stderr() + self.assertIn('could not be prepended', 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.""" @@ -774,7 +800,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 @@ -1137,10 +1167,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') @@ -1219,11 +1250,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', '') @@ -1254,7 +1285,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') @@ -1262,10 +1293,36 @@ 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') + 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([]) + 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_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 []) + 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) - 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']) @@ -1287,9 +1344,25 @@ 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 + # 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") @@ -1579,7 +1652,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')]) self.allow_deprecated_behaviour() with self.mocked_stdout_stderr(): diff --git a/test/framework/toolchain.py b/test/framework/toolchain.py index 11f5b5b0e3..006413631c 100644 --- a/test/framework/toolchain.py +++ b/test/framework/toolchain.py @@ -3284,7 +3284,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