Normalize paths before adding/removing from $MODULEPATH

Author: Flamefire

easybuild/tools/filetools.py

@@ -508,6 +508,24 @@ def det_common_path_prefix(paths):
         return None
 
 
+def normalize_path(path):
+    """Normalize path removing empty and dot components.
+
+    Similar to os.path.normpath but does not resolve '..' which may return a wrong path when symlinks are used
+    """
+    # In POSIX 3 or more leading slashes are equivalent to 1
+    if path.startswith(os.path.sep):
+        if path.startswith(os.path.sep * 2) and not path.startswith(os.path.sep * 3):
+            start_slashes = os.path.sep * 2
+        else:
+            start_slashes = os.path.sep
+    else:
+        start_slashes = ''
+
+    filtered_comps = (comp for comp in path.split(os.path.sep) if comp and comp != '.')
+    return start_slashes + os.path.sep.join(filtered_comps)
+
+
 def is_alt_pypi_url(url):
     """Determine whether specified URL is already an alternate PyPI URL, i.e. whether it contains a hash."""
     # example: .../packages/5b/03/e135b19fadeb9b1ccb45eac9f60ca2dc3afe72d099f6bd84e03cb131f9bf/easybuild-2.7.0.tar.gz

easybuild/tools/modules.py

@@ -46,7 +46,7 @@
 from easybuild.tools.config import EBROOT_ENV_VAR_ACTIONS, LOADED_MODULES_ACTIONS
 from easybuild.tools.config import build_option, get_modules_tool, install_path
 from easybuild.tools.environment import ORIG_OS_ENVIRON, restore_env, setvar, unset_env_vars
-from easybuild.tools.filetools import convert_name, mkdir, path_matches, read_file, which, write_file
+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.run import run_cmd
@@ -381,8 +381,8 @@ def add_module_path(self, path, set_mod_paths=True):
         :param path: path to add to $MODULEPATH via 'use'
         :param set_mod_paths: (re)set self.mod_paths
         """
-        path = path.rstrip(os.path.sep)
-        if path not in curr_module_paths():
+        path = normalize_path(path)
+        if path not in curr_module_paths(normalize=True):
             # add module path via 'module use' and make sure self.mod_paths is synced
             self.use(path)
             if set_mod_paths:
@@ -396,8 +396,8 @@ def remove_module_path(self, path, set_mod_paths=True):
         :param set_mod_paths: (re)set self.mod_paths
         """
         # remove module path via 'module unuse' and make sure self.mod_paths is synced
-        path = path.rstrip(os.path.sep)
-        if path in curr_module_paths():
+        path = normalize_path(path)
+        if path in curr_module_paths(normalize=True):
             self.unuse(path)
 
             if set_mod_paths:
@@ -1294,8 +1294,8 @@ def remove_module_path(self, path, set_mod_paths=True):
         # remove module path via 'module use' and make sure self.mod_paths is synced
         # modulecmd.tcl 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 = path.rstrip(os.path.sep)
-        while path in curr_module_paths():
+        path = normalize_path(path)
+        while path in curr_module_paths(normalize=True):
             self.unuse(path)
         if set_mod_paths:
             self.set_mod_paths()
@@ -1446,7 +1446,7 @@ def use(self, path, priority=None):
             if os.environ.get('__LMOD_Priority_MODULEPATH'):
                 self.run_module(['use', path])
             else:
-                path = path.rstrip(os.path.sep)
+                path = normalize_path(path)
                 cur_mod_path = os.environ.get('MODULEPATH')
                 if cur_mod_path is None:
                     new_mod_path = path
@@ -1467,7 +1467,7 @@ def unuse(self, path):
                 self.log.debug('Changing MODULEPATH from %s to <unset>' % cur_mod_path)
                 del os.environ['MODULEPATH']
             else:
-                path = path.rstrip(os.path.sep)
+                path = normalize_path(path)
                 new_mod_path = ':'.join(p for p in cur_mod_path.split(':') if p != path)
                 if new_mod_path != cur_mod_path:
                     self.log.debug('Changing MODULEPATH from %s to %s' % (cur_mod_path, new_mod_path))
