diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index c6fade8006..2503bb7f04 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1919,7 +1919,7 @@ def clean_up_fake_module(self, fake_mod_data): # self.short_mod_name might not be set (e.g. during unit tests) if fake_mod_path and self.short_mod_name is not None: try: - self.modules_tool.unload([self.short_mod_name], log_changes=False) + self.modules_tool.unload([self.short_mod_name], hide_output=True) self.modules_tool.remove_module_path(os.path.join(fake_mod_path, self.mod_subdir)) remove_dir(os.path.dirname(fake_mod_path)) except OSError as err: diff --git a/easybuild/tools/config.py b/easybuild/tools/config.py index b27d238bd5..3950f96ba3 100644 --- a/easybuild/tools/config.py +++ b/easybuild/tools/config.py @@ -312,6 +312,7 @@ def mk_full_default_path(name, prefix=DEFAULT_PREFIX): 'cuda_sanity_check_strict', 'debug', 'debug_lmod', + 'debug_module_cmds', 'dump_autopep8', 'dump_env_script', 'enforce_checksums', diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py index 75fd040200..508e7bc15b 100644 --- a/easybuild/tools/modules.py +++ b/easybuild/tools/modules.py @@ -1130,12 +1130,12 @@ def load(self, modules, mod_paths=None, purge=False, init_env=None, allow_reload for mod in modules: self.run_module('load', mod) - def unload(self, modules, log_changes=True): + def unload(self, modules, hide_output=False): """ Unload all requested modules. """ for mod in modules: - self.run_module('unload', mod, log_changes=log_changes) + self.run_module('unload', mod, hide_output=hide_output) def purge(self): """ @@ -1259,13 +1259,14 @@ def run_module(self, *args, **kwargs): self.log.debug("Changing %s from '%s' to '%s' in environment for module command", key, old_value, new_value) - log_changes = kwargs.get('log_changes', True) + debug_module_cmds = build_option('debug_module_cmds') + log_output = debug_module_cmds and not kwargs.get('hide_output', False) cmd_list = self.compose_cmd_list(args) cmd = ' '.join(cmd_list) # note: module commands are always run in dry mode, and are kept hidden in trace and dry run output res = run_shell_cmd(cmd_list, env=environ, fail_on_error=False, use_bash=False, split_stderr=True, - hidden=True, in_dry_run=True, output_file=False, - log_output_on_success=log_changes) + hidden=True, in_dry_run=True, output_file=debug_module_cmds, + log_output_on_success=log_output) # stdout will contain python code (to change environment etc) # stderr will contain text (just like the normal module command) @@ -1312,7 +1313,7 @@ def run_module(self, *args, **kwargs): if new_ld_val != curr_ld_val: self.log.debug("Correcting paths in $%s from %s to %s" % (key, curr_ld_val, new_ld_val)) - self.set_path_env_var(key, new_ld_val) + self.set_path_env_var(key, new_ld_val, log_changes=log_output) # Process stderr result = [] @@ -1695,9 +1696,9 @@ class EnvironmentModulesTcl(EnvironmentModulesC): DEPR_VERSION = '9999.9' VERSION_REGEXP = r'^Modules\s+Release\s+Tcl\s+(?P\d\S*)\s' - def set_path_env_var(self, key, paths): + def set_path_env_var(self, key, paths, log_changes=True): """Set environment variable with given name to the given list of paths.""" - super().set_path_env_var(key, paths) + super().set_path_env_var(key, paths, log_changes) # for Tcl Environment Modules, we need to make sure the _modshare env var is kept in sync setvar('%s_modshare' % key, ':1:'.join(paths), verbose=False) @@ -2263,7 +2264,7 @@ def avail_modules_tools(): return class_dict -def modules_tool(mod_paths=None, testing=False): +def modules_tool(mod_paths=None, testing=False) -> ModulesTool: """ Return interface to modules tool (EnvironmentModules, Lmod, ...) """ diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index f4856de579..edc9b74bc3 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -438,6 +438,7 @@ def override_options(self): "more compute capabilities than defined in --cuda-compute-capabilities.", None, 'store_true', False), 'debug-lmod': ("Run Lmod modules tool commands in debug module", None, 'store_true', False), + 'debug-module-cmds': ("Show additional information of module commands", None, 'store_true', False), 'default-opt-level': ("Specify default optimisation level", 'choice', 'store', DEFAULT_OPT_LEVEL, Compiler.COMPILER_OPT_OPTIONS), 'deprecated': ("Run pretending to be (future) version, to test removal of deprecated code.", diff --git a/test/framework/modules.py b/test/framework/modules.py index b69bb69c3e..64aa5d7489 100644 --- a/test/framework/modules.py +++ b/test/framework/modules.py @@ -1070,13 +1070,13 @@ def test_modules_tool_stateless(self): modtxt = read_file(os.path.join(self.test_prefix, 'Core', 'GCC', '6.4.0-2.28')) modpath_extension = os.path.join(self.test_prefix, 'Compiler', 'GCC', '6.4.0-2.28') - modtxt = re.sub('module use .*', 'module use %s' % modpath_extension, modtxt, re.M) + modtxt = re.sub('module use .*', 'module use %s' % modpath_extension, modtxt, flags=re.M) write_file(os.path.join(self.test_prefix, 'Core', 'GCC', '6.4.0-2.28'), modtxt) modtxt = read_file(os.path.join(self.test_prefix, 'Compiler', 'GCC', '6.4.0-2.28', 'OpenMPI', '2.1.2')) modpath_extension = os.path.join(self.test_prefix, 'MPI', 'GCC', '6.4.0-2.28', 'OpenMPI', '2.1.2') mkdir(modpath_extension, parents=True) - modtxt = re.sub('module use .*', 'module use %s' % modpath_extension, modtxt, re.M) + modtxt = re.sub('module use .*', 'module use %s' % modpath_extension, modtxt, flags=re.M) write_file(os.path.join(self.test_prefix, 'Compiler', 'GCC', '6.4.0-2.28', 'OpenMPI', '2.1.2'), modtxt) # force reset of any singletons by reinitiating config diff --git a/test/framework/options.py b/test/framework/options.py index 9ca3bc07f4..4b5b6fd34b 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -1529,11 +1529,11 @@ def test_copy_ec_from_commit(self): stdout = self.mocked_main(args) except URLError as err: print("Ignoring URLError '%s' in test_copy_ec_from_commit" % err) - - pattern = "_%s/e/EasyBuild/EasyBuild-4.8.2.eb copied to " % test_commit - self.assertIn(pattern, stdout) - copied_ecs = os.listdir(test_dir) - self.assertEqual(copied_ecs, ['EasyBuild-4.8.2.eb']) + else: + pattern = "_%s/e/EasyBuild/EasyBuild-4.8.2.eb copied to " % test_commit + self.assertIn(pattern, stdout) + copied_ecs = os.listdir(test_dir) + self.assertEqual(copied_ecs, ['EasyBuild-4.8.2.eb']) # cleanup remove_dir(test_dir) @@ -1547,10 +1547,10 @@ def test_copy_ec_from_commit(self): stdout = self.mocked_main(args) except URLError as err: print("Ignoring URLError '%s' in test_copy_ec_from_commit" % err) - - self.assertIn(pattern, stdout) - copied_ecs = os.listdir(test_dir) - self.assertEqual(copied_ecs, ['EasyBuild-4.8.2.eb']) + else: + self.assertIn(pattern, stdout) + copied_ecs = os.listdir(test_dir) + self.assertEqual(copied_ecs, ['EasyBuild-4.8.2.eb']) # cleanup change_dir(cwd) @@ -1574,10 +1574,10 @@ def test_copy_ec_from_commit(self): stdout = self.mocked_main(args) except URLError as err: print("Ignoring URLError '%s' in test_copy_ec_from_commit" % err) - - copied_ecs = os.listdir(test_dir) - for ec in expected_ecs: - self.assertIn(ec, copied_ecs) + else: + copied_ecs = os.listdir(test_dir) + for ec in expected_ecs: + self.assertIn(ec, copied_ecs) def test_dry_run(self): """Test dry run (long format).""" @@ -2063,7 +2063,6 @@ def test_github_from_pr(self): self.assertRegex(outtxt, r"Extended list of robot search paths with \['%s'\]:" % pr_tmpdir) except URLError as err: print("Ignoring URLError '%s' in test_from_pr" % err) - shutil.rmtree(tmpdir) # test with multiple prs tmpdir = tempfile.mkdtemp() @@ -2096,7 +2095,6 @@ def test_github_from_pr(self): except URLError as err: print("Ignoring URLError '%s' in test_from_pr" % err) - shutil.rmtree(tmpdir) def test_github_from_pr_token_log(self): """Check that --from-pr doesn't leak GitHub token in log.""" @@ -2180,7 +2178,6 @@ def test_github_from_pr_listed_ecs(self): except URLError as err: print("Ignoring URLError '%s' in test_from_pr" % err) - shutil.rmtree(tmpdir) def test_github_from_pr_x(self): """Test combination of --from-pr with --extended-dry-run.""" @@ -2304,7 +2301,6 @@ def test_from_commit(self): self.assertRegex(outtxt, r"Extended list of robot search paths with \['%s'\]:" % pr_tmpdir) except URLError as err: print("Ignoring URLError '%s' in test_from_commit" % err) - shutil.rmtree(tmpdir) # must be run after test for --list-easyblocks, hence the '_xxx_' # cleaning up the imported easyblocks is quite difficult... @@ -2356,7 +2352,6 @@ def test_xxx_include_easyblocks_from_commit(self): except URLError as err: print("Ignoring URLError '%s' in test_include_easyblocks_from_commit" % err) - shutil.rmtree(tmpdir) def test_no_such_software(self): """Test using no arguments.""" @@ -3232,7 +3227,7 @@ def test_http_header_fields_urlpat(self): mentionhdr = 'Custom HTTP header field set: %s' mentionfile = 'File included in parse_http_header_fields_urlpat: %s' - def run_and_assert(args, _msg, words_expected=None, words_unexpected=None): + def run_and_assert(args, words_expected=None, words_unexpected=None): stdout, _stderr = self._run_mock_eb(args, do_build=True, raise_error=True, testing=False) if words_expected is not None: self.assert_multi_regex(words_expected, stdout) @@ -3247,7 +3242,7 @@ def run_and_assert(args, _msg, words_expected=None, words_unexpected=None): ]) # expect to find everything passed on cmdline expected = [mentionhdr % (testdohdr), testdoval, testdonthdr, testdontval] - run_and_assert(args, "case A", expected) + run_and_assert(args, expected) # all subsequent tests share this argument list args = common_args @@ -3263,7 +3258,7 @@ def run_and_assert(args, _msg, words_expected=None, words_unexpected=None): # expect to find only the header key (not its value) and only for the appropriate url expected = [mentionhdr % testdohdr, mentionfile % testcmdfile] not_expected = [testdoval, testdonthdr, testdontval] - run_and_assert(args, "case B", expected, not_expected) + run_and_assert(args, expected, not_expected) # C: recursion one: header value is another file txt = '\n'.join([ @@ -3278,7 +3273,7 @@ def run_and_assert(args, _msg, words_expected=None, words_unexpected=None): expected = [mentionhdr % (testdohdr), mentionfile % (testcmdfile), mentionfile % (testincfile), mentionfile % (testexcfile)] not_expected = [testdoval, testdonthdr, testdontval] - run_and_assert(args, "case C", expected, not_expected) + run_and_assert(args, expected, not_expected) # D: recursion two: header field+value is another file, write_file(testcmdfile, '\n'.join([ @@ -3292,7 +3287,7 @@ def run_and_assert(args, _msg, words_expected=None, words_unexpected=None): expected = [mentionhdr % (testdohdr), mentionfile % (testcmdfile), mentionfile % (testinchdrfile), mentionfile % (testexchdrfile)] not_expected = [testdoval, testdonthdr, testdontval] - run_and_assert(args, "case D", expected, not_expected) + run_and_assert(args, expected, not_expected) # E: recursion three: url pattern + header field + value in another file write_file(testcmdfile, '%s\n' % (testurlpatfile)) @@ -3305,7 +3300,7 @@ def run_and_assert(args, _msg, words_expected=None, words_unexpected=None): # expect to find only the header key (but not the literal filename) and only for the appropriate url expected = [mentionhdr % (testdohdr), mentionfile % (testcmdfile), mentionfile % (testurlpatfile)] not_expected = [testdoval, testdonthdr, testdontval] - run_and_assert(args, "case E", expected, not_expected) + run_and_assert(args, expected, not_expected) # cleanup downloads shutil.rmtree(tmpdir) @@ -5686,6 +5681,23 @@ def test_debug_lmod(self): else: print("Skipping test_debug_lmod, requires Lmod as modules tool") + def test_debug_module_cmds(self): + """Test use of --debug-module-cmds.""" + patterns = [ + "Output of ", + r"os\.env.*EBROOTGCC.*=", + r"os\.env.*PATH.*=", + ] + for enable in (True, False): + with self.subTest(debug_module_cmds=enable): + init_config(build_options={'debug_module_cmds': enable}) + with self.log_to_testlogfile(): + self.modtool.load(['GCC/4.6.3']) + logtxt = read_file(self.logfile) + self.assertRegex(logtxt, "Running .*\n.*load GCC/4.6.3") + self.assert_multi_regex(patterns, logtxt, assert_true=enable) + self.modtool.purge() + def test_use_color(self): """Test use_color function.""" self.assertTrue(use_color('always')) diff --git a/test/framework/utilities.py b/test/framework/utilities.py index 1807085d75..1d0fe1ce38 100644 --- a/test/framework/utilities.py +++ b/test/framework/utilities.py @@ -53,7 +53,7 @@ from easybuild.tools.configobj import ConfigObj from easybuild.tools.environment import modify_env from easybuild.tools.filetools import copy_dir, mkdir, read_file, which -from easybuild.tools.modules import curr_module_paths, modules_tool, reset_module_caches +from easybuild.tools.modules import curr_module_paths, modules_tool, ModulesTool, reset_module_caches from easybuild.tools.options import CONFIG_ENV_VAR_PREFIX, EasyBuildOptions, set_tmpdir @@ -211,7 +211,7 @@ def setUp(self): self.env_path = os.environ.get('PATH') self.env_pythonpath = os.environ.get('PYTHONPATH') - self.modtool = modules_tool() + self.modtool: ModulesTool = modules_tool() self.reset_modulepath([os.path.join(testdir, 'modules')]) reset_module_caches()