From 5243d57cc7535bee9df85f8d0c1ae99fdb281b9c Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Mon, 20 Jan 2025 16:54:45 +0100 Subject: [PATCH 1/2] add TODO and warning about non-path variables not being currently handled in module load environment --- easybuild/framework/easyblock.py | 19 ++++++++++++++----- easybuild/tools/modules.py | 1 + test/framework/easyblock.py | 22 +++++++++++++++++++++- 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 10ab4dcfbb..903a35eb6d 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1655,14 +1655,23 @@ def make_module_req(self): ) self.module_load_environment.update(self.make_module_req_guess()) - # Only inject path-like environment variables into module file - env_var_requirements = sorted([ - (envar_name, envar_val) for envar_name, envar_val in self.module_load_environment.items() + # Expand and inject path-like environment variables into module file + env_var_requirements = { + envar_name: envar_val + for envar_name, envar_val in sorted(self.module_load_environment.items()) if envar_val.is_path - ]) + } self.log.debug(f"Tentative module environment requirements before path expansion: {env_var_requirements}") + # TODO: handle non-path variables in make_module_extra + # in the meantime, just report if any is found + non_path_envars = set(self.module_load_environment) - set(env_var_requirements) + if non_path_envars: + self.log.warning( + f"Non-path variables found in module load environment: {non_path_envars}." + "This is not yet supported by this version of EasyBuild." + ) - for env_var, search_paths in env_var_requirements: + for env_var, search_paths in 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 diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index e063c29b15..6c3c4f29c8 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -272,6 +272,7 @@ def __setattr__(self, name, value): """ if name != name.upper(): raise EasyBuildError(f"Names of ModuleLoadEnvironment attributes must be uppercase, got '{name}'") + try: (contents, kwargs) = value except ValueError: diff --git a/test/framework/easyblock.py b/test/framework/easyblock.py index 009f1ae281..6a62b6cd05 100644 --- a/test/framework/easyblock.py +++ b/test/framework/easyblock.py @@ -592,7 +592,8 @@ def test_make_module_req(self): full_path = os.path.join(eb.installdir, path, 'subpath') os.makedirs(full_path) write_file(os.path.join(full_path, 'any.file'), 'test') - txt = eb.make_module_req() + with eb.module_generator.start_module_creation(): + txt = eb.make_module_req() if get_module_syntax() == 'Tcl': self.assertFalse(re.search(r"prepend-path\s+LD_LIBRARY_PATH\s+\$%s\n" % sub_lib_path, txt, re.M)) @@ -603,6 +604,25 @@ def test_make_module_req(self): txt, re.M)) self.assertFalse(re.search(r'prepend_path\("PATH", pathJoin\(root, "%s"\)\)\n' % sub_path_path, txt, re.M)) + # Module load environement may contain non-path variables + # TODO: remove whenever this is properly supported, in the meantime check warning + eb.module_load_environment.NONPATH = ('non_path_variable', {'var_type': "STRING"}) + eb.module_load_environment.PATH = ['bin'] + with self.mocked_stdout_stderr(): + txt = eb.make_module_req() + + self.assertEqual(list(eb.module_load_environment), ['PATH', 'LD_LIBRARY_PATH', 'NONPATH']) + + if get_module_syntax() == 'Tcl': + self.assertTrue(re.match(r"^\nprepend-path\s+PATH\s+\$root/bin\n$", txt, re.M)) + elif get_module_syntax() == 'Lua': + self.assertTrue(re.match(r'^\nprepend_path\("PATH", pathJoin\(root, "bin"\)\)\n$', txt, re.M)) + else: + self.fail("Unknown module syntax: %s" % get_module_syntax()) + + logtxt = read_file(eb.logfile) + self.assertTrue(re.search(r"WARNING Non-path variables found in module load env.*NONPATH", logtxt, re.M)) + # cleanup eb.close_log() os.remove(eb.logfile) From 64bd0b3c6dbbdc509a955ed6e7734c41736c1903 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Mon, 20 Jan 2025 18:51:55 +0100 Subject: [PATCH 2/2] test that NONPATH variable is not added to module file by make_module_req --- test/framework/easyblock.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/framework/easyblock.py b/test/framework/easyblock.py index 6a62b6cd05..1b70a03d7a 100644 --- a/test/framework/easyblock.py +++ b/test/framework/easyblock.py @@ -606,7 +606,7 @@ def test_make_module_req(self): # Module load environement may contain non-path variables # TODO: remove whenever this is properly supported, in the meantime check warning - eb.module_load_environment.NONPATH = ('non_path_variable', {'var_type': "STRING"}) + eb.module_load_environment.NONPATH = ('non_path', {'var_type': "STRING"}) eb.module_load_environment.PATH = ['bin'] with self.mocked_stdout_stderr(): txt = eb.make_module_req() @@ -615,8 +615,10 @@ def test_make_module_req(self): if get_module_syntax() == 'Tcl': self.assertTrue(re.match(r"^\nprepend-path\s+PATH\s+\$root/bin\n$", txt, re.M)) + self.assertFalse(re.match(r"^\nprepend-path\s+NONPATH\s+\$root/non_path\n$", txt, re.M)) elif get_module_syntax() == 'Lua': self.assertTrue(re.match(r'^\nprepend_path\("PATH", pathJoin\(root, "bin"\)\)\n$', txt, re.M)) + self.assertFalse(re.match(r'^\nprepend_path\("NONPATH", pathJoin\(root, "non_path"\)\)\n$', txt, re.M)) else: self.fail("Unknown module syntax: %s" % get_module_syntax())