new warnings infrastructure to support modern exit codes

- implement updating exit code based on severity, including modern codes
- extend print_warning with kwargs wc (warning code) and wt (warning type)
- update a global warnings_list with warning_info elements
- create a class hierarchy below BorgWarning class similar to Error class
- diff: change harmless warnings about speed to rc == 0
- delete --force --force: change harmless warnings to rc == 0

Also:

- have BackupRaceConditionError as a more precise subclass of BackupError

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``

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
 

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)
 
 

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:

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)

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
 

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

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

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

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.

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

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

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

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