minor changes w.r.t. ModuleLoadEnvironment and ModEnvVarType

easybuild/framework/easyblock.py

@@ -1712,15 +1712,15 @@ def expand_module_search_path(self, search_path, path_type=ModEnvVarType.PATH_WI
             tentative_path = '' if tentative_path == '.' else tentative_path  # use empty string instead of dot
 
             # avoid duplicate entries between symlinked library dirs
-            tentative_sep = tentative_path + os.path.sep
-            if self.install_lib_symlink == LibSymlink.LIB64_TO_LIB and tentative_sep.startswith('lib64' + os.path.sep):
+            tent_path_sep = tentative_path + os.path.sep
+            if self.install_lib_symlink == LibSymlink.LIB64_TO_LIB and tent_path_sep.startswith('lib64' + os.path.sep):
                 self.log.debug("Discarded search path to symlinked lib64 directory: %s", tentative_path)
                 continue
-            if self.install_lib_symlink == LibSymlink.LIB_TO_LIB64 and tentative_sep.startswith('lib' + os.path.sep):
+            if self.install_lib_symlink == LibSymlink.LIB_TO_LIB64 and tent_path_sep.startswith('lib' + os.path.sep):
                 self.log.debug("Discarded search path to symlinked lib directory: %s", tentative_path)
                 continue
 
-            check_dir_files = path_type in [ModEnvVarType.PATH_WITH_FILES, ModEnvVarType.PATH_WITH_TOP_FILES]
+            check_dir_files = path_type in (ModEnvVarType.PATH_WITH_FILES, ModEnvVarType.PATH_WITH_TOP_FILES)
             if os.path.isdir(abs_path) and check_dir_files:
                 # only retain paths to directories that contain at least one file
                 recursive = path_type == ModEnvVarType.PATH_WITH_FILES
@@ -1737,14 +1737,12 @@ def check_install_lib_symlink(self):
         lib_dir = os.path.join(self.installdir, 'lib')
         lib64_dir = os.path.join(self.installdir, 'lib64')
 
-        self.install_lib_symlink = LibSymlink.UNKNOWN
+        self.install_lib_symlink = LibSymlink.NEITHER
         if os.path.exists(lib_dir) and os.path.exists(lib64_dir):
             if os.path.islink(lib_dir) and os.path.samefile(lib_dir, lib64_dir):
                 self.install_lib_symlink = LibSymlink.LIB_TO_LIB64
             elif os.path.islink(lib64_dir) and os.path.samefile(lib_dir, lib64_dir):
                 self.install_lib_symlink = LibSymlink.LIB64_TO_LIB
-            else:
-                self.install_lib_symlink = LibSymlink.NEITHER
 
     def make_module_req_guess(self):
         """

easybuild/tools/modules.py

@@ -163,7 +163,7 @@ def __init__(self, contents, var_type=None, delim=os.pathsep):
         self.delim = delim
 
         if var_type is None:
-            var_type = "PATH_WITH_FILES"
+            var_type = ModEnvVarType.PATH_WITH_FILES
         self.type = var_type
 
         self.log = fancylogger.getLogger(self.__class__.__name__, fname=False)
@@ -201,10 +201,13 @@ def type(self):
     @type.setter
     def type(self, value):
         """Convert type to VarType"""
-        try:
-            self._type = ModEnvVarType[value]
-        except KeyError as err:
-            raise EasyBuildError(f"Cannot create ModuleEnvironmentVariable with type {value}") from err
+        if isinstance(value, ModEnvVarType):
+            self._type = value
+        else:
+            try:
+                self._type = ModEnvVarType[value]
+            except KeyError as err:
+                raise EasyBuildError(f"Cannot create ModuleEnvironmentVariable with type {value}") from err
 
     def append(self, item):
         """Shortcut to append to list of contents"""
@@ -241,7 +244,7 @@ def is_path(self):
 
 
 class ModuleLoadEnvironment:
-    """Environment set by modules on load"""
+    """Changes to environment variables that should be made when environment module is loaded"""
 
     def __init__(self):
         """
@@ -250,7 +253,7 @@ def __init__(self):
         """
         self.ACLOCAL_PATH = [os.path.join('share', 'aclocal')]
         self.CLASSPATH = ['*.jar']
-        self.CMAKE_LIBRARY_PATH = ['lib64']  # only needed for installations whith standalone lib64
+        self.CMAKE_LIBRARY_PATH = ['lib64']  # only needed for installations with standalone lib64
         self.CMAKE_PREFIX_PATH = ['']
         self.CPATH = SEARCH_PATH_HEADER_DIRS
         self.GI_TYPELIB_PATH = [os.path.join(x, 'girepository-*') for x in SEARCH_PATH_LIB_DIRS]
@@ -267,6 +270,8 @@ def __setattr__(self, name, value):
         - attribute names are uppercase
         - attributes are instances of ModuleEnvironmentVariable
         """
+        if name != name.upper():
+            raise EasyBuildError(f"Names of ModuleLoadEnvironment attributes must be uppercase, got '{name}'")
         try:
             (contents, kwargs) = value
         except ValueError:
@@ -277,9 +282,9 @@ def __setattr__(self, name, value):
 
         # special variables that require files in their top directories
         if name in ('LD_LIBRARY_PATH', 'PATH'):
-            kwargs.update({'var_type': 'PATH_WITH_TOP_FILES'})
+            kwargs.update({'var_type': ModEnvVarType.PATH_WITH_TOP_FILES})
 
-        return super().__setattr__(name.upper(), ModuleEnvironmentVariable(contents, **kwargs))
+        return super().__setattr__(name, ModuleEnvironmentVariable(contents, **kwargs))
 
     def __iter__(self):
         """Make the class iterable"""

test/framework/easyblock.py

@@ -3078,8 +3078,9 @@ def test_expand_module_search_path(self):
         write_file(os.path.join(eb.installdir, 'dir_full_subdirs', 'subdir1', 'file12.txt'), 'test file 1.2')
         write_file(os.path.join(eb.installdir, 'dir_full_subdirs', 'subdir2', 'file21.txt'), 'test file 2.1')
 
-        eb.check_install_lib_symlink()
         self.assertEqual(eb.install_lib_symlink, LibSymlink.UNKNOWN)
+        eb.check_install_lib_symlink()
+        self.assertEqual(eb.install_lib_symlink, LibSymlink.NEITHER)
 
         self.assertEqual(test_emsp("nonexistent", ModEnvVarType.PATH), [])
         self.assertEqual(test_emsp("nonexistent", ModEnvVarType.PATH_WITH_FILES), [])
@@ -3096,15 +3097,16 @@ def test_expand_module_search_path(self):
         self.assertEqual(test_emsp("dir_full_subdirs", ModEnvVarType.PATH), ["dir_full_subdirs"])
         self.assertEqual(test_emsp("dir_full_subdirs", ModEnvVarType.PATH_WITH_FILES), ["dir_full_subdirs"])
         self.assertEqual(test_emsp("dir_full_subdirs", ModEnvVarType.PATH_WITH_TOP_FILES), [])
+
         # test globs
         ref_expanded_paths = ["dir_empty_subdir/empty_subdir"]
         self.assertEqual(test_emsp("dir_empty_subdir/*", ModEnvVarType.PATH), ref_expanded_paths)
         self.assertEqual(test_emsp("dir_empty_subdir/*", ModEnvVarType.PATH_WITH_FILES), [])
         self.assertEqual(test_emsp("dir_empty_subdir/*", ModEnvVarType.PATH_WITH_TOP_FILES), [])
         ref_expanded_paths = ["dir_full_subdirs/subdir1", "dir_full_subdirs/subdir2"]
-        self.assertEqual(test_emsp("dir_full_subdirs/*", ModEnvVarType.PATH), ref_expanded_paths)
-        self.assertEqual(test_emsp("dir_full_subdirs/*", ModEnvVarType.PATH_WITH_FILES), ref_expanded_paths)
-        self.assertEqual(test_emsp("dir_full_subdirs/*", ModEnvVarType.PATH_WITH_TOP_FILES), ref_expanded_paths)
+        self.assertEqual(sorted(test_emsp("dir_full_subdirs/*", ModEnvVarType.PATH)), ref_expanded_paths)
+        self.assertEqual(sorted(test_emsp("dir_full_subdirs/*", ModEnvVarType.PATH_WITH_FILES)), ref_expanded_paths)
+        self.assertEqual(sorted(test_emsp("dir_full_subdirs/*", ModEnvVarType.PATH_WITH_TOP_FILES)), ref_expanded_paths)
         ref_expanded_paths = ["dir_full_subdirs/subdir2/file21.txt"]
         self.assertEqual(test_emsp("dir_full_subdirs/subdir2/*", ModEnvVarType.PATH), ref_expanded_paths)
         self.assertEqual(test_emsp("dir_full_subdirs/subdir2/*", ModEnvVarType.PATH_WITH_FILES), ref_expanded_paths)
@@ -3113,31 +3115,34 @@ def test_expand_module_search_path(self):
         self.assertEqual(test_emsp("nonexistent/*", ModEnvVarType.PATH), [])
         self.assertEqual(test_emsp("nonexistent/*", ModEnvVarType.PATH_WITH_FILES), [])
         self.assertEqual(test_emsp("nonexistent/*", ModEnvVarType.PATH_WITH_TOP_FILES), [])
+
         # state of install_lib_symlink should not have changed
-        self.assertEqual(eb.install_lib_symlink, LibSymlink.UNKNOWN)
+        self.assertEqual(eb.install_lib_symlink, LibSymlink.NEITHER)
 
         # test just one lib directory
         os.mkdir(os.path.join(eb.installdir, "lib"))
         eb.check_install_lib_symlink()
-        self.assertEqual(eb.install_lib_symlink, LibSymlink.UNKNOWN)
+        self.assertEqual(eb.install_lib_symlink, LibSymlink.NEITHER)
         self.assertEqual(test_emsp("lib", ModEnvVarType.PATH), ["lib"])
         self.assertEqual(test_emsp("lib", ModEnvVarType.PATH_WITH_FILES), [])
         self.assertEqual(test_emsp("lib", ModEnvVarType.PATH_WITH_TOP_FILES), [])
         write_file(os.path.join(eb.installdir, "lib", "libtest.so"), "not actually a lib")
         self.assertEqual(test_emsp("lib", ModEnvVarType.PATH), ["lib"])
         self.assertEqual(test_emsp("lib", ModEnvVarType.PATH_WITH_FILES), ["lib"])
         self.assertEqual(test_emsp("lib", ModEnvVarType.PATH_WITH_TOP_FILES), ["lib"])
+
         # test both lib and lib64 directories
         os.mkdir(os.path.join(eb.installdir, "lib64"))
         eb.check_install_lib_symlink()
         self.assertEqual(eb.install_lib_symlink, LibSymlink.NEITHER)
-        self.assertEqual(test_emsp("lib*", ModEnvVarType.PATH), ["lib", "lib64"])
+        self.assertEqual(sorted(test_emsp("lib*", ModEnvVarType.PATH)), ["lib", "lib64"])
         self.assertEqual(test_emsp("lib*", ModEnvVarType.PATH_WITH_FILES), ["lib"])
         self.assertEqual(test_emsp("lib*", ModEnvVarType.PATH_WITH_TOP_FILES), ["lib"])
         write_file(os.path.join(eb.installdir, "lib64", "libtest.so"), "not actually a lib")
-        self.assertEqual(test_emsp("lib*", ModEnvVarType.PATH), ["lib", "lib64"])
-        self.assertEqual(test_emsp("lib*", ModEnvVarType.PATH_WITH_FILES), ["lib", "lib64"])
-        self.assertEqual(test_emsp("lib*", ModEnvVarType.PATH_WITH_TOP_FILES), ["lib", "lib64"])
+        self.assertEqual(sorted(test_emsp("lib*", ModEnvVarType.PATH)), ["lib", "lib64"])
+        self.assertEqual(sorted(test_emsp("lib*", ModEnvVarType.PATH_WITH_FILES)), ["lib", "lib64"])
+        self.assertEqual(sorted(test_emsp("lib*", ModEnvVarType.PATH_WITH_TOP_FILES)), ["lib", "lib64"])
+
         # test lib64 symlinked to lib
         remove_dir(os.path.join(eb.installdir, "lib64"))
         os.symlink("lib", os.path.join(eb.installdir, "lib64"))
@@ -3152,6 +3157,7 @@ def test_expand_module_search_path(self):
         self.assertEqual(test_emsp("lib*", ModEnvVarType.PATH), ["lib"])
         self.assertEqual(test_emsp("lib*", ModEnvVarType.PATH_WITH_FILES), ["lib"])
         self.assertEqual(test_emsp("lib*", ModEnvVarType.PATH_WITH_TOP_FILES), ["lib"])
+
         # test lib symlinked to lib64
         remove_dir(os.path.join(eb.installdir, "lib"))
         remove_file(os.path.join(eb.installdir, "lib64"))

test/framework/modules.py

@@ -1622,12 +1622,24 @@ def test_module_environment_variable(self):
         mod_envar_custom_type.type = 'PATH'
         self.assertEqual(mod_envar_custom_type.type, mod.ModEnvVarType.PATH)
         self.assertEqual(mod_envar_custom_type.is_path, True)
+
+        mod_envar_custom_type.type = mod.ModEnvVarType.PATH
+        self.assertEqual(mod_envar_custom_type.is_path, True)
+
         mod_envar_custom_type.type = 'PATH_WITH_FILES'
         self.assertEqual(mod_envar_custom_type.type, mod.ModEnvVarType.PATH_WITH_FILES)
         self.assertEqual(mod_envar_custom_type.is_path, True)
+
+        mod_envar_custom_type.type = mod.ModEnvVarType.PATH_WITH_FILES
+        self.assertEqual(mod_envar_custom_type.is_path, True)
+
         mod_envar_custom_type.type = 'PATH_WITH_TOP_FILES'
         self.assertEqual(mod_envar_custom_type.type, mod.ModEnvVarType.PATH_WITH_TOP_FILES)
         self.assertEqual(mod_envar_custom_type.is_path, True)
+
+        mod_envar_custom_type.type = mod.ModEnvVarType.PATH_WITH_TOP_FILES
+        self.assertEqual(mod_envar_custom_type.is_path, True)
+
         self.assertRaises(EasyBuildError, setattr, mod_envar_custom_type, 'type', 'NONEXISTENT')
         self.assertRaises(EasyBuildError, mod.ModuleEnvironmentVariable, test_paths, 'NONEXISTENT')
 
@@ -1670,34 +1682,43 @@ def test_module_environment_variable(self):
 
     def test_module_load_environment(self):
         """Test for ModuleLoadEnvironment object"""
+        mod_load_env = mod.ModuleLoadEnvironment()
+
         # test setting attributes
         test_contents = ['lib', 'lib64']
-        mod_load_env = mod.ModuleLoadEnvironment()
         mod_load_env.TEST_VAR = test_contents
         self.assertTrue(hasattr(mod_load_env, 'TEST_VAR'))
         self.assertEqual(mod_load_env.TEST_VAR.contents, test_contents)
-        mod_load_env.test_lower = test_contents
-        self.assertTrue(hasattr(mod_load_env, 'TEST_LOWER'))
-        self.assertEqual(mod_load_env.TEST_LOWER.contents, test_contents)
+
+        error_pattern = "Names of ModuleLoadEnvironment attributes must be uppercase, got 'test_lower'"
+        self.assertErrorRegex(EasyBuildError, error_pattern, setattr, mod_load_env, 'test_lower', test_contents)
+
         mod_load_env.TEST_STR = 'some/path'
         self.assertTrue(hasattr(mod_load_env, 'TEST_STR'))
         self.assertEqual(mod_load_env.TEST_STR.contents, ['some/path'])
+
         mod_load_env.TEST_VARTYPE = (test_contents, {'var_type': "STRING"})
         self.assertTrue(hasattr(mod_load_env, 'TEST_VARTYPE'))
         self.assertEqual(mod_load_env.TEST_VARTYPE.contents, test_contents)
         self.assertEqual(mod_load_env.TEST_VARTYPE.type, mod.ModEnvVarType.STRING)
+
         mod_load_env.TEST_VARTYPE.type = "PATH"
         self.assertEqual(mod_load_env.TEST_VARTYPE.type, mod.ModEnvVarType.PATH)
         self.assertRaises(TypeError, setattr, mod_load_env, 'TEST_UNKNONW', (test_contents, {'unkown_param': True}))
+
         # test retrieving environment
         ref_load_env = mod_load_env.__dict__.copy()
         self.assertCountEqual(list(mod_load_env), ref_load_env.keys())
+
         ref_load_env_item_list = list(ref_load_env.items())
         self.assertCountEqual(list(mod_load_env.items()), ref_load_env_item_list)
+
         ref_load_env_item_list = dict(ref_load_env.items())
         self.assertCountEqual(mod_load_env.as_dict, ref_load_env_item_list)
+
         ref_load_env_environ = {key: str(value) for key, value in ref_load_env.items()}
         self.assertDictEqual(mod_load_env.environ, ref_load_env_environ)
+
         # test updating environment
         new_test_env = {
             'TEST_VARTYPE': 'replaced_path',
@@ -1710,11 +1731,10 @@ def test_module_load_environment(self):
         self.assertTrue(hasattr(mod_load_env, 'TEST_NEW_VAR'))
         self.assertEqual(mod_load_env.TEST_NEW_VAR.contents, ['new_path1', 'new_path2'])
         self.assertEqual(mod_load_env.TEST_NEW_VAR.type, mod.ModEnvVarType.PATH_WITH_FILES)
+
         # check that previous variables still exist
         self.assertTrue(hasattr(mod_load_env, 'TEST_VAR'))
         self.assertEqual(mod_load_env.TEST_VAR.contents, test_contents)
-        self.assertTrue(hasattr(mod_load_env, 'TEST_LOWER'))
-        self.assertEqual(mod_load_env.TEST_LOWER.contents, test_contents)
         self.assertTrue(hasattr(mod_load_env, 'TEST_STR'))
         self.assertEqual(mod_load_env.TEST_STR.contents, ['some/path'])