Merge pull request #5045 from Flamefire/silent-module-cmd

Add `--debug-module-cmds` option

Author: Crivella

easybuild/framework/easyblock.py

@@ -1927,7 +1927,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:

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',

easybuild/tools/modules.py

@@ -1130,12 +1130,19 @@ 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, log_changes=None, *, hide_output=None):
         """
         Unload all requested modules.
         """
+        if log_changes is not None:
+            if hide_output is not None:
+                raise EasyBuildError("Cannot specify both log_changes and hide_output")
+            self.log.deprecated("Parameter 'log_changes' is replaced by 'hide_output'", '6.0')
+            hide_output = not log_changes
+        elif hide_output is None:
+            hide_output = False  # TODO: make this the default in 6.0
         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 +1266,23 @@ 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_changes = kwargs.get('log_changes')
+        hide_output = kwargs.get('hide_output')
+
+        if log_changes is not None:
+            if hide_output is not None:
+                raise EasyBuildError("Cannot specify both log_changes and hide_output")
+            self.log.deprecated("Parameter 'log_changes' is replaced by 'hide_output'", '6.0')
+            hide_output = not log_changes
+
+        log_output = debug_module_cmds and not hide_output
         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 +1329,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 +1712,9 @@ class EnvironmentModulesTcl(EnvironmentModulesC):
     DEPR_VERSION = '9999.9'
     VERSION_REGEXP = r'^Modules\s+Release\s+Tcl\s+(?P<version>\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 +2280,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, ...)
     """

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.",

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

test/framework/options.py

@@ -3228,7 +3228,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)
@@ -3243,7 +3243,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
@@ -3259,7 +3259,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([
@@ -3274,7 +3274,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([
@@ -3288,7 +3288,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))
@@ -3301,7 +3301,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)
@@ -5682,6 +5682,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'))

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()