diff --git a/docs/internals/frontends.rst b/docs/internals/frontends.rst index da6270a01..fd2b5b0f4 100644 --- a/docs/internals/frontends.rst +++ b/docs/internals/frontends.rst @@ -706,6 +706,11 @@ Errors Decompression error: {} +Warnings + FileChangedWarning rc: 100 + IncludePatternNeverMatchedWarning rc: 101 + BackupExcWarning rc: 102 (needs more work!) + Operations - cache.begin_transaction - cache.download_chunks, appears with ``borg create --no-cache-sync`` diff --git a/src/borg/archive.py b/src/borg/archive.py index 639f8ef8d..a1c7602ce 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -187,6 +187,12 @@ class BackupError(Exception): """ +class BackupRaceConditionError(BackupError): + """ + Exception raised when encountering a critical race condition while trying to back up a file. + """ + + class BackupOSError(Exception): """ Wrapper for OSError raised while accessing backup files. @@ -259,10 +265,10 @@ def stat_update_check(st_old, st_curr): # are not duplicate in a short timeframe, this check is redundant and solved by the ino check: if stat.S_IFMT(st_old.st_mode) != stat.S_IFMT(st_curr.st_mode): # in this case, we dispatched to wrong handler - abort - raise BackupError("file type changed (race condition), skipping file") + raise BackupRaceConditionError("file type changed (race condition), skipping file") if st_old.st_ino != st_curr.st_ino: # in this case, the hardlinks-related code in create_helper has the wrong inode - abort! - raise BackupError("file inode changed (race condition), skipping file") + raise BackupRaceConditionError("file inode changed (race condition), skipping file") # looks ok, we are still dealing with the same thing - return current stat: return st_curr diff --git a/src/borg/archiver/__init__.py b/src/borg/archiver/__init__.py index 16ec1e1c9..3aba28a79 100644 --- a/src/borg/archiver/__init__.py +++ b/src/borg/archiver/__init__.py @@ -24,8 +24,9 @@ try: from ._common import Highlander from .. import __version__ from ..constants import * # NOQA - from ..helpers import EXIT_SUCCESS, EXIT_WARNING, EXIT_ERROR, EXIT_SIGNAL_BASE - from ..helpers import Error, CommandError, set_ec, modern_ec + from ..helpers import EXIT_SUCCESS, EXIT_WARNING, EXIT_ERROR, EXIT_SIGNAL_BASE, classify_ec + from ..helpers import Error, CommandError, get_ec, modern_ec + from ..helpers import add_warning, BorgWarning from ..helpers import format_file_size from ..helpers import remove_surrogates, text_to_json from ..helpers import DatetimeWrapper, replace_placeholders @@ -128,10 +129,23 @@ class Archiver( self.prog = prog self.last_checkpoint = time.monotonic() - def print_warning(self, msg, *args): - msg = args and msg % args or msg - self.exit_code = EXIT_WARNING # we do not terminate here, so it is a warning - logger.warning(msg) + def print_warning(self, msg, *args, **kw): + warning_code = kw.get("wc", EXIT_WARNING) # note: wc=None can be used to not influence exit code + warning_type = kw.get("wt", "percent") + assert warning_type in ("percent", "curly") + if warning_code is not None: + add_warning(msg, *args, wc=warning_code, wt=warning_type) + if warning_type == "percent": + output = args and msg % args or msg + else: # == "curly" + output = args and msg.format(*args) or msg + logger.warning(output) + + def print_warning_instance(self, warning): + assert isinstance(warning, BorgWarning) + msg = type(warning).__doc__ + args = warning.args + self.print_warning(msg, *args, wc=warning.exit_code, wt="curly") def print_file_status(self, status, path): # if we get called with status == None, the final file status was already printed @@ -514,7 +528,7 @@ class Archiver( variables = dict(locals()) profiler.enable() try: - return set_ec(func(args)) + return get_ec(func(args)) finally: profiler.disable() profiler.snapshot_stats() @@ -531,7 +545,7 @@ class Archiver( # it compatible (see above). msgpack.pack(profiler.stats, fd, use_bin_type=True) else: - return set_ec(func(args)) + return get_ec(func(args)) def sig_info_handler(sig_no, stack): # pragma: no cover @@ -680,16 +694,19 @@ def main(): # pragma: no cover if args.show_rc: rc_logger = logging.getLogger("borg.output.show-rc") exit_msg = "terminating with %s status, rc %d" - if exit_code == EXIT_SUCCESS: - rc_logger.info(exit_msg % ("success", exit_code)) - elif exit_code == EXIT_WARNING or EXIT_WARNING_BASE <= exit_code < EXIT_SIGNAL_BASE: - rc_logger.warning(exit_msg % ("warning", exit_code)) - elif exit_code == EXIT_ERROR or EXIT_ERROR_BASE <= exit_code < EXIT_WARNING_BASE: - rc_logger.error(exit_msg % ("error", exit_code)) - elif exit_code >= EXIT_SIGNAL_BASE: - rc_logger.error(exit_msg % ("signal", exit_code)) - else: + try: + ec_class = classify_ec(exit_code) + except ValueError: rc_logger.error(exit_msg % ("abnormal", exit_code or 666)) + else: + if ec_class == "success": + rc_logger.info(exit_msg % (ec_class, exit_code)) + elif ec_class == "warning": + rc_logger.warning(exit_msg % (ec_class, exit_code)) + elif ec_class == "error": + rc_logger.error(exit_msg % (ec_class, exit_code)) + elif ec_class == "signal": + rc_logger.error(exit_msg % (ec_class, exit_code)) sys.exit(exit_code) diff --git a/src/borg/archiver/create_cmd.py b/src/borg/archiver/create_cmd.py index 5c58d89ee..4ab25575c 100644 --- a/src/borg/archiver/create_cmd.py +++ b/src/borg/archiver/create_cmd.py @@ -29,7 +29,7 @@ from ..helpers import prepare_subprocess_env from ..helpers import sig_int, ignore_sigint from ..helpers import iter_separated from ..helpers import MakePathSafeAction -from ..helpers import Error, CommandError +from ..helpers import Error, CommandError, BackupExcWarning, FileChangedWarning from ..manifest import Manifest from ..patterns import PatternMatcher from ..platform import is_win32 @@ -122,10 +122,10 @@ class CreateMixIn: dry_run=dry_run, ) except (BackupOSError, BackupError) as e: - self.print_warning("%s: %s", path, e) + self.print_warning_instance(BackupExcWarning(path, e)) status = "E" if status == "C": - self.print_warning("%s: file changed while we backed it up", path) + self.print_warning_instance(FileChangedWarning(path)) self.print_file_status(status, path) if not dry_run and status is not None: fso.stats.files_stats[status] += 1 @@ -149,8 +149,8 @@ class CreateMixIn: path=path, cache=cache, fd=sys.stdin.buffer, mode=mode, user=user, group=group ) except BackupOSError as e: + self.print_warning_instance(BackupExcWarning(path, e)) status = "E" - self.print_warning("%s: %s", path, e) else: status = "+" # included self.print_file_status(status, path) @@ -182,7 +182,7 @@ class CreateMixIn: skip_inodes.add((st.st_ino, st.st_dev)) except (BackupOSError, BackupError) as e: # this comes from os.stat, self._rec_walk has own exception handler - self.print_warning("%s: %s", path, e) + self.print_warning_instance(BackupExcWarning(path, e)) continue if not dry_run: if args.progress: @@ -522,10 +522,10 @@ class CreateMixIn: ) except (BackupOSError, BackupError) as e: - self.print_warning("%s: %s", path, e) + self.print_warning_instance(BackupExcWarning(path, e)) status = "E" if status == "C": - self.print_warning("%s: file changed while we backed it up", path) + self.print_warning_instance(FileChangedWarning(path)) if not recurse_excluded_dir: self.print_file_status(status, path) if not dry_run and status is not None: diff --git a/src/borg/archiver/delete_cmd.py b/src/borg/archiver/delete_cmd.py index 48cc59ddf..71467f7ed 100644 --- a/src/borg/archiver/delete_cmd.py +++ b/src/borg/archiver/delete_cmd.py @@ -49,9 +49,9 @@ class DeleteMixIn: manifest.write() # note: might crash in compact() after committing the repo repository.commit(compact=False) - self.print_warning('Done. Run "borg check --repair" to clean up the mess.') + self.print_warning('Done. Run "borg check --repair" to clean up the mess.', wc=None) else: - self.print_warning("Aborted.") + self.print_warning("Aborted.", wc=None) return self.exit_code stats = Statistics(iec=args.iec) diff --git a/src/borg/archiver/diff_cmd.py b/src/borg/archiver/diff_cmd.py index 492a26fc5..7eb0da331 100644 --- a/src/borg/archiver/diff_cmd.py +++ b/src/borg/archiver/diff_cmd.py @@ -37,7 +37,8 @@ class DiffMixIn: self.print_warning( "--chunker-params might be different between archives, diff will be slow.\n" "If you know for certain that they are the same, pass --same-chunker-params " - "to override this check." + "to override this check.", + wc=None, ) matcher = build_matcher(args.patterns, args.paths) @@ -74,7 +75,7 @@ class DiffMixIn: sys.stdout.write(res) for pattern in matcher.get_unmatched_include_patterns(): - self.print_warning("Include pattern '%s' never matched.", pattern) + self.print_warning_instance(IncludePatternNeverMatchedWarning(pattern)) return self.exit_code diff --git a/src/borg/archiver/extract_cmd.py b/src/borg/archiver/extract_cmd.py index 7be8a32fe..1dc7322ee 100644 --- a/src/borg/archiver/extract_cmd.py +++ b/src/borg/archiver/extract_cmd.py @@ -12,6 +12,7 @@ from ..helpers import archivename_validator, PathSpec from ..helpers import remove_surrogates from ..helpers import HardLinkManager from ..helpers import ProgressIndicatorPercent +from ..helpers import BackupExcWarning, IncludePatternNeverMatchedWarning from ..manifest import Manifest from ..logger import create_logger @@ -65,7 +66,7 @@ class ExtractMixIn: try: archive.extract_item(dir_item, stdout=stdout) except BackupOSError as e: - self.print_warning("%s: %s", remove_surrogates(dir_item.path), e) + self.print_warning_instance(BackupExcWarning(remove_surrogates(dir_item.path), e)) if output_list: logging.getLogger("borg.output.list").info(remove_surrogates(item.path)) try: @@ -80,7 +81,7 @@ class ExtractMixIn: item, stdout=stdout, sparse=sparse, hlm=hlm, pi=pi, continue_extraction=continue_extraction ) except (BackupOSError, BackupError) as e: - self.print_warning("%s: %s", remove_surrogates(orig_path), e) + self.print_warning_instance(BackupExcWarning(remove_surrogates(orig_path), e)) if pi: pi.finish() @@ -95,9 +96,9 @@ class ExtractMixIn: try: archive.extract_item(dir_item, stdout=stdout) except BackupOSError as e: - self.print_warning("%s: %s", remove_surrogates(dir_item.path), e) + self.print_warning_instance(BackupExcWarning(remove_surrogates(dir_item.path), e)) for pattern in matcher.get_unmatched_include_patterns(): - self.print_warning("Include pattern '%s' never matched.", pattern) + self.print_warning_instance(IncludePatternNeverMatchedWarning(pattern)) if pi: # clear progress output pi.finish() diff --git a/src/borg/archiver/tar_cmds.py b/src/borg/archiver/tar_cmds.py index e54e3facc..284481585 100644 --- a/src/borg/archiver/tar_cmds.py +++ b/src/borg/archiver/tar_cmds.py @@ -240,7 +240,7 @@ class TarMixIn: tar.close() for pattern in matcher.get_unmatched_include_patterns(): - self.print_warning("Include pattern '%s' never matched.", pattern) + self.print_warning_instance(IncludePatternNeverMatchedWarning(pattern)) return self.exit_code @with_repository(cache=True, exclusive=True, compatibility=(Manifest.Operation.WRITE,)) diff --git a/src/borg/helpers/__init__.py b/src/borg/helpers/__init__.py index 6a3ee2adb..7970b1fcd 100644 --- a/src/borg/helpers/__init__.py +++ b/src/borg/helpers/__init__.py @@ -6,12 +6,14 @@ Code used to be in borg/helpers.py but was split into the modules in this package, which are imported into here for compatibility. """ import os +from collections import namedtuple from ..constants import * # NOQA from .checks import check_extension_modules, check_python from .datastruct import StableDict, Buffer, EfficientCollectionQueue from .errors import Error, ErrorWithTraceback, IntegrityError, DecompressionError, CancelledByUser, CommandError from .errors import RTError, modern_ec +from .errors import BorgWarning, FileChangedWarning, BackupExcWarning, IncludePatternNeverMatchedWarning from .fs import ensure_dir, join_base_dir, get_socket_filename from .fs import get_security_dir, get_keys_dir, get_base_dir, get_cache_dir, get_config_dir, get_runtime_dir from .fs import dir_is_tagged, dir_is_cachedir, remove_dotdot_prefixes, make_path_safe, scandir_inorder @@ -44,11 +46,37 @@ from .yes_no import yes, TRUISH, FALSISH, DEFAULTISH from .msgpack import is_slow_msgpack, is_supported_msgpack, get_limited_unpacker from . import msgpack +from ..logger import create_logger + +logger = create_logger() + + # generic mechanism to enable users to invoke workarounds by setting the # BORG_WORKAROUNDS environment variable to a list of comma-separated strings. # see the docs for a list of known workaround strings. workarounds = tuple(os.environ.get("BORG_WORKAROUNDS", "").split(",")) + +# element data type for warnings_list: +warning_info = namedtuple("warning_info", "wc,msg,args,wt") + +""" +The global warnings_list variable is used to collect warning_info elements while borg is running. + +Note: keep this in helpers/__init__.py as the code expects to be able to assign to helpers.warnings_list. +""" +warnings_list = [] + + +def add_warning(msg, *args, **kwargs): + global warnings_list + warning_code = kwargs.get("wc", EXIT_WARNING) + assert isinstance(warning_code, int) + warning_type = kwargs.get("wt", "percent") + assert warning_type in ("percent", "curly") + warnings_list.append(warning_info(warning_code, msg, args, warning_type)) + + """ The global exit_code variable is used so that modules other than archiver can increase the program exit code if a warning or error occurred during their operation. This is different from archiver.exit_code, which is only accessible @@ -59,13 +87,72 @@ Note: keep this in helpers/__init__.py as the code expects to be able to assign exit_code = EXIT_SUCCESS +def classify_ec(ec): + if not isinstance(ec, int): + raise TypeError("ec must be of type int") + if EXIT_SIGNAL_BASE <= ec <= 255: + return "signal" + elif ec == EXIT_ERROR or EXIT_ERROR_BASE <= ec < EXIT_WARNING_BASE: + return "error" + elif ec == EXIT_WARNING or EXIT_WARNING_BASE <= ec < EXIT_SIGNAL_BASE: + return "warning" + elif ec == EXIT_SUCCESS: + return "success" + else: + raise ValueError(f"invalid error code: {ec}") + + +def max_ec(ec1, ec2): + """return the more severe error code of ec1 and ec2""" + # note: usually, there can be only 1 error-class ec, the other ec is then either success or warning. + ec1_class = classify_ec(ec1) + ec2_class = classify_ec(ec2) + if ec1_class == "signal": + return ec1 + if ec2_class == "signal": + return ec2 + if ec1_class == "error": + return ec1 + if ec2_class == "error": + return ec2 + if ec1_class == "warning": + return ec1 + if ec2_class == "warning": + return ec2 + assert ec1 == ec2 == EXIT_SUCCESS + return EXIT_SUCCESS + + def set_ec(ec): """ - Sets the exit code of the program, if an exit code higher or equal than this is set, this does nothing. This - makes EXIT_ERROR override EXIT_WARNING, etc.. - - ec: exit code to set + Sets the exit code of the program to ec IF ec is more severe than the current exit code. """ global exit_code - exit_code = max(exit_code, ec) - return exit_code + exit_code = max_ec(exit_code, ec) + + +def get_ec(ec=None): + """ + compute the final return code of the borg process + """ + if ec is not None: + set_ec(ec) + + global exit_code + exit_code_class = classify_ec(exit_code) + if exit_code_class in ("signal", "error", "warning"): + # there was a signal/error/warning, return its exit code + return exit_code + assert exit_code_class == "success" + global warnings_list + if not warnings_list: + # we do not have any warnings in warnings list, return success exit code + return exit_code + # looks like we have some warning(s) + rcs = sorted(set(w_info.wc for w_info in warnings_list)) + logger.debug(f"rcs: {rcs!r}") + if len(rcs) == 1: + # easy: there was only one kind of warning, so we can be specific + return rcs[0] + # there were different kinds of warnings + return EXIT_WARNING # generic warning rc, user has to look into the logs diff --git a/src/borg/helpers/errors.py b/src/borg/helpers/errors.py index fe800ed81..85a620849 100644 --- a/src/borg/helpers/errors.py +++ b/src/borg/helpers/errors.py @@ -70,3 +70,47 @@ class CommandError(Error): """Command Error: {}""" exit_mcode = 4 + + +class BorgWarning: + """Warning: {}""" + + # Warning base class + + # please note that this class and its subclasses are NOT exceptions, we do not raise them. + # so this is just to have inheritance, inspectability and the exit_code property. + exit_mcode = EXIT_WARNING # modern, more specific exit code (defaults to EXIT_WARNING) + + def __init__(self, *args): + self.args = args + + def get_message(self): + return type(self).__doc__.format(*self.args) + + __str__ = get_message + + @property + def exit_code(self): + # legacy: borg used to always use rc 1 (EXIT_WARNING) for all warnings. + # modern: users can opt in to more specific return codes, using BORG_EXIT_CODES: + return self.exit_mcode if modern_ec else EXIT_WARNING + + +class FileChangedWarning(BorgWarning): + """{}: file changed while we backed it up""" + + exit_mcode = 100 + + +class IncludePatternNeverMatchedWarning(BorgWarning): + """Include pattern '{}' never matched.""" + + exit_mcode = 101 + + +class BackupExcWarning(BorgWarning): + """{}: {}""" + + exit_mcode = 102 + + # TODO: override exit_code and compute the exit code based on the wrapped exception. diff --git a/src/borg/testsuite/archiver/__init__.py b/src/borg/testsuite/archiver/__init__.py index 2804ad44f..f45b34d26 100644 --- a/src/borg/testsuite/archiver/__init__.py +++ b/src/borg/testsuite/archiver/__init__.py @@ -78,6 +78,7 @@ def exec_cmd(*args, archiver=None, fork=False, exe=None, input=b"", binary_outpu archiver.prerun_checks = lambda *args: None archiver.exit_code = EXIT_SUCCESS helpers.exit_code = EXIT_SUCCESS + helpers.warnings_list = [] try: args = archiver.parse_args(list(args)) # argparse parsing may raise SystemExit when the command line is bad or diff --git a/src/borg/testsuite/archiver/delete_cmd.py b/src/borg/testsuite/archiver/delete_cmd.py index e5cc2d4e7..30727cac2 100644 --- a/src/borg/testsuite/archiver/delete_cmd.py +++ b/src/borg/testsuite/archiver/delete_cmd.py @@ -76,7 +76,7 @@ def test_delete_double_force(archivers, request): id = archive.metadata.items[0] repository.put(id, b"corrupted items metadata stream chunk") repository.commit(compact=False) - cmd(archiver, "delete", "-a", "test", "--force", "--force", exit_code=1) + cmd(archiver, "delete", "-a", "test", "--force", "--force") cmd(archiver, "check", "--repair") output = cmd(archiver, "rlist") assert "test" not in output diff --git a/src/borg/testsuite/archiver/diff_cmd.py b/src/borg/testsuite/archiver/diff_cmd.py index c87f4bf0b..e997e850d 100644 --- a/src/borg/testsuite/archiver/diff_cmd.py +++ b/src/borg/testsuite/archiver/diff_cmd.py @@ -220,8 +220,7 @@ def test_basic_functionality(archivers, request): output = cmd(archiver, "diff", "test0", "test1a") do_asserts(output, True) - # We expect exit_code=1 due to the chunker params warning - output = cmd(archiver, "diff", "test0", "test1b", "--content-only", exit_code=1) + output = cmd(archiver, "diff", "test0", "test1b", "--content-only") do_asserts(output, False, content_only=True) output = cmd(archiver, "diff", "test0", "test1a", "--json-lines") diff --git a/src/borg/testsuite/helpers.py b/src/borg/testsuite/helpers.py index 9a09739e6..605fe1bb1 100644 --- a/src/borg/testsuite/helpers.py +++ b/src/borg/testsuite/helpers.py @@ -13,7 +13,7 @@ import pytest from ..archiver.prune_cmd import prune_within, prune_split from .. import platform -from ..constants import MAX_DATA_SIZE +from ..constants import * # NOQA from ..helpers import Location from ..helpers import Buffer from ..helpers import ( @@ -44,6 +44,7 @@ from ..helpers import iter_separated from ..helpers import eval_escapes from ..helpers import safe_unlink from ..helpers import text_to_json, binary_to_json +from ..helpers import classify_ec, max_ec from ..helpers.passphrase import Passphrase, PasswordRetriesExceeded from ..platform import is_cygwin, is_win32, is_darwin from . import FakeInputs, are_hardlinks_supported @@ -1408,3 +1409,63 @@ class TestPassphrase: def test_passphrase_repr(self): assert "secret" not in repr(Passphrase("secret")) + + +@pytest.mark.parametrize( + "ec_range,ec_class", + ( + # inclusive range start, exclusive range end + ((0, 1), "success"), + ((1, 2), "warning"), + ((2, 3), "error"), + ((EXIT_ERROR_BASE, EXIT_WARNING_BASE), "error"), + ((EXIT_WARNING_BASE, EXIT_SIGNAL_BASE), "warning"), + ((EXIT_SIGNAL_BASE, 256), "signal"), + ), +) +def test_classify_ec(ec_range, ec_class): + for ec in range(*ec_range): + classify_ec(ec) == ec_class + + +def test_ec_invalid(): + with pytest.raises(ValueError): + classify_ec(666) + with pytest.raises(ValueError): + classify_ec(-1) + with pytest.raises(TypeError): + classify_ec(None) + + +@pytest.mark.parametrize( + "ec1,ec2,ec_max", + ( + # same for modern / legacy + (EXIT_SUCCESS, EXIT_SUCCESS, EXIT_SUCCESS), + (EXIT_SUCCESS, EXIT_SIGNAL_BASE, EXIT_SIGNAL_BASE), + # legacy exit codes + (EXIT_SUCCESS, EXIT_WARNING, EXIT_WARNING), + (EXIT_SUCCESS, EXIT_ERROR, EXIT_ERROR), + (EXIT_WARNING, EXIT_SUCCESS, EXIT_WARNING), + (EXIT_WARNING, EXIT_WARNING, EXIT_WARNING), + (EXIT_WARNING, EXIT_ERROR, EXIT_ERROR), + (EXIT_WARNING, EXIT_SIGNAL_BASE, EXIT_SIGNAL_BASE), + (EXIT_ERROR, EXIT_SUCCESS, EXIT_ERROR), + (EXIT_ERROR, EXIT_WARNING, EXIT_ERROR), + (EXIT_ERROR, EXIT_ERROR, EXIT_ERROR), + (EXIT_ERROR, EXIT_SIGNAL_BASE, EXIT_SIGNAL_BASE), + # some modern codes + (EXIT_SUCCESS, EXIT_WARNING_BASE, EXIT_WARNING_BASE), + (EXIT_SUCCESS, EXIT_ERROR_BASE, EXIT_ERROR_BASE), + (EXIT_WARNING_BASE, EXIT_SUCCESS, EXIT_WARNING_BASE), + (EXIT_WARNING_BASE + 1, EXIT_WARNING_BASE + 2, EXIT_WARNING_BASE + 1), + (EXIT_WARNING_BASE, EXIT_ERROR_BASE, EXIT_ERROR_BASE), + (EXIT_WARNING_BASE, EXIT_SIGNAL_BASE, EXIT_SIGNAL_BASE), + (EXIT_ERROR_BASE, EXIT_SUCCESS, EXIT_ERROR_BASE), + (EXIT_ERROR_BASE, EXIT_WARNING_BASE, EXIT_ERROR_BASE), + (EXIT_ERROR_BASE + 1, EXIT_ERROR_BASE + 2, EXIT_ERROR_BASE + 1), + (EXIT_ERROR_BASE, EXIT_SIGNAL_BASE, EXIT_SIGNAL_BASE), + ), +) +def test_max_ec(ec1, ec2, ec_max): + assert max_ec(ec1, ec2) == ec_max