Merge pull request #4666 from boegel/run_shell_cmd_cmd.sh_script

include path to `cmd.sh` script in output generated by `run_shell_cmd` when a command fails + use colors: red for ERROR line, yellow for path to output files + `cmd.sh` script

Author: branfosj

easybuild/tools/output.py

@@ -33,6 +33,7 @@
 """
 import functools
 from collections import OrderedDict
+import sys
 
 from easybuild.tools.build_log import EasyBuildError
 from easybuild.tools.config import OUTPUT_STYLE_RICH, build_option, get_output_style
@@ -328,9 +329,7 @@ def print_checks(checks_data):
 
     if use_rich():
         console = Console()
-        # don't use console.print, which causes SyntaxError in Python 2
-        console_print = getattr(console, 'print')  # noqa: B009
-        console_print('')
+        console.print('')
 
     for section in checks_data:
         section_checks = checks_data[section]
@@ -382,11 +381,25 @@ def print_checks(checks_data):
             lines.append('')
 
         if use_rich():
-            console_print(table)
+            console.print(table)
         else:
             print('\n'.join(lines))
 
 
+def print_error(error_msg, rich_highlight=True):
+    """
+    Print error message, using a Rich Console instance if possible.
+    Newlines before/after message are automatically added.
+
+    :param rich_highlight: boolean indicating whether automatic highlighting by Rich should be enabled
+    """
+    if use_rich():
+        console = Console(stderr=True)
+        console.print('\n\n' + error_msg + '\n', highlight=rich_highlight)
+    else:
+        sys.stderr.write('\n' + error_msg + '\n\n')
+
+
 # this constant must be defined at the end, since functions used as values need to be defined
 PROGRESS_BAR_TYPES = {
     PROGRESS_BAR_DOWNLOAD_ALL: download_all_progress_bar,

easybuild/tools/run.py

@@ -45,7 +45,6 @@
 import shutil
 import string
 import subprocess
-import sys
 import tempfile
 import time
 from collections import namedtuple
@@ -68,6 +67,7 @@
 from easybuild.tools.build_log import dry_run_msg, print_msg, time_str_since
 from easybuild.tools.config import build_option
 from easybuild.tools.hooks import RUN_SHELL_CMD, load_hooks, run_hook
+from easybuild.tools.output import COLOR_RED, COLOR_YELLOW, colorize, print_error
 from easybuild.tools.utilities import trace_msg
 
 
@@ -86,7 +86,20 @@
 )
 
 RunShellCmdResult = namedtuple('RunShellCmdResult', ('cmd', 'exit_code', 'output', 'stderr', 'work_dir',
-                                                     'out_file', 'err_file', 'thread_id', 'task_id'))
+                                                     'out_file', 'err_file', 'cmd_sh', 'thread_id', 'task_id'))
+RunShellCmdResult.__doc__ = """A namedtuple that represents the result of a call to run_shell_cmd,
+with the following fields:
+- cmd: the command that was executed;
+- exit_code: the exit code of the command (zero if it was successful, non-zero if not);
+- output: output of the command (stdout+stderr combined, only stdout if stderr was caught separately);
+- stderr: stderr output produced by the command, if caught separately (None otherwise);
+- work_dir: the working directory of the command;
+- out_file: path to file with output of command (stdout+stderr combined, only stdout if stderr was caught separately);
+- err_file: path to file with stderr output of command, if caught separately (None otherwise);
+- cmd_sh: path to script to set up interactive shell with environment in which command was executed;
+- thread_id: thread ID of command that was executed (None unless asynchronous mode was enabled for running command);
+- task_id: task ID of command, if it was specified (None otherwise);
+"""
 
 
 class RunShellCmdError(BaseException):
@@ -101,6 +114,7 @@ def __init__(self, cmd_result, caller_info, *args, **kwargs):
         self.out_file = cmd_result.out_file
         self.stderr = cmd_result.stderr
         self.err_file = cmd_result.err_file
+        self.cmd_sh = cmd_result.cmd_sh
 
         self.caller_info = caller_info
 
@@ -112,33 +126,36 @@ def print(self):
         Report failed shell command for this RunShellCmdError instance
         """
 
-        def pad_4_spaces(msg):
-            return ' ' * 4 + msg
+        def pad_4_spaces(msg, color=None):
+            padded_msg = ' ' * 4 + msg
+            if color:
+                return colorize(padded_msg, color)
+            else:
+                return padded_msg
+
+        caller_file_name, caller_line_nr, caller_function_name = self.caller_info
+        called_from_info = f"'{caller_function_name}' function in {caller_file_name} (line {caller_line_nr})"
 
         error_info = [
-            '',
-            "ERROR: Shell command failed!",
+            colorize("ERROR: Shell command failed!", COLOR_RED),
             pad_4_spaces(f"full command              ->  {self.cmd}"),
             pad_4_spaces(f"exit code                 ->  {self.exit_code}"),
+            pad_4_spaces(f"called from               ->  {called_from_info}"),
             pad_4_spaces(f"working directory         ->  {self.work_dir}"),
         ]
 
         if self.out_file is not None:
             # if there's no separate file for error/warnings, then out_file includes both stdout + stderr
             out_info_msg = "output (stdout + stderr)" if self.err_file is None else "output (stdout)         "
-            error_info.append(pad_4_spaces(f"{out_info_msg}  ->  {self.out_file}"))
+            error_info.append(pad_4_spaces(f"{out_info_msg}  ->  {self.out_file}", color=COLOR_YELLOW))
 
         if self.err_file is not None:
-            error_info.append(pad_4_spaces(f"error/warnings (stderr)   ->  {self.err_file}"))
+            error_info.append(pad_4_spaces(f"error/warnings (stderr)   ->  {self.err_file}", color=COLOR_YELLOW))
 
-        caller_file_name, caller_line_nr, caller_function_name = self.caller_info
-        called_from_info = f"'{caller_function_name}' function in {caller_file_name} (line {caller_line_nr})"
-        error_info.extend([
-            pad_4_spaces(f"called from               ->  {called_from_info}"),
-            '',
-        ])
+        if self.cmd_sh is not None:
+            error_info.append(pad_4_spaces(f"interactive shell script  ->  {self.cmd_sh}", color=COLOR_YELLOW))
 
-        sys.stderr.write('\n'.join(error_info) + '\n')
+        print_error('\n'.join(error_info), rich_highlight=False)
 
 
 def raise_run_shell_cmd_error(cmd_res):
@@ -254,6 +271,8 @@ def create_cmd_scripts(cmd_str, work_dir, env, tmpdir, out_file, err_file):
             ]))
     os.chmod(cmd_fp, 0o775)
 
+    return cmd_fp
+
 
 def _answer_question(stdout, proc, qa_patterns, qa_wait_patterns):
     """
@@ -430,9 +449,9 @@ def to_cmd_str(cmd):
         else:
             cmd_err_fp = None
 
-        create_cmd_scripts(cmd_str, work_dir, env, tmpdir, cmd_out_fp, cmd_err_fp)
+        cmd_sh = create_cmd_scripts(cmd_str, work_dir, env, tmpdir, cmd_out_fp, cmd_err_fp)
     else:
-        tmpdir, cmd_out_fp, cmd_err_fp = None, None, None
+        tmpdir, cmd_out_fp, cmd_err_fp, cmd_sh = None, None, None, None
 
     interactive_msg = 'interactive ' if interactive else ''
 
@@ -445,7 +464,8 @@ def to_cmd_str(cmd):
             dry_run_msg(msg, silent=silent)
 
         return RunShellCmdResult(cmd=cmd_str, exit_code=0, output='', stderr=None, work_dir=work_dir,
-                                 out_file=cmd_out_fp, err_file=cmd_err_fp, thread_id=thread_id, task_id=task_id)
+                                 out_file=cmd_out_fp, err_file=cmd_err_fp, cmd_sh=cmd_sh,
+                                 thread_id=thread_id, task_id=task_id)
 
     start_time = datetime.now()
     if not hidden:
@@ -574,8 +594,9 @@ def to_cmd_str(cmd):
         except IOError as err:
             raise EasyBuildError(f"Failed to dump command output to temporary file: {err}")
 
-    res = RunShellCmdResult(cmd=cmd_str, exit_code=proc.returncode, output=output, stderr=stderr, work_dir=work_dir,
-                            out_file=cmd_out_fp, err_file=cmd_err_fp, thread_id=thread_id, task_id=task_id)
+    res = RunShellCmdResult(cmd=cmd_str, exit_code=proc.returncode, output=output, stderr=stderr,
+                            work_dir=work_dir, out_file=cmd_out_fp, err_file=cmd_err_fp, cmd_sh=cmd_sh,
+                            thread_id=thread_id, task_id=task_id)
 
     # always log command output
     cmd_name = cmd_str.split(' ')[0]

test/framework/output.py

@@ -35,7 +35,7 @@
 from easybuild.tools.build_log import EasyBuildError
 from easybuild.tools.config import build_option, get_output_style, update_build_option
 from easybuild.tools.output import PROGRESS_BAR_EXTENSIONS, PROGRESS_BAR_TYPES
-from easybuild.tools.output import DummyRich, colorize, get_progress_bar, show_progress_bars
+from easybuild.tools.output import DummyRich, colorize, get_progress_bar, print_error, show_progress_bars
 from easybuild.tools.output import start_progress_bar, status_bar, stop_progress_bar, update_progress_bar, use_rich
 
 try:
@@ -139,6 +139,26 @@ def test_colorize(self):
 
         self.assertErrorRegex(EasyBuildError, "Unknown color: nosuchcolor", colorize, 'test', 'nosuchcolor')
 
+    def test_print_error(self):
+        """
+        Test print_error function
+        """
+        msg = "This is yellow: " + colorize("a banana", color='yellow')
+        self.mock_stderr(True)
+        self.mock_stdout(True)
+        print_error(msg)
+        stderr = self.get_stderr()
+        stdout = self.get_stdout()
+        self.mock_stderr(False)
+        self.mock_stdout(False)
+        self.assertEqual(stdout, '')
+        if HAVE_RICH:
+            # when using Rich, message printed to stderr won't have funny terminal escape characters for the color
+            expected = '\n\nThis is yellow: a banana\n\n'
+        else:
+            expected = '\nThis is yellow: \x1b[1;33ma banana\x1b[0m\n\n'
+        self.assertEqual(stderr, expected)
+
     def test_get_progress_bar(self):
         """
         Test get_progress_bar.

test/framework/run.py

@@ -491,12 +491,14 @@ def handler(signum, _):
                 # check error reporting output
                 stderr = stderr.getvalue()
                 patterns = [
-                    r"^ERROR: Shell command failed!",
-                    r"^\s+full command\s* ->  kill -9 \$\$",
-                    r"^\s+exit code\s* ->  -9",
-                    r"^\s+working directory\s* ->  " + work_dir,
-                    r"^\s+called from\s* ->  'test_run_shell_cmd_fail' function in .*/test/.*/run.py \(line [0-9]+\)",
-                    r"^\s+output \(stdout \+ stderr\)\s* ->  .*/run-shell-cmd-output/kill-.*/out.txt",
+                    r"ERROR: Shell command failed!",
+                    r"\s+full command\s* ->  kill -9 \$\$",
+                    r"\s+exit code\s* ->  -9",
+                    r"\s+working directory\s* ->  " + work_dir,
+                    r"\s+called from\s* ->  'test_run_shell_cmd_fail' function in "
+                    r"(.|\n)*/test/(.|\n)*/run.py \(line [0-9]+\)",
+                    r"\s+output \(stdout \+ stderr\)\s* ->  (.|\n)*/run-shell-cmd-output/kill-(.|\n)*/out.txt",
+                    r"\s+interactive shell script\s* ->  (.|\n)*/run-shell-cmd-output/kill-(.|\n)*/cmd.sh",
                 ]
                 for pattern in patterns:
                     regex = re.compile(pattern, re.M)
@@ -526,13 +528,15 @@ def handler(signum, _):
                 # check error reporting output
                 stderr = stderr.getvalue()
                 patterns = [
-                    r"^ERROR: Shell command failed!",
-                    r"^\s+full command\s+ ->  kill -9 \$\$",
-                    r"^\s+exit code\s+ ->  -9",
-                    r"^\s+working directory\s+ ->  " + work_dir,
-                    r"^\s+called from\s+ ->  'test_run_shell_cmd_fail' function in .*/test/.*/run.py \(line [0-9]+\)",
-                    r"^\s+output \(stdout\)\s+ -> .*/run-shell-cmd-output/kill-.*/out.txt",
-                    r"^\s+error/warnings \(stderr\)\s+ -> .*/run-shell-cmd-output/kill-.*/err.txt",
+                    r"ERROR: Shell command failed!",
+                    r"\s+full command\s+ ->  kill -9 \$\$",
+                    r"\s+exit code\s+ ->  -9",
+                    r"\s+working directory\s+ ->  " + work_dir,
+                    r"\s+called from\s+ ->  'test_run_shell_cmd_fail' function in "
+                    r"(.|\n)*/test/(.|\n)*/run.py \(line [0-9]+\)",
+                    r"\s+output \(stdout\)\s+ -> (.|\n)*/run-shell-cmd-output/kill-(.|\n)*/out.txt",
+                    r"\s+error/warnings \(stderr\)\s+ -> (.|\n)*/run-shell-cmd-output/kill-(.|\n)*/err.txt",
+                    r"\s+interactive shell script\s* ->  (.|\n)*/run-shell-cmd-output/kill-(.|\n)*/cmd.sh",
                 ]
                 for pattern in patterns:
                     regex = re.compile(pattern, re.M)
@@ -1383,7 +1387,7 @@ def test_run_shell_cmd_cache(self):
         with self.mocked_stdout_stderr():
             cached_res = RunShellCmdResult(cmd=cmd, output="123456", exit_code=123, stderr=None,
                                            work_dir='/test_ulimit', out_file='/tmp/foo.out', err_file=None,
-                                           thread_id=None, task_id=None)
+                                           cmd_sh='/tmp/cmd.sh', thread_id=None, task_id=None)
             run_shell_cmd.update_cache({(cmd, None): cached_res})
             res = run_shell_cmd(cmd)
         self.assertEqual(res.cmd, cmd)
@@ -1403,7 +1407,7 @@ def test_run_shell_cmd_cache(self):
         with self.mocked_stdout_stderr():
             cached_res = RunShellCmdResult(cmd=cmd, output="bar", exit_code=123, stderr=None,
                                            work_dir='/test_cat', out_file='/tmp/cat.out', err_file=None,
-                                           thread_id=None, task_id=None)
+                                           cmd_sh='/tmp/cmd.sh', thread_id=None, task_id=None)
             run_shell_cmd.update_cache({(cmd, 'foo'): cached_res})
             res = run_shell_cmd(cmd, stdin='foo')
         self.assertEqual(res.cmd, cmd)

test/framework/systemtools.py

@@ -341,7 +341,7 @@ def mocked_run_shell_cmd(cmd, **kwargs):
     }
     if cmd in known_cmds:
         return RunShellCmdResult(cmd=cmd, exit_code=0, output=known_cmds[cmd], stderr=None, work_dir=os.getcwd(),
-                                 out_file=None, err_file=None, thread_id=None, task_id=None)
+                                 out_file=None, err_file=None, cmd_sh=None, thread_id=None, task_id=None)
     else:
         return run_shell_cmd(cmd, **kwargs)
 
@@ -774,7 +774,7 @@ def test_gcc_version_darwin(self):
         out = "Apple LLVM version 7.0.0 (clang-700.1.76)"
         cwd = os.getcwd()
         mocked_run_res = RunShellCmdResult(cmd="gcc --version", exit_code=0, output=out, stderr=None, work_dir=cwd,
-                                           out_file=None, err_file=None, thread_id=None, task_id=None)
+                                           out_file=None, err_file=None, cmd_sh=None, thread_id=None, task_id=None)
         st.run_shell_cmd = lambda *args, **kwargs: mocked_run_res
         self.assertEqual(get_gcc_version(), None)