Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions easybuild/base/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,15 @@ def mocked_stdout(self):
finally:
self.mock_stdout(False)

@contextmanager
def mocked_stderr(self):
"""Context manager to mock stdout"""
self.mock_stderr(True)
try:
yield sys.stderr
finally:
self.mock_stderr(False)

@contextmanager
def mocked_stdout_stderr(self, mock_stdout=True, mock_stderr=True):
"""Context manager to mock stdout and stderr"""
Expand Down
9 changes: 5 additions & 4 deletions easybuild/framework/easyblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
from easybuild.tools import LooseVersion, config
from easybuild.tools.build_details import get_build_stats
from easybuild.tools.build_log import EasyBuildError, EasyBuildExit, dry_run_msg, dry_run_warning, dry_run_set_dirs
from easybuild.tools.build_log import print_error, print_msg, print_warning
from easybuild.tools.build_log import print_error_and_exit, print_msg, print_warning
from easybuild.tools.config import CHECKSUM_PRIORITY_JSON, DEFAULT_ENVVAR_USERS_MODULES
from easybuild.tools.config import EASYBUILD_SOURCES_URL, EBPYTHONPREFIXES # noqa
from easybuild.tools.config import FORCE_DOWNLOAD_ALL, FORCE_DOWNLOAD_PATCHES, FORCE_DOWNLOAD_SOURCES
Expand Down Expand Up @@ -2982,7 +2982,8 @@ def prepare_step(self, start_dir=True, load_tc_deps_modules=True):
if os.path.isabs(self.rpath_wrappers_dir):
_log.info(f"Using {self.rpath_wrappers_dir} to store/use RPATH wrappers")
else:
raise EasyBuildError(f"Path used for rpath_wrappers_dir is not an absolute path: {path}")
raise EasyBuildError("Path used for rpath_wrappers_dir is not an absolute path: %s",
self.rpath_wrappers_dir)

if self.iter_idx > 0:
# reset toolchain for iterative runs before preparing it again
Expand Down Expand Up @@ -5064,8 +5065,8 @@ def build_and_install_one(ecdict, init_env):
app = app_class(ecdict['ec'])
_log.info("Obtained application instance for %s (easyblock: %s)" % (name, easyblock))
except EasyBuildError as err:
print_error("Failed to get application instance for %s (easyblock: %s): %s" % (name, easyblock, err.msg),
silent=silent)
print_error_and_exit("Failed to get application instance for %s (easyblock: %s): %s", name, easyblock, err.msg,
silent=silent, exit_code=err.exit_code)

# application settings
stop = build_option('stop')
Expand Down
4 changes: 2 additions & 2 deletions easybuild/framework/easyconfig/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
from easybuild.framework.easyconfig.easyconfig import process_easyconfig
from easybuild.framework.easyconfig.style import cmdline_easyconfigs_style_check
from easybuild.tools import LooseVersion
from easybuild.tools.build_log import EasyBuildError, EasyBuildExit, print_error, print_msg, print_warning
from easybuild.tools.build_log import EasyBuildError, EasyBuildExit, print_error_and_exit, print_msg, print_warning
from easybuild.tools.config import build_option
from easybuild.tools.environment import restore_env
from easybuild.tools.filetools import EASYBLOCK_CLASS_PREFIX, get_cwd, find_easyconfigs, is_patch_file
Expand Down Expand Up @@ -216,7 +216,7 @@ def mk_node_name(spec):
if _dep_graph_dump(dgr, filename):
print_msg("Wrote " + what, silent=silent)
else:
print_error("Failed writing " + what, silent=silent)
print_error_and_exit("Failed writing " + what, silent=silent)


@only_if_module_is_available('pygraph.readwrite.dot', pkgname='python-graph-dot')
Expand Down
23 changes: 12 additions & 11 deletions easybuild/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@

# IMPORTANT this has to be the first easybuild import as it customises the logging
# expect missing log output when this not the case!
from easybuild.tools.build_log import EasyBuildError, print_error, print_msg, print_warning, stop_logging
from easybuild.tools.build_log import EasyBuildError, print_error_and_exit, print_msg, print_warning, stop_logging
from easybuild.tools.build_log import EasyBuildExit

from easybuild.framework.easyblock import build_and_install_one, inject_checksums, inject_checksums_to_json
Expand Down Expand Up @@ -428,9 +428,12 @@ def process_eb_args(eb_args, eb_go, cfg_settings, modtool, testing, init_session
elif any(no_ec_opts):
paths = determined_paths
else:
print_error("Please provide one or multiple easyconfig files, or use software build " +
"options to make EasyBuild search for easyconfigs",
log=_log, opt_parser=eb_go.parser, exit_on_error=not testing)
msg = ("Please provide one or multiple easyconfig files, or use software build "
"options to make EasyBuild search for easyconfigs")
if testing:
raise EasyBuildError(msg)
eb_go.parser.print_shorthelp()
print_error_and_exit(msg)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

End out produced output will look like this:

All long option names can be passed as environment variables. Variable name is
EASYBUILD_<LONGNAME> eg. --some-opt is same as setting EASYBUILD_SOME_OPT in
the environment.
ERROR: Please provide one or multiple easyconfig files, or use software build options to make EasyBuild search for easyconfigs

So the error message "drowns" a bit because it blends in with the short help output.

Let's add one (or two) newlines in between?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this was the case before.

We have output.print_error which adds a newline before and after the message. Would be great if we could use that. A bit confusing that print_error are in output but print_msg and print_warning in build_log. I'd move all to one of them but we can't as we either get a cycle or break backwards compat with build_log.print_error (which used to exit)

I'm now calling output.print_error from here to have a uniform appearance of printed errors.

_log.debug("Paths: %s", paths)

# run regtest
Expand Down Expand Up @@ -560,10 +563,8 @@ def process_eb_args(eb_args, eb_go, cfg_settings, modtool, testing, init_session

elif options.check_conflicts:
if check_conflicts(easyconfigs, modtool):
print_error("One or more conflicts detected!")
sys.exit(1)
else:
print_msg("\nNo conflicts detected!\n", prefix=False)
print_error_and_exit("One or more conflicts detected!")
print_msg("\nNo conflicts detected!\n", prefix=False)

# dump source script to set up build environment
elif options.dump_env_script:
Expand Down Expand Up @@ -836,7 +837,7 @@ def main_with_hooks(args=None):
try:
init_session_state, eb_go, cfg_settings = prepare_main(args=args)
except EasyBuildError as err:
print_error(err.msg, exit_code=err.exit_code)
print_error_and_exit(err.msg, exit_code=err.exit_code)

hooks = load_hooks(eb_go.options.hooks)

Expand All @@ -845,10 +846,10 @@ def main_with_hooks(args=None):
sys.exit(int(exit_code))
except EasyBuildError as err:
run_hook(FAIL, hooks, args=[err])
print_error(err.msg, exit_on_error=True, exit_code=err.exit_code)
print_error_and_exit(err.msg, exit_code=err.exit_code)
except KeyboardInterrupt as err:
run_hook(CANCEL, hooks, args=[err])
print_error("Cancelled by user: %s" % err)
print_error_and_exit("Cancelled by user: %s", err)
except Exception as err:
run_hook(CRASH, hooks, args=[err])
sys.stderr.write("EasyBuild crashed! Please consider reporting a bug, this should not happen...\n\n")
Expand Down
36 changes: 25 additions & 11 deletions easybuild/tools/build_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -394,8 +394,9 @@ def print_error(msg, *args, **kwargs):
"""
Print error message and exit EasyBuild
"""
if args:
msg = msg % args

_init_easybuildlog.deprecated("Function 'print_error' from easybuild.tools.build_log is replaced "
"with 'print_error_and_exit'", '6.0')

# grab exit code, if specified;
# also consider deprecated 'exitCode' option
Expand All @@ -404,9 +405,6 @@ def print_error(msg, *args, **kwargs):
if exitCode is not None:
_init_easybuildlog.deprecated("'exitCode' option in print_error function is replaced with 'exit_code'", '6.0')

if exit_code is None:
exit_code = EasyBuildExit.ERROR

log = kwargs.pop('log', None)
opt_parser = kwargs.pop('opt_parser', None)
exit_on_error = kwargs.pop('exit_on_error', True)
Expand All @@ -415,13 +413,29 @@ def print_error(msg, *args, **kwargs):
raise EasyBuildError("Unknown named arguments passed to print_error: %s", kwargs)

if exit_on_error:
if not silent:
if opt_parser:
opt_parser.print_shorthelp()
sys.stderr.write("ERROR: %s\n" % msg)
sys.exit(int(exit_code))
if not silent and opt_parser:
opt_parser.print_shorthelp()
if exit_code is None:
exit_code = EasyBuildExit.ERROR
print_error_and_exit(msg, *args, exit_code=exit_code, silent=silent)
elif log is not None:
raise EasyBuildError(msg)
raise EasyBuildError(msg) # Handle legacy weirdness


def print_error_and_exit(msg, *args, exit_code=EasyBuildExit.ERROR, silent=False):
"""
Print error message and exit EasyBuild, supports format strings

:param msg: Message to show
:exit_code: EasyBuildExit or integer to exit with
:silent: When True don't print to stderr
"""
if args:
msg = msg % args
if not silent:
from easybuild.tools.output import print_error as show_error
show_error("ERROR: " + msg, disable_rich=True)
sys.exit(int(exit_code))


def print_warning(msg, *args, **kwargs):
Expand Down
11 changes: 6 additions & 5 deletions easybuild/tools/output.py
Original file line number Diff line number Diff line change
Expand Up @@ -392,18 +392,19 @@ def print_checks(checks_data):
print('\n'.join(lines))


def print_error(error_msg, rich_highlight=True):
def print_error(error_msg, rich_highlight=True, disable_rich=False):
"""
Print error message, using a Rich Console instance if possible.
Print error message, using a Rich Console instance if possible unless disable_rich=True.
Newlines before/after message are automatically added.

:param rich_highlight: boolean indicating whether automatic highlighting by Rich should be enabled
"""
if use_rich():
error_msg = f'\n\n{error_msg}\n'
if not disable_rich and use_rich():
console = Console(stderr=True)
console.print('\n\n' + error_msg + '\n', highlight=rich_highlight)
console.print(error_msg, highlight=rich_highlight)
else:
sys.stderr.write('\n' + error_msg + '\n\n')
print(error_msg, file=sys.stderr)


# this constant must be defined at the end, since functions used as values need to be defined
Expand Down
41 changes: 23 additions & 18 deletions test/framework/build_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@
from easybuild.base.fancylogger import getLogger, logToFile, setLogFormat
from easybuild.framework.easyconfig.tweak import tweak_one
from easybuild.tools.build_log import (
LOGGING_FORMAT, EasyBuildError, EasyBuildLog, dry_run_msg, dry_run_warning, init_logging, print_error, print_msg,
print_warning, stop_logging, time_str_since, raise_nosupport)
LOGGING_FORMAT, EasyBuildError, EasyBuildLog, dry_run_msg, dry_run_warning, init_logging, print_error,
print_error_and_exit, print_msg, print_warning, stop_logging, time_str_since, raise_nosupport)
from easybuild.tools.filetools import read_file, write_file


Expand Down Expand Up @@ -275,28 +275,33 @@ def run_check(args, silent=False, expected_stderr='', **kwargs):
log_txt = read_file(tmp_logfile)
self.assertIn("WARNING Test log message with a logger involved.", log_txt)

def test_print_error(self):
"""Test print_error"""
def test_print_error_and_exit(self):
"""Test print_error_and_exit and (deprecated) print_error functions"""
def run_check(args, silent=False, expected_stderr=''):
"""Helper function to check stdout/stderr produced via print_error."""
self.mock_stderr(True)
self.mock_stdout(True)
self.assertErrorRegex(SystemExit, '1', print_error, *args, silent=silent)
stderr = self.get_stderr()
stdout = self.get_stdout()
self.mock_stdout(False)
self.mock_stderr(False)
self.assertEqual(stdout, '')
self.assertTrue(stderr.startswith(expected_stderr))

run_check(['You have failed.'], expected_stderr="ERROR: You have failed.\n")
run_check(['You have %s.', 'failed'], expected_stderr="ERROR: You have failed.\n")
run_check(['%s %s %s.', 'You', 'have', 'failed'], expected_stderr="ERROR: You have failed.\n")
for func in ("print_error_and_exit", "print_error"):
with self.subTest(f"Function {func}"):
with self.mocked_stdout_stderr():
if func == "print_error": # Deprecated variant
with self.temporarily_allow_deprecated_behaviour():
self.assertRaisesRegex(SystemExit, '1', print_error, *args, silent=silent)
stderr = re.sub(r'\nWARNING: Deprecated.*\n\n', '', self.get_stderr())
else:
self.assertRaisesRegex(SystemExit, '1', print_error_and_exit, *args, silent=silent)
stderr = self.get_stderr()
stdout = self.get_stdout()
self.assertEqual(stdout, '')
self.assertEqual(stderr, expected_stderr)

run_check(['You have failed.'], expected_stderr="\n\nERROR: You have failed.\n\n")
run_check(['You have %s.', 'failed'], expected_stderr="\n\nERROR: You have failed.\n\n")
run_check(['%s %s %s.', 'You', 'have', 'failed'], expected_stderr="\n\nERROR: You have failed.\n\n")
run_check(['You have failed.'], silent=True)
run_check(['You have %s.', 'failed'], silent=True)
run_check(['%s %s %s.', 'You', 'have', 'failed'], silent=True)

self.assertErrorRegex(EasyBuildError, "Unknown named arguments", print_error, 'foo', unknown_arg='bar')
with self.temporarily_allow_deprecated_behaviour(), self.mocked_stderr():
self.assertErrorRegex(EasyBuildError, "Unknown named arguments", print_error, 'foo', unknown_arg='bar')

def test_print_msg(self):
"""Test print_msg"""
Expand Down