From d5fef2c77e60a83760734ffb8370ea26eec2af07 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Thu, 24 Apr 2025 13:34:08 +0200 Subject: [PATCH 01/13] replace PATH_JOIN_TEMPLATE in ModuleGeneratorLua with _path_join_cmd() method --- easybuild/tools/module_generator.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index fb9b29ac6b..243ac3789c 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -52,7 +52,6 @@ from easybuild.tools.modules import ROOT_ENV_VAR_NAME_PREFIX, EnvironmentModulesC, Lmod, modules_tool from easybuild.tools.utilities import get_subclasses, nub, quote_str - _log = fancylogger.getLogger('module_generator', fname=False) @@ -1152,7 +1151,7 @@ class ModuleGeneratorLua(ModuleGenerator): LOAD_TEMPLATE_DEPENDS_ON = 'depends_on("%(mod_name)s")' IS_LOADED_TEMPLATE = 'isloaded("%s")' - PATH_JOIN_TEMPLATE = 'pathJoin(root, "%s")' + PATH_JOIN_TEMPLATE = 'pathJoin(root, "%s")' # TODO: remove after 6.0, replaced by _path_join_cmd() UPDATE_PATH_TEMPLATE = '%s_path("%s", %s)' UPDATE_PATH_TEMPLATE_DELIM = '%s_path("%s", %s, "%s")' @@ -1167,6 +1166,19 @@ def __init__(self, *args, **kwargs): if self.modules_tool.version and LooseVersion(self.modules_tool.version) >= LooseVersion('7.7.38'): self.DOT_MODULERC = '.modulerc.lua' + @staticmethod + def _path_join_cmd(path): + "Return 'pathJoin' command for given path string" + path_components = [quote_str(p) for p in path.split(os.path.sep) if p] + + path_root = quote_str(os.path.sep) if os.path.isabs(path) else 'root' + path_components.insert(0, path_root) + + if len(path_components) > 1: + return f'pathJoin({", ".join(path_components)})' + # no need for a pathJoin for single component paths + return os.path.join(*path_components) + def check_version(self, minimal_version_maj, minimal_version_min, minimal_version_patch='0'): """ Check the minimal version of the moduletool in the module file @@ -1448,7 +1460,7 @@ def update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpath # use pathJoin for (non-empty) relative paths if path: if expand_relpaths: - abspaths.append(self.PATH_JOIN_TEMPLATE % path) + abspaths.append(self._path_join_cmd(path)) else: abspaths.append(quote_str(path)) else: From 8e46ad8837db6fab58d0b518d77a4dbd38bfe450 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Thu, 24 Apr 2025 13:35:39 +0200 Subject: [PATCH 02/13] add support for environment variables in set_environment method of module generators --- easybuild/tools/module_generator.py | 41 +++++++++++++++++------------ test/framework/module_generator.py | 41 ++++++++++++++++++++++++----- 2 files changed, 58 insertions(+), 24 deletions(-) diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index 243ac3789c..eb2e449693 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -132,6 +132,11 @@ class ModuleGenerator: # a single level of indentation INDENTATION = ' ' * 4 + # shell environment variable name: ${__}VAR_NAME_00_SUFFIX + REGEX_SHELL_VAR_PATTERN = r'[A-Z_]+[A-Z0-9_]+' + REGEX_SHELL_VAR = re.compile(rf'\$({REGEX_SHELL_VAR_PATTERN})') + REGEX_QUOTE_SHELL_VAR = re.compile(rf'[\"\']\$({REGEX_SHELL_VAR_PATTERN})[\"\']') + def __init__(self, application, fake=False): """ModuleGenerator constructor.""" self.app = application @@ -1055,19 +1060,18 @@ def set_environment(self, key, value, relpath=False): self.log.info("Not including statement to define environment variable $%s, as specified", key) return '' - value, use_pushenv = self.unpack_setenv_value(key, value) + set_value, use_pushenv = self.unpack_setenv_value(key, value) - # quotes are needed, to ensure smooth working of EBDEVEL* modulefiles if relpath: - if value: - val = quote_str(os.path.join('$root', value), tcl=True) - else: - val = '"$root"' - else: - val = quote_str(value, tcl=True) + set_value = os.path.join('$root', set_value) if set_value else '$root' + + set_value = self.REGEX_SHELL_VAR.sub(r'$::env(\1)', set_value) + + # quotes are needed, to ensure smooth working of EBDEVEL* modulefiles + set_value = quote_str(set_value, tcl=True) env_setter = 'pushenv' if use_pushenv else 'setenv' - return '%s\t%s\t\t%s\n' % (env_setter, key, val) + return f'{env_setter}\t{key}\t\t{set_value}\n' def swap_module(self, mod_name_out, mod_name_in, guarded=True): """ @@ -1525,19 +1529,22 @@ def set_environment(self, key, value, relpath=False): self.log.info("Not including statement to define environment variable $%s, as specified", key) return '' - value, use_pushenv = self.unpack_setenv_value(key, value) + set_value, use_pushenv = self.unpack_setenv_value(key, value) if relpath: - if value: - val = self.PATH_JOIN_TEMPLATE % value - else: - val = 'root' + set_value = self._path_join_cmd(set_value) + set_value = self.REGEX_QUOTE_SHELL_VAR.sub(r'os.getenv("\1")', set_value) else: - val = quote_str(value) + lua_concat = ' .. ' + set_value = self.REGEX_SHELL_VAR.sub(rf'{lua_concat}os.getenv("\1"){lua_concat}', set_value) + set_value = lua_concat.join([ + # quote any substrings that are not lua commands + quote_str(x) if not x.startswith('os.') else x + for x in set_value.strip(lua_concat).split(lua_concat) + ]) env_setter = 'pushenv' if use_pushenv else 'setenv' - - return '%s("%s", %s)\n' % (env_setter, key, val) + return f'{env_setter}("{key}", {set_value})\n' def swap_module(self, mod_name_out, mod_name_in, guarded=True): """ diff --git a/test/framework/module_generator.py b/test/framework/module_generator.py index 003c19aea7..0f64f47342 100644 --- a/test/framework/module_generator.py +++ b/test/framework/module_generator.py @@ -953,14 +953,41 @@ def test_use(self): def test_env(self): """Test setting of environment variables.""" + collection = ( + # value, relpath, Tcl reference, Lua reference + ("value", False, 'setenv\tkey\t\t"value"\n', 'setenv("key", "value")\n'), #noqa + ('va"lue', False, 'setenv\tkey\t\t"va\\"lue"\n', 'setenv("key", \'va"lue\')\n'), #noqa + ("va'lue", False, 'setenv\tkey\t\t"va\'lue"\n', 'setenv("key", "va\'lue")\n'), #noqa + ("""va"l'ue""", False, 'setenv\tkey\t\t"""va"l\'ue"""\n', 'setenv("key", """va"l\'ue""")\n'), #noqa + ("relative/path", False, 'setenv\tkey\t\t"relative/path"\n', 'setenv("key", "relative/path")\n'), #noqa + ("relative/path", True, 'setenv\tkey\t\t"$root/relative/path"\n', 'setenv("key", pathJoin(root, "relative", "path"))\n'), #noqa + ("relative/path/", False, 'setenv\tkey\t\t"relative/path/"\n', 'setenv("key", "relative/path/")\n'), #noqa + ("relative/path/", True, 'setenv\tkey\t\t"$root/relative/path/"\n', 'setenv("key", pathJoin(root, "relative", "path"))\n'), #noqa + ("", False, 'setenv\tkey\t\t""\n', 'setenv("key", "")\n'), #noqa + ("", True, 'setenv\tkey\t\t"$root"\n', 'setenv("key", root)\n'), #noqa + ("/absolute/path", False, 'setenv\tkey\t\t"/absolute/path"\n', 'setenv("key", "/absolute/path")\n'), #noqa + ("/absolute/path", True, 'setenv\tkey\t\t"/absolute/path"\n', 'setenv("key", pathJoin("/", "absolute", "path"))\n'), #noqa + ("/", False, 'setenv\tkey\t\t"/"\n', 'setenv("key", "/")\n'), #noqa + ("/", True, 'setenv\tkey\t\t"/"\n', 'setenv("key", "/")\n'), #noqa + ("$VAR", False, 'setenv\tkey\t\t"$::env(VAR)"\n', 'setenv("key", os.getenv("VAR"))\n'), #noqa + ("$VAR", True, 'setenv\tkey\t\t"$root/$::env(VAR)"\n', 'setenv("key", pathJoin(root, os.getenv("VAR")))\n'), #noqa + ("$VAR/in/path", False, 'setenv\tkey\t\t"$::env(VAR)/in/path"\n', 'setenv("key", os.getenv("VAR") .. "/in/path")\n'), #noqa + ("$VAR/in/path", True, 'setenv\tkey\t\t"$root/$::env(VAR)/in/path"\n', 'setenv("key", pathJoin(root, os.getenv("VAR"), "in", "path"))\n'), #noqa + ("path/with/$VAR", False, 'setenv\tkey\t\t"path/with/$::env(VAR)"\n', 'setenv("key", "path/with/" .. os.getenv("VAR"))\n'), #noqa + ("path/with/$VAR", True, 'setenv\tkey\t\t"$root/path/with/$::env(VAR)"\n', 'setenv("key", pathJoin(root, "path", "with", os.getenv("VAR")))\n'), #noqa + ("path/$VAR/dir", False, 'setenv\tkey\t\t"path/$::env(VAR)/dir"\n', 'setenv("key", "path/" .. os.getenv("VAR") .. "/dir")\n'), #noqa + ("path/$VAR/dir", True, 'setenv\tkey\t\t"$root/path/$::env(VAR)/dir"\n', 'setenv("key", pathJoin(root, "path", os.getenv("VAR"), "dir"))\n'), #noqa + ("/$VAR/in/abspath", False, 'setenv\tkey\t\t"/$::env(VAR)/in/abspath"\n', 'setenv("key", "/" .. os.getenv("VAR") .. "/in/abspath")\n'), #noqa + ("/$VAR/in/abspath", True, 'setenv\tkey\t\t"/$::env(VAR)/in/abspath"\n', 'setenv("key", pathJoin("/", os.getenv("VAR"), "in", "abspath"))\n'), #noqa + ("/abspath/with/$VAR", False, 'setenv\tkey\t\t"/abspath/with/$::env(VAR)"\n', 'setenv("key", "/abspath/with/" .. os.getenv("VAR"))\n'), #noqa + ("/abspath/with/$VAR", True, 'setenv\tkey\t\t"/abspath/with/$::env(VAR)"\n', 'setenv("key", pathJoin("/", "abspath", "with", os.getenv("VAR")))\n'), #noqa + ("/abspath/$VAR/dir", False, 'setenv\tkey\t\t"/abspath/$::env(VAR)/dir"\n', 'setenv("key", "/abspath/" .. os.getenv("VAR") .. "/dir")\n'), #noqa + ("/abspath/$VAR/dir", True, 'setenv\tkey\t\t"/abspath/$::env(VAR)/dir"\n', 'setenv("key", pathJoin("/", "abspath", os.getenv("VAR"), "dir"))\n'), #noqa + ) # test set_environment - if self.MODULE_GENERATOR_CLASS == ModuleGeneratorTcl: - self.assertEqual('setenv\tkey\t\t"value"\n', self.modgen.set_environment("key", "value")) - self.assertEqual('setenv\tkey\t\t"va\\"lue"\n', self.modgen.set_environment("key", 'va"lue')) - self.assertEqual('setenv\tkey\t\t"va\'lue"\n', self.modgen.set_environment("key", "va'lue")) - self.assertEqual('setenv\tkey\t\t"""va"l\'ue"""\n', self.modgen.set_environment("key", """va"l'ue""")) - else: - self.assertEqual('setenv("key", "value")\n', self.modgen.set_environment("key", "value")) + for test_value, test_relpath, ref_tcl, ref_lua in collection: + reference = ref_tcl if self.MODULE_GENERATOR_CLASS == ModuleGeneratorTcl else ref_lua + self.assertEqual(self.modgen.set_environment("key", test_value, test_relpath), reference) def test_getenv_cmd(self): """Test getting value of environment variable.""" From 5f3b4d15dcc61efa72fc5cce68c629cf5795ce64 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Thu, 24 Apr 2025 13:49:46 +0200 Subject: [PATCH 03/13] fix formatting of inline comments in test_env --- test/framework/module_generator.py | 56 +++++++++++++++--------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/test/framework/module_generator.py b/test/framework/module_generator.py index 0f64f47342..ad8b5c0912 100644 --- a/test/framework/module_generator.py +++ b/test/framework/module_generator.py @@ -955,34 +955,34 @@ def test_env(self): """Test setting of environment variables.""" collection = ( # value, relpath, Tcl reference, Lua reference - ("value", False, 'setenv\tkey\t\t"value"\n', 'setenv("key", "value")\n'), #noqa - ('va"lue', False, 'setenv\tkey\t\t"va\\"lue"\n', 'setenv("key", \'va"lue\')\n'), #noqa - ("va'lue", False, 'setenv\tkey\t\t"va\'lue"\n', 'setenv("key", "va\'lue")\n'), #noqa - ("""va"l'ue""", False, 'setenv\tkey\t\t"""va"l\'ue"""\n', 'setenv("key", """va"l\'ue""")\n'), #noqa - ("relative/path", False, 'setenv\tkey\t\t"relative/path"\n', 'setenv("key", "relative/path")\n'), #noqa - ("relative/path", True, 'setenv\tkey\t\t"$root/relative/path"\n', 'setenv("key", pathJoin(root, "relative", "path"))\n'), #noqa - ("relative/path/", False, 'setenv\tkey\t\t"relative/path/"\n', 'setenv("key", "relative/path/")\n'), #noqa - ("relative/path/", True, 'setenv\tkey\t\t"$root/relative/path/"\n', 'setenv("key", pathJoin(root, "relative", "path"))\n'), #noqa - ("", False, 'setenv\tkey\t\t""\n', 'setenv("key", "")\n'), #noqa - ("", True, 'setenv\tkey\t\t"$root"\n', 'setenv("key", root)\n'), #noqa - ("/absolute/path", False, 'setenv\tkey\t\t"/absolute/path"\n', 'setenv("key", "/absolute/path")\n'), #noqa - ("/absolute/path", True, 'setenv\tkey\t\t"/absolute/path"\n', 'setenv("key", pathJoin("/", "absolute", "path"))\n'), #noqa - ("/", False, 'setenv\tkey\t\t"/"\n', 'setenv("key", "/")\n'), #noqa - ("/", True, 'setenv\tkey\t\t"/"\n', 'setenv("key", "/")\n'), #noqa - ("$VAR", False, 'setenv\tkey\t\t"$::env(VAR)"\n', 'setenv("key", os.getenv("VAR"))\n'), #noqa - ("$VAR", True, 'setenv\tkey\t\t"$root/$::env(VAR)"\n', 'setenv("key", pathJoin(root, os.getenv("VAR")))\n'), #noqa - ("$VAR/in/path", False, 'setenv\tkey\t\t"$::env(VAR)/in/path"\n', 'setenv("key", os.getenv("VAR") .. "/in/path")\n'), #noqa - ("$VAR/in/path", True, 'setenv\tkey\t\t"$root/$::env(VAR)/in/path"\n', 'setenv("key", pathJoin(root, os.getenv("VAR"), "in", "path"))\n'), #noqa - ("path/with/$VAR", False, 'setenv\tkey\t\t"path/with/$::env(VAR)"\n', 'setenv("key", "path/with/" .. os.getenv("VAR"))\n'), #noqa - ("path/with/$VAR", True, 'setenv\tkey\t\t"$root/path/with/$::env(VAR)"\n', 'setenv("key", pathJoin(root, "path", "with", os.getenv("VAR")))\n'), #noqa - ("path/$VAR/dir", False, 'setenv\tkey\t\t"path/$::env(VAR)/dir"\n', 'setenv("key", "path/" .. os.getenv("VAR") .. "/dir")\n'), #noqa - ("path/$VAR/dir", True, 'setenv\tkey\t\t"$root/path/$::env(VAR)/dir"\n', 'setenv("key", pathJoin(root, "path", os.getenv("VAR"), "dir"))\n'), #noqa - ("/$VAR/in/abspath", False, 'setenv\tkey\t\t"/$::env(VAR)/in/abspath"\n', 'setenv("key", "/" .. os.getenv("VAR") .. "/in/abspath")\n'), #noqa - ("/$VAR/in/abspath", True, 'setenv\tkey\t\t"/$::env(VAR)/in/abspath"\n', 'setenv("key", pathJoin("/", os.getenv("VAR"), "in", "abspath"))\n'), #noqa - ("/abspath/with/$VAR", False, 'setenv\tkey\t\t"/abspath/with/$::env(VAR)"\n', 'setenv("key", "/abspath/with/" .. os.getenv("VAR"))\n'), #noqa - ("/abspath/with/$VAR", True, 'setenv\tkey\t\t"/abspath/with/$::env(VAR)"\n', 'setenv("key", pathJoin("/", "abspath", "with", os.getenv("VAR")))\n'), #noqa - ("/abspath/$VAR/dir", False, 'setenv\tkey\t\t"/abspath/$::env(VAR)/dir"\n', 'setenv("key", "/abspath/" .. os.getenv("VAR") .. "/dir")\n'), #noqa - ("/abspath/$VAR/dir", True, 'setenv\tkey\t\t"/abspath/$::env(VAR)/dir"\n', 'setenv("key", pathJoin("/", "abspath", os.getenv("VAR"), "dir"))\n'), #noqa + ("value", False, 'setenv\tkey\t\t"value"\n', 'setenv("key", "value")\n'), # noqa + ('va"lue', False, 'setenv\tkey\t\t"va\\"lue"\n', 'setenv("key", \'va"lue\')\n'), # noqa + ("va'lue", False, 'setenv\tkey\t\t"va\'lue"\n', 'setenv("key", "va\'lue")\n'), # noqa + ("""va"l'ue""", False, 'setenv\tkey\t\t"""va"l\'ue"""\n', 'setenv("key", """va"l\'ue""")\n'), # noqa + ("relative/path", False, 'setenv\tkey\t\t"relative/path"\n', 'setenv("key", "relative/path")\n'), # noqa + ("relative/path", True, 'setenv\tkey\t\t"$root/relative/path"\n', 'setenv("key", pathJoin(root, "relative", "path"))\n'), # noqa + ("relative/path/", False, 'setenv\tkey\t\t"relative/path/"\n', 'setenv("key", "relative/path/")\n'), # noqa + ("relative/path/", True, 'setenv\tkey\t\t"$root/relative/path/"\n', 'setenv("key", pathJoin(root, "relative", "path"))\n'), # noqa + ("", False, 'setenv\tkey\t\t""\n', 'setenv("key", "")\n'), # noqa + ("", True, 'setenv\tkey\t\t"$root"\n', 'setenv("key", root)\n'), # noqa + ("/absolute/path", False, 'setenv\tkey\t\t"/absolute/path"\n', 'setenv("key", "/absolute/path")\n'), # noqa + ("/absolute/path", True, 'setenv\tkey\t\t"/absolute/path"\n', 'setenv("key", pathJoin("/", "absolute", "path"))\n'), # noqa + ("/", False, 'setenv\tkey\t\t"/"\n', 'setenv("key", "/")\n'), # noqa + ("/", True, 'setenv\tkey\t\t"/"\n', 'setenv("key", "/")\n'), # noqa + ("$VAR", False, 'setenv\tkey\t\t"$::env(VAR)"\n', 'setenv("key", os.getenv("VAR"))\n'), # noqa + ("$VAR", True, 'setenv\tkey\t\t"$root/$::env(VAR)"\n', 'setenv("key", pathJoin(root, os.getenv("VAR")))\n'), # noqa + ("$VAR/in/path", False, 'setenv\tkey\t\t"$::env(VAR)/in/path"\n', 'setenv("key", os.getenv("VAR") .. "/in/path")\n'), # noqa + ("$VAR/in/path", True, 'setenv\tkey\t\t"$root/$::env(VAR)/in/path"\n', 'setenv("key", pathJoin(root, os.getenv("VAR"), "in", "path"))\n'), # noqa + ("path/with/$VAR", False, 'setenv\tkey\t\t"path/with/$::env(VAR)"\n', 'setenv("key", "path/with/" .. os.getenv("VAR"))\n'), # noqa + ("path/with/$VAR", True, 'setenv\tkey\t\t"$root/path/with/$::env(VAR)"\n', 'setenv("key", pathJoin(root, "path", "with", os.getenv("VAR")))\n'), # noqa + ("path/$VAR/dir", False, 'setenv\tkey\t\t"path/$::env(VAR)/dir"\n', 'setenv("key", "path/" .. os.getenv("VAR") .. "/dir")\n'), # noqa + ("path/$VAR/dir", True, 'setenv\tkey\t\t"$root/path/$::env(VAR)/dir"\n', 'setenv("key", pathJoin(root, "path", os.getenv("VAR"), "dir"))\n'), # noqa + ("/$VAR/in/abspath", False, 'setenv\tkey\t\t"/$::env(VAR)/in/abspath"\n', 'setenv("key", "/" .. os.getenv("VAR") .. "/in/abspath")\n'), # noqa + ("/$VAR/in/abspath", True, 'setenv\tkey\t\t"/$::env(VAR)/in/abspath"\n', 'setenv("key", pathJoin("/", os.getenv("VAR"), "in", "abspath"))\n'), # noqa + ("/abspath/with/$VAR", False, 'setenv\tkey\t\t"/abspath/with/$::env(VAR)"\n', 'setenv("key", "/abspath/with/" .. os.getenv("VAR"))\n'), # noqa + ("/abspath/with/$VAR", True, 'setenv\tkey\t\t"/abspath/with/$::env(VAR)"\n', 'setenv("key", pathJoin("/", "abspath", "with", os.getenv("VAR")))\n'), # noqa + ("/abspath/$VAR/dir", False, 'setenv\tkey\t\t"/abspath/$::env(VAR)/dir"\n', 'setenv("key", "/abspath/" .. os.getenv("VAR") .. "/dir")\n'), # noqa + ("/abspath/$VAR/dir", True, 'setenv\tkey\t\t"/abspath/$::env(VAR)/dir"\n', 'setenv("key", pathJoin("/", "abspath", os.getenv("VAR"), "dir"))\n'), # noqa ) # test set_environment for test_value, test_relpath, ref_tcl, ref_lua in collection: From 788e925f58e1e5ead7446251e0714b73a7ad46f9 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Thu, 24 Apr 2025 14:18:22 +0200 Subject: [PATCH 04/13] update test_make_module_req to adapt to new _path_join_cmd method in ModuleGeneratorLua --- test/framework/easyblock.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/test/framework/easyblock.py b/test/framework/easyblock.py index 0cbefe3387..e31d0ec50b 100644 --- a/test/framework/easyblock.py +++ b/test/framework/easyblock.py @@ -462,7 +462,7 @@ def test_make_module_req(self): elif get_module_syntax() == 'Lua': self.assertTrue(re.search(r'^prepend_path\("CLASSPATH", pathJoin\(root, "bla.jar"\)\)$', guess, re.M)) self.assertTrue(re.search(r'^prepend_path\("CLASSPATH", pathJoin\(root, "foo.jar"\)\)$', guess, re.M)) - self.assertTrue(re.search(r'^prepend_path\("MANPATH", pathJoin\(root, "share/man"\)\)$', guess, re.M)) + self.assertTrue(re.search(r'^prepend_path\("MANPATH", pathJoin\(root, "share", "man"\)\)$', guess, re.M)) self.assertIn('prepend_path("CMAKE_PREFIX_PATH", root)', guess) # bin/ is not added to $PATH if it doesn't include files self.assertFalse(re.search(r'^prepend_path\("PATH", pathJoin\(root, "bin"\)\)$', guess, re.M)) @@ -573,12 +573,12 @@ def test_make_module_req(self): r"prepend-path\s+LD_LIBRARY_PATH\s+\$root/lib/pathA\n", txt, re.M)) elif get_module_syntax() == 'Lua': - self.assertTrue(re.search(r'\nprepend_path\("LD_LIBRARY_PATH", pathJoin\(root, "lib/pathC"\)\)\n' + - r'prepend_path\("LD_LIBRARY_PATH", pathJoin\(root, "lib/pathA"\)\)\n' + - r'prepend_path\("LD_LIBRARY_PATH", pathJoin\(root, "lib/pathB"\)\)\n', + self.assertTrue(re.search(r'\nprepend_path\("LD_LIBRARY_PATH", pathJoin\(root, "lib", "pathC"\)\)\n' + + r'prepend_path\("LD_LIBRARY_PATH", pathJoin\(root, "lib", "pathA"\)\)\n' + + r'prepend_path\("LD_LIBRARY_PATH", pathJoin\(root, "lib", "pathB"\)\)\n', txt, re.M)) - self.assertFalse(re.search(r'\nprepend_path\("LD_LIBRARY_PATH", pathJoin\(root, "lib/pathB"\)\)\n' + - r'prepend_path\("LD_LIBRARY_PATH", pathJoin\(root, "lib/pathA"\)\)\n', + self.assertFalse(re.search(r'\nprepend_path\("LD_LIBRARY_PATH", pathJoin\(root, "lib", "pathB"\)\)\n' + + r'prepend_path\("LD_LIBRARY_PATH", pathJoin\(root, "lib", "pathA"\)\)\n', txt, re.M)) else: self.fail("Unknown module syntax: %s" % get_module_syntax()) @@ -651,10 +651,12 @@ def test_make_module_req(self): self.assertTrue(re.search(r"^prepend-path\s+LD_LIBRARY_PATH\s+\$root/libraries/intel64_lin$", txt, re.M)) self.assertTrue(re.search(r"^prepend-path\s+LIBRARY_PATH\s+\$root/libraries/intel64_lin\n$", txt, re.M)) elif get_module_syntax() == 'Lua': - self.assertTrue(re.search(r'^prepend_path\("LD_LIBRARY_PATH", pathJoin\(root, "libraries/intel64_lin"\)\)$', - txt, re.M)) - self.assertTrue(re.search(r'^prepend_path\("LIBRARY_PATH", pathJoin\(root, "libraries/intel64_lin"\)\)$', - txt, re.M)) + self.assertTrue(re.search( + r'^prepend_path\("LD_LIBRARY_PATH", pathJoin\(root, "libraries", "intel64_lin"\)\)$', txt, re.M + )) + self.assertTrue(re.search( + r'^prepend_path\("LIBRARY_PATH", pathJoin\(root, "libraries", "intel64_lin"\)\)$', txt, re.M + )) else: self.fail("Unknown module syntax: %s" % get_module_syntax()) @@ -712,7 +714,7 @@ def test_make_module_req(self): expected_patterns = [ r"^append[-_]path.*TEST_VAR_CUSTOM.*root.*foo.*", r"^prepend[-_]path.*CPATH.*root.*include.*", - r"^prepend[-_]path.*CPATH.*root.*include/foo.*", + r"^prepend[-_]path.*CPATH.*root.*include.*foo.*", r"^prepend[-_]path.*LD_LIBRARY_PATH.*root.*lib", r"^prepend[-_]path.*LD_LIBRARY_PATH.*root.*foo", r"^prepend[-_]path.*TEST_VAR.*root.*foo", From 510f517109d0fe8ed19c53ac2f841dcd00b503b7 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Thu, 24 Apr 2025 14:24:32 +0200 Subject: [PATCH 05/13] update test_make_module_step to adapt to new _path_join_cmd method in ModuleGeneratorLua --- test/framework/easyblock.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/framework/easyblock.py b/test/framework/easyblock.py index e31d0ec50b..721cc57043 100644 --- a/test/framework/easyblock.py +++ b/test/framework/easyblock.py @@ -1616,7 +1616,7 @@ def test_make_module_step(self): if val == '': full_val = 'root' else: - full_val = fr'pathJoin\(root, "{val}"\)' + full_val = fr'pathJoin\(root, "{val.replace("/",".*")}"\)' regex = re.compile(fr'^{placement}_path\("{key}", {full_val}{delim_lua}\)$', re.M) else: self.fail(f"Unknown module syntax: {get_module_syntax()}") From 67aeaa2edb555fe976705939f4ee04c4b05e4d0b Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Thu, 24 Apr 2025 14:36:21 +0200 Subject: [PATCH 06/13] update test_toy_module_fulltxt to adapt to new _path_join_cmd method in ModuleGeneratorLua --- test/framework/toy_build.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/framework/toy_build.py b/test/framework/toy_build.py index 5f02ea2f1e..d48ff0b10c 100644 --- a/test/framework/toy_build.py +++ b/test/framework/toy_build.py @@ -428,9 +428,9 @@ def test_toy_tweaked(self): self.assertTrue(re.search(r'^puts stderr "oh hai!"$', toy_module_txt, re.M)) elif get_module_syntax() == 'Lua': self.assertTrue(re.search(r'^setenv\("FOO", "bar"\)', toy_module_txt, re.M)) - pattern = r'^prepend_path\("SOMEPATH", pathJoin\(root, "foo/bar"\)\)$' + pattern = r'^prepend_path\("SOMEPATH", pathJoin\(root, "foo", "bar"\)\)$' self.assertTrue(re.search(pattern, toy_module_txt, re.M)) - pattern = r'^append_path\("SOMEPATH_APPEND", pathJoin\(root, "qux/fred"\)\)$' + pattern = r'^append_path\("SOMEPATH_APPEND", pathJoin\(root, "qux", "fred"\)\)$' self.assertTrue(re.search(pattern, toy_module_txt, re.M)) pattern = r'^append_path\("SOMEPATH_APPEND", pathJoin\(root, "thud"\)\)$' self.assertTrue(re.search(pattern, toy_module_txt, re.M)) @@ -1647,15 +1647,15 @@ def test_toy_module_fulltxt(self): r'prepend_path\("LD_LIBRARY_PATH", pathJoin\(root, "lib"\)\)', r'prepend_path\("LIBRARY_PATH", pathJoin\(root, "lib"\)\)', r'prepend_path\("PATH", pathJoin\(root, "bin"\)\)', - r'prepend_path\("SOMEPATH", pathJoin\(root, "foo/bar"\)\)', + r'prepend_path\("SOMEPATH", pathJoin\(root, "foo", "bar"\)\)', r'prepend_path\("SOMEPATH", pathJoin\(root, "baz"\)\)', r'prepend_path\("SOMEPATH", root\)', - r'append_path\("SOMEPATH_APPEND", pathJoin\(root, "qux/fred"\)\)', + r'append_path\("SOMEPATH_APPEND", pathJoin\(root, "qux", "fred"\)\)', r'append_path\("SOMEPATH_APPEND", pathJoin\(root, "thud"\)\)', r'append_path\("SOMEPATH_APPEND", root\)', r'setenv\("EBROOTTOY", root\)', r'setenv\("EBVERSIONTOY", "0.0"\)', - r'setenv\("EBDEVELTOY", pathJoin\(root, "easybuild/toy-0.0-tweaked-easybuild-devel"\)\)', + r'setenv\("EBDEVELTOY", pathJoin\(root, "easybuild", "toy-0.0-tweaked-easybuild-devel"\)\)', r'', r'setenv\("FOO", "bar"\)', r'', From 72cdac793b9f291f5be3f8f8ecc7a607449c0fa2 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Thu, 24 Apr 2025 14:37:37 +0200 Subject: [PATCH 07/13] update test_toy_python to adapt to new _path_join_cmd method in ModuleGeneratorLua --- test/framework/toy_build.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/framework/toy_build.py b/test/framework/toy_build.py index d48ff0b10c..b948a8a5b9 100644 --- a/test/framework/toy_build.py +++ b/test/framework/toy_build.py @@ -4461,7 +4461,7 @@ def test_toy_python(self): toy_mod += '.lua' toy_mod_txt = read_file(toy_mod) - pythonpath_regex = re.compile('^prepend.path.*PYTHONPATH.*lib/python3.6/site-packages', re.M) + pythonpath_regex = re.compile('^prepend.path.*PYTHONPATH.*lib.*python3.6.*site-packages', re.M) self.assertTrue(pythonpath_regex.search(toy_mod_txt), f"Pattern '{pythonpath_regex.pattern}' found in: {toy_mod_txt}") From 5c594ad603db8ed408a45710979538e159a27fbe Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Thu, 24 Apr 2025 14:40:21 +0200 Subject: [PATCH 08/13] fix code formatting in test_make_module_step --- test/framework/easyblock.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/framework/easyblock.py b/test/framework/easyblock.py index 721cc57043..fa6473fad2 100644 --- a/test/framework/easyblock.py +++ b/test/framework/easyblock.py @@ -1616,7 +1616,7 @@ def test_make_module_step(self): if val == '': full_val = 'root' else: - full_val = fr'pathJoin\(root, "{val.replace("/",".*")}"\)' + full_val = fr'pathJoin\(root, "{val.replace("/", ".*")}"\)' regex = re.compile(fr'^{placement}_path\("{key}", {full_val}{delim_lua}\)$', re.M) else: self.fail(f"Unknown module syntax: {get_module_syntax()}") From ad20bd04062103ec81fd1a9d4bed0ab74a4ba889 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Thu, 24 Apr 2025 16:50:18 +0200 Subject: [PATCH 09/13] update test_make_module_extra to adapt to new _path_join_cmd method in ModuleGeneratorLua --- test/framework/easyblock.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/framework/easyblock.py b/test/framework/easyblock.py index fa6473fad2..ae625d5a52 100644 --- a/test/framework/easyblock.py +++ b/test/framework/easyblock.py @@ -870,12 +870,12 @@ def test_make_module_extra(self): expected_default = re.compile(r'\n'.join([ r'setenv\("EBROOTPI", root\)', r'setenv\("EBVERSIONPI", "3.14"\)', - r'setenv\("EBDEVELPI", pathJoin\(root, "easybuild/pi-3.14-gompi-2018a-easybuild-devel"\)\)', + r'setenv\("EBDEVELPI", pathJoin\(root, "easybuild", "pi-3.14-gompi-2018a-easybuild-devel"\)\)', ])) expected_alt = re.compile(r'\n'.join([ r'setenv\("EBROOTPI", "/opt/software/tau/6.28"\)', r'setenv\("EBVERSIONPI", "6.28"\)', - r'setenv\("EBDEVELPI", pathJoin\(root, "easybuild/pi-3.14-gompi-2018a-easybuild-devel"\)\)', + r'setenv\("EBDEVELPI", pathJoin\(root, "easybuild", "pi-3.14-gompi-2018a-easybuild-devel"\)\)', ])) else: self.fail("Unknown module syntax: %s" % get_module_syntax()) From bb0a777e825f5b7695e7975106c8246979bb5931 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Fri, 16 May 2025 12:04:05 +0200 Subject: [PATCH 10/13] add 'shell_vars' option to modextravars to control resolution of shell variables --- easybuild/tools/module_generator.py | 36 ++++++++++++++++++----------- test/framework/module_generator.py | 22 ++++++++++++++++++ 2 files changed, 44 insertions(+), 14 deletions(-) diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index eb2e449693..17b5abe005 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -137,6 +137,10 @@ class ModuleGenerator: REGEX_SHELL_VAR = re.compile(rf'\$({REGEX_SHELL_VAR_PATTERN})') REGEX_QUOTE_SHELL_VAR = re.compile(rf'[\"\']\$({REGEX_SHELL_VAR_PATTERN})[\"\']') + # default options for modextravars + DEFAULT_MODEXTRAVARS_PUSHENV = False + DEFAULT_MODEXTRAVARS_SHELL_VARS = True + def __init__(self, application, fake=False): """ModuleGenerator constructor.""" self.app = application @@ -430,24 +434,25 @@ def unpack_setenv_value(self, env_var_name, env_var_val): """ Unpack value that specifies how to define an environment variable with specified name. """ - use_pushenv = False + use_pushenv = self.DEFAULT_MODEXTRAVARS_PUSHENV + shell_vars = self.DEFAULT_MODEXTRAVARS_SHELL_VARS # value may be specified as a string, or as a dict for special cases if isinstance(env_var_val, str): value = env_var_val - elif isinstance(env_var_val, dict): - use_pushenv = env_var_val.get('pushenv', False) + use_pushenv = env_var_val.get('pushenv', self.DEFAULT_MODEXTRAVARS_PUSHENV) + shell_vars = env_var_val.get('shell_vars', self.DEFAULT_MODEXTRAVARS_SHELL_VARS) try: value = env_var_val['value'] - except KeyError: + except KeyError as err: raise EasyBuildError("Required key 'value' is missing in dict that specifies how to set $%s: %s", - env_var_name, env_var_val) + env_var_name, env_var_val) from err else: raise EasyBuildError("Incorrect value type for setting $%s environment variable (%s): %s", env_var_name, type(env_var_val), env_var_val) - return value, use_pushenv + return value, use_pushenv, shell_vars # From this point on just not implemented methods @@ -1060,12 +1065,13 @@ def set_environment(self, key, value, relpath=False): self.log.info("Not including statement to define environment variable $%s, as specified", key) return '' - set_value, use_pushenv = self.unpack_setenv_value(key, value) + set_value, use_pushenv, shell_vars = self.unpack_setenv_value(key, value) if relpath: set_value = os.path.join('$root', set_value) if set_value else '$root' - set_value = self.REGEX_SHELL_VAR.sub(r'$::env(\1)', set_value) + if shell_vars: + set_value = self.REGEX_SHELL_VAR.sub(r'$::env(\1)', set_value) # quotes are needed, to ensure smooth working of EBDEVEL* modulefiles set_value = quote_str(set_value, tcl=True) @@ -1161,6 +1167,7 @@ class ModuleGeneratorLua(ModuleGenerator): START_STR = '[==[' END_STR = ']==]' + CONCAT_STR = ' .. ' def __init__(self, *args, **kwargs): """ModuleGeneratorLua constructor.""" @@ -1529,18 +1536,19 @@ def set_environment(self, key, value, relpath=False): self.log.info("Not including statement to define environment variable $%s, as specified", key) return '' - set_value, use_pushenv = self.unpack_setenv_value(key, value) + set_value, use_pushenv, shell_vars = self.unpack_setenv_value(key, value) if relpath: set_value = self._path_join_cmd(set_value) - set_value = self.REGEX_QUOTE_SHELL_VAR.sub(r'os.getenv("\1")', set_value) + if shell_vars: + set_value = self.REGEX_QUOTE_SHELL_VAR.sub(r'os.getenv("\1")', set_value) else: - lua_concat = ' .. ' - set_value = self.REGEX_SHELL_VAR.sub(rf'{lua_concat}os.getenv("\1"){lua_concat}', set_value) - set_value = lua_concat.join([ + if shell_vars: + set_value = self.REGEX_SHELL_VAR.sub(rf'{self.CONCAT_STR}os.getenv("\1"){self.CONCAT_STR}', set_value) + set_value = self.CONCAT_STR.join([ # quote any substrings that are not lua commands quote_str(x) if not x.startswith('os.') else x - for x in set_value.strip(lua_concat).split(lua_concat) + for x in set_value.strip(self.CONCAT_STR).split(self.CONCAT_STR) ]) env_setter = 'pushenv' if use_pushenv else 'setenv' diff --git a/test/framework/module_generator.py b/test/framework/module_generator.py index ad8b5c0912..711cd3ae60 100644 --- a/test/framework/module_generator.py +++ b/test/framework/module_generator.py @@ -983,6 +983,28 @@ def test_env(self): ("/abspath/with/$VAR", True, 'setenv\tkey\t\t"/abspath/with/$::env(VAR)"\n', 'setenv("key", pathJoin("/", "abspath", "with", os.getenv("VAR")))\n'), # noqa ("/abspath/$VAR/dir", False, 'setenv\tkey\t\t"/abspath/$::env(VAR)/dir"\n', 'setenv("key", "/abspath/" .. os.getenv("VAR") .. "/dir")\n'), # noqa ("/abspath/$VAR/dir", True, 'setenv\tkey\t\t"/abspath/$::env(VAR)/dir"\n', 'setenv("key", pathJoin("/", "abspath", os.getenv("VAR"), "dir"))\n'), # noqa + # modextravars defined with dicts + ({'value': 'value'}, False, 'setenv\tkey\t\t"value"\n', 'setenv("key", "value")\n'), # noqa + ({'value': 'value', + 'pushenv': True}, False, 'pushenv\tkey\t\t"value"\n', 'pushenv("key", "value")\n'), # noqa + ({'value': 'value', + 'pushenv': False}, False, 'setenv\tkey\t\t"value"\n', 'setenv("key", "value")\n'), # noqa + ({'value': "$VAR", + 'shell_vars': True}, False, 'setenv\tkey\t\t"$::env(VAR)"\n', 'setenv("key", os.getenv("VAR"))\n'), # noqa + ({'value': "$VAR", + 'shell_vars': True}, True, 'setenv\tkey\t\t"$root/$::env(VAR)"\n', 'setenv("key", pathJoin(root, os.getenv("VAR")))\n'), # noqa + ({'value': "$VAR", + 'shell_vars': False}, False, 'setenv\tkey\t\t"$VAR"\n', 'setenv("key", "$VAR")\n'), # noqa + ({'value': "$VAR", + 'shell_vars': False}, True, 'setenv\tkey\t\t"$root/$VAR"\n', 'setenv("key", pathJoin(root, "$VAR"))\n'), # noqa + ({'value': "path/$VAR/dir", + 'shell_vars': True}, False, 'setenv\tkey\t\t"path/$::env(VAR)/dir"\n', 'setenv("key", "path/" .. os.getenv("VAR") .. "/dir")\n'), # noqa + ({'value': "path/$VAR/dir", + 'shell_vars': True}, True, 'setenv\tkey\t\t"$root/path/$::env(VAR)/dir"\n', 'setenv("key", pathJoin(root, "path", os.getenv("VAR"), "dir"))\n'), # noqa + ({'value': "path/$VAR/dir", + 'shell_vars': False}, False, 'setenv\tkey\t\t"path/$VAR/dir"\n', 'setenv("key", "path/$VAR/dir")\n'), # noqa + ({'value': "path/$VAR/dir", + 'shell_vars': False}, True, 'setenv\tkey\t\t"$root/path/$VAR/dir"\n', 'setenv("key", pathJoin(root, "path", "$VAR", "dir"))\n'), # noqa ) # test set_environment for test_value, test_relpath, ref_tcl, ref_lua in collection: From 510e8f0942598ec8856ffb5d7fefd0c56fac235a Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Wed, 21 May 2025 13:54:26 +0200 Subject: [PATCH 11/13] minor changes to ModuleGenerator w.r.t. resolve environment variables in values of modextravars --- easybuild/tools/module_generator.py | 53 ++++++++++++++++++----------- 1 file changed, 33 insertions(+), 20 deletions(-) diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index 17b5abe005..4df2200b40 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -430,19 +430,26 @@ def det_installdir(self, modfile): return res - def unpack_setenv_value(self, env_var_name, env_var_val): + def unpack_setenv_value(self, *args, **kwargs): + """ + """ + self.log.deprecated("...", '6.0') + value, use_pushenv, _ = self._unpack_setenv_value(*args, **kwargs) + return value, use_pushenv + + def _unpack_setenv_value(self, env_var_name, env_var_val): """ Unpack value that specifies how to define an environment variable with specified name. """ use_pushenv = self.DEFAULT_MODEXTRAVARS_PUSHENV - shell_vars = self.DEFAULT_MODEXTRAVARS_SHELL_VARS + resolve_shell_vars = self.DEFAULT_MODEXTRAVARS_SHELL_VARS # value may be specified as a string, or as a dict for special cases if isinstance(env_var_val, str): value = env_var_val elif isinstance(env_var_val, dict): use_pushenv = env_var_val.get('pushenv', self.DEFAULT_MODEXTRAVARS_PUSHENV) - shell_vars = env_var_val.get('shell_vars', self.DEFAULT_MODEXTRAVARS_SHELL_VARS) + resolve_shell_vars = env_var_val.get('shell_vars', self.DEFAULT_MODEXTRAVARS_SHELL_VARS) try: value = env_var_val['value'] except KeyError as err: @@ -452,7 +459,7 @@ def unpack_setenv_value(self, env_var_name, env_var_val): raise EasyBuildError("Incorrect value type for setting $%s environment variable (%s): %s", env_var_name, type(env_var_val), env_var_val) - return value, use_pushenv, shell_vars + return value, use_pushenv, resolve_shell_vars # From this point on just not implemented methods @@ -1065,12 +1072,12 @@ def set_environment(self, key, value, relpath=False): self.log.info("Not including statement to define environment variable $%s, as specified", key) return '' - set_value, use_pushenv, shell_vars = self.unpack_setenv_value(key, value) + set_value, use_pushenv, resolve_shell_vars = self._unpack_setenv_value(key, value) if relpath: set_value = os.path.join('$root', set_value) if set_value else '$root' - if shell_vars: + if resolve_shell_vars: set_value = self.REGEX_SHELL_VAR.sub(r'$::env(\1)', set_value) # quotes are needed, to ensure smooth working of EBDEVEL* modulefiles @@ -1161,7 +1168,8 @@ class ModuleGeneratorLua(ModuleGenerator): LOAD_TEMPLATE_DEPENDS_ON = 'depends_on("%(mod_name)s")' IS_LOADED_TEMPLATE = 'isloaded("%s")' - PATH_JOIN_TEMPLATE = 'pathJoin(root, "%s")' # TODO: remove after 6.0, replaced by _path_join_cmd() + OS_GETENV_TEMPLATE = r'os.getenv("%s")' + PATH_JOIN_TEMPLATE = 'pathJoin(root, "%s")' UPDATE_PATH_TEMPLATE = '%s_path("%s", %s)' UPDATE_PATH_TEMPLATE_DELIM = '%s_path("%s", %s, "%s")' @@ -1186,9 +1194,10 @@ def _path_join_cmd(path): path_components.insert(0, path_root) if len(path_components) > 1: - return f'pathJoin({", ".join(path_components)})' + return 'pathJoin(' + ', '.join(path_components) + ')' + # no need for a pathJoin for single component paths - return os.path.join(*path_components) + return path_components[0] def check_version(self, minimal_version_maj, minimal_version_min, minimal_version_patch='0'): """ @@ -1315,10 +1324,9 @@ def getenv_cmd(self, envvar, default=None): """ Return module-syntax specific code to get value of specific environment variable. """ - if default is None: - cmd = 'os.getenv("%s")' % envvar - else: - cmd = 'os.getenv("%s") or "%s"' % (envvar, default) + cmd = self.OS_GETENV_TEMPLATE % envvar + if default is not None: + cmd += f' or "{default}"' return cmd def load_module(self, mod_name, recursive_unload=None, depends_on=None, unload_modules=None, multi_dep_mods=None): @@ -1536,18 +1544,23 @@ def set_environment(self, key, value, relpath=False): self.log.info("Not including statement to define environment variable $%s, as specified", key) return '' - set_value, use_pushenv, shell_vars = self.unpack_setenv_value(key, value) + set_value, use_pushenv, resolve_shell_vars = self._unpack_setenv_value(key, value) if relpath: set_value = self._path_join_cmd(set_value) - if shell_vars: - set_value = self.REGEX_QUOTE_SHELL_VAR.sub(r'os.getenv("\1")', set_value) + if resolve_shell_vars: + # replace quoted substring with env var with os.getenv statement + # example: pathJoin(root, "$HOME") -> pathJoin(root, os.getenv("HOME")) + set_value = self.REGEX_QUOTE_SHELL_VAR.sub(self.OS_GETENV_TEMPLATE % r"\1", set_value) else: - if shell_vars: - set_value = self.REGEX_SHELL_VAR.sub(rf'{self.CONCAT_STR}os.getenv("\1"){self.CONCAT_STR}', set_value) + if resolve_shell_vars: + # replace env var with os.getenv statement + # example: $HOME -> os.getenv("HOME") + concat_getenv = self.CONCAT_STR + self.OS_GETENV_TEMPLATE % r"\1" + self.CONCAT_STR + set_value = self.REGEX_SHELL_VAR.sub(concat_getenv, set_value) set_value = self.CONCAT_STR.join([ - # quote any substrings that are not lua commands - quote_str(x) if not x.startswith('os.') else x + # quote any substrings that are not a os.getenv Lua statement + x if x.startswith(self.OS_GETENV_TEMPLATE[:10]) else quote_str(x) for x in set_value.strip(self.CONCAT_STR).split(self.CONCAT_STR) ]) From b3842b4c2b945f8de8bb70a6d22ca3956db8a46e Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Wed, 21 May 2025 14:12:04 +0200 Subject: [PATCH 12/13] fix docstring + deprecation warning for ModuleGenerator.unpack_setenv_value --- easybuild/tools/module_generator.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index 4df2200b40..5582af2375 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -432,8 +432,10 @@ def det_installdir(self, modfile): def unpack_setenv_value(self, *args, **kwargs): """ + DEPRECATED method, should not be used. + Replaced with (internal) _unpack_setenv_value method. """ - self.log.deprecated("...", '6.0') + self.log.deprecated("unpack_setenv_value should not be used directly (replaced by internal method)", '6.0') value, use_pushenv, _ = self._unpack_setenv_value(*args, **kwargs) return value, use_pushenv From db919e4b3d6a3c23b062649fb367e2376d831af4 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Wed, 21 May 2025 14:40:00 +0200 Subject: [PATCH 13/13] rename 'shell_vars' options in modextravars to 'resolve_env_vars' (+ rename DEFAULT_MODEXTRAVARS* constants in ModuleGenerator class) --- easybuild/tools/module_generator.py | 24 ++++++++++++------------ test/framework/module_generator.py | 16 ++++++++-------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index 5582af2375..4f4b5aefbf 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -138,8 +138,8 @@ class ModuleGenerator: REGEX_QUOTE_SHELL_VAR = re.compile(rf'[\"\']\$({REGEX_SHELL_VAR_PATTERN})[\"\']') # default options for modextravars - DEFAULT_MODEXTRAVARS_PUSHENV = False - DEFAULT_MODEXTRAVARS_SHELL_VARS = True + DEFAULT_MODEXTRAVARS_USE_PUSHENV = False + DEFAULT_MODEXTRAVARS_RESOLVE_ENV_VARS = True def __init__(self, application, fake=False): """ModuleGenerator constructor.""" @@ -443,15 +443,15 @@ def _unpack_setenv_value(self, env_var_name, env_var_val): """ Unpack value that specifies how to define an environment variable with specified name. """ - use_pushenv = self.DEFAULT_MODEXTRAVARS_PUSHENV - resolve_shell_vars = self.DEFAULT_MODEXTRAVARS_SHELL_VARS + use_pushenv = self.DEFAULT_MODEXTRAVARS_USE_PUSHENV + resolve_env_vars = self.DEFAULT_MODEXTRAVARS_RESOLVE_ENV_VARS # value may be specified as a string, or as a dict for special cases if isinstance(env_var_val, str): value = env_var_val elif isinstance(env_var_val, dict): - use_pushenv = env_var_val.get('pushenv', self.DEFAULT_MODEXTRAVARS_PUSHENV) - resolve_shell_vars = env_var_val.get('shell_vars', self.DEFAULT_MODEXTRAVARS_SHELL_VARS) + use_pushenv = env_var_val.get('pushenv', self.DEFAULT_MODEXTRAVARS_USE_PUSHENV) + resolve_env_vars = env_var_val.get('resolve_env_vars', self.DEFAULT_MODEXTRAVARS_RESOLVE_ENV_VARS) try: value = env_var_val['value'] except KeyError as err: @@ -461,7 +461,7 @@ def _unpack_setenv_value(self, env_var_name, env_var_val): raise EasyBuildError("Incorrect value type for setting $%s environment variable (%s): %s", env_var_name, type(env_var_val), env_var_val) - return value, use_pushenv, resolve_shell_vars + return value, use_pushenv, resolve_env_vars # From this point on just not implemented methods @@ -1074,12 +1074,12 @@ def set_environment(self, key, value, relpath=False): self.log.info("Not including statement to define environment variable $%s, as specified", key) return '' - set_value, use_pushenv, resolve_shell_vars = self._unpack_setenv_value(key, value) + set_value, use_pushenv, resolve_env_vars = self._unpack_setenv_value(key, value) if relpath: set_value = os.path.join('$root', set_value) if set_value else '$root' - if resolve_shell_vars: + if resolve_env_vars: set_value = self.REGEX_SHELL_VAR.sub(r'$::env(\1)', set_value) # quotes are needed, to ensure smooth working of EBDEVEL* modulefiles @@ -1546,16 +1546,16 @@ def set_environment(self, key, value, relpath=False): self.log.info("Not including statement to define environment variable $%s, as specified", key) return '' - set_value, use_pushenv, resolve_shell_vars = self._unpack_setenv_value(key, value) + set_value, use_pushenv, resolve_env_vars = self._unpack_setenv_value(key, value) if relpath: set_value = self._path_join_cmd(set_value) - if resolve_shell_vars: + if resolve_env_vars: # replace quoted substring with env var with os.getenv statement # example: pathJoin(root, "$HOME") -> pathJoin(root, os.getenv("HOME")) set_value = self.REGEX_QUOTE_SHELL_VAR.sub(self.OS_GETENV_TEMPLATE % r"\1", set_value) else: - if resolve_shell_vars: + if resolve_env_vars: # replace env var with os.getenv statement # example: $HOME -> os.getenv("HOME") concat_getenv = self.CONCAT_STR + self.OS_GETENV_TEMPLATE % r"\1" + self.CONCAT_STR diff --git a/test/framework/module_generator.py b/test/framework/module_generator.py index 711cd3ae60..0196860d40 100644 --- a/test/framework/module_generator.py +++ b/test/framework/module_generator.py @@ -990,21 +990,21 @@ def test_env(self): ({'value': 'value', 'pushenv': False}, False, 'setenv\tkey\t\t"value"\n', 'setenv("key", "value")\n'), # noqa ({'value': "$VAR", - 'shell_vars': True}, False, 'setenv\tkey\t\t"$::env(VAR)"\n', 'setenv("key", os.getenv("VAR"))\n'), # noqa + 'resolve_env_vars': True}, False, 'setenv\tkey\t\t"$::env(VAR)"\n', 'setenv("key", os.getenv("VAR"))\n'), # noqa ({'value': "$VAR", - 'shell_vars': True}, True, 'setenv\tkey\t\t"$root/$::env(VAR)"\n', 'setenv("key", pathJoin(root, os.getenv("VAR")))\n'), # noqa + 'resolve_env_vars': True}, True, 'setenv\tkey\t\t"$root/$::env(VAR)"\n', 'setenv("key", pathJoin(root, os.getenv("VAR")))\n'), # noqa ({'value': "$VAR", - 'shell_vars': False}, False, 'setenv\tkey\t\t"$VAR"\n', 'setenv("key", "$VAR")\n'), # noqa + 'resolve_env_vars': False}, False, 'setenv\tkey\t\t"$VAR"\n', 'setenv("key", "$VAR")\n'), # noqa ({'value': "$VAR", - 'shell_vars': False}, True, 'setenv\tkey\t\t"$root/$VAR"\n', 'setenv("key", pathJoin(root, "$VAR"))\n'), # noqa + 'resolve_env_vars': False}, True, 'setenv\tkey\t\t"$root/$VAR"\n', 'setenv("key", pathJoin(root, "$VAR"))\n'), # noqa ({'value': "path/$VAR/dir", - 'shell_vars': True}, False, 'setenv\tkey\t\t"path/$::env(VAR)/dir"\n', 'setenv("key", "path/" .. os.getenv("VAR") .. "/dir")\n'), # noqa + 'resolve_env_vars': True}, False, 'setenv\tkey\t\t"path/$::env(VAR)/dir"\n', 'setenv("key", "path/" .. os.getenv("VAR") .. "/dir")\n'), # noqa ({'value': "path/$VAR/dir", - 'shell_vars': True}, True, 'setenv\tkey\t\t"$root/path/$::env(VAR)/dir"\n', 'setenv("key", pathJoin(root, "path", os.getenv("VAR"), "dir"))\n'), # noqa + 'resolve_env_vars': True}, True, 'setenv\tkey\t\t"$root/path/$::env(VAR)/dir"\n', 'setenv("key", pathJoin(root, "path", os.getenv("VAR"), "dir"))\n'), # noqa ({'value': "path/$VAR/dir", - 'shell_vars': False}, False, 'setenv\tkey\t\t"path/$VAR/dir"\n', 'setenv("key", "path/$VAR/dir")\n'), # noqa + 'resolve_env_vars': False}, False, 'setenv\tkey\t\t"path/$VAR/dir"\n', 'setenv("key", "path/$VAR/dir")\n'), # noqa ({'value': "path/$VAR/dir", - 'shell_vars': False}, True, 'setenv\tkey\t\t"$root/path/$VAR/dir"\n', 'setenv("key", pathJoin(root, "path", "$VAR", "dir"))\n'), # noqa + 'resolve_env_vars': False}, True, 'setenv\tkey\t\t"$root/path/$VAR/dir"\n', 'setenv("key", pathJoin(root, "path", "$VAR", "dir"))\n'), # noqa ) # test set_environment for test_value, test_relpath, ref_tcl, ref_lua in collection: