Merge pull request #3099 from Micket/consistent_module_paths

Fix inconsistent module path usage that leads to repeated reloading in HMNS

Author: migueldiascosta

easybuild/tools/modules.py

@@ -342,7 +342,7 @@ def set_mod_paths(self, mod_paths=None):
         # make sure we don't have the same path twice, using nub
         if mod_paths is None:
             # no paths specified, so grab list of (existing) module paths from $MODULEPATH
-            self.mod_paths = [p for p in nub(curr_module_paths()) if os.path.exists(p)]
+            self.mod_paths = nub(curr_module_paths())
         else:
             for mod_path in nub(mod_paths):
                 self.prepend_module_path(mod_path, set_mod_paths=False)
@@ -1475,8 +1475,8 @@ def curr_module_paths():
     """
     Return a list of current module paths.
     """
-    # avoid empty entries, which don't make any sense
-    return [p for p in os.environ.get('MODULEPATH', '').split(':') if p]
+    # 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)]
 
 
 def mk_module_path(paths):

test/framework/modules.py

@@ -282,6 +282,106 @@ def test_load(self):
         self.assertEqual(os.environ.get('EBROOTGCC'), None)
         self.assertFalse(loaded_modules[-1] == 'GCC/6.4.0-2.28')
 
+    def test_curr_module_paths(self):
+        """Test for curr_module_paths function."""
+
+        # first, create a couple of (empty) directories to use as entries in $MODULEPATH
+        test1 = os.path.join(self.test_prefix, 'test1')
+        mkdir(test1)
+        test2 = os.path.join(self.test_prefix, 'test2')
+        mkdir(test2)
+        test3 = os.path.join(self.test_prefix, 'test3')
+        mkdir(test3)
+
+        os.environ['MODULEPATH'] = ''
+        self.assertEqual(curr_module_paths(), [])
+
+        os.environ['MODULEPATH'] = '%s:%s:%s' % (test1, test2, test3)
+        self.assertEqual(curr_module_paths(), [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])
+
+    def test_check_module_path(self):
+        """Test ModulesTool.check_module_path() method"""
+
+        # first, create a couple of (empty) directories to use as entries in $MODULEPATH
+        test1 = os.path.join(self.test_prefix, 'test1')
+        mkdir(test1)
+        test2 = os.path.join(self.test_prefix, 'test2')
+        mkdir(test2)
+        test3 = os.path.join(self.test_prefix, 'test3')
+        mkdir(test3)
+
+        os.environ['MODULEPATH'] = test1
+
+        modtool = modules_tool()
+
+        # directory where modules are installed based on current configuration is automatically added in front
+        mod_install_dir = os.path.join(self.test_installpath, 'modules', 'all')
+        self.assertEqual(modtool.mod_paths, [mod_install_dir, test1])
+
+        # if mod_paths is reset, it can be restored using check_module_path
+        modtool.mod_paths = None
+        modtool.check_module_path()
+        self.assertEqual(modtool.mod_paths, [mod_install_dir, test1])
+
+        # no harm done with multiple subsequent calls
+        modtool.check_module_path()
+        self.assertEqual(modtool.mod_paths, [mod_install_dir, test1])
+
+        # if $MODULEPATH is tweaked, mod_paths and $MODULEPATH can be corrected with check_module_path
+        os.environ['MODULEPATH'] = test2
+        modtool.check_module_path()
+        self.assertEqual(modtool.mod_paths, [mod_install_dir, test1, test2])
+        self.assertEqual(os.environ['MODULEPATH'], 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)
+        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])
+        self.assertEqual(os.environ['MODULEPATH'], ':'.join([mod_install_dir, test1, test3, '/does/not/exist', test2]))
+
+    def test_check_module_path_hmns(self):
+        """Test behaviour of check_module_path with HierarchicalMNS."""
+
+        # to verify that https://github.com/easybuilders/easybuild-framework/issues/3084 is fixed
+        # (see also https://github.com/easybuilders/easybuild-framework/issues/2226);
+        # this bug can be triggered by having at least one non-existing directory in $MODULEPATH,
+        # and using HierarchicalMNS
+
+        os.environ['EASYBUILD_MODULE_NAMING_SCHEME'] = 'HierarchicalMNS'
+        init_config()
+
+        top_mod_dir = os.path.join(self.test_installpath, 'modules', 'all')
+        core_mod_dir = os.path.join(top_mod_dir, 'Core')
+        mkdir(core_mod_dir, parents=True)
+
+        doesnotexist = os.path.join(self.test_prefix, 'doesnotexist')
+        self.assertFalse(os.path.exists(doesnotexist))
+
+        os.environ['MODULEPATH'] = '%s:%s' % (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))
+
+        # 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,
+        # see https://github.com/easybuilders/easybuild-framework/issues/3084
+        def broken_prepend_module_path(*args, **kwargs):
+            raise EasyBuildError("broken prepend_module_path")
+
+        modtool.prepend_module_path = broken_prepend_module_path
+
+        # if this doesn't trigger a raised error from the hacked prepend_module_path, the bug is fixed
+        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))
+
     def test_prepend_module_path(self):
         """Test prepend_module_path method."""
         test_path = tempfile.mkdtemp(prefix=self.test_prefix)