Avoid using a priority in prepend_module_path (Lmod) to avoid costly module calls

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)

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)

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
 
@@ -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):
         """
@@ -359,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)",
@@ -378,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):
@@ -390,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:
@@ -1538,12 +1548,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, 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',
+                '<unset>' if cur_mod_path is None else cur_mod_path,
+                '<unset>' 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")
@@ -1557,35 +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 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' %
-                               ('<unset>' 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 <unset>' % 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):
         """
@@ -1598,7 +1629,19 @@ 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 set_mod_paths:
                 self.set_mod_paths()
 
@@ -1728,14 +1771,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)
@@ -1745,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():

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)