From 734cd9915926ab7a0ace0ef736aa09fd26046aea Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Mon, 23 Sep 2024 16:43:43 +0200 Subject: [PATCH 01/16] refactor generation of required environment variables in module files --- easybuild/framework/easyblock.py | 146 ++++++++++++++----------------- easybuild/tools/config.py | 5 +- 2 files changed, 72 insertions(+), 79 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 88da255d1d..d2977cede4 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -74,6 +74,7 @@ from easybuild.tools.build_log import print_error, print_msg, print_warning from easybuild.tools.config import CHECKSUM_PRIORITY_JSON, DEFAULT_ENVVAR_USERS_MODULES from easybuild.tools.config import FORCE_DOWNLOAD_ALL, FORCE_DOWNLOAD_PATCHES, FORCE_DOWNLOAD_SOURCES +from easybuild.tools.config import SEARCH_PATH_BIN_DIRS, SEARCH_PATH_HEADER_DIRS, SEARCH_PATH_LIB_DIRS from easybuild.tools.config import EASYBUILD_SOURCES_URL # noqa from easybuild.tools.config import build_option, build_path, get_log_filename, get_repository, get_repositorypath from easybuild.tools.config import install_path, log_path, package_path, source_paths @@ -107,10 +108,13 @@ from easybuild.tools.utilities import remove_unwanted_chars, time2str, trace_msg from easybuild.tools.version import this_is_easybuild, VERBOSE_VERSION, VERSION -DEFAULT_BIN_LIB_SUBDIRS = ('bin', 'lib', 'lib64') +DEFAULT_BIN_LIB_SUBDIRS = SEARCH_PATH_BIN_DIRS + SEARCH_PATH_LIB_DIRS MODULE_ONLY_STEPS = [MODULE_STEP, PREPARE_STEP, READY_STEP, POSTITER_STEP, SANITYCHECK_STEP] +# search paths that require some file in their top directory +NON_RECURSIVE_SEARCH_PATHS = ["PATH", "LD_LIBRARY_PATH"] + # string part of URL for Python packages on PyPI that indicates needs to be rewritten (see derive_alt_pypi_url) PYPI_PKG_URL_PATTERN = 'pypi.python.org/packages/source/' @@ -1557,107 +1561,93 @@ def make_module_group_check(self): def make_module_req(self): """ - Generate the environment-variables to run the module. + Generate the environment-variables required to run the module. """ - requirements = self.make_module_req_guess() - - lines = ['\n'] - if os.path.isdir(self.installdir): - old_dir = change_dir(self.installdir) - else: - old_dir = None + mod_lines = ['\n'] if self.dry_run: self.dry_run_msg("List of paths that would be searched and added to module file:\n") note = "note: glob patterns are not expanded and existence checks " note += "for paths are skipped for the statements below due to dry run" - lines.append(self.module_generator.comment(note)) - - # For these environment variables, the corresponding directory must include at least one file. - # The values determine if detection is done recursively, i.e. if it accepts directories where files - # are only in subdirectories. - keys_requiring_files = { - 'PATH': False, - 'LD_LIBRARY_PATH': False, - 'LIBRARY_PATH': True, - 'CPATH': True, - 'CMAKE_PREFIX_PATH': True, - 'CMAKE_LIBRARY_PATH': True, - } + mod_lines.append(self.module_generator.comment(note)) + + env_var_requirements = self.make_module_req_guess() + for env_var, search_paths in sorted(env_var_requirements.items()): + if isinstance(search_paths, str): + self.log.warning("Hoisting string value %s into a list before iterating over it", search_paths) + search_paths = [search_paths] - for key, reqs in sorted(requirements.items()): - if isinstance(reqs, str): - self.log.warning("Hoisting string value %s into a list before iterating over it", reqs) - reqs = [reqs] + mod_env_paths = [] + recursive = env_var not in NON_RECURSIVE_SEARCH_PATHS if self.dry_run: - self.dry_run_msg(" $%s: %s" % (key, ', '.join(reqs))) - # Don't expand globs or do any filtering below for dry run - paths = reqs + self.dry_run_msg(f" ${env_var}:{', '.join(search_paths)}") + # Don't expand globs or do any filtering for dry run + mod_env_paths = search_paths else: - # Expand globs but only if the string is non-empty - # empty string is a valid value here (i.e. to prepend the installation prefix, cfr $CUDA_HOME) - paths = sum((glob.glob(path) if path else [path] for path in reqs), []) # sum flattens to list - - # If lib64 is just a symlink to lib we fixup the paths to avoid duplicates - lib64_is_symlink = (all(os.path.isdir(path) for path in ['lib', 'lib64']) and - os.path.samefile('lib', 'lib64')) - if lib64_is_symlink: - fixed_paths = [] - for path in paths: - if (path + os.path.sep).startswith('lib64' + os.path.sep): - # We only need CMAKE_LIBRARY_PATH if there is a separate lib64 path, so skip symlink - if key == 'CMAKE_LIBRARY_PATH': - continue - path = path.replace('lib64', 'lib', 1) - fixed_paths.append(path) - if fixed_paths != paths: - self.log.info("Fixed symlink lib64 in paths for %s: %s -> %s", key, paths, fixed_paths) - paths = fixed_paths - # remove duplicate paths preserving order - paths = nub(paths) - if key in keys_requiring_files: - # only retain paths that contain at least one file - recursive = keys_requiring_files[key] - retained_paths = [] - for pth in paths: - fullpath = os.path.join(self.installdir, pth) - if os.path.isdir(fullpath) and dir_contains_files(fullpath, recursive=recursive): - retained_paths.append(pth) - if retained_paths != paths: - self.log.info("Only retaining paths for %s that contain at least one file: %s -> %s", - key, paths, retained_paths) - paths = retained_paths - - if paths: - lines.append(self.module_generator.prepend_paths(key, paths)) + for sp in search_paths: + mod_env_paths.extend(self._expand_module_search_path(sp, recursive)) + + if mod_env_paths: + mod_env_paths = nub(mod_env_paths) # remove duplicates + mod_lines.append(self.module_generator.prepend_paths(env_var, mod_env_paths)) + if self.dry_run: self.dry_run_msg('') - if old_dir is not None: - change_dir(old_dir) - - return ''.join(lines) + return "".join(mod_lines) def make_module_req_guess(self): """ - A dictionary of possible directories to look for. + A dictionary of common search path variables to be loaded by environment modules + Each key contains the list of known directories related to the search path """ - lib_paths = ['lib', 'lib32', 'lib64'] return { - 'PATH': ['bin', 'sbin'], - 'LD_LIBRARY_PATH': lib_paths, - 'LIBRARY_PATH': lib_paths, - 'CPATH': ['include'], + 'PATH': SEARCH_PATH_BIN_DIRS + ['sbin'], + 'LD_LIBRARY_PATH': SEARCH_PATH_LIB_DIRS, + 'LIBRARY_PATH': SEARCH_PATH_LIB_DIRS, + 'CPATH': SEARCH_PATH_HEADER_DIRS, 'MANPATH': ['man', os.path.join('share', 'man')], - 'PKG_CONFIG_PATH': [os.path.join(x, 'pkgconfig') for x in lib_paths + ['share']], + 'PKG_CONFIG_PATH': [os.path.join(x, 'pkgconfig') for x in SEARCH_PATH_LIB_DIRS + ['share']], 'ACLOCAL_PATH': [os.path.join('share', 'aclocal')], 'CLASSPATH': ['*.jar'], 'XDG_DATA_DIRS': ['share'], - 'GI_TYPELIB_PATH': [os.path.join(x, 'girepository-*') for x in lib_paths], + 'GI_TYPELIB_PATH': [os.path.join(x, 'girepository-*') for x in SEARCH_PATH_LIB_DIRS], 'CMAKE_PREFIX_PATH': [''], - 'CMAKE_LIBRARY_PATH': ['lib64'], # lib and lib32 are searched through the above + 'CMAKE_LIBRARY_PATH': ['lib64'], # only needed for installations whith standalone lib64 } + def _expand_module_search_path(self, search_path, recursive): + """ + Expand given path glob and return list of paths that are suitable to be + used as search paths in environment module + """ + # Expand globs but only if the string is non-empty + # empty string is a valid value here (i.e. to prepend the installation prefix root directory) + abs_search_path = os.path.join(self.installdir, search_path) + exp_search_paths = [abs_search_path] if search_path == "" else glob.glob(abs_search_path) + + retained_search_paths = [] + for abs_path in exp_search_paths: + tentative_path = os.path.relpath(abs_path, start=self.installdir) + tentative_path = "" if tentative_path == "." else tentative_path # use empty string instead of dot + + # avoid duplicate entries if lib64 is just a symlink to lib + if (tentative_path + os.path.sep).startswith("lib64" + os.path.sep): + abs_lib_path = os.path.join(self.installdir, "lib") + abs_lib64_path = os.path.join(self.installdir, "lib64") + if os.path.islink(abs_lib64_path) and os.path.samefile(abs_lib_path, abs_lib64_path): + self.log.debug("Discarded search path to symlink lib64: %s", tentative_path) + break + + # only retain paths to directories that contain at least one file + if os.path.isdir(abs_path) and not dir_contains_files(abs_path, recursive=recursive): + self.log.debug("Discarded search path to empty directory: %s", tentative_path) + break + + retained_search_paths.append(tentative_path) + + return retained_search_paths + def load_module(self, mod_paths=None, purge=True, extra_modules=None, verbose=True): """ Load module for this software package/version, after purging all currently loaded modules. diff --git a/easybuild/tools/config.py b/easybuild/tools/config.py index 13f9722bce..c9a914d478 100644 --- a/easybuild/tools/config.py +++ b/easybuild/tools/config.py @@ -168,13 +168,16 @@ LOCAL_VAR_NAMING_CHECK_WARN = WARN LOCAL_VAR_NAMING_CHECKS = [LOCAL_VAR_NAMING_CHECK_ERROR, LOCAL_VAR_NAMING_CHECK_LOG, LOCAL_VAR_NAMING_CHECK_WARN] - OUTPUT_STYLE_AUTO = 'auto' OUTPUT_STYLE_BASIC = 'basic' OUTPUT_STYLE_NO_COLOR = 'no_color' OUTPUT_STYLE_RICH = 'rich' OUTPUT_STYLES = (OUTPUT_STYLE_AUTO, OUTPUT_STYLE_BASIC, OUTPUT_STYLE_NO_COLOR, OUTPUT_STYLE_RICH) +SEARCH_PATH_BIN_DIRS = ["bin"] +SEARCH_PATH_HEADER_DIRS = ["include"] +SEARCH_PATH_LIB_DIRS = ["lib", "lib64"] + class Singleton(ABCMeta): """Serves as metaclass for classes that should implement the Singleton pattern. From df505cf7da212f6d81dd3ca1933789c5fe64e6bc Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Mon, 23 Sep 2024 16:45:08 +0200 Subject: [PATCH 02/16] update test_make_module_req to add a file into MANPATH of test installation --- test/framework/easyblock.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/framework/easyblock.py b/test/framework/easyblock.py index 505a1c1d2f..bdf15504f4 100644 --- a/test/framework/easyblock.py +++ b/test/framework/easyblock.py @@ -433,12 +433,13 @@ def test_make_module_req(self): # create fake directories and files that should be guessed os.makedirs(eb.installdir) - write_file(os.path.join(eb.installdir, 'foo.jar'), 'foo.jar') - write_file(os.path.join(eb.installdir, 'bla.jar'), 'bla.jar') for path in ('bin', ('bin', 'testdir'), 'sbin', 'share', ('share', 'man'), 'lib', 'lib64'): if isinstance(path, str): path = (path, ) os.mkdir(os.path.join(eb.installdir, *path)) + write_file(os.path.join(eb.installdir, 'foo.jar'), 'foo.jar') + write_file(os.path.join(eb.installdir, 'bla.jar'), 'bla.jar') + write_file(os.path.join(eb.installdir, 'share', 'man', 'pi'), 'Man page') # this is not a path that should be picked up os.mkdir(os.path.join(eb.installdir, 'CPATH')) From 56800386935a56b23bb024c104d4c209a0a4d31f Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Tue, 24 Sep 2024 01:58:59 +0200 Subject: [PATCH 03/16] disable non-empty check on search path drectories for fake module files --- easybuild/framework/easyblock.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index d2977cede4..559c6630df 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1559,9 +1559,10 @@ def make_module_group_check(self): return txt - def make_module_req(self): + def make_module_req(self, fake=False): """ Generate the environment-variables required to run the module. + Fake modules can set search paths to empty directories. """ mod_lines = ['\n'] @@ -1585,7 +1586,7 @@ def make_module_req(self): mod_env_paths = search_paths else: for sp in search_paths: - mod_env_paths.extend(self._expand_module_search_path(sp, recursive)) + mod_env_paths.extend(self._expand_module_search_path(sp, recursive, fake=fake)) if mod_env_paths: mod_env_paths = nub(mod_env_paths) # remove duplicates @@ -1616,10 +1617,12 @@ def make_module_req_guess(self): 'CMAKE_LIBRARY_PATH': ['lib64'], # only needed for installations whith standalone lib64 } - def _expand_module_search_path(self, search_path, recursive): + def _expand_module_search_path(self, search_path, recursive, fake=False): """ - Expand given path glob and return list of paths that are suitable to be - used as search paths in environment module + Expand given path glob and return list of suitable paths to be used as search paths: + - Files must exist and directories be non-empty + - Fake modules can set search paths to empty directories + - Search paths to 'lib64' symlinked to 'lib' are discarded """ # Expand globs but only if the string is non-empty # empty string is a valid value here (i.e. to prepend the installation prefix root directory) @@ -1628,6 +1631,7 @@ def _expand_module_search_path(self, search_path, recursive): retained_search_paths = [] for abs_path in exp_search_paths: + # return relative paths tentative_path = os.path.relpath(abs_path, start=self.installdir) tentative_path = "" if tentative_path == "." else tentative_path # use empty string instead of dot @@ -1640,7 +1644,7 @@ def _expand_module_search_path(self, search_path, recursive): break # only retain paths to directories that contain at least one file - if os.path.isdir(abs_path) and not dir_contains_files(abs_path, recursive=recursive): + if os.path.isdir(abs_path) and not dir_contains_files(abs_path, recursive=recursive) and not fake: self.log.debug("Discarded search path to empty directory: %s", tentative_path) break @@ -3785,7 +3789,7 @@ def make_module_step(self, fake=False): txt += self.make_module_deppaths() txt += self.make_module_dep() txt += self.make_module_extend_modpath() - txt += self.make_module_req() + txt += self.make_module_req(fake=fake) txt += self.make_module_extra() txt += self.make_module_footer() From 5b17f2e8e85b0abd822c70c792db253074dc7e70 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Tue, 24 Sep 2024 23:17:18 +0200 Subject: [PATCH 04/16] change position of easyblock methods make_module_req_guess and _expand_module_search_path --- easybuild/framework/easyblock.py | 40 ++++++++++++++++---------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 559c6630df..660af36ccf 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1597,26 +1597,6 @@ def make_module_req(self, fake=False): return "".join(mod_lines) - def make_module_req_guess(self): - """ - A dictionary of common search path variables to be loaded by environment modules - Each key contains the list of known directories related to the search path - """ - return { - 'PATH': SEARCH_PATH_BIN_DIRS + ['sbin'], - 'LD_LIBRARY_PATH': SEARCH_PATH_LIB_DIRS, - 'LIBRARY_PATH': SEARCH_PATH_LIB_DIRS, - 'CPATH': SEARCH_PATH_HEADER_DIRS, - 'MANPATH': ['man', os.path.join('share', 'man')], - 'PKG_CONFIG_PATH': [os.path.join(x, 'pkgconfig') for x in SEARCH_PATH_LIB_DIRS + ['share']], - 'ACLOCAL_PATH': [os.path.join('share', 'aclocal')], - 'CLASSPATH': ['*.jar'], - 'XDG_DATA_DIRS': ['share'], - 'GI_TYPELIB_PATH': [os.path.join(x, 'girepository-*') for x in SEARCH_PATH_LIB_DIRS], - 'CMAKE_PREFIX_PATH': [''], - 'CMAKE_LIBRARY_PATH': ['lib64'], # only needed for installations whith standalone lib64 - } - def _expand_module_search_path(self, search_path, recursive, fake=False): """ Expand given path glob and return list of suitable paths to be used as search paths: @@ -1652,6 +1632,26 @@ def _expand_module_search_path(self, search_path, recursive, fake=False): return retained_search_paths + def make_module_req_guess(self): + """ + A dictionary of common search path variables to be loaded by environment modules + Each key contains the list of known directories related to the search path + """ + return { + 'PATH': SEARCH_PATH_BIN_DIRS + ['sbin'], + 'LD_LIBRARY_PATH': SEARCH_PATH_LIB_DIRS, + 'LIBRARY_PATH': SEARCH_PATH_LIB_DIRS, + 'CPATH': SEARCH_PATH_HEADER_DIRS, + 'MANPATH': ['man', os.path.join('share', 'man')], + 'PKG_CONFIG_PATH': [os.path.join(x, 'pkgconfig') for x in SEARCH_PATH_LIB_DIRS + ['share']], + 'ACLOCAL_PATH': [os.path.join('share', 'aclocal')], + 'CLASSPATH': ['*.jar'], + 'XDG_DATA_DIRS': ['share'], + 'GI_TYPELIB_PATH': [os.path.join(x, 'girepository-*') for x in SEARCH_PATH_LIB_DIRS], + 'CMAKE_PREFIX_PATH': [''], + 'CMAKE_LIBRARY_PATH': ['lib64'], # only needed for installations whith standalone lib64 + } + def load_module(self, mod_paths=None, purge=True, extra_modules=None, verbose=True): """ Load module for this software package/version, after purging all currently loaded modules. From 9205c3b5731d4b678d68b5904ee62eed6703253f Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Tue, 1 Oct 2024 00:03:31 +0200 Subject: [PATCH 05/16] add new class ModuleEnvironmentVariable to hold definitions of environment variables for module files --- easybuild/tools/modules.py | 36 ++++++++++++++++++++++++++++++++++++ test/framework/modules.py | 26 ++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index a4e1f565ae..e4699941de 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -130,6 +130,42 @@ _log = fancylogger.getLogger('modules', fname=False) +class ModuleEnvironmentVariable: + """Environment variable data structure for modules""" + def __init__(self, paths, empty=False, top_level_file=False): + self.paths = paths + self.empty = bool(empty) + self.top_level_file = bool(top_level_file) + + def __str__(self): + return ":".join(self.paths) + + @property + def paths(self): + return self._paths + + @paths.setter + def paths(self, value): + """Enforce that paths is a list of strings""" + if isinstance(value, str): + value = [value] + try: + self._paths = [str(path) for path in value] + except TypeError: + raise TypeError("ModuleEnvironmentVariable.paths must be a list of strings") from None + + def append(self, *args): + """Shortcut to append to list of paths""" + self.paths.append(*args) + + def extend(self, *args): + """Shortcut to extend list of paths""" + self.paths.extend(*args) + + def prepend(self, item): + """Shortcut to append to list of paths""" + self.paths.insert(0, item) + class ModulesTool(object): """An abstract interface to a tool that deals with modules.""" diff --git a/test/framework/modules.py b/test/framework/modules.py index 5cd783694d..1d847db6b1 100644 --- a/test/framework/modules.py +++ b/test/framework/modules.py @@ -1588,6 +1588,32 @@ def test_get_setenv_value_from_modulefile(self): res = self.modtool.get_setenv_value_from_modulefile('toy/0.0', 'NO_SUCH_VARIABLE_SET') self.assertEqual(res, None) + def test_module_environment_variable(self): + """Test for ModuleEnvironmentVariable object""" + test_paths = ['lib', 'lib64'] + mod_envar = mod.ModuleEnvironmentVariable(test_paths) + self.assertTrue(hasattr(mod_envar, "paths")) + self.assertTrue(hasattr(mod_envar, "empty")) + self.assertTrue(hasattr(mod_envar, "top_level_file")) + self.assertEqual(mod_envar.paths, test_paths) + self.assertEqual(str(mod_envar), "lib:lib64") + + mod_envar.paths = [] + self.assertEqual(mod_envar.paths, []) + self.assertRaises(TypeError, setattr, mod_envar, "paths", None) + + mod_envar.paths = (1, 2, 3) + self.assertEqual(mod_envar.paths, ["1", "2", "3"]) + + mod_envar.paths = "include" + self.assertEqual(mod_envar.paths, ["include"]) + + mod_envar.append("share") + self.assertEqual(mod_envar.paths, ["include", "share"]) + mod_envar.extend(test_paths) + self.assertEqual(mod_envar.paths, ["include", "share", "lib", "lib64"]) + mod_envar.prepend("bin") + self.assertEqual(mod_envar.paths, ["bin", "include", "share", "lib", "lib64"]) def suite(): """ returns all the testcases in this module """ From 29affc984bda000780fc6f8b91d83c2d519eb1bb Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Tue, 1 Oct 2024 00:05:06 +0200 Subject: [PATCH 06/16] add new class ModuleLoadEnvironment to hold environment definition for modules on load --- easybuild/tools/modules.py | 84 +++++++++++++++++++++++++++++++++++++- test/framework/modules.py | 21 ++++++++++ 2 files changed, 104 insertions(+), 1 deletion(-) diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index e4699941de..5bef0e82bb 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -43,11 +43,13 @@ import shlex from easybuild.base import fancylogger +from easybuild.base.wrapper import create_base_metaclass from easybuild.tools import LooseVersion from easybuild.tools.build_log import EasyBuildError, EasyBuildExit, print_warning from easybuild.tools.config import ERROR, IGNORE, PURGE, UNLOAD, UNSET 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.config import Singleton, build_option, get_modules_tool, install_path +from easybuild.tools.config import SEARCH_PATH_BIN_DIRS, SEARCH_PATH_HEADER_DIRS, SEARCH_PATH_LIB_DIRS 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 @@ -167,6 +169,86 @@ def prepend(self, item): self.paths.insert(0, item) +# singleton metaclass: only one instance is created +BaseModuleEnvironment = create_base_metaclass('BaseModuleEnvironment', Singleton, object) + + +class ModuleLoadEnvironment(BaseModuleEnvironment): + """Environment set by modules on load""" + + def __init__(self): + """ + Initialize default environment definition + Paths are relative to root of installation directory + """ + + self.PATH = ModuleEnvironmentVariable( + SEARCH_PATH_BIN_DIRS + ['sbin'], + top_level_file=True, + ) + self.LD_LIBRARY_PATH = ModuleEnvironmentVariable( + SEARCH_PATH_LIB_DIRS, + top_level_file=True, + ) + self.LIBRARY_PATH = ModuleEnvironmentVariable( + SEARCH_PATH_LIB_DIRS, + ) + self.CPATH = ModuleEnvironmentVariable( + SEARCH_PATH_HEADER_DIRS, + ) + self.MANPATH = ModuleEnvironmentVariable( + ['man', os.path.join('share', 'man')], + ) + self.PKG_CONFIG_PATH = ModuleEnvironmentVariable( + [os.path.join(x, 'pkgconfig') for x in SEARCH_PATH_LIB_DIRS + ['share']], + ) + self.ACLOCAL_PATH = ModuleEnvironmentVariable( + [os.path.join('share', 'aclocal')], + ) + self.CLASSPATH = ModuleEnvironmentVariable( + ['*.jar'], + ) + self.XDG_DATA_DIRS = ModuleEnvironmentVariable( + ['share'], + ) + self.GI_TYPELIB_PATH = ModuleEnvironmentVariable( + [os.path.join(x, 'girepository-*') for x in SEARCH_PATH_LIB_DIRS] + ) + self.CMAKE_PREFIX_PATH = ModuleEnvironmentVariable( + [''], + ) + # only needed for installations whith standalone lib64 + self.CMAKE_LIBRARY_PATH = ModuleEnvironmentVariable( + ['lib64'], + ) + + def __iter__(self): + """Make the class iterable""" + yield from (envar for envar in self.__dict__ if isinstance(getattr(self, envar), ModuleEnvironmentVariable)) + + def items(self): + """ + Return key-value pairs for each attribute that is a ModuleEnvironmentVariable + - key = attribute name + - value = its "paths" attribute + """ + for attr in self.__dict__: + envar = getattr(self, attr) + if isinstance(envar, ModuleEnvironmentVariable): + yield attr, envar.paths + + @property + def environ(self): + """ + Return dict with mapping of ModuleEnvironmentVariables names with their paths + Equivalent in shape to os.environ + """ + mapping = {} + for envar_name, envar_paths in self.items(): + mapping.update({envar_name: envar_paths}) + return mapping + + class ModulesTool(object): """An abstract interface to a tool that deals with modules.""" # name of this modules tool (used in log/warning/error messages) diff --git a/test/framework/modules.py b/test/framework/modules.py index 1d847db6b1..4437360359 100644 --- a/test/framework/modules.py +++ b/test/framework/modules.py @@ -1615,6 +1615,27 @@ def test_module_environment_variable(self): mod_envar.prepend("bin") self.assertEqual(mod_envar.paths, ["bin", "include", "share", "lib", "lib64"]) + def test_module_load_environment(self): + """Test for ModuleLoadEnvironment object""" + mod_load_env = mod.ModuleLoadEnvironment() + mod_load_env.TEST_VAR = mod.ModuleEnvironmentVariable(["lib", "lib64"]) + self.assertTrue(hasattr(mod_load_env, "TEST_VAR")) + self.assertEqual(mod_load_env.TEST_VAR.paths, ["lib", "lib64"]) + + # copy current state as reference + ref_load_env = mod_load_env.__dict__.copy() + # and add some garbage + mod_load_env.garbage = "not_an_env_var" + self.assertTrue(hasattr(mod_load_env, "garbage")) + self.assertEqual(mod_load_env.garbage, "not_an_env_var") + + self.assertCountEqual(list(mod_load_env), ref_load_env.keys()) + ref_load_env_item_list = [(key, value.paths) for key, value in ref_load_env.items()] + self.assertCountEqual(list(mod_load_env.items()), ref_load_env_item_list) + ref_load_env_environ = {key: value.paths for key, value in ref_load_env.items()} + self.assertDictEqual(mod_load_env.environ, ref_load_env_environ) + + def suite(): """ returns all the testcases in this module """ return TestLoaderFiltered().loadTestsFromTestCase(ModulesTest, sys.argv[1:]) From 8d3a0511dc9dac7f6dee1d69d2e8104234142fed Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Tue, 1 Oct 2024 00:06:43 +0200 Subject: [PATCH 07/16] add LibSymlink enum to easyblock to define possible states of symlinked library directories --- easybuild/framework/easyblock.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 660af36ccf..3c13b45866 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -54,6 +54,7 @@ import traceback from concurrent.futures import ThreadPoolExecutor from datetime import datetime +from enum import Enum from textwrap import indent import easybuild.tools.environment as env @@ -124,6 +125,11 @@ _log = fancylogger.getLogger('easyblock') +class LibSymlink(Enum): + """Possible states for symlinking of library directories""" + NONE, LIB, LIB64, NEITHER = range(0, 4) + + class EasyBlock(object): """Generic support for building and installing software, base class for actual easyblocks.""" From ee37ae744b930dda932ea7a60cf11aa650b2ce27 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Tue, 1 Oct 2024 00:07:30 +0200 Subject: [PATCH 08/16] set library symlink state at the end of post_install_step --- easybuild/framework/easyblock.py | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 3c13b45866..a267cc86c1 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -214,6 +214,9 @@ def __init__(self, ec): # determine install subdirectory, based on module name self.install_subdir = None + # track status of symlink between library directories + self.install_lib_symlink = LibSymlink.NONE + # indicates whether build should be performed in installation dir self.build_in_installdir = self.cfg['buildininstalldir'] @@ -3059,18 +3062,24 @@ def post_install_step(self): # However for each in $LIBRARY_PATH (where is often /lib) it searches /../lib64 first. # So we create /lib64 as a symlink to /lib to make it prefer EB installed libraries. # See https://github.com/easybuilders/easybuild-easyconfigs/issues/5776 - if build_option('lib64_lib_symlink'): - if os.path.exists(lib_dir) and not os.path.exists(lib64_dir): - # create *relative* 'lib64' symlink to 'lib'; - # see https://github.com/easybuilders/easybuild-framework/issues/3564 - symlink('lib', lib64_dir, use_abspath_source=False) + if build_option('lib64_lib_symlink') and os.path.exists(lib_dir) and not os.path.exists(lib64_dir): + # create *relative* 'lib64' symlink to 'lib'; + # see https://github.com/easybuilders/easybuild-framework/issues/3564 + symlink('lib', lib64_dir, use_abspath_source=False) # symlink lib to lib64, which is helpful on OpenSUSE; # see https://github.com/easybuilders/easybuild-framework/issues/3549 - if build_option('lib_lib64_symlink'): - if os.path.exists(lib64_dir) and not os.path.exists(lib_dir): - # create *relative* 'lib' symlink to 'lib64'; - symlink('lib64', lib_dir, use_abspath_source=False) + if build_option('lib_lib64_symlink') and os.path.exists(lib64_dir) and not os.path.exists(lib_dir): + # create *relative* 'lib' symlink to 'lib64'; + symlink('lib64', lib_dir, use_abspath_source=False) + + # check symlink state + if os.path.exists(lib_dir) and os.path.exists(lib64_dir): + self.install_lib_symlink = LibSymlink.NEITHER + if os.path.islink(lib_dir) and os.path.samefile(lib_dir, lib64_dir): + self.install_lib_symlink = LibSymlink.LIB + elif os.path.islink(lib64_dir) and os.path.samefile(lib_dir, lib64_dir): + self.install_lib_symlink = LibSymlink.LIB64 self.run_post_install_commands() self.apply_post_install_patches() From b6e1821d38637a202a7aa214e34b98bd60dcd7b7 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Tue, 1 Oct 2024 00:10:37 +0200 Subject: [PATCH 09/16] use environment definition from ModuleLoadEnvironment in make_module_req_guess --- easybuild/framework/easyblock.py | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index a267cc86c1..afef6c2c53 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -97,7 +97,8 @@ from easybuild.tools.module_generator import ModuleGeneratorLua, ModuleGeneratorTcl, module_generator, dependencies_for from easybuild.tools.module_naming_scheme.utilities import det_full_ec_version from easybuild.tools.modules import ROOT_ENV_VAR_NAME_PREFIX, VERSION_ENV_VAR_NAME_PREFIX, DEVEL_ENV_VAR_NAME_PREFIX -from easybuild.tools.modules import Lmod, curr_module_paths, invalidate_module_caches_for, get_software_root +from easybuild.tools.modules import Lmod, ModuleLoadEnvironment +from easybuild.tools.modules import curr_module_paths, invalidate_module_caches_for, get_software_root from easybuild.tools.modules import get_software_root_env_var_name, get_software_version_env_var_name from easybuild.tools.output import PROGRESS_BAR_DOWNLOAD_ALL, PROGRESS_BAR_EASYCONFIG, PROGRESS_BAR_EXTENSIONS from easybuild.tools.output import show_progress_bars, start_progress_bar, stop_progress_bar, update_progress_bar @@ -211,6 +212,9 @@ def __init__(self, ec): if modules_header_path is not None: self.modules_header = read_file(modules_header_path) + # environment variables on module load + self.module_load_environment = ModuleLoadEnvironment() + # determine install subdirectory, based on module name self.install_subdir = None @@ -1646,20 +1650,7 @@ def make_module_req_guess(self): A dictionary of common search path variables to be loaded by environment modules Each key contains the list of known directories related to the search path """ - return { - 'PATH': SEARCH_PATH_BIN_DIRS + ['sbin'], - 'LD_LIBRARY_PATH': SEARCH_PATH_LIB_DIRS, - 'LIBRARY_PATH': SEARCH_PATH_LIB_DIRS, - 'CPATH': SEARCH_PATH_HEADER_DIRS, - 'MANPATH': ['man', os.path.join('share', 'man')], - 'PKG_CONFIG_PATH': [os.path.join(x, 'pkgconfig') for x in SEARCH_PATH_LIB_DIRS + ['share']], - 'ACLOCAL_PATH': [os.path.join('share', 'aclocal')], - 'CLASSPATH': ['*.jar'], - 'XDG_DATA_DIRS': ['share'], - 'GI_TYPELIB_PATH': [os.path.join(x, 'girepository-*') for x in SEARCH_PATH_LIB_DIRS], - 'CMAKE_PREFIX_PATH': [''], - 'CMAKE_LIBRARY_PATH': ['lib64'], # only needed for installations whith standalone lib64 - } + return self.module_load_environment.environ def load_module(self, mod_paths=None, purge=True, extra_modules=None, verbose=True): """ From 544a3165ba22f340f53c976495d767f9e075d1a7 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Tue, 1 Oct 2024 00:12:27 +0200 Subject: [PATCH 10/16] deprecate make_module_req_guess in favor of directly using ModuleLoadEnvironment --- easybuild/framework/easyblock.py | 44 +++++++++++++++++++------------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index afef6c2c53..5a27fae724 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -75,7 +75,7 @@ from easybuild.tools.build_log import print_error, print_msg, print_warning from easybuild.tools.config import CHECKSUM_PRIORITY_JSON, DEFAULT_ENVVAR_USERS_MODULES from easybuild.tools.config import FORCE_DOWNLOAD_ALL, FORCE_DOWNLOAD_PATCHES, FORCE_DOWNLOAD_SOURCES -from easybuild.tools.config import SEARCH_PATH_BIN_DIRS, SEARCH_PATH_HEADER_DIRS, SEARCH_PATH_LIB_DIRS +from easybuild.tools.config import SEARCH_PATH_BIN_DIRS, SEARCH_PATH_LIB_DIRS from easybuild.tools.config import EASYBUILD_SOURCES_URL # noqa from easybuild.tools.config import build_option, build_path, get_log_filename, get_repository, get_repositorypath from easybuild.tools.config import install_path, log_path, package_path, source_paths @@ -114,9 +114,6 @@ MODULE_ONLY_STEPS = [MODULE_STEP, PREPARE_STEP, READY_STEP, POSTITER_STEP, SANITYCHECK_STEP] -# search paths that require some file in their top directory -NON_RECURSIVE_SEARCH_PATHS = ["PATH", "LD_LIBRARY_PATH"] - # string part of URL for Python packages on PyPI that indicates needs to be rewritten (see derive_alt_pypi_url) PYPI_PKG_URL_PATTERN = 'pypi.python.org/packages/source/' @@ -1585,25 +1582,36 @@ def make_module_req(self, fake=False): note += "for paths are skipped for the statements below due to dry run" mod_lines.append(self.module_generator.comment(note)) - env_var_requirements = self.make_module_req_guess() - for env_var, search_paths in sorted(env_var_requirements.items()): - if isinstance(search_paths, str): - self.log.warning("Hoisting string value %s into a list before iterating over it", search_paths) - search_paths = [search_paths] + # prefer deprecated make_module_req_guess on custom easyblocks + if self.make_module_req_guess.__qualname__ == "EasyBlock.make_module_req_guess": + # No custom method in child Easyblock, deprecated method is defined by base EasyBlock class + env_var_requirements = self.module_load_environment.environ + else: + # Custom deprecated method used by child EasyBlock + self.log.devel( + "make_module_req_guess() is deprecated, use module_load_environment object instead.", + "6.0", + ) + env_var_requirements = self.make_module_req_guess() + # backward compatibility: manually convert paths defined as string to lists + env_var_requirements.update({ + envar: [path] for envar, path in env_var_requirements.items() if isinstance(path, str) + }) - mod_env_paths = [] - recursive = env_var not in NON_RECURSIVE_SEARCH_PATHS + for env_var, search_paths in sorted(env_var_requirements.items()): if self.dry_run: self.dry_run_msg(f" ${env_var}:{', '.join(search_paths)}") # Don't expand globs or do any filtering for dry run - mod_env_paths = search_paths + mod_req_paths = search_paths else: - for sp in search_paths: - mod_env_paths.extend(self._expand_module_search_path(sp, recursive, fake=fake)) - - if mod_env_paths: - mod_env_paths = nub(mod_env_paths) # remove duplicates - mod_lines.append(self.module_generator.prepend_paths(env_var, mod_env_paths)) + mod_req_paths = [] + top_level = getattr(self.module_load_environment, env_var).top_level_file + for path in search_paths: + mod_req_paths.extend(self._expand_module_search_path(path, top_level, fake=fake)) + + if mod_req_paths: + mod_req_paths = nub(mod_req_paths) # remove duplicates + mod_lines.append(self.module_generator.prepend_paths(env_var, mod_req_paths)) if self.dry_run: self.dry_run_msg('') From dc09eeeb7716af644e96992530f8bf7cf256fb19 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Tue, 1 Oct 2024 00:13:33 +0200 Subject: [PATCH 11/16] consider both possible symplink states between lib and lib64 in expanding paths for environment of modules --- easybuild/framework/easyblock.py | 28 +++++++++++++++------------- test/framework/easyblock.py | 5 ++++- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 5a27fae724..245f45e0e1 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1618,17 +1618,18 @@ def make_module_req(self, fake=False): return "".join(mod_lines) - def _expand_module_search_path(self, search_path, recursive, fake=False): + def _expand_module_search_path(self, search_path, top_level, fake=False): """ Expand given path glob and return list of suitable paths to be used as search paths: - - Files must exist and directories be non-empty + - Paths are relative to installation prefix root + - Paths to files must exist and directories be non-empty - Fake modules can set search paths to empty directories - - Search paths to 'lib64' symlinked to 'lib' are discarded + - Search paths to a 'lib64' symlinked to 'lib' are discarded to avoid duplicates """ # Expand globs but only if the string is non-empty # empty string is a valid value here (i.e. to prepend the installation prefix root directory) - abs_search_path = os.path.join(self.installdir, search_path) - exp_search_paths = [abs_search_path] if search_path == "" else glob.glob(abs_search_path) + abs_glob = os.path.join(self.installdir, search_path) + exp_search_paths = [abs_glob] if search_path == "" else glob.glob(abs_glob) retained_search_paths = [] for abs_path in exp_search_paths: @@ -1636,16 +1637,17 @@ def _expand_module_search_path(self, search_path, recursive, fake=False): tentative_path = os.path.relpath(abs_path, start=self.installdir) tentative_path = "" if tentative_path == "." else tentative_path # use empty string instead of dot - # avoid duplicate entries if lib64 is just a symlink to lib - if (tentative_path + os.path.sep).startswith("lib64" + os.path.sep): - abs_lib_path = os.path.join(self.installdir, "lib") - abs_lib64_path = os.path.join(self.installdir, "lib64") - if os.path.islink(abs_lib64_path) and os.path.samefile(abs_lib_path, abs_lib64_path): - self.log.debug("Discarded search path to symlink lib64: %s", tentative_path) - break + # avoid duplicate entries between symlinked library dirs + tentative_sep = tentative_path + os.path.sep + if self.install_lib_symlink == LibSymlink.LIB64 and tentative_sep.startswith("lib64" + os.path.sep): + self.log.debug("Discarded search path to symlinked lib64 directory: %s", tentative_path) + break + if self.install_lib_symlink == LibSymlink.LIB and tentative_sep.startswith("lib" + os.path.sep): + self.log.debug("Discarded search path to symlinked lib directory: %s", tentative_path) + break # only retain paths to directories that contain at least one file - if os.path.isdir(abs_path) and not dir_contains_files(abs_path, recursive=recursive) and not fake: + if os.path.isdir(abs_path) and not dir_contains_files(abs_path, recursive=not top_level) and not fake: self.log.debug("Discarded search path to empty directory: %s", tentative_path) break diff --git a/test/framework/easyblock.py b/test/framework/easyblock.py index bdf15504f4..5c8091be01 100644 --- a/test/framework/easyblock.py +++ b/test/framework/easyblock.py @@ -40,7 +40,7 @@ import easybuild.tools.systemtools as st from easybuild.base import fancylogger -from easybuild.framework.easyblock import EasyBlock, get_easyblock_instance +from easybuild.framework.easyblock import LibSymlink, EasyBlock, get_easyblock_instance from easybuild.framework.easyconfig import CUSTOM from easybuild.framework.easyconfig.easyconfig import EasyConfig from easybuild.framework.easyconfig.tools import avail_easyblocks, process_easyconfig @@ -437,6 +437,8 @@ def test_make_module_req(self): if isinstance(path, str): path = (path, ) os.mkdir(os.path.join(eb.installdir, *path)) + eb.install_lib_symlink = LibSymlink.NEITHER + write_file(os.path.join(eb.installdir, 'foo.jar'), 'foo.jar') write_file(os.path.join(eb.installdir, 'bla.jar'), 'bla.jar') write_file(os.path.join(eb.installdir, 'share', 'man', 'pi'), 'Man page') @@ -501,6 +503,7 @@ def test_make_module_req(self): write_file(os.path.join(eb.installdir, 'lib', 'libfoo.so'), 'test') shutil.rmtree(os.path.join(eb.installdir, 'lib64')) os.symlink('lib', os.path.join(eb.installdir, 'lib64')) + eb.install_lib_symlink = LibSymlink.LIB64 with eb.module_generator.start_module_creation(): guess = eb.make_module_req() if get_module_syntax() == 'Tcl': From 562c98d08918bff0722c938b0127224a2f1e924a Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Tue, 1 Oct 2024 00:17:23 +0200 Subject: [PATCH 12/16] fix code style around ModuleEnvironmentVariable --- easybuild/tools/modules.py | 1 + 1 file changed, 1 insertion(+) diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index 5bef0e82bb..4efc78f696 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -132,6 +132,7 @@ _log = fancylogger.getLogger('modules', fname=False) + class ModuleEnvironmentVariable: """Environment variable data structure for modules""" def __init__(self, paths, empty=False, top_level_file=False): From 17cc73f81772fe7b589615f72c25654260acfce8 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Tue, 1 Oct 2024 01:04:10 +0200 Subject: [PATCH 13/16] add check_install_lib_symlink method to EasyBlock to be able to trigger explicit checks in the module step --- easybuild/framework/easyblock.py | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 245f45e0e1..fcae1fc3bd 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1631,6 +1631,10 @@ def _expand_module_search_path(self, search_path, top_level, fake=False): abs_glob = os.path.join(self.installdir, search_path) exp_search_paths = [abs_glob] if search_path == "" else glob.glob(abs_glob) + # Explicitly check symlink state between lib dirs if it is still undefined (e.g. --module-only) + if self.install_lib_symlink == LibSymlink.NONE: + self.check_install_lib_symlink() + retained_search_paths = [] for abs_path in exp_search_paths: # return relative paths @@ -1655,6 +1659,17 @@ def _expand_module_search_path(self, search_path, top_level, fake=False): return retained_search_paths + def check_install_lib_symlink(self): + """Check symlink state between library directories in installation prefix""" + lib_dir = os.path.join(self.installdir, 'lib') + lib64_dir = os.path.join(self.installdir, 'lib64') + if os.path.exists(lib_dir) and os.path.exists(lib64_dir): + self.install_lib_symlink = LibSymlink.NEITHER + if os.path.islink(lib_dir) and os.path.samefile(lib_dir, lib64_dir): + self.install_lib_symlink = LibSymlink.LIB + elif os.path.islink(lib64_dir) and os.path.samefile(lib_dir, lib64_dir): + self.install_lib_symlink = LibSymlink.LIB64 + def make_module_req_guess(self): """ A dictionary of common search path variables to be loaded by environment modules @@ -3074,13 +3089,8 @@ def post_install_step(self): # create *relative* 'lib' symlink to 'lib64'; symlink('lib64', lib_dir, use_abspath_source=False) - # check symlink state - if os.path.exists(lib_dir) and os.path.exists(lib64_dir): - self.install_lib_symlink = LibSymlink.NEITHER - if os.path.islink(lib_dir) and os.path.samefile(lib_dir, lib64_dir): - self.install_lib_symlink = LibSymlink.LIB - elif os.path.islink(lib64_dir) and os.path.samefile(lib_dir, lib64_dir): - self.install_lib_symlink = LibSymlink.LIB64 + # refresh symlink state + self.check_install_lib_symlink() self.run_post_install_commands() self.apply_post_install_patches() From 16e47436d11af407d503a50a3b122e829bbb0211 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Tue, 1 Oct 2024 01:59:49 +0200 Subject: [PATCH 14/16] remove unused empty attribute from ModuleEnvironmentVariable --- easybuild/tools/modules.py | 3 +-- test/framework/modules.py | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index 4efc78f696..582137f85c 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -135,9 +135,8 @@ class ModuleEnvironmentVariable: """Environment variable data structure for modules""" - def __init__(self, paths, empty=False, top_level_file=False): + def __init__(self, paths, top_level_file=False): self.paths = paths - self.empty = bool(empty) self.top_level_file = bool(top_level_file) def __str__(self): diff --git a/test/framework/modules.py b/test/framework/modules.py index 4437360359..8e7b1962e9 100644 --- a/test/framework/modules.py +++ b/test/framework/modules.py @@ -1593,7 +1593,6 @@ def test_module_environment_variable(self): test_paths = ['lib', 'lib64'] mod_envar = mod.ModuleEnvironmentVariable(test_paths) self.assertTrue(hasattr(mod_envar, "paths")) - self.assertTrue(hasattr(mod_envar, "empty")) self.assertTrue(hasattr(mod_envar, "top_level_file")) self.assertEqual(mod_envar.paths, test_paths) self.assertEqual(str(mod_envar), "lib:lib64") From f68f383cdea1726f55f98585e1b2da987e36f992 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Tue, 1 Oct 2024 02:00:44 +0200 Subject: [PATCH 15/16] attributes in ModuleLoadEnvironment can only be instances of ModuleEnvironmentVariable --- easybuild/tools/modules.py | 70 +++++++++++++++++--------------------- test/framework/modules.py | 23 ++++++++----- 2 files changed, 47 insertions(+), 46 deletions(-) diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index 582137f85c..a0bc842d41 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -182,49 +182,45 @@ def __init__(self): Paths are relative to root of installation directory """ - self.PATH = ModuleEnvironmentVariable( + self.PATH = ( SEARCH_PATH_BIN_DIRS + ['sbin'], - top_level_file=True, + {"top_level_file": True}, ) - self.LD_LIBRARY_PATH = ModuleEnvironmentVariable( + self.LD_LIBRARY_PATH = ( SEARCH_PATH_LIB_DIRS, - top_level_file=True, - ) - self.LIBRARY_PATH = ModuleEnvironmentVariable( - SEARCH_PATH_LIB_DIRS, - ) - self.CPATH = ModuleEnvironmentVariable( - SEARCH_PATH_HEADER_DIRS, - ) - self.MANPATH = ModuleEnvironmentVariable( - ['man', os.path.join('share', 'man')], - ) - self.PKG_CONFIG_PATH = ModuleEnvironmentVariable( - [os.path.join(x, 'pkgconfig') for x in SEARCH_PATH_LIB_DIRS + ['share']], - ) - self.ACLOCAL_PATH = ModuleEnvironmentVariable( - [os.path.join('share', 'aclocal')], - ) - self.CLASSPATH = ModuleEnvironmentVariable( - ['*.jar'], - ) - self.XDG_DATA_DIRS = ModuleEnvironmentVariable( - ['share'], - ) - self.GI_TYPELIB_PATH = ModuleEnvironmentVariable( - [os.path.join(x, 'girepository-*') for x in SEARCH_PATH_LIB_DIRS] - ) - self.CMAKE_PREFIX_PATH = ModuleEnvironmentVariable( - [''], + {"top_level_file": True}, ) + self.LIBRARY_PATH = SEARCH_PATH_LIB_DIRS + self.CPATH = SEARCH_PATH_HEADER_DIRS + self.MANPATH = ['man', os.path.join('share', 'man')] + self.PKG_CONFIG_PATH = [os.path.join(x, 'pkgconfig') for x in SEARCH_PATH_LIB_DIRS + ['share']] + self.ACLOCAL_PATH = [os.path.join('share', 'aclocal')] + self.CLASSPATH = ['*.jar'] + self.XDG_DATA_DIRS = ['share'] + self.GI_TYPELIB_PATH = [os.path.join(x, 'girepository-*') for x in SEARCH_PATH_LIB_DIRS] + self.CMAKE_PREFIX_PATH = [''] # only needed for installations whith standalone lib64 - self.CMAKE_LIBRARY_PATH = ModuleEnvironmentVariable( - ['lib64'], - ) + self.CMAKE_LIBRARY_PATH = ['lib64'] + + def __setattr__(self, name, value): + """ + Specific restrictions for ModuleLoadEnvironment attributes: + - attribute names are uppercase + - attributes are instances of ModuleEnvironmentVariable + """ + try: + (paths, kwargs) = value + except ValueError: + paths, kwargs = value, {} + else: + if not isinstance(kwargs, dict): + paths, kwargs = value, {} + + return super().__setattr__(name.upper(), ModuleEnvironmentVariable(paths, **kwargs)) def __iter__(self): """Make the class iterable""" - yield from (envar for envar in self.__dict__ if isinstance(getattr(self, envar), ModuleEnvironmentVariable)) + yield from self.__dict__ def items(self): """ @@ -233,9 +229,7 @@ def items(self): - value = its "paths" attribute """ for attr in self.__dict__: - envar = getattr(self, attr) - if isinstance(envar, ModuleEnvironmentVariable): - yield attr, envar.paths + yield attr, getattr(self, attr).paths @property def environ(self): diff --git a/test/framework/modules.py b/test/framework/modules.py index 8e7b1962e9..bfbb6666a2 100644 --- a/test/framework/modules.py +++ b/test/framework/modules.py @@ -1616,24 +1616,31 @@ def test_module_environment_variable(self): def test_module_load_environment(self): """Test for ModuleLoadEnvironment object""" + test_paths = ['lib', 'lib64'] mod_load_env = mod.ModuleLoadEnvironment() - mod_load_env.TEST_VAR = mod.ModuleEnvironmentVariable(["lib", "lib64"]) + mod_load_env.TEST_VAR = test_paths self.assertTrue(hasattr(mod_load_env, "TEST_VAR")) - self.assertEqual(mod_load_env.TEST_VAR.paths, ["lib", "lib64"]) + self.assertEqual(mod_load_env.TEST_VAR.paths, test_paths) - # copy current state as reference ref_load_env = mod_load_env.__dict__.copy() - # and add some garbage - mod_load_env.garbage = "not_an_env_var" - self.assertTrue(hasattr(mod_load_env, "garbage")) - self.assertEqual(mod_load_env.garbage, "not_an_env_var") - self.assertCountEqual(list(mod_load_env), ref_load_env.keys()) ref_load_env_item_list = [(key, value.paths) for key, value in ref_load_env.items()] self.assertCountEqual(list(mod_load_env.items()), ref_load_env_item_list) ref_load_env_environ = {key: value.paths for key, value in ref_load_env.items()} self.assertDictEqual(mod_load_env.environ, ref_load_env_environ) + mod_load_env.test_lower = test_paths + self.assertTrue(hasattr(mod_load_env, "TEST_LOWER")) + self.assertEqual(mod_load_env.TEST_LOWER.paths, test_paths) + mod_load_env.TEST_STR = "some/path" + self.assertTrue(hasattr(mod_load_env, "TEST_STR")) + self.assertEqual(mod_load_env.TEST_STR.paths, ["some/path"]) + mod_load_env.TEST_EXTRA = (test_paths, {"top_level_file": True}) + self.assertTrue(hasattr(mod_load_env, "TEST_EXTRA")) + self.assertEqual(mod_load_env.TEST_EXTRA.paths, test_paths) + self.assertEqual(mod_load_env.TEST_EXTRA.top_level_file, True) + self.assertRaises(TypeError, setattr, mod_load_env, "TEST_UNKNONW", (test_paths, {"unkown_param": True})) + def suite(): """ returns all the testcases in this module """ From 36d4f9274ffd429dfd935be3d1b35a3767ec4ff2 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Mon, 7 Oct 2024 10:18:54 +0200 Subject: [PATCH 16/16] add test to verify that environment variables don't leak into module file of subsequent installations --- .../sandbox/easybuild/easyblocks/t/toy.py | 7 ++++ test/framework/toy_build.py | 36 +++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/test/framework/sandbox/easybuild/easyblocks/t/toy.py b/test/framework/sandbox/easybuild/easyblocks/t/toy.py index 5727dcd0e0..552b5d8a37 100644 --- a/test/framework/sandbox/easybuild/easyblocks/t/toy.py +++ b/test/framework/sandbox/easybuild/easyblocks/t/toy.py @@ -192,3 +192,10 @@ def make_module_extra(self): txt = super(EB_toy, self).make_module_extra() txt += self.module_generator.set_environment('TOY', os.getenv('TOY', '')) return txt + + def make_module_req_guess(self): + """Extra paths for environment variables to consider""" + guesses = super(EB_toy, self).make_module_req_guess() + if self.name == 'toy': + guesses['CPATH'].append('toy-headers') + return guesses diff --git a/test/framework/toy_build.py b/test/framework/toy_build.py index edd9e94fcc..6edef42c5d 100644 --- a/test/framework/toy_build.py +++ b/test/framework/toy_build.py @@ -4302,6 +4302,42 @@ def test_toy_python(self): self.assertTrue(ebpythonprefixes_regex.search(toy_mod_txt), f"Pattern '{ebpythonprefixes_regex.pattern}' found in: {toy_mod_txt}") + def test_toy_multiple_ecs_module(self): + """ + Verify whether module file is correct when multiple easyconfigs are being installed. + """ + test_ecs = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'easyconfigs', 'test_ecs') + toy_ec = os.path.join(test_ecs, 't', 'toy', 'toy-0.0.eb') + + # modify 'toy' easyconfig so toy-headers subdirectory is created, + # which is taken into account by EB_toy easyblock for $CPATH + test_toy_ec = os.path.join(self.test_prefix, 'test-toy.eb') + toy_ec_txt = read_file(toy_ec) + toy_ec_txt += "\npostinstallcmds += ['mkdir %(installdir)s/toy-headers']" + toy_ec_txt += "\npostinstallcmds += ['touch %(installdir)s/toy-headers/toy.h']" + write_file(test_toy_ec, toy_ec_txt) + + # modify 'toy-app' easyconfig so toy-headers subdirectory is created, + # which is consider by EB_toy easyblock for $CPATH, + # but should *not* be actually used because software name is not 'toy' + toy_app_ec = os.path.join(test_ecs, 't', 'toy-app', 'toy-app-0.0.eb') + test_toy_app_ec = os.path.join(self.test_prefix, 'test-toy-app.eb') + toy_ec_txt = read_file(toy_app_ec) + toy_ec_txt += "\npostinstallcmds += ['mkdir %(installdir)s/toy-headers']" + toy_ec_txt += "\npostinstallcmds += ['touch %(installdir)s/toy-headers/toy.h']" + write_file(test_toy_app_ec, toy_ec_txt) + + self.run_test_toy_build_with_output(ec_file=test_toy_ec, extra_args=[test_toy_app_ec]) + + toy_modtxt = read_file(os.path.join(self.test_installpath, 'modules', 'all', 'toy', '0.0.lua')) + regex = re.compile('prepend[-_]path.*CPATH.*toy-headers', re.M) + self.assertTrue(regex.search(toy_modtxt), + f"Pattern '{regex.pattern}' should be found in: {toy_modtxt}") + + toy_app_modtxt = read_file(os.path.join(self.test_installpath, 'modules', 'all', 'toy-app', '0.0.lua')) + self.assertFalse(regex.search(toy_app_modtxt), + f"Pattern '{regex.pattern}' should *not* be found in: {toy_app_modtxt}") + def suite(): """ return all the tests in this file """