@@ -1618,12 +1618,17 @@ def get_software_version(name):
     return version
 
 
-def curr_module_paths():
+def curr_module_paths(normalize=False):
     """
     Return a list of current module paths.
+
+    :param normalize: Normalize the paths
     """
     # avoid empty or nonexistent paths, which don't make any sense
-    return [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(':') if p and os.path.exists(p))
+    if normalize:
+        module_paths = (normalize_path(p) for p in module_paths)
+    return list(module_paths)
 
 
 def mk_module_path(paths):

test/framework/filetools.py

@@ -351,6 +351,21 @@ def test_common_path_prefix(self):
         self.assertEqual(ft.det_common_path_prefix(['foo']), None)
         self.assertEqual(ft.det_common_path_prefix([]), None)
 
+    def test_normalize_path(self):
+        """Test normalize_path"""
+        self.assertEqual(ft.normalize_path(''), '')
+        self.assertEqual(ft.normalize_path('/'), '/')
+        self.assertEqual(ft.normalize_path('//'), '//')
+        self.assertEqual(ft.normalize_path('///'), '/')
+        self.assertEqual(ft.normalize_path('/foo/bar/baz'), '/foo/bar/baz')
+        self.assertEqual(ft.normalize_path('/foo//bar/././baz/'), '/foo/bar/baz')
+        self.assertEqual(ft.normalize_path('foo//bar/././baz/'), 'foo/bar/baz')
+        self.assertEqual(ft.normalize_path('//foo//bar/././baz/'), '//foo/bar/baz')
+        self.assertEqual(ft.normalize_path('///foo//bar/././baz/'), '/foo/bar/baz')
+        self.assertEqual(ft.normalize_path('////foo//bar/././baz/'), '/foo/bar/baz')
+        self.assertEqual(ft.normalize_path('/././foo//bar/././baz/'), '/foo/bar/baz')
+        self.assertEqual(ft.normalize_path('//././foo//bar/././baz/'), '//foo/bar/baz')
+
     def test_download_file(self):
         """Test download_file function."""
         fn = 'toy-0.0.tar.gz'

test/framework/modules.py

@@ -1259,6 +1259,27 @@ def test_add_and_remove_module_path(self):
         self.modtool.remove_module_path(os.path.join(test_dir1, ''))
         self.assertEqual(os.environ.get('MODULEPATH', ''), '')
 
+        # And with some more trickery
+        test_dir1_relative = os.path.join(test_dir1, '..', os.path.basename(test_dir1))
+        test_dir2_dot = os.path.join(os.path.dirname(test_dir2), '.', os.path.basename(test_dir2))
+        self.modtool.add_module_path(test_dir1_relative)
+        self.assertEqual(os.environ['MODULEPATH'], test_dir1_relative)
+        self.modtool.add_module_path(test_dir1)
+        self.assertEqual(os.environ['MODULEPATH'], test_dir1 + ':' + test_dir1_relative)
+        self.modtool.remove_module_path(test_dir1)
+        self.assertEqual(os.environ['MODULEPATH'], test_dir1_relative)
+        self.modtool.add_module_path(test_dir2_dot)
+        self.assertEqual(os.environ['MODULEPATH'], test_dir2 + ':' + test_dir1_relative)
+        self.modtool.remove_module_path(test_dir2_dot)
+        self.assertEqual(os.environ['MODULEPATH'], test_dir1_relative)
+        # Force adding such a dot path which can be removed with either variant
+        os.environ['MODULEPATH'] = test_dir2_dot + ':' + test_dir1_relative
+        self.modtool.remove_module_path(test_dir2_dot)
+        self.assertEqual(os.environ['MODULEPATH'], test_dir1_relative)
+        os.environ['MODULEPATH'] = test_dir2_dot + ':' + test_dir1_relative
+        self.modtool.remove_module_path(test_dir2)
+        self.assertEqual(os.environ['MODULEPATH'], test_dir1_relative)
+
         os.environ['MODULEPATH'] = old_module_path  # Restore
 
     def test_module_use_bash(self):