From 1b09d0efd7253a3cbf55144eed09c5270a331320 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Thu, 9 Nov 2023 05:40:34 +0100 Subject: [PATCH 01/22] BORG_EXIT_CODES=modern can be set to get more specific process exit codes If not set, it will default to "legacy" (always return 2 for errors). This commit only changes the Error exception class and its subclasses. The more specific exit codes need to be defined via .exit_mcode in the subclasses. --- src/borg/helpers/errors.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/borg/helpers/errors.py b/src/borg/helpers/errors.py index ae71cc56..882b65fc 100644 --- a/src/borg/helpers/errors.py +++ b/src/borg/helpers/errors.py @@ -1,3 +1,5 @@ +import os + from ..constants import * # NOQA from ..crypto.low_level import IntegrityError as IntegrityErrorBase @@ -10,8 +12,9 @@ class Error(Exception): # if we raise such an Error and it is only caught by the uppermost # exception handler (that exits short after with the given exit_code), - # it is always a (fatal and abrupt) EXIT_ERROR, never just a warning. - exit_code = EXIT_ERROR + # it is always a (fatal and abrupt) error, never just a warning. + exit_mcode = EXIT_ERROR # modern, more specific exit code (defaults to EXIT_ERROR) + # show a traceback? traceback = False @@ -24,6 +27,13 @@ class Error(Exception): __str__ = get_message + @property + def exit_code(self): + # legacy: borg used to always use rc 2 (EXIT_ERROR) for all errors. + # modern: users can opt in to more specific return codes, using BORG_RC_STYLE: + modern = os.environ.get("BORG_EXIT_CODES", "legacy") == "modern" + return self.exit_mcode if modern else EXIT_ERROR + class ErrorWithTraceback(Error): """Error: {}""" From 34bbce8e713a003078783d9b32afbc75c9487a10 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Thu, 9 Nov 2023 06:50:23 +0100 Subject: [PATCH 02/22] scripts/errorlist.py: improve error list docs generation - also output modern rc and traceback yes/no - recursive list of Error subclasses --- scripts/errorlist.py | 40 ++++++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/scripts/errorlist.py b/scripts/errorlist.py index 6eae5056..735f1125 100755 --- a/scripts/errorlist.py +++ b/scripts/errorlist.py @@ -1,14 +1,42 @@ #!/usr/bin/env python3 +# this script automatically generates the error list for the docs by +# looking at the "Error" class and its subclasses. from textwrap import indent -import borg.archiver # noqa: F401 - need import to get Error and ErrorWithTraceback subclasses. -from borg.helpers import Error, ErrorWithTraceback +import borg.archiver # noqa: F401 - need import to get Error subclasses. +from borg.helpers import Error -classes = Error.__subclasses__() + ErrorWithTraceback.__subclasses__() + +def subclasses(cls): + direct_subclasses = cls.__subclasses__() + return set(direct_subclasses).union([s for c in direct_subclasses for s in subclasses(c)]) + + +# 0, 1, 2 are used for success, generic warning, generic error +# 3..99 are available for specific errors +# 100..127 are available for specific warnings +# 128+ are reserved for signals +free_rcs = set(range(3, 99 + 1)) # 3 .. 99 (we only deal with errors here) + +# these classes map to rc 2 +generic_rc_classes = set() + +classes = {Error}.union(subclasses(Error)) for cls in sorted(classes, key=lambda cls: (cls.__module__, cls.__qualname__)): - if cls is ErrorWithTraceback: - continue - print(" ", cls.__qualname__) + traceback = "yes" if cls.traceback else "no" + rc = cls.exit_mcode + print(" ", cls.__qualname__, "rc:", rc, "traceback:", traceback) print(indent(cls.__doc__, " " * 8)) + if rc in free_rcs: + free_rcs.remove(rc) + elif rc == 2: + generic_rc_classes.add(cls.__qualname__) + else: # rc != 2 + # if we did not intentionally map this to the generic error rc, this might be an issue: + print(f"ERROR: {rc} is not a free/available RC, but either duplicate or invalid") + +print() +print("free RCs:", sorted(free_rcs)) +print("generic errors:", sorted(generic_rc_classes)) From 9de07ebd464a4d12fad7a5eb7114072f1e09ea00 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Thu, 9 Nov 2023 06:00:13 +0100 Subject: [PATCH 03/22] update "modern" error RCs (docs and code) --- docs/internals/frontends.rst | 196 +++++++++++++------- docs/usage/general/environment.rst.inc | 3 + docs/usage/general/return-codes.rst.inc | 6 +- src/borg/archive.py | 12 +- src/borg/archiver/__init__.py | 13 +- src/borg/archiver/check_cmd.py | 13 +- src/borg/archiver/config_cmd.py | 6 +- src/borg/archiver/create_cmd.py | 18 +- src/borg/archiver/debug_cmd.py | 20 +- src/borg/archiver/delete_cmd.py | 2 +- src/borg/archiver/key_cmds.py | 20 +- src/borg/archiver/mount_cmds.py | 13 +- src/borg/archiver/prune_cmd.py | 8 +- src/borg/archiver/rdelete_cmd.py | 5 +- src/borg/archiver/recreate_cmd.py | 5 +- src/borg/cache.py | 26 ++- src/borg/constants.py | 7 +- src/borg/crypto/file_integrity.py | 2 + src/borg/crypto/key.py | 14 ++ src/borg/crypto/keymanager.py | 22 ++- src/borg/helpers/__init__.py | 3 +- src/borg/helpers/errors.py | 28 ++- src/borg/helpers/parseformat.py | 4 + src/borg/helpers/passphrase.py | 14 +- src/borg/locking.py | 18 +- src/borg/manifest.py | 10 +- src/borg/remote.py | 14 ++ src/borg/repository.py | 52 ++++-- src/borg/testsuite/archiver/config_cmd.py | 8 +- src/borg/testsuite/archiver/create_cmd.py | 17 +- src/borg/testsuite/archiver/key_cmds.py | 20 +- src/borg/testsuite/archiver/rdelete_cmd.py | 9 +- src/borg/testsuite/archiver/recreate_cmd.py | 9 +- 33 files changed, 408 insertions(+), 209 deletions(-) diff --git a/docs/internals/frontends.rst b/docs/internals/frontends.rst index 544ec2fd..26cf2edc 100644 --- a/docs/internals/frontends.rst +++ b/docs/internals/frontends.rst @@ -565,92 +565,150 @@ Message IDs are strings that essentially give a log message or operation a name, full text, since texts change more frequently. Message IDs are unambiguous and reduce the need to parse log messages. -Assigned message IDs are: +Assigned message IDs and related error RCs (exit codes) are: .. See scripts/errorlist.py; this is slightly edited. Errors - Archive.AlreadyExists - Archive {} already exists - Archive.DoesNotExist - Archive {} does not exist - Archive.IncompatibleFilesystemEncodingError - Failed to encode filename "{}" into file system encoding "{}". Consider configuring the LANG environment variable. - Cache.CacheInitAbortedError - Cache initialization aborted - Cache.EncryptionMethodMismatch - Repository encryption method changed since last access, refusing to continue - Cache.RepositoryAccessAborted - Repository access aborted - Cache.RepositoryIDNotUnique - Cache is newer than repository - do you have multiple, independently updated repos with same ID? - Cache.RepositoryReplay - Cache is newer than repository - this is either an attack or unsafe (multiple repos with same ID) - Buffer.MemoryLimitExceeded + Error rc: 2 traceback: no + Error: {} + ErrorWithTraceback rc: 2 traceback: yes + Error: {} + + ExtensionModuleError rc: 2 traceback: no + The Borg binary extension modules do not seem to be properly installed. + PythonLibcTooOld rc: 2 traceback: no + FATAL: this Python was compiled for a too old (g)libc and misses required functionality. + Buffer.MemoryLimitExceeded rc: 2 traceback: no Requested buffer size {} is above the limit of {}. - ExtensionModuleError - The Borg binary extension modules do not seem to be installed properly - IntegrityError - Data integrity error: {} - NoManifestError - Repository has no manifest. - PlaceholderError + EfficientCollectionQueue.SizeUnderflow rc: 2 traceback: no + Could not pop_front first {} elements, collection only has {} elements.. + RTError rc: 2 traceback: no + Runtime Error: {} + + CancelledByUser rc: 3 traceback: no + Cancelled by user. + + CommandError rc: 4 traceback: no + Command Error: {} + PlaceholderError rc: 5 traceback: no Formatting Error: "{}".format({}): {}({}) - KeyfileInvalidError - Invalid key file for repository {} found in {}. - KeyfileMismatchError - Mismatch between repository {} and key file {}. - KeyfileNotFoundError - No key file for repository {} found in {}. - PassphraseWrong - passphrase supplied in BORG_PASSPHRASE is incorrect - PasswordRetriesExceeded - exceeded the maximum password retries - RepoKeyNotFoundError - No key entry found in the config of repository {}. - UnsupportedManifestError + InvalidPlaceholder rc: 6 traceback: no + Invalid placeholder "{}" in string: {} + + Repository.AlreadyExists rc: 10 traceback: no + A repository already exists at {}. + Repository.CheckNeeded rc: 12 traceback: yes + Inconsistency detected. Please run "borg check {}". + Repository.DoesNotExist rc: 13 traceback: no + Repository {} does not exist. + Repository.InsufficientFreeSpaceError rc: 14 traceback: no + Insufficient free space to complete transaction (required: {}, available: {}). + Repository.InvalidRepository rc: 15 traceback: no + {} is not a valid repository. Check repo config. + Repository.InvalidRepositoryConfig rc: 16 traceback: no + {} does not have a valid configuration. Check repo config [{}]. + Repository.ObjectNotFound rc: 17 traceback: yes + Object with key {} not found in repository {}. + Repository.ParentPathDoesNotExist rc: 18 traceback: no + The parent path of the repo directory [{}] does not exist. + Repository.PathAlreadyExists rc: 19 traceback: no + There is already something at {}. + Repository.StorageQuotaExceeded rc: 20 traceback: no + The storage quota ({}) has been exceeded ({}). Try deleting some archives. + + MandatoryFeatureUnsupported rc: 25 traceback: no + Unsupported repository feature(s) {}. A newer version of borg is required to access this repository. + NoManifestError rc: 26 traceback: no + Repository has no manifest. + UnsupportedManifestError rc: 27 traceback: no Unsupported manifest envelope. A newer version is required to access this repository. - UnsupportedPayloadError - Unsupported payload type {}. A newer version is required to access this repository. - NotABorgKeyFile + + Archive.AlreadyExists rc: 30 traceback: no + Archive {} already exists + Archive.DoesNotExist rc: 31 traceback: no + Archive {} does not exist + Archive.IncompatibleFilesystemEncodingError rc: 32 traceback: no + Failed to encode filename "{}" into file system encoding "{}". Consider configuring the LANG environment variable. + + KeyfileInvalidError rc: 40 traceback: no + Invalid key file for repository {} found in {}. + KeyfileMismatchError rc: 41 traceback: no + Mismatch between repository {} and key file {}. + KeyfileNotFoundError rc: 42 traceback: no + No key file for repository {} found in {}. + NotABorgKeyFile rc: 43 traceback: no This file is not a borg key backup, aborting. - RepoIdMismatch + RepoKeyNotFoundError rc: 44 traceback: no + No key entry found in the config of repository {}. + RepoIdMismatch rc: 45 traceback: no This key backup seems to be for a different backup repository, aborting. - UnencryptedRepo - Keymanagement not available for unencrypted repositories. - UnknownKeyType - Keytype {0} is unknown. - LockError + UnencryptedRepo rc: 46 traceback: no + Key management not available for unencrypted repositories. + UnknownKeyType rc: 47 traceback: no + Key type {0} is unknown. + UnsupportedPayloadError rc: 48 traceback: no + Unsupported payload type {}. A newer version is required to access this repository. + UnsupportedKeyFormatError rc: 49 traceback:no + Your borg key is stored in an unsupported format. Try using a newer version of borg. + + + NoPassphraseFailure rc: 50 traceback: no + can not acquire a passphrase: {} + PasscommandFailure rc: 51 traceback: no + passcommand supplied in BORG_PASSCOMMAND failed: {} + PassphraseWrong rc: 52 traceback: no + passphrase supplied in BORG_PASSPHRASE, by BORG_PASSCOMMAND or via BORG_PASSPHRASE_FD is incorrect. + PasswordRetriesExceeded rc: 53 traceback: no + exceeded the maximum password retries + + Cache.CacheInitAbortedError rc: 60 traceback: no + Cache initialization aborted + Cache.EncryptionMethodMismatch rc: 61 traceback: no + Repository encryption method changed since last access, refusing to continue + Cache.RepositoryAccessAborted rc: 62 traceback: no + Repository access aborted + Cache.RepositoryIDNotUnique rc: 63 traceback: no + Cache is newer than repository - do you have multiple, independently updated repos with same ID? + Cache.RepositoryReplay rc: 64 traceback: no + Cache, or information obtained from the security directory is newer than repository - this is either an attack or unsafe (multiple repos with same ID) + + LockError rc: 70 traceback: no Failed to acquire the lock {}. - LockErrorT + LockErrorT rc: 71 traceback: yes Failed to acquire the lock {}. - ConnectionClosed + LockFailed rc: 72 traceback: yes + Failed to create/acquire the lock {} ({}). + LockTimeout rc: 73 traceback: no + Failed to create/acquire the lock {} (timeout). + NotLocked rc: 74 traceback: yes + Failed to release the lock {} (was not locked). + NotMyLock rc: 75 traceback: yes + Failed to release the lock {} (was/is locked, but not by me). + + ConnectionClosed rc: 80 traceback: no Connection closed by remote host - InvalidRPCMethod + ConnectionClosedWithHint rc: 81 traceback: no + Connection closed by remote host. {} + InvalidRPCMethod rc: 82 traceback: no RPC method {} is not valid - PathNotAllowed - Repository path not allowed - RemoteRepository.RPCServerOutdated + PathNotAllowed rc: 83 traceback: no + Repository path not allowed: {} + RemoteRepository.RPCServerOutdated rc: 84 traceback: no Borg server is too old for {}. Required version {} - UnexpectedRPCDataFormatFromClient + UnexpectedRPCDataFormatFromClient rc: 85 traceback: no Borg {}: Got unexpected RPC data format from client. - UnexpectedRPCDataFormatFromServer + UnexpectedRPCDataFormatFromServer rc: 86 traceback: no Got unexpected RPC data format from server: {} - Repository.AlreadyExists - Repository {} already exists. - Repository.CheckNeeded - Inconsistency detected. Please run "borg check {}". - Repository.DoesNotExist - Repository {} does not exist. - Repository.InsufficientFreeSpaceError - Insufficient free space to complete transaction (required: {}, available: {}). - Repository.InvalidRepository - {} is not a valid repository. Check repo config. - Repository.AtticRepository - Attic repository detected. Please run "borg upgrade {}". - Repository.ObjectNotFound - Object with key {} not found in repository {}. + + IntegrityError rc: 90 traceback: yes + Data integrity error: {} + FileIntegrityError rc: 91 traceback: yes + File failed integrity check: {} + DecompressionError rc: 92 traceback: yes + Decompression error: {} + Operations - cache.begin_transaction diff --git a/docs/usage/general/environment.rst.inc b/docs/usage/general/environment.rst.inc index 4902c0ae..fe06afde 100644 --- a/docs/usage/general/environment.rst.inc +++ b/docs/usage/general/environment.rst.inc @@ -36,6 +36,9 @@ General: Main usecase for this is to automate fully ``borg change-passphrase``. BORG_DISPLAY_PASSPHRASE When set, use the value to answer the "display the passphrase for verification" question when defining a new passphrase for encrypted repositories. + BORG_EXIT_CODES + When set to "modern", the borg process will return more specific exit codes (rc). + Default is "legacy" and returns rc 2 for all errors, 1 for all warnings, 0 for success. BORG_HOST_ID Borg usually computes a host id from the FQDN plus the results of ``uuid.getnode()`` (which usually returns a unique id based on the MAC address of the network interface. Except if that MAC happens to be all-zero - in diff --git a/docs/usage/general/return-codes.rst.inc b/docs/usage/general/return-codes.rst.inc index 68f458c4..e908c928 100644 --- a/docs/usage/general/return-codes.rst.inc +++ b/docs/usage/general/return-codes.rst.inc @@ -7,10 +7,12 @@ Borg can exit with the following return codes (rc): Return code Meaning =========== ======= 0 success (logged as INFO) -1 warning (operation reached its normal end, but there were warnings -- +1 generic warning (operation reached its normal end, but there were warnings -- you should check the log, logged as WARNING) -2 error (like a fatal error, a local or remote exception, the operation +2 generic error (like a fatal error, a local or remote exception, the operation did not reach its normal end, logged as ERROR) +3..99 specific error (enabled by BORG_EXIT_CODES=modern) +100..127 specific warning (enabled by BORG_EXIT_CODES=modern) 128+N killed by signal N (e.g. 137 == kill -9) =========== ======= diff --git a/src/borg/archive.py b/src/borg/archive.py index b048d5c1..639f8ef8 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -454,15 +454,21 @@ def archive_put_items(chunk_ids, *, repo_objs, cache=None, stats=None, add_refer class Archive: - class DoesNotExist(Error): - """Archive {} does not exist""" - class AlreadyExists(Error): """Archive {} already exists""" + exit_mcode = 30 + + class DoesNotExist(Error): + """Archive {} does not exist""" + + exit_mcode = 31 + class IncompatibleFilesystemEncodingError(Error): """Failed to encode filename "{}" into file system encoding "{}". Consider configuring the LANG environment variable.""" + exit_mcode = 32 + def __init__( self, manifest, diff --git a/src/borg/archiver/__init__.py b/src/borg/archiver/__init__.py index 8e360d31..f0f0ca5c 100644 --- a/src/borg/archiver/__init__.py +++ b/src/borg/archiver/__init__.py @@ -25,7 +25,7 @@ try: from .. import __version__ from ..constants import * # NOQA from ..helpers import EXIT_SUCCESS, EXIT_WARNING, EXIT_ERROR, EXIT_SIGNAL_BASE - from ..helpers import Error, set_ec + from ..helpers import Error, CommandError, set_ec, modern_ec from ..helpers import format_file_size from ..helpers import remove_surrogates, text_to_json from ..helpers import DatetimeWrapper, replace_placeholders @@ -128,11 +128,6 @@ class Archiver( self.prog = prog self.last_checkpoint = time.monotonic() - def print_error(self, msg, *args): - msg = args and msg % args or msg - self.exit_code = EXIT_ERROR - logger.error(msg) - 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 @@ -631,7 +626,7 @@ def main(): # pragma: no cover except argparse.ArgumentTypeError as e: # we might not have logging setup yet, so get out quickly print(str(e), file=sys.stderr) - sys.exit(EXIT_ERROR) + sys.exit(CommandError.exit_mcode if modern_ec else EXIT_ERROR) except Exception: msg = "Local Exception" tb = f"{traceback.format_exc()}\n{sysinfo()}" @@ -687,9 +682,9 @@ def main(): # pragma: no cover 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: + 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: + 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)) diff --git a/src/borg/archiver/check_cmd.py b/src/borg/archiver/check_cmd.py index 52563fc5..5ef4ebbc 100644 --- a/src/borg/archiver/check_cmd.py +++ b/src/borg/archiver/check_cmd.py @@ -2,7 +2,7 @@ import argparse from ._common import with_repository, Highlander from ..archive import ArchiveChecker from ..constants import * # NOQA -from ..helpers import EXIT_SUCCESS, EXIT_WARNING, EXIT_ERROR +from ..helpers import EXIT_SUCCESS, EXIT_WARNING, CancelledByUser, CommandError from ..helpers import yes from ..logger import create_logger @@ -30,22 +30,19 @@ class CheckMixIn: retry=False, env_var_override="BORG_CHECK_I_KNOW_WHAT_I_AM_DOING", ): - return EXIT_ERROR + raise CancelledByUser() if args.repo_only and any((args.verify_data, args.first, args.last, args.match_archives)): - self.print_error( + raise CommandError( "--repository-only contradicts --first, --last, -a / --match-archives and --verify-data arguments." ) - return EXIT_ERROR if args.repair and args.max_duration: - self.print_error("--repair does not allow --max-duration argument.") - return EXIT_ERROR + raise CommandError("--repair does not allow --max-duration argument.") if args.max_duration and not args.repo_only: # when doing a partial repo check, we can only check crc32 checksums in segment files, # we can't build a fresh repo index in memory to verify the on-disk index against it. # thus, we should not do an archives check based on a unknown-quality on-disk repo index. # also, there is no max_duration support in the archives check code anyway. - self.print_error("--repository-only is required for --max-duration support.") - return EXIT_ERROR + raise CommandError("--repository-only is required for --max-duration support.") if not args.archives_only: if not repository.check(repair=args.repair, max_duration=args.max_duration): return EXIT_WARNING diff --git a/src/borg/archiver/config_cmd.py b/src/borg/archiver/config_cmd.py index 67595441..6704d716 100644 --- a/src/borg/archiver/config_cmd.py +++ b/src/borg/archiver/config_cmd.py @@ -7,7 +7,7 @@ from ._common import with_repository from ..cache import Cache, assert_secure from ..constants import * # NOQA from ..helpers import EXIT_SUCCESS, EXIT_WARNING -from ..helpers import Error +from ..helpers import Error, CommandError from ..helpers import Location from ..helpers import parse_file_size from ..manifest import Manifest @@ -99,9 +99,7 @@ class ConfigMixIn: if not args.list: if args.name is None: - self.print_error("No config key name was provided.") - return self.exit_code - + raise CommandError("No config key name was provided.") try: section, name = args.name.split(".") except ValueError: diff --git a/src/borg/archiver/create_cmd.py b/src/borg/archiver/create_cmd.py index fb9f3cfa..5c58d89e 100644 --- a/src/borg/archiver/create_cmd.py +++ b/src/borg/archiver/create_cmd.py @@ -29,6 +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 ..manifest import Manifest from ..patterns import PatternMatcher from ..platform import is_win32 @@ -79,18 +80,15 @@ class CreateMixIn: preexec_fn=None if is_win32 else ignore_sigint, ) except (FileNotFoundError, PermissionError) as e: - self.print_error("Failed to execute command: %s", e) - return self.exit_code + raise CommandError("Failed to execute command: %s", e) status = fso.process_pipe( path=path, cache=cache, fd=proc.stdout, mode=mode, user=user, group=group ) rc = proc.wait() if rc != 0: - self.print_error("Command %r exited with status %d", args.paths[0], rc) - return self.exit_code + raise CommandError("Command %r exited with status %d", args.paths[0], rc) except BackupOSError as e: - self.print_error("%s: %s", path, e) - return self.exit_code + raise Error("%s: %s", path, e) else: status = "+" # included self.print_file_status(status, path) @@ -103,8 +101,7 @@ class CreateMixIn: args.paths, stdout=subprocess.PIPE, env=env, preexec_fn=None if is_win32 else ignore_sigint ) except (FileNotFoundError, PermissionError) as e: - self.print_error("Failed to execute command: %s", e) - return self.exit_code + raise CommandError("Failed to execute command: %s", e) pipe_bin = proc.stdout else: # args.paths_from_stdin == True pipe_bin = sys.stdin.buffer @@ -135,8 +132,7 @@ class CreateMixIn: if args.paths_from_command: rc = proc.wait() if rc != 0: - self.print_error("Command %r exited with status %d", args.paths[0], rc) - return self.exit_code + raise CommandError("Command %r exited with status %d", args.paths[0], rc) else: for path in args.paths: if path == "": # issue #5637 @@ -197,7 +193,7 @@ class CreateMixIn: if sig_int: # do not save the archive if the user ctrl-c-ed - it is valid, but incomplete. # we already have a checkpoint archive in this case. - self.print_error("Got Ctrl-C / SIGINT.") + raise Error("Got Ctrl-C / SIGINT.") else: archive.save(comment=args.comment, timestamp=args.timestamp) args.stats |= args.json diff --git a/src/borg/archiver/debug_cmd.py b/src/borg/archiver/debug_cmd.py index cab8075c..d7265a5b 100644 --- a/src/borg/archiver/debug_cmd.py +++ b/src/borg/archiver/debug_cmd.py @@ -13,6 +13,7 @@ from ..helpers import bin_to_hex, prepare_dump_dict from ..helpers import dash_open from ..helpers import StableDict from ..helpers import positive_int_validator, archivename_validator +from ..helpers import CommandError, RTError from ..manifest import Manifest from ..platform import get_process_id from ..repository import Repository, LIST_SCAN_LIMIT, TAG_PUT, TAG_DELETE, TAG_COMMIT @@ -191,8 +192,7 @@ class DebugMixIn: except (ValueError, UnicodeEncodeError): wanted = None if not wanted: - self.print_error("search term needs to be hex:123abc or str:foobar style") - return EXIT_ERROR + raise CommandError("search term needs to be hex:123abc or str:foobar style") from ..crypto.key import key_factory @@ -245,13 +245,11 @@ class DebugMixIn: if len(id) != 32: # 256bit raise ValueError("id must be 256bits or 64 hex digits") except ValueError as err: - print(f"object id {hex_id} is invalid [{str(err)}].") - return EXIT_ERROR + raise CommandError(f"object id {hex_id} is invalid [{str(err)}].") try: data = repository.get(id) except Repository.ObjectNotFound: - print("object %s not found." % hex_id) - return EXIT_ERROR + raise RTError("object %s not found." % hex_id) with open(args.path, "wb") as f: f.write(data) print("object %s fetched." % hex_id) @@ -278,8 +276,7 @@ class DebugMixIn: if len(id) != 32: # 256bit raise ValueError("id must be 256bits or 64 hex digits") except ValueError as err: - print(f"object id {hex_id} is invalid [{str(err)}].") - return EXIT_ERROR + raise CommandError(f"object id {hex_id} is invalid [{str(err)}].") with open(args.object_path, "rb") as f: cdata = f.read() @@ -306,8 +303,7 @@ class DebugMixIn: if len(id) != 32: # 256bit raise ValueError("id must be 256bits or 64 hex digits") except ValueError as err: - print(f"object id {hex_id} is invalid [{str(err)}].") - return EXIT_ERROR + raise CommandError(f"object id {hex_id} is invalid [{str(err)}].") with open(args.binary_path, "rb") as f: data = f.read() @@ -334,8 +330,8 @@ class DebugMixIn: if len(id) != 32: # 256bit raise ValueError("id must be 256bits or 64 hex digits") except ValueError as err: - print(f"object id {hex_id} is invalid [{str(err)}].") - return EXIT_ERROR + raise CommandError(f"object id {hex_id} is invalid [{str(err)}].") + repository.put(id, data) print("object %s put." % hex_id) repository.commit(compact=False) diff --git a/src/borg/archiver/delete_cmd.py b/src/borg/archiver/delete_cmd.py index e9444dcf..9e2b4e8b 100644 --- a/src/borg/archiver/delete_cmd.py +++ b/src/borg/archiver/delete_cmd.py @@ -87,7 +87,7 @@ class DeleteMixIn: uncommitted_deletes = 0 if checkpointed else (uncommitted_deletes + 1) if sig_int: # Ctrl-C / SIGINT: do not checkpoint (commit) again, we already have a checkpoint in this case. - self.print_error("Got Ctrl-C / SIGINT.") + raise Error("Got Ctrl-C / SIGINT.") elif uncommitted_deletes > 0: checkpoint_func() if args.stats: diff --git a/src/borg/archiver/key_cmds.py b/src/borg/archiver/key_cmds.py index cd658b72..74b388ac 100644 --- a/src/borg/archiver/key_cmds.py +++ b/src/borg/archiver/key_cmds.py @@ -6,7 +6,7 @@ from ..constants import * # NOQA from ..crypto.key import AESOCBRepoKey, CHPORepoKey, Blake2AESOCBRepoKey, Blake2CHPORepoKey from ..crypto.key import AESOCBKeyfileKey, CHPOKeyfileKey, Blake2AESOCBKeyfileKey, Blake2CHPOKeyfileKey from ..crypto.keymanager import KeyManager -from ..helpers import PathSpec +from ..helpers import PathSpec, CommandError from ..manifest import Manifest from ._common import with_repository @@ -22,8 +22,7 @@ class KeysMixIn: """Change repository key file passphrase""" key = manifest.key if not hasattr(key, "change_passphrase"): - print("This repository is not encrypted, cannot change the passphrase.") - return EXIT_ERROR + raise CommandError("This repository is not encrypted, cannot change the passphrase.") key.change_passphrase() logger.info("Key updated") if hasattr(key, "find_key"): @@ -36,8 +35,7 @@ class KeysMixIn: """Change repository key location""" key = manifest.key if not hasattr(key, "change_passphrase"): - print("This repository is not encrypted, cannot change the key location.") - return EXIT_ERROR + raise CommandError("This repository is not encrypted, cannot change the key location.") if args.key_mode == "keyfile": if isinstance(key, AESOCBRepoKey): @@ -109,8 +107,7 @@ class KeysMixIn: else: manager.export(args.path) except IsADirectoryError: - self.print_error(f"'{args.path}' must be a file, not a directory") - return EXIT_ERROR + raise CommandError(f"'{args.path}' must be a file, not a directory") return EXIT_SUCCESS @with_repository(lock=False, exclusive=False, manifest=False, cache=False) @@ -119,16 +116,13 @@ class KeysMixIn: manager = KeyManager(repository) if args.paper: if args.path: - self.print_error("with --paper import from file is not supported") - return EXIT_ERROR + raise CommandError("with --paper import from file is not supported") manager.import_paperkey(args) else: if not args.path: - self.print_error("input file to import key from expected") - return EXIT_ERROR + raise CommandError("expected input file to import key from") if args.path != "-" and not os.path.exists(args.path): - self.print_error("input file does not exist: " + args.path) - return EXIT_ERROR + raise CommandError("input file does not exist: " + args.path) manager.import_keyfile(args) return EXIT_SUCCESS diff --git a/src/borg/archiver/mount_cmds.py b/src/borg/archiver/mount_cmds.py index 74c70f24..1bd96617 100644 --- a/src/borg/archiver/mount_cmds.py +++ b/src/borg/archiver/mount_cmds.py @@ -3,7 +3,7 @@ import os from ._common import with_repository, Highlander from ..constants import * # NOQA -from ..helpers import EXIT_ERROR +from ..helpers import RTError from ..helpers import PathSpec from ..helpers import umount from ..manifest import Manifest @@ -22,16 +22,13 @@ class MountMixIn: from ..fuse_impl import llfuse, BORG_FUSE_IMPL if llfuse is None: - self.print_error("borg mount not available: no FUSE support, BORG_FUSE_IMPL=%s." % BORG_FUSE_IMPL) - return self.exit_code + raise RTError("borg mount not available: no FUSE support, BORG_FUSE_IMPL=%s." % BORG_FUSE_IMPL) if not os.path.isdir(args.mountpoint): - self.print_error(f"{args.mountpoint}: Mountpoint must be an **existing directory**") - return self.exit_code + raise RTError(f"{args.mountpoint}: Mountpoint must be an **existing directory**") if not os.access(args.mountpoint, os.R_OK | os.W_OK | os.X_OK): - self.print_error(f"{args.mountpoint}: Mountpoint must be a **writable** directory") - return self.exit_code + raise RTError(f"{args.mountpoint}: Mountpoint must be a **writable** directory") return self._do_mount(args) @@ -46,7 +43,7 @@ class MountMixIn: operations.mount(args.mountpoint, args.options, args.foreground) except RuntimeError: # Relevant error message already printed to stderr by FUSE - self.exit_code = EXIT_ERROR + raise RTError("FUSE mount failed") return self.exit_code def do_umount(self, args): diff --git a/src/borg/archiver/prune_cmd.py b/src/borg/archiver/prune_cmd.py index c0cf3aed..467fb76a 100644 --- a/src/borg/archiver/prune_cmd.py +++ b/src/borg/archiver/prune_cmd.py @@ -10,7 +10,7 @@ from ._common import with_repository, Highlander from ..archive import Archive, Statistics from ..cache import Cache from ..constants import * # NOQA -from ..helpers import ArchiveFormatter, interval, sig_int, log_multi, ProgressIndicatorPercent +from ..helpers import ArchiveFormatter, interval, sig_int, log_multi, ProgressIndicatorPercent, CommandError, Error from ..manifest import Manifest from ..logger import create_logger @@ -77,12 +77,12 @@ class PruneMixIn: if not any( (args.secondly, args.minutely, args.hourly, args.daily, args.weekly, args.monthly, args.yearly, args.within) ): - self.print_error( + raise CommandError( 'At least one of the "keep-within", "keep-last", ' '"keep-secondly", "keep-minutely", "keep-hourly", "keep-daily", ' '"keep-weekly", "keep-monthly" or "keep-yearly" settings must be specified.' ) - return self.exit_code + if args.format is not None: format = args.format elif args.short: @@ -173,7 +173,7 @@ class PruneMixIn: pi.finish() if sig_int: # Ctrl-C / SIGINT: do not checkpoint (commit) again, we already have a checkpoint in this case. - self.print_error("Got Ctrl-C / SIGINT.") + raise Error("Got Ctrl-C / SIGINT.") elif uncommitted_deletes > 0: checkpoint_func() if args.stats: diff --git a/src/borg/archiver/rdelete_cmd.py b/src/borg/archiver/rdelete_cmd.py index 40ccbc72..dead4c31 100644 --- a/src/borg/archiver/rdelete_cmd.py +++ b/src/borg/archiver/rdelete_cmd.py @@ -3,7 +3,7 @@ import argparse from ._common import with_repository from ..cache import Cache, SecurityManager from ..constants import * # NOQA -from ..helpers import EXIT_ERROR +from ..helpers import CancelledByUser from ..helpers import format_archive from ..helpers import bin_to_hex from ..helpers import yes @@ -72,8 +72,7 @@ class RDeleteMixIn: retry=False, env_var_override="BORG_DELETE_I_KNOW_WHAT_I_AM_DOING", ): - self.exit_code = EXIT_ERROR - return self.exit_code + raise CancelledByUser() if not dry_run: repository.destroy() logger.info("Repository deleted.") diff --git a/src/borg/archiver/recreate_cmd.py b/src/borg/archiver/recreate_cmd.py index f1803a48..7f4b5da6 100644 --- a/src/borg/archiver/recreate_cmd.py +++ b/src/borg/archiver/recreate_cmd.py @@ -5,7 +5,7 @@ from ._common import build_matcher from ..archive import ArchiveRecreater from ..constants import * # NOQA from ..compress import CompressionSpec -from ..helpers import archivename_validator, comment_validator, PathSpec, ChunkerParams +from ..helpers import archivename_validator, comment_validator, PathSpec, ChunkerParams, CommandError from ..helpers import timestamp from ..manifest import Manifest @@ -42,8 +42,7 @@ class RecreateMixIn: archive_names = tuple(archive.name for archive in manifest.archives.list_considering(args)) if args.target is not None and len(archive_names) != 1: - self.print_error("--target: Need to specify single archive") - return self.exit_code + raise CommandError("--target: Need to specify single archive") for name in archive_names: if recreater.is_temporary_archive(name): continue diff --git a/src/borg/cache.py b/src/borg/cache.py index c18f315a..a55dadc0 100644 --- a/src/borg/cache.py +++ b/src/borg/cache.py @@ -365,20 +365,30 @@ class CacheConfig: class Cache: """Client Side cache""" - class RepositoryIDNotUnique(Error): - """Cache is newer than repository - do you have multiple, independently updated repos with same ID?""" - - class RepositoryReplay(Error): - """Cache, or information obtained from the security directory is newer than repository - this is either an attack or unsafe (multiple repos with same ID)""" - class CacheInitAbortedError(Error): """Cache initialization aborted""" + exit_mcode = 60 + + class EncryptionMethodMismatch(Error): + """Repository encryption method changed since last access, refusing to continue""" + + exit_mcode = 61 + class RepositoryAccessAborted(Error): """Repository access aborted""" - class EncryptionMethodMismatch(Error): - """Repository encryption method changed since last access, refusing to continue""" + exit_mcode = 62 + + class RepositoryIDNotUnique(Error): + """Cache is newer than repository - do you have multiple, independently updated repos with same ID?""" + + exit_mcode = 63 + + class RepositoryReplay(Error): + """Cache, or information obtained from the security directory is newer than repository - this is either an attack or unsafe (multiple repos with same ID)""" + + exit_mcode = 64 @staticmethod def break_lock(repository, path=None): diff --git a/src/borg/constants.py b/src/borg/constants.py index 7f4cbc31..76e63f79 100644 --- a/src/borg/constants.py +++ b/src/borg/constants.py @@ -114,10 +114,11 @@ FILES_CACHE_MODE_UI_DEFAULT = "ctime,size,inode" # default for "borg create" co FILES_CACHE_MODE_DISABLED = "d" # most borg commands do not use the files cache at all (disable) # return codes returned by borg command -# when borg is killed by signal N, rc = 128 + N EXIT_SUCCESS = 0 # everything done, no problems -EXIT_WARNING = 1 # reached normal end of operation, but there were issues -EXIT_ERROR = 2 # terminated abruptly, did not reach end of operation +EXIT_WARNING = 1 # reached normal end of operation, but there were issues (generic warning) +EXIT_ERROR = 2 # terminated abruptly, did not reach end of operation (generic error) +EXIT_ERROR_BASE = 3 # specific error codes are 3..99 (enabled by BORG_EXIT_CODES=modern) +EXIT_WARNING_BASE = 100 # specific warning codes are 100..127 (enabled by BORG_EXIT_CODES=modern) EXIT_SIGNAL_BASE = 128 # terminated due to signal, rc = 128 + sig_no ISO_FORMAT_NO_USECS = "%Y-%m-%dT%H:%M:%S" diff --git a/src/borg/crypto/file_integrity.py b/src/borg/crypto/file_integrity.py index 33b503a6..2bac18f2 100644 --- a/src/borg/crypto/file_integrity.py +++ b/src/borg/crypto/file_integrity.py @@ -124,6 +124,8 @@ SUPPORTED_ALGORITHMS = { class FileIntegrityError(IntegrityError): """File failed integrity check: {}""" + exit_mcode = 91 + class IntegrityCheckedFile(FileLikeWrapper): def __init__(self, path, write, filename=None, override_fd=None, integrity_data=None): diff --git a/src/borg/crypto/key.py b/src/borg/crypto/key.py index ee820c8b..cf72c8c3 100644 --- a/src/borg/crypto/key.py +++ b/src/borg/crypto/key.py @@ -38,30 +38,44 @@ AUTHENTICATED_NO_KEY = "authenticated_no_key" in workarounds class UnsupportedPayloadError(Error): """Unsupported payload type {}. A newer version is required to access this repository.""" + exit_mcode = 48 + class UnsupportedManifestError(Error): """Unsupported manifest envelope. A newer version is required to access this repository.""" + exit_mcode = 27 + class KeyfileNotFoundError(Error): """No key file for repository {} found in {}.""" + exit_mcode = 42 + class KeyfileInvalidError(Error): """Invalid key file for repository {} found in {}.""" + exit_mcode = 40 + class KeyfileMismatchError(Error): """Mismatch between repository {} and key file {}.""" + exit_mcode = 41 + class RepoKeyNotFoundError(Error): """No key entry found in the config of repository {}.""" + exit_mcode = 44 + class UnsupportedKeyFormatError(Error): """Your borg key is stored in an unsupported format. Try using a newer version of borg.""" + exit_mcode = 49 + def key_creator(repository, args, *, other_key=None): for key in AVAILABLE_KEY_TYPES: diff --git a/src/borg/crypto/keymanager.py b/src/borg/crypto/keymanager.py index eb4962bf..4051f469 100644 --- a/src/borg/crypto/keymanager.py +++ b/src/borg/crypto/keymanager.py @@ -13,20 +13,28 @@ from ..repoobj import RepoObj from .key import CHPOKeyfileKey, RepoKeyNotFoundError, KeyBlobStorage, identify_key -class UnencryptedRepo(Error): - """Keymanagement not available for unencrypted repositories.""" +class NotABorgKeyFile(Error): + """This file is not a borg key backup, aborting.""" - -class UnknownKeyType(Error): - """Keytype {0} is unknown.""" + exit_mcode = 43 class RepoIdMismatch(Error): """This key backup seems to be for a different backup repository, aborting.""" + exit_mcode = 45 -class NotABorgKeyFile(Error): - """This file is not a borg key backup, aborting.""" + +class UnencryptedRepo(Error): + """Key management not available for unencrypted repositories.""" + + exit_mcode = 46 + + +class UnknownKeyType(Error): + """Key type {0} is unknown.""" + + exit_mcode = 47 def sha256_truncated(data, num): diff --git a/src/borg/helpers/__init__.py b/src/borg/helpers/__init__.py index 64f66078..6a3ee2ad 100644 --- a/src/borg/helpers/__init__.py +++ b/src/borg/helpers/__init__.py @@ -10,7 +10,8 @@ import os 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 +from .errors import Error, ErrorWithTraceback, IntegrityError, DecompressionError, CancelledByUser, CommandError +from .errors import RTError, modern_ec 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 diff --git a/src/borg/helpers/errors.py b/src/borg/helpers/errors.py index 882b65fc..fe800ed8 100644 --- a/src/borg/helpers/errors.py +++ b/src/borg/helpers/errors.py @@ -5,6 +5,9 @@ from ..constants import * # NOQA from ..crypto.low_level import IntegrityError as IntegrityErrorBase +modern_ec = os.environ.get("BORG_EXIT_CODES", "legacy") == "modern" + + class Error(Exception): """Error: {}""" @@ -30,9 +33,8 @@ class Error(Exception): @property def exit_code(self): # legacy: borg used to always use rc 2 (EXIT_ERROR) for all errors. - # modern: users can opt in to more specific return codes, using BORG_RC_STYLE: - modern = os.environ.get("BORG_EXIT_CODES", "legacy") == "modern" - return self.exit_mcode if modern else EXIT_ERROR + # modern: users can opt in to more specific return codes, using BORG_EXIT_CODES: + return self.exit_mcode if modern_ec else EXIT_ERROR class ErrorWithTraceback(Error): @@ -45,6 +47,26 @@ class ErrorWithTraceback(Error): class IntegrityError(ErrorWithTraceback, IntegrityErrorBase): """Data integrity error: {}""" + exit_mcode = 90 + class DecompressionError(IntegrityError): """Decompression error: {}""" + + exit_mcode = 92 + + +class CancelledByUser(Error): + """Cancelled by user.""" + + exit_mcode = 3 + + +class RTError(Error): + """Runtime Error: {}""" + + +class CommandError(Error): + """Command Error: {}""" + + exit_mcode = 4 diff --git a/src/borg/helpers/parseformat.py b/src/borg/helpers/parseformat.py index aeb07f83..e5752d34 100644 --- a/src/borg/helpers/parseformat.py +++ b/src/borg/helpers/parseformat.py @@ -224,10 +224,14 @@ class DatetimeWrapper: class PlaceholderError(Error): """Formatting Error: "{}".format({}): {}({})""" + exit_mcode = 5 + class InvalidPlaceholder(PlaceholderError): """Invalid placeholder "{}" in string: {}""" + exit_mcode = 6 + def format_line(format, data): for _, key, _, conversion in Formatter().parse(format): diff --git a/src/borg/helpers/passphrase.py b/src/borg/helpers/passphrase.py index 643709d3..963e1836 100644 --- a/src/borg/helpers/passphrase.py +++ b/src/borg/helpers/passphrase.py @@ -17,18 +17,26 @@ logger = create_logger() class NoPassphraseFailure(Error): """can not acquire a passphrase: {}""" - -class PassphraseWrong(Error): - """passphrase supplied in BORG_PASSPHRASE, by BORG_PASSCOMMAND or via BORG_PASSPHRASE_FD is incorrect.""" + exit_mcode = 50 class PasscommandFailure(Error): """passcommand supplied in BORG_PASSCOMMAND failed: {}""" + exit_mcode = 51 + + +class PassphraseWrong(Error): + """passphrase supplied in BORG_PASSPHRASE, by BORG_PASSCOMMAND or via BORG_PASSPHRASE_FD is incorrect.""" + + exit_mcode = 52 + class PasswordRetriesExceeded(Error): """exceeded the maximum password retries""" + exit_mcode = 53 + class Passphrase(str): @classmethod diff --git a/src/borg/locking.py b/src/borg/locking.py index 59508180..67b94a4d 100644 --- a/src/borg/locking.py +++ b/src/borg/locking.py @@ -71,26 +71,38 @@ class TimeoutTimer: class LockError(Error): """Failed to acquire the lock {}.""" + exit_mcode = 70 + class LockErrorT(ErrorWithTraceback): """Failed to acquire the lock {}.""" - -class LockTimeout(LockError): - """Failed to create/acquire the lock {} (timeout).""" + exit_mcode = 71 class LockFailed(LockErrorT): """Failed to create/acquire the lock {} ({}).""" + exit_mcode = 72 + + +class LockTimeout(LockError): + """Failed to create/acquire the lock {} (timeout).""" + + exit_mcode = 73 + class NotLocked(LockErrorT): """Failed to release the lock {} (was not locked).""" + exit_mcode = 74 + class NotMyLock(LockErrorT): """Failed to release the lock {} (was/is locked, but not by me).""" + exit_mcode = 75 + class ExclusiveLock: """An exclusive Lock based on mkdir fs operation being atomic. diff --git a/src/borg/manifest.py b/src/borg/manifest.py index 8a6f9a34..9b23cb63 100644 --- a/src/borg/manifest.py +++ b/src/borg/manifest.py @@ -18,12 +18,16 @@ from .patterns import get_regex_from_pattern from .repoobj import RepoObj +class MandatoryFeatureUnsupported(Error): + """Unsupported repository feature(s) {}. A newer version of borg is required to access this repository.""" + + exit_mcode = 25 + + class NoManifestError(Error): """Repository has no manifest.""" - -class MandatoryFeatureUnsupported(Error): - """Unsupported repository feature(s) {}. A newer version of borg is required to access this repository.""" + exit_mcode = 26 ArchiveInfo = namedtuple("ArchiveInfo", "name id ts") diff --git a/src/borg/remote.py b/src/borg/remote.py index 4a1551f0..1471b495 100644 --- a/src/borg/remote.py +++ b/src/borg/remote.py @@ -69,26 +69,38 @@ def os_write(fd, data): class ConnectionClosed(Error): """Connection closed by remote host""" + exit_mcode = 80 + class ConnectionClosedWithHint(ConnectionClosed): """Connection closed by remote host. {}""" + exit_mcode = 81 + class PathNotAllowed(Error): """Repository path not allowed: {}""" + exit_mcode = 83 + class InvalidRPCMethod(Error): """RPC method {} is not valid""" + exit_mcode = 82 + class UnexpectedRPCDataFormatFromClient(Error): """Borg {}: Got unexpected RPC data format from client.""" + exit_mcode = 85 + class UnexpectedRPCDataFormatFromServer(Error): """Got unexpected RPC data format from server:\n{}""" + exit_mcode = 86 + def __init__(self, data): try: data = data.decode()[:128] @@ -513,6 +525,8 @@ class RemoteRepository: class RPCServerOutdated(Error): """Borg server is too old for {}. Required version {}""" + exit_mcode = 84 + @property def method(self): return self.args[0] diff --git a/src/borg/repository.py b/src/borg/repository.py index 7bc2b08a..dd59e488 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -134,41 +134,61 @@ class Repository: will still get rid of them. """ - class DoesNotExist(Error): - """Repository {} does not exist.""" - class AlreadyExists(Error): """A repository already exists at {}.""" - class PathAlreadyExists(Error): - """There is already something at {}.""" - - class ParentPathDoesNotExist(Error): - """The parent path of the repo directory [{}] does not exist.""" - - class InvalidRepository(Error): - """{} is not a valid repository. Check repo config.""" - - class InvalidRepositoryConfig(Error): - """{} does not have a valid configuration. Check repo config [{}].""" + exit_mcode = 10 class CheckNeeded(ErrorWithTraceback): """Inconsistency detected. Please run "borg check {}".""" + exit_mcode = 12 + + class DoesNotExist(Error): + """Repository {} does not exist.""" + + exit_mcode = 13 + + class InsufficientFreeSpaceError(Error): + """Insufficient free space to complete transaction (required: {}, available: {}).""" + + exit_mcode = 14 + + class InvalidRepository(Error): + """{} is not a valid repository. Check repo config.""" + + exit_mcode = 15 + + class InvalidRepositoryConfig(Error): + """{} does not have a valid configuration. Check repo config [{}].""" + + exit_mcode = 16 + class ObjectNotFound(ErrorWithTraceback): """Object with key {} not found in repository {}.""" + exit_mcode = 17 + def __init__(self, id, repo): if isinstance(id, bytes): id = bin_to_hex(id) super().__init__(id, repo) - class InsufficientFreeSpaceError(Error): - """Insufficient free space to complete transaction (required: {}, available: {}).""" + class ParentPathDoesNotExist(Error): + """The parent path of the repo directory [{}] does not exist.""" + + exit_mcode = 18 + + class PathAlreadyExists(Error): + """There is already something at {}.""" + + exit_mcode = 19 class StorageQuotaExceeded(Error): """The storage quota ({}) has been exceeded ({}). Try deleting some archives.""" + exit_mcode = 20 + def __init__( self, path, diff --git a/src/borg/testsuite/archiver/config_cmd.py b/src/borg/testsuite/archiver/config_cmd.py index 29ef32f6..2215b55a 100644 --- a/src/borg/testsuite/archiver/config_cmd.py +++ b/src/borg/testsuite/archiver/config_cmd.py @@ -1,7 +1,9 @@ import os +import pytest from ...constants import * # NOQA from . import RK_ENCRYPTION, create_test_files, cmd, generate_archiver_tests +from ...helpers import CommandError pytest_generate_tests = lambda metafunc: generate_archiver_tests(metafunc, kinds="local,binary") # NOQA @@ -40,5 +42,9 @@ def test_config(archivers, request): cmd(archiver, "config", cfg_key, exit_code=1) cmd(archiver, "config", "--list", "--delete", exit_code=2) - cmd(archiver, "config", exit_code=2) + if archiver.FORK_DEFAULT: + cmd(archiver, "config", exit_code=2) + else: + with pytest.raises(CommandError): + cmd(archiver, "config") cmd(archiver, "config", "invalid-option", exit_code=1) diff --git a/src/borg/testsuite/archiver/create_cmd.py b/src/borg/testsuite/archiver/create_cmd.py index 48d8fb23..d536c435 100644 --- a/src/borg/testsuite/archiver/create_cmd.py +++ b/src/borg/testsuite/archiver/create_cmd.py @@ -16,6 +16,7 @@ from ...constants import * # NOQA from ...manifest import Manifest from ...platform import is_cygwin, is_win32, is_darwin from ...repository import Repository +from ...helpers import CommandError from .. import has_lchflags from .. import changedir from .. import ( @@ -360,8 +361,12 @@ def test_create_content_from_command(archivers, request): def test_create_content_from_command_with_failed_command(archivers, request): archiver = request.getfixturevalue(archivers) cmd(archiver, "rcreate", RK_ENCRYPTION) - output = cmd(archiver, "create", "--content-from-command", "test", "--", "sh", "-c", "exit 73;", exit_code=2) - assert output.endswith("Command 'sh' exited with status 73" + os.linesep) + if archiver.FORK_DEFAULT: + output = cmd(archiver, "create", "--content-from-command", "test", "--", "sh", "-c", "exit 73;", exit_code=2) + assert output.endswith("Command 'sh' exited with status 73" + os.linesep) + else: + with pytest.raises(CommandError): + cmd(archiver, "create", "--content-from-command", "test", "--", "sh", "-c", "exit 73;") archive_list = json.loads(cmd(archiver, "rlist", "--json")) assert archive_list["archives"] == [] @@ -408,8 +413,12 @@ def test_create_paths_from_command(archivers, request): def test_create_paths_from_command_with_failed_command(archivers, request): archiver = request.getfixturevalue(archivers) cmd(archiver, "rcreate", RK_ENCRYPTION) - output = cmd(archiver, "create", "--paths-from-command", "test", "--", "sh", "-c", "exit 73;", exit_code=2) - assert output.endswith("Command 'sh' exited with status 73" + os.linesep) + if archiver.FORK_DEFAULT: + output = cmd(archiver, "create", "--paths-from-command", "test", "--", "sh", "-c", "exit 73;", exit_code=2) + assert output.endswith("Command 'sh' exited with status 73" + os.linesep) + else: + with pytest.raises(CommandError): + cmd(archiver, "create", "--paths-from-command", "test", "--", "sh", "-c", "exit 73;") archive_list = json.loads(cmd(archiver, "rlist", "--json")) assert archive_list["archives"] == [] diff --git a/src/borg/testsuite/archiver/key_cmds.py b/src/borg/testsuite/archiver/key_cmds.py index dfddd41d..6a596cf4 100644 --- a/src/borg/testsuite/archiver/key_cmds.py +++ b/src/borg/testsuite/archiver/key_cmds.py @@ -6,7 +6,7 @@ import pytest from ...constants import * # NOQA from ...crypto.key import AESOCBRepoKey, AESOCBKeyfileKey, CHPOKeyfileKey, Passphrase from ...crypto.keymanager import RepoIdMismatch, NotABorgKeyFile -from ...helpers import EXIT_ERROR +from ...helpers import EXIT_ERROR, CommandError from ...helpers import bin_to_hex from ...helpers import msgpack from ...repository import Repository @@ -170,7 +170,11 @@ def test_key_export_directory(archivers, request): export_directory = archiver.output_path + "/exported" os.mkdir(export_directory) cmd(archiver, "rcreate", RK_ENCRYPTION) - cmd(archiver, "key", "export", export_directory, exit_code=EXIT_ERROR) + if archiver.FORK_DEFAULT: + cmd(archiver, "key", "export", export_directory, exit_code=EXIT_ERROR) + else: + with pytest.raises(CommandError): + cmd(archiver, "key", "export", export_directory) def test_key_export_qr_directory(archivers, request): @@ -178,14 +182,22 @@ def test_key_export_qr_directory(archivers, request): export_directory = archiver.output_path + "/exported" os.mkdir(export_directory) cmd(archiver, "rcreate", RK_ENCRYPTION) - cmd(archiver, "key", "export", "--qr-html", export_directory, exit_code=EXIT_ERROR) + if archiver.FORK_DEFAULT: + cmd(archiver, "key", "export", "--qr-html", export_directory, exit_code=EXIT_ERROR) + else: + with pytest.raises(CommandError): + cmd(archiver, "key", "export", "--qr-html", export_directory) def test_key_import_errors(archivers, request): archiver = request.getfixturevalue(archivers) export_file = archiver.output_path + "/exported" cmd(archiver, "rcreate", KF_ENCRYPTION) - cmd(archiver, "key", "import", export_file, exit_code=EXIT_ERROR) + if archiver.FORK_DEFAULT: + cmd(archiver, "key", "import", export_file, exit_code=EXIT_ERROR) + else: + with pytest.raises(CommandError): + cmd(archiver, "key", "import", export_file) with open(export_file, "w") as fd: fd.write("something not a key\n") diff --git a/src/borg/testsuite/archiver/rdelete_cmd.py b/src/borg/testsuite/archiver/rdelete_cmd.py index 688dae99..e18eee46 100644 --- a/src/borg/testsuite/archiver/rdelete_cmd.py +++ b/src/borg/testsuite/archiver/rdelete_cmd.py @@ -1,6 +1,9 @@ import os +import pytest + from ...constants import * # NOQA +from ...helpers import CancelledByUser from . import create_regular_file, cmd, generate_archiver_tests, RK_ENCRYPTION pytest_generate_tests = lambda metafunc: generate_archiver_tests(metafunc, kinds="local,remote,binary") # NOQA @@ -14,7 +17,11 @@ def test_delete_repo(archivers, request): cmd(archiver, "create", "test", "input") cmd(archiver, "create", "test.2", "input") os.environ["BORG_DELETE_I_KNOW_WHAT_I_AM_DOING"] = "no" - cmd(archiver, "rdelete", exit_code=2) + if archiver.FORK_DEFAULT: + cmd(archiver, "rdelete", exit_code=2) + else: + with pytest.raises(CancelledByUser): + cmd(archiver, "rdelete") assert os.path.exists(archiver.repository_path) os.environ["BORG_DELETE_I_KNOW_WHAT_I_AM_DOING"] = "YES" cmd(archiver, "rdelete") diff --git a/src/borg/testsuite/archiver/recreate_cmd.py b/src/borg/testsuite/archiver/recreate_cmd.py index d30460b9..078ec1ed 100644 --- a/src/borg/testsuite/archiver/recreate_cmd.py +++ b/src/borg/testsuite/archiver/recreate_cmd.py @@ -5,6 +5,7 @@ from datetime import datetime import pytest from ...constants import * # NOQA +from ...helpers import CommandError from .. import changedir, are_hardlinks_supported from . import ( _create_test_caches, @@ -82,8 +83,12 @@ def test_recreate_hardlinked_tags(archivers, request): # test for issue #4911 def test_recreate_target_rc(archivers, request): archiver = request.getfixturevalue(archivers) cmd(archiver, "rcreate", RK_ENCRYPTION) - output = cmd(archiver, "recreate", "--target=asdf", exit_code=2) - assert "Need to specify single archive" in output + if archiver.FORK_DEFAULT: + output = cmd(archiver, "recreate", "--target=asdf", exit_code=2) + assert "Need to specify single archive" in output + else: + with pytest.raises(CommandError): + cmd(archiver, "recreate", "--target=asdf") def test_recreate_target(archivers, request): From bec02a36c8a35fa1c6cf194fcfc527a017713868 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 12 Nov 2023 18:35:25 +0100 Subject: [PATCH 04/22] use print_warning also in borg delete ::archive --force --force --- src/borg/archiver/delete_cmd.py | 9 ++++----- src/borg/testsuite/archiver/delete_cmd.py | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/borg/archiver/delete_cmd.py b/src/borg/archiver/delete_cmd.py index 9e2b4e8b..48cc59dd 100644 --- a/src/borg/archiver/delete_cmd.py +++ b/src/borg/archiver/delete_cmd.py @@ -37,8 +37,7 @@ class DeleteMixIn: try: current_archive = manifest.archives.pop(archive_name) except KeyError: - self.exit_code = EXIT_WARNING - logger.warning(f"Archive {archive_name} not found ({i}/{len(archive_names)}).") + self.print_warning(f"Archive {archive_name} not found ({i}/{len(archive_names)}).") else: deleted = True if self.output_list: @@ -50,9 +49,9 @@ class DeleteMixIn: manifest.write() # note: might crash in compact() after committing the repo repository.commit(compact=False) - logger.warning('Done. Run "borg check --repair" to clean up the mess.') + self.print_warning('Done. Run "borg check --repair" to clean up the mess.') else: - logger.warning("Aborted.") + self.print_warning("Aborted.") return self.exit_code stats = Statistics(iec=args.iec) @@ -73,7 +72,7 @@ class DeleteMixIn: try: archive_info = manifest.archives[archive_name] except KeyError: - logger.warning(msg_not_found.format(archive_name, i, len(archive_names))) + self.print_warning(msg_not_found.format(archive_name, i, len(archive_names))) else: if self.output_list: logger_list.info(msg_delete.format(format_archive(archive_info), i, len(archive_names))) diff --git a/src/borg/testsuite/archiver/delete_cmd.py b/src/borg/testsuite/archiver/delete_cmd.py index 30727cac..e5cc2d4e 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") + cmd(archiver, "delete", "-a", "test", "--force", "--force", exit_code=1) cmd(archiver, "check", "--repair") output = cmd(archiver, "rlist") assert "test" not in output From cb8b718a967cbd370879711a0aa3a2b9765e9d4f Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 12 Nov 2023 20:01:14 +0100 Subject: [PATCH 05/22] refactor set_ec usage - msgpack version check: raise Error instead of calling set_ec --- src/borg/archiver/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/borg/archiver/__init__.py b/src/borg/archiver/__init__.py index f0f0ca5c..48480b5e 100644 --- a/src/borg/archiver/__init__.py +++ b/src/borg/archiver/__init__.py @@ -498,7 +498,7 @@ class Archiver( logger.error("You do not have a supported version of the msgpack python package installed. Terminating.") logger.error("This should never happen as specific, supported versions are required by our pyproject.toml.") logger.error("Do not contact borgbackup support about this.") - return set_ec(EXIT_ERROR) + raise Error("unsupported msgpack version") if is_slow_msgpack(): logger.warning(PURE_PYTHON_MSGPACK_WARNING) if args.debug_profile: From 0504dee0d92d693f6a1c7d41bc155b70cabf4372 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 12 Nov 2023 21:09:08 +0100 Subject: [PATCH 06/22] fix dealing with remote repo Locking Exceptions previously, this was handled in RPCError handler and always resulted in rc 2. now re-raise Lock Exceptions locally, so it gives rc 2 (legacy) or 7x (modern). --- src/borg/archiver/__init__.py | 2 +- src/borg/remote.py | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/borg/archiver/__init__.py b/src/borg/archiver/__init__.py index 48480b5e..16ec1e1c 100644 --- a/src/borg/archiver/__init__.py +++ b/src/borg/archiver/__init__.py @@ -644,7 +644,7 @@ def main(): # pragma: no cover tb = format_tb(e) exit_code = e.exit_code except RemoteRepository.RPCError as e: - important = e.exception_class not in ("LockTimeout",) and e.traceback + important = e.traceback msg = e.exception_full if important else e.get_message() msgid = e.exception_class tb_log_level = logging.ERROR if important else logging.DEBUG diff --git a/src/borg/remote.py b/src/borg/remote.py index 1471b495..5d9cb1af 100644 --- a/src/borg/remote.py +++ b/src/borg/remote.py @@ -30,6 +30,7 @@ from .helpers import format_file_size from .helpers import safe_unlink from .helpers import prepare_subprocess_env, ignore_sigint from .helpers import get_socket_filename +from .locking import LockTimeout, NotLocked, NotMyLock, LockFailed from .logger import create_logger, borg_serve_log_queue from .helpers import msgpack from .repository import Repository @@ -781,6 +782,14 @@ class RemoteRepository: raise Repository.ObjectNotFound(args[0], self.location.processed) elif error == "InvalidRPCMethod": raise InvalidRPCMethod(args[0]) + elif error == "LockTimeout": + raise LockTimeout(args[0]) + elif error == "LockFailed": + raise LockFailed(args[0], args[1]) + elif error == "NotLocked": + raise NotLocked(args[0]) + elif error == "NotMyLock": + raise NotMyLock(args[0]) else: raise self.RPCError(unpacked) From b18e6136275eb539943fd5c7fd7a097054759bcc Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Mon, 13 Nov 2023 20:48:41 +0100 Subject: [PATCH 07/22] get rid of some rare error classes, use RTError instead --- docs/internals/frontends.rst | 4 ---- src/borg/helpers/checks.py | 25 +++++++++---------------- 2 files changed, 9 insertions(+), 20 deletions(-) diff --git a/docs/internals/frontends.rst b/docs/internals/frontends.rst index 26cf2edc..da6270a0 100644 --- a/docs/internals/frontends.rst +++ b/docs/internals/frontends.rst @@ -575,10 +575,6 @@ Errors ErrorWithTraceback rc: 2 traceback: yes Error: {} - ExtensionModuleError rc: 2 traceback: no - The Borg binary extension modules do not seem to be properly installed. - PythonLibcTooOld rc: 2 traceback: no - FATAL: this Python was compiled for a too old (g)libc and misses required functionality. Buffer.MemoryLimitExceeded rc: 2 traceback: no Requested buffer size {} is above the limit of {}. EfficientCollectionQueue.SizeUnderflow rc: 2 traceback: no diff --git a/src/borg/helpers/checks.py b/src/borg/helpers/checks.py index acc38ffb..63170977 100644 --- a/src/borg/helpers/checks.py +++ b/src/borg/helpers/checks.py @@ -1,38 +1,31 @@ import os -from .errors import Error +from .errors import RTError from ..platformflags import is_win32 -class PythonLibcTooOld(Error): - """FATAL: this Python was compiled for a too old (g)libc and misses required functionality.""" - - def check_python(): if is_win32: required_funcs = {os.stat} else: required_funcs = {os.stat, os.utime, os.chown} if not os.supports_follow_symlinks.issuperset(required_funcs): - raise PythonLibcTooOld - - -class ExtensionModuleError(Error): - """The Borg binary extension modules do not seem to be properly installed.""" + raise RTError("""FATAL: this Python was compiled for a too old (g)libc and misses required functionality.""") def check_extension_modules(): from .. import platform, compress, crypto, item, chunker, hashindex + msg = """The Borg binary extension modules do not seem to be properly installed.""" if hashindex.API_VERSION != "1.2_01": - raise ExtensionModuleError + raise RTError(msg) if chunker.API_VERSION != "1.2_01": - raise ExtensionModuleError + raise RTError(msg) if compress.API_VERSION != "1.2_02": - raise ExtensionModuleError + raise RTError(msg) if crypto.low_level.API_VERSION != "1.3_01": - raise ExtensionModuleError + raise RTError(msg) if item.API_VERSION != "1.2_01": - raise ExtensionModuleError + raise RTError(msg) if platform.API_VERSION != platform.OS_API_VERSION or platform.API_VERSION != "1.2_05": - raise ExtensionModuleError + raise RTError(msg) From c704e5ea9e2f8d9839207e7d7edbb54051db8ca1 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Wed, 15 Nov 2023 01:44:01 +0100 Subject: [PATCH 08/22] 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 | 5 ++ src/borg/archive.py | 10 ++- src/borg/archiver/__init__.py | 51 ++++++++---- src/borg/archiver/create_cmd.py | 14 ++-- src/borg/archiver/delete_cmd.py | 4 +- src/borg/archiver/diff_cmd.py | 5 +- src/borg/archiver/extract_cmd.py | 9 ++- src/borg/archiver/tar_cmds.py | 2 +- src/borg/helpers/__init__.py | 99 +++++++++++++++++++++-- src/borg/helpers/errors.py | 44 ++++++++++ src/borg/testsuite/archiver/__init__.py | 1 + src/borg/testsuite/archiver/delete_cmd.py | 2 +- src/borg/testsuite/archiver/diff_cmd.py | 3 +- src/borg/testsuite/helpers.py | 63 ++++++++++++++- 14 files changed, 267 insertions(+), 45 deletions(-) diff --git a/docs/internals/frontends.rst b/docs/internals/frontends.rst index da6270a0..fd2b5b0f 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 639f8ef8..a1c7602c 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 16ec1e1c..3aba28a7 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 5c58d89e..4ab25575 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 48cc59dd..71467f7e 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 492a26fc..7eb0da33 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 7be8a32f..1dc7322e 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 e54e3fac..28448158 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 6a3ee2ad..7970b1fc 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 fe800ed8..85a62084 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 2804ad44..f45b34d2 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 e5cc2d4e..30727cac 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 c87f4bf0..e997e850 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 9a09739e..605fe1bb 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 From 900a1674dfdabdf31e31c30a5dd94be159ca0677 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Wed, 15 Nov 2023 18:04:54 +0100 Subject: [PATCH 09/22] move Backup*Error to errors module --- src/borg/archive.py | 38 +----------------------------------- src/borg/helpers/__init__.py | 1 + src/borg/helpers/errors.py | 37 +++++++++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 37 deletions(-) diff --git a/src/borg/archive.py b/src/borg/archive.py index a1c7602c..e8ce53fc 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -26,6 +26,7 @@ from .crypto.key import key_factory, UnsupportedPayloadError from .compress import CompressionSpec from .constants import * # NOQA from .crypto.low_level import IntegrityError as IntegrityErrorBase +from .helpers import BackupError, BackupOSError, BackupRaceConditionError from .hashindex import ChunkIndex, ChunkIndexEntry, CacheSynchronizer from .helpers import HardLinkManager from .helpers import ChunkIteratorFileWrapper, open_item @@ -181,43 +182,6 @@ def is_special(mode): return stat.S_ISBLK(mode) or stat.S_ISCHR(mode) or stat.S_ISFIFO(mode) -class BackupError(Exception): - """ - Exception raised for non-OSError-based exceptions while accessing backup files. - """ - - -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. - - Borg does different kinds of IO, and IO failures have different consequences. - This wrapper represents failures of input file or extraction IO. - These are non-critical and are only reported (exit code = 1, warning). - - Any unwrapped IO error is critical and aborts execution (for example repository IO failure). - """ - - def __init__(self, op, os_error): - self.op = op - self.os_error = os_error - self.errno = os_error.errno - self.strerror = os_error.strerror - self.filename = os_error.filename - - def __str__(self): - if self.op: - return f"{self.op}: {self.os_error}" - else: - return str(self.os_error) - - class BackupIO: op = "" diff --git a/src/borg/helpers/__init__.py b/src/borg/helpers/__init__.py index 7970b1fc..d898fe09 100644 --- a/src/borg/helpers/__init__.py +++ b/src/borg/helpers/__init__.py @@ -14,6 +14,7 @@ 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 .errors import BackupError, BackupOSError, BackupRaceConditionError 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 diff --git a/src/borg/helpers/errors.py b/src/borg/helpers/errors.py index 85a62084..24917a0a 100644 --- a/src/borg/helpers/errors.py +++ b/src/borg/helpers/errors.py @@ -114,3 +114,40 @@ class BackupExcWarning(BorgWarning): exit_mcode = 102 # TODO: override exit_code and compute the exit code based on the wrapped exception. + + +class BackupError(Exception): + """ + Exception raised for non-OSError-based exceptions while accessing backup files. + """ + + +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. + + Borg does different kinds of IO, and IO failures have different consequences. + This wrapper represents failures of input file or extraction IO. + These are non-critical and are only reported (exit code = 1, warning). + + Any unwrapped IO error is critical and aborts execution (for example repository IO failure). + """ + + def __init__(self, op, os_error): + self.op = op + self.os_error = os_error + self.errno = os_error.errno + self.strerror = os_error.strerror + self.filename = os_error.filename + + def __str__(self): + if self.op: + return f"{self.op}: {self.os_error}" + else: + return str(self.os_error) From e8fa4986cc0688b036cbb8d7f4f6498b998f9782 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Wed, 15 Nov 2023 18:41:02 +0100 Subject: [PATCH 10/22] BackupError->BackupWarning, BackupOSError->BackupOSWarning --- src/borg/archiver/create_cmd.py | 22 ++++++++++++++-------- src/borg/archiver/extract_cmd.py | 13 +++++++------ src/borg/helpers/__init__.py | 2 +- src/borg/helpers/errors.py | 8 ++++++-- 4 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/borg/archiver/create_cmd.py b/src/borg/archiver/create_cmd.py index 4ab25575..0dfaee95 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, BackupExcWarning, FileChangedWarning +from ..helpers import Error, CommandError, BackupWarning, BackupOSWarning, FileChangedWarning from ..manifest import Manifest from ..patterns import PatternMatcher from ..platform import is_win32 @@ -121,8 +121,11 @@ class CreateMixIn: read_special=args.read_special, dry_run=dry_run, ) - except (BackupOSError, BackupError) as e: - self.print_warning_instance(BackupExcWarning(path, e)) + except BackupOSError as e: + self.print_warning_instance(BackupOSWarning(path, e)) + status = "E" + except BackupError as e: + self.print_warning_instance(BackupWarning(path, e)) status = "E" if status == "C": self.print_warning_instance(FileChangedWarning(path)) @@ -149,7 +152,7 @@ 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)) + self.print_warning_instance(BackupOSWarning(path, e)) status = "E" else: status = "+" # included @@ -180,9 +183,9 @@ class CreateMixIn: # if we get back here, we've finished recursing into , # we do not ever want to get back in there (even if path is given twice as recursion root) skip_inodes.add((st.st_ino, st.st_dev)) - except (BackupOSError, BackupError) as e: + except BackupOSError as e: # this comes from os.stat, self._rec_walk has own exception handler - self.print_warning_instance(BackupExcWarning(path, e)) + self.print_warning_instance(BackupOSWarning(path, e)) continue if not dry_run: if args.progress: @@ -521,8 +524,11 @@ class CreateMixIn: dry_run=dry_run, ) - except (BackupOSError, BackupError) as e: - self.print_warning_instance(BackupExcWarning(path, e)) + except BackupOSError as e: + self.print_warning_instance(BackupOSWarning(path, e)) + status = "E" + except BackupError as e: + self.print_warning_instance(BackupWarning(path, e)) status = "E" if status == "C": self.print_warning_instance(FileChangedWarning(path)) diff --git a/src/borg/archiver/extract_cmd.py b/src/borg/archiver/extract_cmd.py index 1dc7322e..5732f2bf 100644 --- a/src/borg/archiver/extract_cmd.py +++ b/src/borg/archiver/extract_cmd.py @@ -12,7 +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 ..helpers import BackupWarning, BackupOSWarning, IncludePatternNeverMatchedWarning from ..manifest import Manifest from ..logger import create_logger @@ -66,7 +66,7 @@ class ExtractMixIn: try: archive.extract_item(dir_item, stdout=stdout) except BackupOSError as e: - self.print_warning_instance(BackupExcWarning(remove_surrogates(dir_item.path), e)) + self.print_warning_instance(BackupOSWarning(remove_surrogates(dir_item.path), e)) if output_list: logging.getLogger("borg.output.list").info(remove_surrogates(item.path)) try: @@ -80,9 +80,10 @@ class ExtractMixIn: archive.extract_item( item, stdout=stdout, sparse=sparse, hlm=hlm, pi=pi, continue_extraction=continue_extraction ) - except (BackupOSError, BackupError) as e: - self.print_warning_instance(BackupExcWarning(remove_surrogates(orig_path), e)) - + except BackupOSError as e: + self.print_warning_instance(BackupOSWarning(remove_surrogates(orig_path), e)) + except BackupError as e: + self.print_warning_instance(BackupWarning(remove_surrogates(orig_path), e)) if pi: pi.finish() @@ -96,7 +97,7 @@ class ExtractMixIn: try: archive.extract_item(dir_item, stdout=stdout) except BackupOSError as e: - self.print_warning_instance(BackupExcWarning(remove_surrogates(dir_item.path), e)) + self.print_warning_instance(BackupOSWarning(remove_surrogates(dir_item.path), e)) for pattern in matcher.get_unmatched_include_patterns(): self.print_warning_instance(IncludePatternNeverMatchedWarning(pattern)) if pi: diff --git a/src/borg/helpers/__init__.py b/src/borg/helpers/__init__.py index d898fe09..ef8c9f97 100644 --- a/src/borg/helpers/__init__.py +++ b/src/borg/helpers/__init__.py @@ -13,7 +13,7 @@ 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 .errors import BorgWarning, FileChangedWarning, BackupWarning, BackupOSWarning, IncludePatternNeverMatchedWarning from .errors import BackupError, BackupOSError, BackupRaceConditionError 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 diff --git a/src/borg/helpers/errors.py b/src/borg/helpers/errors.py index 24917a0a..0aca4729 100644 --- a/src/borg/helpers/errors.py +++ b/src/borg/helpers/errors.py @@ -108,12 +108,16 @@ class IncludePatternNeverMatchedWarning(BorgWarning): exit_mcode = 101 -class BackupExcWarning(BorgWarning): +class BackupWarning(BorgWarning): """{}: {}""" exit_mcode = 102 - # TODO: override exit_code and compute the exit code based on the wrapped exception. + +class BackupOSWarning(BorgWarning): + """{}: {}""" + + exit_mcode = 104 class BackupError(Exception): From 982aeb56a7773b5d16df0a86a232f0557d6009d8 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Wed, 15 Nov 2023 22:39:42 +0100 Subject: [PATCH 11/22] more detailled warnings for source file OSErrors --- src/borg/helpers/errors.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/borg/helpers/errors.py b/src/borg/helpers/errors.py index 0aca4729..931d6daf 100644 --- a/src/borg/helpers/errors.py +++ b/src/borg/helpers/errors.py @@ -1,3 +1,4 @@ +import errno import os from ..constants import * # NOQA @@ -119,6 +120,27 @@ class BackupOSWarning(BorgWarning): exit_mcode = 104 + @property + def exit_code(self): + if not modern_ec: + return EXIT_WARNING + exc = self.args[1] + assert isinstance(exc, BackupOSError) + if exc.errno in (errno.EPERM, errno.EACCES): + return PermissionWarning.exit_mcode + elif exc.errno in (errno.EIO,): + return IOWarning.exit_mcode + else: + return self.exit_mcode + + +class PermissionWarning(BorgWarning): + exit_mcode = 105 + + +class IOWarning(BorgWarning): + exit_mcode = 106 + class BackupError(Exception): """ From 5168a97782134699fb31ff7bdf74c6f053bf85f0 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Wed, 15 Nov 2023 23:01:26 +0100 Subject: [PATCH 12/22] extend errorlist script to warnings, update docs --- docs/internals/frontends.rst | 13 +++++++++++- scripts/errorlist.py | 41 +++++++++++++++++++++++++++--------- src/borg/helpers/errors.py | 4 ++++ 3 files changed, 47 insertions(+), 11 deletions(-) diff --git a/docs/internals/frontends.rst b/docs/internals/frontends.rst index fd2b5b0f..b054331a 100644 --- a/docs/internals/frontends.rst +++ b/docs/internals/frontends.rst @@ -707,9 +707,20 @@ Errors Warnings + BorgWarning rc: 1 + Warning: {} FileChangedWarning rc: 100 + {}: file changed while we backed it up IncludePatternNeverMatchedWarning rc: 101 - BackupExcWarning rc: 102 (needs more work!) + Include pattern '{}' never matched. + BackupWarning rc: 102 + {}: {} + BackupOSWarning rc: 104 + {}: {} + PermissionWarning rc: 105 + {}: {} + IOWarning rc: 106 + {}: {} Operations - cache.begin_transaction diff --git a/scripts/errorlist.py b/scripts/errorlist.py index 735f1125..dba19aae 100755 --- a/scripts/errorlist.py +++ b/scripts/errorlist.py @@ -5,7 +5,8 @@ from textwrap import indent import borg.archiver # noqa: F401 - need import to get Error subclasses. -from borg.helpers import Error +from borg.constants import * # NOQA +from borg.helpers import Error, BorgWarning def subclasses(cls): @@ -17,26 +18,46 @@ def subclasses(cls): # 3..99 are available for specific errors # 100..127 are available for specific warnings # 128+ are reserved for signals -free_rcs = set(range(3, 99 + 1)) # 3 .. 99 (we only deal with errors here) +free_error_rcs = set(range(EXIT_ERROR_BASE, EXIT_WARNING_BASE)) # 3 .. 99 +free_warning_rcs = set(range(EXIT_WARNING_BASE, EXIT_SIGNAL_BASE)) # 100 .. 127 # these classes map to rc 2 -generic_rc_classes = set() +generic_error_rc_classes = set() +generic_warning_rc_classes = set() -classes = {Error}.union(subclasses(Error)) +error_classes = {Error}.union(subclasses(Error)) -for cls in sorted(classes, key=lambda cls: (cls.__module__, cls.__qualname__)): +for cls in sorted(error_classes, key=lambda cls: (cls.__module__, cls.__qualname__)): traceback = "yes" if cls.traceback else "no" rc = cls.exit_mcode print(" ", cls.__qualname__, "rc:", rc, "traceback:", traceback) print(indent(cls.__doc__, " " * 8)) - if rc in free_rcs: - free_rcs.remove(rc) + if rc in free_error_rcs: + free_error_rcs.remove(rc) elif rc == 2: - generic_rc_classes.add(cls.__qualname__) + generic_error_rc_classes.add(cls.__qualname__) else: # rc != 2 # if we did not intentionally map this to the generic error rc, this might be an issue: print(f"ERROR: {rc} is not a free/available RC, but either duplicate or invalid") print() -print("free RCs:", sorted(free_rcs)) -print("generic errors:", sorted(generic_rc_classes)) +print("free error RCs:", sorted(free_error_rcs)) +print("generic errors:", sorted(generic_error_rc_classes)) + +warning_classes = {BorgWarning}.union(subclasses(BorgWarning)) + +for cls in sorted(warning_classes, key=lambda cls: (cls.__module__, cls.__qualname__)): + rc = cls.exit_mcode + print(" ", cls.__qualname__, "rc:", rc) + print(indent(cls.__doc__, " " * 8)) + if rc in free_warning_rcs: + free_warning_rcs.remove(rc) + elif rc == 1: + generic_warning_rc_classes.add(cls.__qualname__) + else: # rc != 1 + # if we did not intentionally map this to the generic warning rc, this might be an issue: + print(f"ERROR: {rc} is not a free/available RC, but either duplicate or invalid") + +print("\n") +print("free warning RCs:", sorted(free_warning_rcs)) +print("generic warnings:", sorted(generic_warning_rc_classes)) diff --git a/src/borg/helpers/errors.py b/src/borg/helpers/errors.py index 931d6daf..10012ef4 100644 --- a/src/borg/helpers/errors.py +++ b/src/borg/helpers/errors.py @@ -135,10 +135,14 @@ class BackupOSWarning(BorgWarning): class PermissionWarning(BorgWarning): + """{}: {}""" + exit_mcode = 105 class IOWarning(BorgWarning): + """{}: {}""" + exit_mcode = 106 From 4adc782100437a2af689ea1ab60d11f774a54f77 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 18 Nov 2023 22:37:38 +0100 Subject: [PATCH 13/22] print_warning*: support warning msgids, fixes #7080 --- src/borg/archiver/__init__.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/borg/archiver/__init__.py b/src/borg/archiver/__init__.py index 3aba28a7..20abf5ef 100644 --- a/src/borg/archiver/__init__.py +++ b/src/borg/archiver/__init__.py @@ -133,19 +133,21 @@ class Archiver( 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") + warning_msgid = kw.get("msgid") 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) + logger.warning(output, msgid=warning_msgid) if warning_msgid else logger.warning(output) def print_warning_instance(self, warning): assert isinstance(warning, BorgWarning) msg = type(warning).__doc__ + msgid = type(warning).__qualname__ args = warning.args - self.print_warning(msg, *args, wc=warning.exit_code, wt="curly") + self.print_warning(msg, *args, wc=warning.exit_code, wt="curly", msgid=msgid) def print_file_status(self, status, path): # if we get called with status == None, the final file status was already printed From 9ac4672254a7be79d42d22e36d595727799130b0 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 18 Nov 2023 22:49:56 +0100 Subject: [PATCH 14/22] add NotFoundWarning --- docs/internals/frontends.rst | 2 ++ src/borg/helpers/errors.py | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/docs/internals/frontends.rst b/docs/internals/frontends.rst index b054331a..2003bad3 100644 --- a/docs/internals/frontends.rst +++ b/docs/internals/frontends.rst @@ -721,6 +721,8 @@ Warnings {}: {} IOWarning rc: 106 {}: {} + NotFoundWarning rc: 107 + {}: {} Operations - cache.begin_transaction diff --git a/src/borg/helpers/errors.py b/src/borg/helpers/errors.py index 10012ef4..b40005d9 100644 --- a/src/borg/helpers/errors.py +++ b/src/borg/helpers/errors.py @@ -128,6 +128,8 @@ class BackupOSWarning(BorgWarning): assert isinstance(exc, BackupOSError) if exc.errno in (errno.EPERM, errno.EACCES): return PermissionWarning.exit_mcode + elif exc.errno in (errno.ENOENT,): + return NotFoundWarning.exit_mcode elif exc.errno in (errno.EIO,): return IOWarning.exit_mcode else: @@ -146,6 +148,12 @@ class IOWarning(BorgWarning): exit_mcode = 106 +class NotFoundWarning(BorgWarning): + """{}: {}""" + + exit_mcode = 107 + + class BackupError(Exception): """ Exception raised for non-OSError-based exceptions while accessing backup files. From 97dd2875841e658dabc11e9e307c8418cc1b9af7 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Mon, 27 Nov 2023 17:02:23 +0100 Subject: [PATCH 15/22] raise BackupOSError subclasses --- docs/internals/frontends.rst | 12 +++-- src/borg/archive.py | 13 ++++- src/borg/archiver/__init__.py | 10 ++-- src/borg/archiver/create_cmd.py | 20 +++---- src/borg/archiver/extract_cmd.py | 14 +++-- src/borg/helpers/__init__.py | 3 +- src/borg/helpers/errors.py | 89 ++++++++++++++------------------ 7 files changed, 77 insertions(+), 84 deletions(-) diff --git a/docs/internals/frontends.rst b/docs/internals/frontends.rst index 2003bad3..777a4964 100644 --- a/docs/internals/frontends.rst +++ b/docs/internals/frontends.rst @@ -713,15 +713,17 @@ Warnings {}: file changed while we backed it up IncludePatternNeverMatchedWarning rc: 101 Include pattern '{}' never matched. - BackupWarning rc: 102 + BackupError rc: 102 {}: {} - BackupOSWarning rc: 104 + BackupRaceConditionError rc: 103 + Error: {} + BackupOSError rc: 104 {}: {} - PermissionWarning rc: 105 + BackupPermissionError rc: 105 {}: {} - IOWarning rc: 106 + BackupIOError rc: 106 {}: {} - NotFoundWarning rc: 107 + BackupFileNotFoundError rc: 107 {}: {} Operations diff --git a/src/borg/archive.py b/src/borg/archive.py index e8ce53fc..d4449ec4 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -1,4 +1,5 @@ import base64 +import errno import json import os import stat @@ -26,7 +27,8 @@ from .crypto.key import key_factory, UnsupportedPayloadError from .compress import CompressionSpec from .constants import * # NOQA from .crypto.low_level import IntegrityError as IntegrityErrorBase -from .helpers import BackupError, BackupOSError, BackupRaceConditionError +from .helpers import BackupError, BackupRaceConditionError +from .helpers import BackupOSError, BackupPermissionError, BackupFileNotFoundError, BackupIOError from .hashindex import ChunkIndex, ChunkIndexEntry, CacheSynchronizer from .helpers import HardLinkManager from .helpers import ChunkIteratorFileWrapper, open_item @@ -194,7 +196,14 @@ class BackupIO: def __exit__(self, exc_type, exc_val, exc_tb): if exc_type and issubclass(exc_type, OSError): - raise BackupOSError(self.op, exc_val) from exc_val + E_MAP = { + errno.EPERM: BackupPermissionError, + errno.EACCES: BackupPermissionError, + errno.ENOENT: BackupFileNotFoundError, + errno.EIO: BackupIOError, + } + e_cls = E_MAP.get(exc_val.errno, BackupOSError) + raise e_cls(self.op, exc_val) from exc_val backup_io = BackupIO() diff --git a/src/borg/archiver/__init__.py b/src/borg/archiver/__init__.py index 20abf5ef..d05ba9b5 100644 --- a/src/borg/archiver/__init__.py +++ b/src/borg/archiver/__init__.py @@ -26,7 +26,7 @@ try: from ..constants import * # NOQA 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 add_warning, BorgWarning, BackupWarning from ..helpers import format_file_size from ..helpers import remove_surrogates, text_to_json from ..helpers import DatetimeWrapper, replace_placeholders @@ -144,10 +144,10 @@ class Archiver( def print_warning_instance(self, warning): assert isinstance(warning, BorgWarning) - msg = type(warning).__doc__ - msgid = type(warning).__qualname__ - args = warning.args - self.print_warning(msg, *args, wc=warning.exit_code, wt="curly", msgid=msgid) + # if it is a BackupWarning, use the wrapped BackupError exception instance: + cls = type(warning.args[1]) if isinstance(warning, BackupWarning) else type(warning) + msg, msgid, args, wc = cls.__doc__, cls.__qualname__, warning.args, warning.exit_code + self.print_warning(msg, *args, wc=wc, wt="curly", msgid=msgid) def print_file_status(self, status, path): # if we get called with status == None, the final file status was already printed diff --git a/src/borg/archiver/create_cmd.py b/src/borg/archiver/create_cmd.py index 0dfaee95..67fe121d 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, BackupWarning, BackupOSWarning, FileChangedWarning +from ..helpers import Error, CommandError, BackupWarning, FileChangedWarning from ..manifest import Manifest from ..patterns import PatternMatcher from ..platform import is_win32 @@ -87,7 +87,7 @@ class CreateMixIn: rc = proc.wait() if rc != 0: raise CommandError("Command %r exited with status %d", args.paths[0], rc) - except BackupOSError as e: + except BackupError as e: raise Error("%s: %s", path, e) else: status = "+" # included @@ -121,9 +121,6 @@ class CreateMixIn: read_special=args.read_special, dry_run=dry_run, ) - except BackupOSError as e: - self.print_warning_instance(BackupOSWarning(path, e)) - status = "E" except BackupError as e: self.print_warning_instance(BackupWarning(path, e)) status = "E" @@ -151,8 +148,8 @@ class CreateMixIn: status = fso.process_pipe( path=path, cache=cache, fd=sys.stdin.buffer, mode=mode, user=user, group=group ) - except BackupOSError as e: - self.print_warning_instance(BackupOSWarning(path, e)) + except BackupError as e: + self.print_warning_instance(BackupWarning(path, e)) status = "E" else: status = "+" # included @@ -183,9 +180,9 @@ class CreateMixIn: # if we get back here, we've finished recursing into , # we do not ever want to get back in there (even if path is given twice as recursion root) skip_inodes.add((st.st_ino, st.st_dev)) - except BackupOSError as e: + except BackupError as e: # this comes from os.stat, self._rec_walk has own exception handler - self.print_warning_instance(BackupOSWarning(path, e)) + self.print_warning_instance(BackupWarning(path, e)) continue if not dry_run: if args.progress: @@ -368,7 +365,7 @@ class CreateMixIn: else: self.print_warning("Unknown file type: %s", path) return - except (BackupError, BackupOSError) as err: + except BackupError as err: if isinstance(err, BackupOSError): if err.errno in (errno.EPERM, errno.EACCES): # Do not try again, such errors can not be fixed by retrying. @@ -524,9 +521,6 @@ class CreateMixIn: dry_run=dry_run, ) - except BackupOSError as e: - self.print_warning_instance(BackupOSWarning(path, e)) - status = "E" except BackupError as e: self.print_warning_instance(BackupWarning(path, e)) status = "E" diff --git a/src/borg/archiver/extract_cmd.py b/src/borg/archiver/extract_cmd.py index 5732f2bf..92cc7fa2 100644 --- a/src/borg/archiver/extract_cmd.py +++ b/src/borg/archiver/extract_cmd.py @@ -6,13 +6,13 @@ import stat from ._common import with_repository, with_archive from ._common import build_filter, build_matcher -from ..archive import BackupError, BackupOSError +from ..archive import BackupError from ..constants import * # NOQA from ..helpers import archivename_validator, PathSpec from ..helpers import remove_surrogates from ..helpers import HardLinkManager from ..helpers import ProgressIndicatorPercent -from ..helpers import BackupWarning, BackupOSWarning, IncludePatternNeverMatchedWarning +from ..helpers import BackupWarning, IncludePatternNeverMatchedWarning from ..manifest import Manifest from ..logger import create_logger @@ -65,8 +65,8 @@ class ExtractMixIn: dir_item = dirs.pop(-1) try: archive.extract_item(dir_item, stdout=stdout) - except BackupOSError as e: - self.print_warning_instance(BackupOSWarning(remove_surrogates(dir_item.path), e)) + except BackupError as e: + self.print_warning_instance(BackupWarning(remove_surrogates(dir_item.path), e)) if output_list: logging.getLogger("borg.output.list").info(remove_surrogates(item.path)) try: @@ -80,8 +80,6 @@ class ExtractMixIn: archive.extract_item( item, stdout=stdout, sparse=sparse, hlm=hlm, pi=pi, continue_extraction=continue_extraction ) - except BackupOSError as e: - self.print_warning_instance(BackupOSWarning(remove_surrogates(orig_path), e)) except BackupError as e: self.print_warning_instance(BackupWarning(remove_surrogates(orig_path), e)) if pi: @@ -96,8 +94,8 @@ class ExtractMixIn: dir_item = dirs.pop(-1) try: archive.extract_item(dir_item, stdout=stdout) - except BackupOSError as e: - self.print_warning_instance(BackupOSWarning(remove_surrogates(dir_item.path), e)) + except BackupError as e: + self.print_warning_instance(BackupWarning(remove_surrogates(dir_item.path), e)) for pattern in matcher.get_unmatched_include_patterns(): self.print_warning_instance(IncludePatternNeverMatchedWarning(pattern)) if pi: diff --git a/src/borg/helpers/__init__.py b/src/borg/helpers/__init__.py index ef8c9f97..af914e67 100644 --- a/src/borg/helpers/__init__.py +++ b/src/borg/helpers/__init__.py @@ -13,8 +13,9 @@ 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, BackupWarning, BackupOSWarning, IncludePatternNeverMatchedWarning +from .errors import BorgWarning, FileChangedWarning, BackupWarning, IncludePatternNeverMatchedWarning from .errors import BackupError, BackupOSError, BackupRaceConditionError +from .errors import BackupPermissionError, BackupIOError, BackupFileNotFoundError 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 diff --git a/src/borg/helpers/errors.py b/src/borg/helpers/errors.py index b40005d9..015dda1b 100644 --- a/src/borg/helpers/errors.py +++ b/src/borg/helpers/errors.py @@ -1,4 +1,3 @@ -import errno import os from ..constants import * # NOQA @@ -112,70 +111,42 @@ class IncludePatternNeverMatchedWarning(BorgWarning): class BackupWarning(BorgWarning): """{}: {}""" - exit_mcode = 102 - - -class BackupOSWarning(BorgWarning): - """{}: {}""" - - exit_mcode = 104 + # this is to wrap a caught BackupError exception, so it can be given to print_warning_instance @property def exit_code(self): if not modern_ec: return EXIT_WARNING exc = self.args[1] - assert isinstance(exc, BackupOSError) - if exc.errno in (errno.EPERM, errno.EACCES): - return PermissionWarning.exit_mcode - elif exc.errno in (errno.ENOENT,): - return NotFoundWarning.exit_mcode - elif exc.errno in (errno.EIO,): - return IOWarning.exit_mcode - else: - return self.exit_mcode + assert isinstance(exc, BackupError) + return exc.exit_mcode -class PermissionWarning(BorgWarning): - """{}: {}""" +class BackupError(Error): + """{}: backup error""" - exit_mcode = 105 - - -class IOWarning(BorgWarning): - """{}: {}""" - - exit_mcode = 106 - - -class NotFoundWarning(BorgWarning): - """{}: {}""" - - exit_mcode = 107 - - -class BackupError(Exception): - """ - Exception raised for non-OSError-based exceptions while accessing backup files. - """ + # Exception raised for non-OSError-based exceptions while accessing backup files. + exit_mcode = 102 class BackupRaceConditionError(BackupError): - """ - Exception raised when encountering a critical race condition while trying to back up a file. - """ + """{}: file type or inode changed while we backed it up (race condition, skipped file)""" + + # Exception raised when encountering a critical race condition while trying to back up a file. + exit_mcode = 103 -class BackupOSError(Exception): - """ - Wrapper for OSError raised while accessing backup files. +class BackupOSError(BackupError): + """{}: {}""" - Borg does different kinds of IO, and IO failures have different consequences. - This wrapper represents failures of input file or extraction IO. - These are non-critical and are only reported (exit code = 1, warning). - - Any unwrapped IO error is critical and aborts execution (for example repository IO failure). - """ + # Wrapper for OSError raised while accessing backup files. + # + # Borg does different kinds of IO, and IO failures have different consequences. + # This wrapper represents failures of input file or extraction IO. + # These are non-critical and are only reported (warnings). + # + # Any unwrapped IO error is critical and aborts execution (for example repository IO failure). + exit_mcode = 104 def __init__(self, op, os_error): self.op = op @@ -189,3 +160,21 @@ class BackupOSError(Exception): return f"{self.op}: {self.os_error}" else: return str(self.os_error) + + +class BackupPermissionError(BackupOSError): + """{}: {}""" + + exit_mcode = 105 + + +class BackupIOError(BackupOSError): + """{}: {}""" + + exit_mcode = 106 + + +class BackupFileNotFoundError(BackupOSError): + """{}: {}""" + + exit_mcode = 107 From 2e05c234b62f22140fb853e246936f4f8b745d7f Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 3 Dec 2023 00:01:57 +0100 Subject: [PATCH 16/22] BackupErrors get caught and give warning RCs also: use more union operators rather than .union() --- scripts/errorlist.py | 8 ++++---- src/borg/helpers/errors.py | 10 +++++++--- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/scripts/errorlist.py b/scripts/errorlist.py index dba19aae..8bc6f0bc 100755 --- a/scripts/errorlist.py +++ b/scripts/errorlist.py @@ -6,12 +6,12 @@ from textwrap import indent import borg.archiver # noqa: F401 - need import to get Error subclasses. from borg.constants import * # NOQA -from borg.helpers import Error, BorgWarning +from borg.helpers import Error, BackupError, BorgWarning def subclasses(cls): direct_subclasses = cls.__subclasses__() - return set(direct_subclasses).union([s for c in direct_subclasses for s in subclasses(c)]) + return set(direct_subclasses) | set(s for c in direct_subclasses for s in subclasses(c)) # 0, 1, 2 are used for success, generic warning, generic error @@ -25,7 +25,7 @@ free_warning_rcs = set(range(EXIT_WARNING_BASE, EXIT_SIGNAL_BASE)) # 100 .. 127 generic_error_rc_classes = set() generic_warning_rc_classes = set() -error_classes = {Error}.union(subclasses(Error)) +error_classes = {Error} | subclasses(Error) for cls in sorted(error_classes, key=lambda cls: (cls.__module__, cls.__qualname__)): traceback = "yes" if cls.traceback else "no" @@ -44,7 +44,7 @@ print() print("free error RCs:", sorted(free_error_rcs)) print("generic errors:", sorted(generic_error_rc_classes)) -warning_classes = {BorgWarning}.union(subclasses(BorgWarning)) +warning_classes = {BorgWarning} | subclasses(BorgWarning) | {BackupError} | subclasses(BackupError) for cls in sorted(warning_classes, key=lambda cls: (cls.__module__, cls.__qualname__)): rc = cls.exit_mcode diff --git a/src/borg/helpers/errors.py b/src/borg/helpers/errors.py index 015dda1b..d19048a6 100644 --- a/src/borg/helpers/errors.py +++ b/src/borg/helpers/errors.py @@ -8,8 +8,8 @@ from ..crypto.low_level import IntegrityError as IntegrityErrorBase modern_ec = os.environ.get("BORG_EXIT_CODES", "legacy") == "modern" -class Error(Exception): - """Error: {}""" +class ErrorBase(Exception): + """ErrorBase: {}""" # Error base class @@ -37,6 +37,10 @@ class Error(Exception): return self.exit_mcode if modern_ec else EXIT_ERROR +class Error(ErrorBase): + """Error: {}""" + + class ErrorWithTraceback(Error): """Error: {}""" @@ -122,7 +126,7 @@ class BackupWarning(BorgWarning): return exc.exit_mcode -class BackupError(Error): +class BackupError(ErrorBase): """{}: backup error""" # Exception raised for non-OSError-based exceptions while accessing backup files. From 5caf747011430f7ad8c71a122f5890c52babc9ed Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 3 Dec 2023 00:27:19 +0100 Subject: [PATCH 17/22] update frontends.rst error/warning list --- docs/internals/frontends.rst | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/internals/frontends.rst b/docs/internals/frontends.rst index 777a4964..d63f9786 100644 --- a/docs/internals/frontends.rst +++ b/docs/internals/frontends.rst @@ -643,7 +643,7 @@ Errors Key management not available for unencrypted repositories. UnknownKeyType rc: 47 traceback: no Key type {0} is unknown. - UnsupportedPayloadError rc: 48 traceback: no + UnsupportedPayloadError rc: 48 traceback: no Unsupported payload type {}. A newer version is required to access this repository. UnsupportedKeyFormatError rc: 49 traceback:no Your borg key is stored in an unsupported format. Try using a newer version of borg. @@ -709,14 +709,17 @@ Errors Warnings BorgWarning rc: 1 Warning: {} + BackupWarning rc: 1 + {}: {} + FileChangedWarning rc: 100 {}: file changed while we backed it up IncludePatternNeverMatchedWarning rc: 101 Include pattern '{}' never matched. BackupError rc: 102 - {}: {} + {}: backup error BackupRaceConditionError rc: 103 - Error: {} + {}: file type or inode changed while we backed it up (race condition, skipped file) BackupOSError rc: 104 {}: {} BackupPermissionError rc: 105 From abe6545853f14e1f39b4c3e74d32b42c88829b01 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Wed, 6 Dec 2023 00:14:46 +0100 Subject: [PATCH 18/22] do not return the rc from Archiver methods this is not needed and getting rid of it makes the code / behaviour simpler to understand: if a fatal error is detected, we throw an exception. if we encounter something warning worthy, we emit and collect the warning. in a few cases, we directly call set_ec to set the exit code as needed, e.g. if passing it through from a subprocess. also: - get rid of Archiver.exit_code - assert that return value of archiver methods is None - fix a print_warning call to use the correct formatting method --- src/borg/archiver/__init__.py | 7 +++-- src/borg/archiver/benchmark_cmd.py | 36 ++++++++++++++-------- src/borg/archiver/check_cmd.py | 8 ++--- src/borg/archiver/compact_cmd.py | 2 -- src/borg/archiver/config_cmd.py | 6 +--- src/borg/archiver/create_cmd.py | 1 - src/borg/archiver/debug_cmd.py | 16 ---------- src/borg/archiver/delete_cmd.py | 8 ++--- src/borg/archiver/diff_cmd.py | 2 -- src/borg/archiver/extract_cmd.py | 1 - src/borg/archiver/help_cmd.py | 2 -- src/borg/archiver/info_cmd.py | 3 -- src/borg/archiver/key_cmds.py | 9 ++---- src/borg/archiver/list_cmd.py | 2 -- src/borg/archiver/lock_cmds.py | 6 ++-- src/borg/archiver/mount_cmds.py | 5 ++- src/borg/archiver/prune_cmd.py | 1 - src/borg/archiver/rcompress_cmd.py | 2 -- src/borg/archiver/rcreate_cmd.py | 5 ++- src/borg/archiver/rdelete_cmd.py | 1 - src/borg/archiver/recreate_cmd.py | 1 - src/borg/archiver/rename_cmd.py | 1 - src/borg/archiver/rinfo_cmd.py | 1 - src/borg/archiver/rlist_cmd.py | 2 -- src/borg/archiver/serve_cmd.py | 2 -- src/borg/archiver/tar_cmds.py | 5 --- src/borg/archiver/transfer_cmd.py | 28 ++++++----------- src/borg/archiver/version_cmd.py | 2 -- src/borg/helpers/__init__.py | 3 +- src/borg/helpers/fs.py | 7 +++-- src/borg/testsuite/archiver/__init__.py | 1 - src/borg/testsuite/archiver/config_cmd.py | 23 +++++++++++--- src/borg/testsuite/archiver/corruption.py | 8 +++-- src/borg/testsuite/archiver/rcreate_cmd.py | 9 ++++-- 34 files changed, 91 insertions(+), 125 deletions(-) diff --git a/src/borg/archiver/__init__.py b/src/borg/archiver/__init__.py index d05ba9b5..9b9e6036 100644 --- a/src/borg/archiver/__init__.py +++ b/src/borg/archiver/__init__.py @@ -24,7 +24,7 @@ try: from ._common import Highlander from .. import __version__ from ..constants import * # NOQA - from ..helpers import EXIT_SUCCESS, EXIT_WARNING, EXIT_ERROR, EXIT_SIGNAL_BASE, classify_ec + from ..helpers import EXIT_WARNING, EXIT_ERROR, EXIT_SIGNAL_BASE, classify_ec from ..helpers import Error, CommandError, get_ec, modern_ec from ..helpers import add_warning, BorgWarning, BackupWarning from ..helpers import format_file_size @@ -124,7 +124,6 @@ class Archiver( VersionMixIn, ): def __init__(self, lock_wait=None, prog=None): - self.exit_code = EXIT_SUCCESS self.lock_wait = lock_wait self.prog = prog self.last_checkpoint = time.monotonic() @@ -547,7 +546,9 @@ class Archiver( # it compatible (see above). msgpack.pack(profiler.stats, fd, use_bin_type=True) else: - return get_ec(func(args)) + rc = func(args) + assert rc is None + return get_ec(rc) def sig_info_handler(sig_no, stack): # pragma: no cover diff --git a/src/borg/archiver/benchmark_cmd.py b/src/borg/archiver/benchmark_cmd.py index e8a362c8..78bf9b39 100644 --- a/src/borg/archiver/benchmark_cmd.py +++ b/src/borg/archiver/benchmark_cmd.py @@ -9,6 +9,7 @@ from ..constants import * # NOQA from ..crypto.key import FlexiKey from ..helpers import format_file_size from ..helpers import msgpack +from ..helpers import get_ec from ..item import Item from ..platform import SyncFile @@ -21,38 +22,49 @@ class BenchmarkMixIn: compression = "--compression=none" # measure create perf (without files cache to always have it chunking) t_start = time.monotonic() - rc = self.do_create( - self.parse_args( - [f"--repo={repo}", "create", compression, "--files-cache=disabled", "borg-benchmark-crud1", path] + rc = get_ec( + self.do_create( + self.parse_args( + [ + f"--repo={repo}", + "create", + compression, + "--files-cache=disabled", + "borg-benchmark-crud1", + path, + ] + ) ) ) t_end = time.monotonic() dt_create = t_end - t_start assert rc == 0 # now build files cache - rc1 = self.do_create( - self.parse_args([f"--repo={repo}", "create", compression, "borg-benchmark-crud2", path]) + rc1 = get_ec( + self.do_create(self.parse_args([f"--repo={repo}", "create", compression, "borg-benchmark-crud2", path])) ) - rc2 = self.do_delete(self.parse_args([f"--repo={repo}", "delete", "-a", "borg-benchmark-crud2"])) + rc2 = get_ec(self.do_delete(self.parse_args([f"--repo={repo}", "delete", "-a", "borg-benchmark-crud2"]))) assert rc1 == rc2 == 0 # measure a no-change update (archive1 is still present) t_start = time.monotonic() - rc1 = self.do_create( - self.parse_args([f"--repo={repo}", "create", compression, "borg-benchmark-crud3", path]) + rc1 = get_ec( + self.do_create(self.parse_args([f"--repo={repo}", "create", compression, "borg-benchmark-crud3", path])) ) t_end = time.monotonic() dt_update = t_end - t_start - rc2 = self.do_delete(self.parse_args([f"--repo={repo}", "delete", "-a", "borg-benchmark-crud3"])) + rc2 = get_ec(self.do_delete(self.parse_args([f"--repo={repo}", "delete", "-a", "borg-benchmark-crud3"]))) assert rc1 == rc2 == 0 # measure extraction (dry-run: without writing result to disk) t_start = time.monotonic() - rc = self.do_extract(self.parse_args([f"--repo={repo}", "extract", "borg-benchmark-crud1", "--dry-run"])) + rc = get_ec( + self.do_extract(self.parse_args([f"--repo={repo}", "extract", "borg-benchmark-crud1", "--dry-run"])) + ) t_end = time.monotonic() dt_extract = t_end - t_start assert rc == 0 # measure archive deletion (of LAST present archive with the data) t_start = time.monotonic() - rc = self.do_delete(self.parse_args([f"--repo={repo}", "delete", "-a", "borg-benchmark-crud1"])) + rc = get_ec(self.do_delete(self.parse_args([f"--repo={repo}", "delete", "-a", "borg-benchmark-crud1"]))) t_end = time.monotonic() dt_delete = t_end - t_start assert rc == 0 @@ -93,8 +105,6 @@ class BenchmarkMixIn: print(fmt % ("U", msg, total_size_MB / dt_update, count, file_size_formatted, content, dt_update)) print(fmt % ("D", msg, total_size_MB / dt_delete, count, file_size_formatted, content, dt_delete)) - return 0 - def do_benchmark_cpu(self, args): """Benchmark CPU bound operations.""" from timeit import timeit diff --git a/src/borg/archiver/check_cmd.py b/src/borg/archiver/check_cmd.py index 5ef4ebbc..b3ca070a 100644 --- a/src/borg/archiver/check_cmd.py +++ b/src/borg/archiver/check_cmd.py @@ -2,7 +2,7 @@ import argparse from ._common import with_repository, Highlander from ..archive import ArchiveChecker from ..constants import * # NOQA -from ..helpers import EXIT_SUCCESS, EXIT_WARNING, CancelledByUser, CommandError +from ..helpers import set_ec, EXIT_WARNING, CancelledByUser, CommandError from ..helpers import yes from ..logger import create_logger @@ -45,7 +45,7 @@ class CheckMixIn: raise CommandError("--repository-only is required for --max-duration support.") if not args.archives_only: if not repository.check(repair=args.repair, max_duration=args.max_duration): - return EXIT_WARNING + set_ec(EXIT_WARNING) if not args.repo_only and not ArchiveChecker().check( repository, verify_data=args.verify_data, @@ -59,8 +59,8 @@ class CheckMixIn: oldest=args.oldest, newest=args.newest, ): - return EXIT_WARNING - return EXIT_SUCCESS + set_ec(EXIT_WARNING) + return def build_parser_check(self, subparsers, common_parser, mid_common_parser): from ._common import process_epilog diff --git a/src/borg/archiver/compact_cmd.py b/src/borg/archiver/compact_cmd.py index ba8cf893..d5847741 100644 --- a/src/borg/archiver/compact_cmd.py +++ b/src/borg/archiver/compact_cmd.py @@ -2,7 +2,6 @@ import argparse from ._common import with_repository, Highlander from ..constants import * # NOQA -from ..helpers import EXIT_SUCCESS from ..manifest import Manifest from ..logger import create_logger @@ -19,7 +18,6 @@ class CompactMixIn: repository.put(Manifest.MANIFEST_ID, data) threshold = args.threshold / 100 repository.commit(compact=True, threshold=threshold) - return EXIT_SUCCESS def build_parser_compact(self, subparsers, common_parser, mid_common_parser): from ._common import process_epilog diff --git a/src/borg/archiver/config_cmd.py b/src/borg/archiver/config_cmd.py index 6704d716..b67f072d 100644 --- a/src/borg/archiver/config_cmd.py +++ b/src/borg/archiver/config_cmd.py @@ -1,4 +1,3 @@ -import sys import argparse import configparser from binascii import unhexlify @@ -6,7 +5,6 @@ from binascii import unhexlify from ._common import with_repository from ..cache import Cache, assert_secure from ..constants import * # NOQA -from ..helpers import EXIT_SUCCESS, EXIT_WARNING from ..helpers import Error, CommandError from ..helpers import Location from ..helpers import parse_file_size @@ -140,9 +138,7 @@ class ConfigMixIn: try: print(config.get(section, name)) except (configparser.NoOptionError, configparser.NoSectionError) as e: - print(e, file=sys.stderr) - return EXIT_WARNING - return EXIT_SUCCESS + raise Error(e) finally: if args.cache: cache.close() diff --git a/src/borg/archiver/create_cmd.py b/src/borg/archiver/create_cmd.py index 67fe121d..37226cf4 100644 --- a/src/borg/archiver/create_cmd.py +++ b/src/borg/archiver/create_cmd.py @@ -273,7 +273,6 @@ class CreateMixIn: create_inner(archive, cache, fso) else: create_inner(None, None, None) - return self.exit_code def _process_any(self, *, path, parent_fd, name, st, fso, cache, read_special, dry_run): """ diff --git a/src/borg/archiver/debug_cmd.py b/src/borg/archiver/debug_cmd.py index d7265a5b..ce4f6f15 100644 --- a/src/borg/archiver/debug_cmd.py +++ b/src/borg/archiver/debug_cmd.py @@ -28,7 +28,6 @@ class DebugMixIn: """display system information for debugging / bug reports""" print(sysinfo()) print("Process ID:", get_process_id()) - return EXIT_SUCCESS @with_repository(compatibility=Manifest.NO_OPERATION_CHECK) def do_debug_dump_archive_items(self, args, repository, manifest): @@ -42,7 +41,6 @@ class DebugMixIn: with open(filename, "wb") as fd: fd.write(data) print("Done.") - return EXIT_SUCCESS @with_repository(compatibility=Manifest.NO_OPERATION_CHECK) def do_debug_dump_archive(self, args, repository, manifest): @@ -97,7 +95,6 @@ class DebugMixIn: with dash_open(args.path, "w") as fd: output(fd) - return EXIT_SUCCESS @with_repository(compatibility=Manifest.NO_OPERATION_CHECK) def do_debug_dump_manifest(self, args, repository, manifest): @@ -109,7 +106,6 @@ class DebugMixIn: with dash_open(args.path, "w") as fd: json.dump(meta, fd, indent=4) - return EXIT_SUCCESS @with_repository(manifest=False) def do_debug_dump_repo_objs(self, args, repository): @@ -165,7 +161,6 @@ class DebugMixIn: decrypt_dump(i, id, cdata) i += 1 print("Done.") - return EXIT_SUCCESS @with_repository(manifest=False) def do_debug_search_repo_objs(self, args, repository): @@ -234,7 +229,6 @@ class DebugMixIn: if i % 10000 == 0: print("%d objects processed." % i) print("Done.") - return EXIT_SUCCESS @with_repository(manifest=False) def do_debug_get_obj(self, args, repository): @@ -253,7 +247,6 @@ class DebugMixIn: with open(args.path, "wb") as f: f.write(data) print("object %s fetched." % hex_id) - return EXIT_SUCCESS @with_repository(compatibility=Manifest.NO_OPERATION_CHECK) def do_debug_id_hash(self, args, repository, manifest): @@ -263,7 +256,6 @@ class DebugMixIn: key = manifest.key id = key.id_hash(data) print(id.hex()) - return EXIT_SUCCESS @with_repository(compatibility=Manifest.NO_OPERATION_CHECK) def do_debug_parse_obj(self, args, repository, manifest): @@ -290,8 +282,6 @@ class DebugMixIn: with open(args.binary_path, "wb") as f: f.write(data) - return EXIT_SUCCESS - @with_repository(compatibility=Manifest.NO_OPERATION_CHECK) def do_debug_format_obj(self, args, repository, manifest): """format file and metadata into borg object file""" @@ -317,7 +307,6 @@ class DebugMixIn: with open(args.object_path, "wb") as f: f.write(data_encrypted) - return EXIT_SUCCESS @with_repository(manifest=False, exclusive=True) def do_debug_put_obj(self, args, repository): @@ -335,7 +324,6 @@ class DebugMixIn: repository.put(id, data) print("object %s put." % hex_id) repository.commit(compact=False) - return EXIT_SUCCESS @with_repository(manifest=False, exclusive=True) def do_debug_delete_obj(self, args, repository): @@ -356,7 +344,6 @@ class DebugMixIn: if modified: repository.commit(compact=False) print("Done.") - return EXIT_SUCCESS @with_repository(manifest=False, exclusive=True, cache=True, compatibility=Manifest.NO_OPERATION_CHECK) def do_debug_refcount_obj(self, args, repository, manifest, cache): @@ -372,7 +359,6 @@ class DebugMixIn: print("object %s has %d referrers [info from chunks cache]." % (hex_id, refcount)) except KeyError: print("object %s not found [info from chunks cache]." % hex_id) - return EXIT_SUCCESS @with_repository(manifest=False, exclusive=True) def do_debug_dump_hints(self, args, repository): @@ -390,7 +376,6 @@ class DebugMixIn: json.dump(hints, fd, indent=4) finally: repository.rollback() - return EXIT_SUCCESS def do_debug_convert_profile(self, args): """convert Borg profile to Python profile""" @@ -398,7 +383,6 @@ class DebugMixIn: with args.output, args.input: marshal.dump(msgpack.unpack(args.input, use_list=False, raw=False), args.output) - return EXIT_SUCCESS def build_parser_debug(self, subparsers, common_parser, mid_common_parser): debug_epilog = process_epilog( diff --git a/src/borg/archiver/delete_cmd.py b/src/borg/archiver/delete_cmd.py index 71467f7e..00c3169c 100644 --- a/src/borg/archiver/delete_cmd.py +++ b/src/borg/archiver/delete_cmd.py @@ -22,13 +22,13 @@ class DeleteMixIn: manifest = Manifest.load(repository, (Manifest.Operation.DELETE,)) archive_names = tuple(x.name for x in manifest.archives.list_considering(args)) if not archive_names: - return self.exit_code + return if args.match_archives is None and args.first == 0 and args.last == 0: self.print_error( "Aborting: if you really want to delete all archives, please use -a 'sh:*' " "or just delete the whole repository (might be much faster)." ) - return EXIT_ERROR + return if args.forced == 2: deleted = False @@ -52,7 +52,7 @@ class DeleteMixIn: self.print_warning('Done. Run "borg check --repair" to clean up the mess.', wc=None) else: self.print_warning("Aborted.", wc=None) - return self.exit_code + return stats = Statistics(iec=args.iec) with Cache(repository, manifest, progress=args.progress, lock_wait=self.lock_wait, iec=args.iec) as cache: @@ -92,8 +92,6 @@ class DeleteMixIn: if args.stats: log_multi(str(stats), logger=logging.getLogger("borg.output.stats")) - return self.exit_code - def build_parser_delete(self, subparsers, common_parser, mid_common_parser): from ._common import process_epilog, define_archive_filters_group diff --git a/src/borg/archiver/diff_cmd.py b/src/borg/archiver/diff_cmd.py index 7eb0da33..d6349424 100644 --- a/src/borg/archiver/diff_cmd.py +++ b/src/borg/archiver/diff_cmd.py @@ -77,8 +77,6 @@ class DiffMixIn: for pattern in matcher.get_unmatched_include_patterns(): self.print_warning_instance(IncludePatternNeverMatchedWarning(pattern)) - return self.exit_code - def build_parser_diff(self, subparsers, common_parser, mid_common_parser): from ._common import process_epilog from ._common import define_exclusion_group diff --git a/src/borg/archiver/extract_cmd.py b/src/borg/archiver/extract_cmd.py index 92cc7fa2..af22fc56 100644 --- a/src/borg/archiver/extract_cmd.py +++ b/src/borg/archiver/extract_cmd.py @@ -101,7 +101,6 @@ class ExtractMixIn: if pi: # clear progress output pi.finish() - return self.exit_code def build_parser_extract(self, subparsers, common_parser, mid_common_parser): from ._common import process_epilog diff --git a/src/borg/archiver/help_cmd.py b/src/borg/archiver/help_cmd.py index f616635f..f848c27d 100644 --- a/src/borg/archiver/help_cmd.py +++ b/src/borg/archiver/help_cmd.py @@ -475,12 +475,10 @@ class HelpMixIn: msg_lines += [" Commands: %s" % ", ".join(sorted(commands.keys()))] msg_lines += [" Topics: %s" % ", ".join(sorted(self.helptext.keys()))] parser.error("\n".join(msg_lines)) - return self.exit_code def do_subcommand_help(self, parser, args): """display infos about subcommand""" parser.print_help() - return EXIT_SUCCESS do_maincommand_help = do_subcommand_help diff --git a/src/borg/archiver/info_cmd.py b/src/borg/archiver/info_cmd.py index d16c1fe8..78ccf105 100644 --- a/src/borg/archiver/info_cmd.py +++ b/src/borg/archiver/info_cmd.py @@ -50,14 +50,11 @@ class InfoMixIn: .strip() .format(**info) ) - if self.exit_code: - break if not args.json and len(archive_names) - i: print() if args.json: json_print(basic_json_data(manifest, cache=cache, extra={"archives": output_data})) - return self.exit_code def build_parser_info(self, subparsers, common_parser, mid_common_parser): from ._common import process_epilog, define_archive_filters_group diff --git a/src/borg/archiver/key_cmds.py b/src/borg/archiver/key_cmds.py index 74b388ac..81e1f056 100644 --- a/src/borg/archiver/key_cmds.py +++ b/src/borg/archiver/key_cmds.py @@ -28,7 +28,6 @@ class KeysMixIn: if hasattr(key, "find_key"): # print key location to make backing it up easier logger.info("Key location: %s", key.find_key()) - return EXIT_SUCCESS @with_repository(exclusive=True, manifest=True, cache=True, compatibility=(Manifest.Operation.CHECK,)) def do_change_location(self, args, repository, manifest, cache): @@ -48,7 +47,7 @@ class KeysMixIn: key_new = Blake2CHPOKeyfileKey(repository) else: print("Change not needed or not supported.") - return EXIT_WARNING + return if args.key_mode == "repokey": if isinstance(key, AESOCBKeyfileKey): key_new = AESOCBRepoKey(repository) @@ -60,7 +59,7 @@ class KeysMixIn: key_new = Blake2CHPORepoKey(repository) else: print("Change not needed or not supported.") - return EXIT_WARNING + return for name in ("repository_id", "crypt_key", "id_key", "chunk_seed", "sessionid", "cipher"): value = getattr(key, name) @@ -89,8 +88,6 @@ class KeysMixIn: key.remove(key.target) # remove key from current location logger.info(f"Key moved to {loc}") - return EXIT_SUCCESS - @with_repository(lock=False, exclusive=False, manifest=False, cache=False) def do_key_export(self, args, repository): """Export the repository key for backup""" @@ -108,7 +105,6 @@ class KeysMixIn: manager.export(args.path) except IsADirectoryError: raise CommandError(f"'{args.path}' must be a file, not a directory") - return EXIT_SUCCESS @with_repository(lock=False, exclusive=False, manifest=False, cache=False) def do_key_import(self, args, repository): @@ -124,7 +120,6 @@ class KeysMixIn: if args.path != "-" and not os.path.exists(args.path): raise CommandError("input file does not exist: " + args.path) manager.import_keyfile(args) - return EXIT_SUCCESS def build_parser_keys(self, subparsers, common_parser, mid_common_parser): from ._common import process_epilog diff --git a/src/borg/archiver/list_cmd.py b/src/borg/archiver/list_cmd.py index c2a2acb0..8b4ccede 100644 --- a/src/borg/archiver/list_cmd.py +++ b/src/borg/archiver/list_cmd.py @@ -40,8 +40,6 @@ class ListMixIn: else: _list_inner(cache=None) - return self.exit_code - def build_parser_list(self, subparsers, common_parser, mid_common_parser): from ._common import process_epilog, define_exclusion_group diff --git a/src/borg/archiver/lock_cmds.py b/src/borg/archiver/lock_cmds.py index 1ea294ac..53668cca 100644 --- a/src/borg/archiver/lock_cmds.py +++ b/src/borg/archiver/lock_cmds.py @@ -4,7 +4,7 @@ import subprocess from ._common import with_repository from ..cache import Cache from ..constants import * # NOQA -from ..helpers import prepare_subprocess_env +from ..helpers import prepare_subprocess_env, set_ec from ..manifest import Manifest from ..logger import create_logger @@ -33,7 +33,8 @@ class LocksMixIn: env = prepare_subprocess_env(system=True) try: # we exit with the return code we get from the subprocess - return subprocess.call([args.command] + args.args, env=env) + rc = subprocess.call([args.command] + args.args, env=env) + set_ec(rc) finally: # we need to commit the "no change" operation we did to the manifest # because it created a new segment file in the repository. if we would @@ -48,7 +49,6 @@ class LocksMixIn: """Break the repository lock (e.g. in case it was left by a dead borg.""" repository.break_lock() Cache.break_lock(repository) - return self.exit_code def build_parser_locks(self, subparsers, common_parser, mid_common_parser): from ._common import process_epilog diff --git a/src/borg/archiver/mount_cmds.py b/src/borg/archiver/mount_cmds.py index 1bd96617..15eade7a 100644 --- a/src/borg/archiver/mount_cmds.py +++ b/src/borg/archiver/mount_cmds.py @@ -30,7 +30,7 @@ class MountMixIn: if not os.access(args.mountpoint, os.R_OK | os.W_OK | os.X_OK): raise RTError(f"{args.mountpoint}: Mountpoint must be a **writable** directory") - return self._do_mount(args) + self._do_mount(args) @with_repository(compatibility=(Manifest.Operation.READ,)) def _do_mount(self, args, repository, manifest): @@ -44,11 +44,10 @@ class MountMixIn: except RuntimeError: # Relevant error message already printed to stderr by FUSE raise RTError("FUSE mount failed") - return self.exit_code def do_umount(self, args): """un-mount the FUSE filesystem""" - return umount(args.mountpoint) + umount(args.mountpoint) def build_parser_mount_umount(self, subparsers, common_parser, mid_common_parser): from ._common import process_epilog diff --git a/src/borg/archiver/prune_cmd.py b/src/borg/archiver/prune_cmd.py index 467fb76a..30d4d428 100644 --- a/src/borg/archiver/prune_cmd.py +++ b/src/borg/archiver/prune_cmd.py @@ -178,7 +178,6 @@ class PruneMixIn: checkpoint_func() if args.stats: log_multi(str(stats), logger=logging.getLogger("borg.output.stats")) - return self.exit_code def build_parser_prune(self, subparsers, common_parser, mid_common_parser): from ._common import process_epilog diff --git a/src/borg/archiver/rcompress_cmd.py b/src/borg/archiver/rcompress_cmd.py index 40d7571d..3d5d358c 100644 --- a/src/borg/archiver/rcompress_cmd.py +++ b/src/borg/archiver/rcompress_cmd.py @@ -178,8 +178,6 @@ class RCompressMixIn: print(f"Kept as is: {stats_process['kept_count']}") print(f"Total: {stats_process['recompressed_count'] + stats_process['kept_count']}") - return self.exit_code - def build_parser_rcompress(self, subparsers, common_parser, mid_common_parser): from ._common import process_epilog diff --git a/src/borg/archiver/rcreate_cmd.py b/src/borg/archiver/rcreate_cmd.py index 67ee6ebf..b4a66645 100644 --- a/src/borg/archiver/rcreate_cmd.py +++ b/src/borg/archiver/rcreate_cmd.py @@ -4,7 +4,7 @@ from ._common import with_repository, with_other_repository, Highlander from ..cache import Cache from ..constants import * # NOQA from ..crypto.key import key_creator, key_argument_names -from ..helpers import EXIT_WARNING +from ..helpers import CancelledByUser from ..helpers import location_validator, Location from ..helpers import parse_storage_quota from ..manifest import Manifest @@ -28,7 +28,7 @@ class RCreateMixIn: key = key_creator(repository, args, other_key=other_key) except (EOFError, KeyboardInterrupt): repository.destroy() - return EXIT_WARNING + raise CancelledByUser() manifest = Manifest(key, repository) manifest.key = key manifest.write() @@ -51,7 +51,6 @@ class RCreateMixIn: " borg key export -r REPOSITORY --qr-html encrypted-key-backup.html\n" "2. Write down the borg key passphrase and store it at safe place.\n" ) - return self.exit_code def build_parser_rcreate(self, subparsers, common_parser, mid_common_parser): from ._common import process_epilog diff --git a/src/borg/archiver/rdelete_cmd.py b/src/borg/archiver/rdelete_cmd.py index dead4c31..30bb66d9 100644 --- a/src/borg/archiver/rdelete_cmd.py +++ b/src/borg/archiver/rdelete_cmd.py @@ -86,7 +86,6 @@ class RDeleteMixIn: logger.info("Cache deleted.") else: logger.info("Would delete cache.") - return self.exit_code def build_parser_rdelete(self, subparsers, common_parser, mid_common_parser): from ._common import process_epilog diff --git a/src/borg/archiver/recreate_cmd.py b/src/borg/archiver/recreate_cmd.py index 7f4b5da6..9ba12579 100644 --- a/src/borg/archiver/recreate_cmd.py +++ b/src/borg/archiver/recreate_cmd.py @@ -53,7 +53,6 @@ class RecreateMixIn: manifest.write() repository.commit(compact=False) cache.commit() - return self.exit_code def build_parser_recreate(self, subparsers, common_parser, mid_common_parser): from ._common import process_epilog diff --git a/src/borg/archiver/rename_cmd.py b/src/borg/archiver/rename_cmd.py index 8e6c413f..67dff512 100644 --- a/src/borg/archiver/rename_cmd.py +++ b/src/borg/archiver/rename_cmd.py @@ -19,7 +19,6 @@ class RenameMixIn: manifest.write() repository.commit(compact=False) cache.commit() - return self.exit_code def build_parser_rename(self, subparsers, common_parser, mid_common_parser): from ._common import process_epilog diff --git a/src/borg/archiver/rinfo_cmd.py b/src/borg/archiver/rinfo_cmd.py index fa5e6012..221d05fc 100644 --- a/src/borg/archiver/rinfo_cmd.py +++ b/src/borg/archiver/rinfo_cmd.py @@ -72,7 +72,6 @@ class RInfoMixIn: print(output) print(str(cache)) - return self.exit_code def build_parser_rinfo(self, subparsers, common_parser, mid_common_parser): from ._common import process_epilog diff --git a/src/borg/archiver/rlist_cmd.py b/src/borg/archiver/rlist_cmd.py index 1e1f7b4e..0ef621f1 100644 --- a/src/borg/archiver/rlist_cmd.py +++ b/src/borg/archiver/rlist_cmd.py @@ -36,8 +36,6 @@ class RListMixIn: if args.json: json_print(basic_json_data(manifest, extra={"archives": output_data})) - return self.exit_code - def build_parser_rlist(self, subparsers, common_parser, mid_common_parser): from ._common import process_epilog, define_archive_filters_group diff --git a/src/borg/archiver/serve_cmd.py b/src/borg/archiver/serve_cmd.py index 2044e0cd..8cc613c5 100644 --- a/src/borg/archiver/serve_cmd.py +++ b/src/borg/archiver/serve_cmd.py @@ -2,7 +2,6 @@ import argparse from ._common import Highlander from ..constants import * # NOQA -from ..helpers import EXIT_SUCCESS from ..helpers import parse_storage_quota from ..remote import RepositoryServer @@ -21,7 +20,6 @@ class ServeMixIn: storage_quota=args.storage_quota, use_socket=args.use_socket, ).serve() - return EXIT_SUCCESS def build_parser_serve(self, subparsers, common_parser, mid_common_parser): from ._common import process_epilog diff --git a/src/borg/archiver/tar_cmds.py b/src/borg/archiver/tar_cmds.py index 28448158..c68d37c2 100644 --- a/src/borg/archiver/tar_cmds.py +++ b/src/borg/archiver/tar_cmds.py @@ -86,8 +86,6 @@ class TarMixIn: with create_filter_process(filter, stream=tarstream, stream_close=tarstream_close, inbound=False) as _stream: self._export_tar(args, archive, _stream) - return self.exit_code - def _export_tar(self, args, archive, tarstream): matcher = build_matcher(args.patterns, args.paths) @@ -241,7 +239,6 @@ class TarMixIn: for pattern in matcher.get_unmatched_include_patterns(): self.print_warning_instance(IncludePatternNeverMatchedWarning(pattern)) - return self.exit_code @with_repository(cache=True, exclusive=True, compatibility=(Manifest.Operation.WRITE,)) def do_import_tar(self, args, repository, manifest, cache): @@ -257,8 +254,6 @@ class TarMixIn: with create_filter_process(filter, stream=tarstream, stream_close=tarstream_close, inbound=True) as _stream: self._import_tar(args, repository, manifest, manifest.key, cache, _stream) - return self.exit_code - def _import_tar(self, args, repository, manifest, key, cache, tarstream): t0 = archive_ts_now() t0_monotonic = time.monotonic() diff --git a/src/borg/archiver/transfer_cmd.py b/src/borg/archiver/transfer_cmd.py index f820ef63..1922cf1c 100644 --- a/src/borg/archiver/transfer_cmd.py +++ b/src/borg/archiver/transfer_cmd.py @@ -5,7 +5,7 @@ from ..archive import Archive from ..compress import CompressionSpec from ..constants import * # NOQA from ..crypto.key import uses_same_id_hash, uses_same_chunker_secret -from ..helpers import EXIT_SUCCESS, EXIT_ERROR, Error +from ..helpers import Error from ..helpers import location_validator, Location, archivename_validator, comment_validator from ..helpers import format_file_size from ..manifest import Manifest @@ -23,22 +23,20 @@ class TransferMixIn: key = manifest.key other_key = other_manifest.key if not uses_same_id_hash(other_key, key): - self.print_error( + raise Error( "You must keep the same ID hash ([HMAC-]SHA256 or BLAKE2b) or deduplication will break. " "Use a related repository!" ) - return EXIT_ERROR if not uses_same_chunker_secret(other_key, key): - self.print_error( + raise Error( "You must use the same chunker secret or deduplication will break. " "Use a related repository!" ) - return EXIT_ERROR dry_run = args.dry_run args.consider_checkpoints = True archive_names = tuple(x.name for x in other_manifest.archives.list_considering(args)) if not archive_names: - return EXIT_SUCCESS + return an_errors = [] for archive_name in archive_names: @@ -47,10 +45,8 @@ class TransferMixIn: except argparse.ArgumentTypeError as err: an_errors.append(str(err)) if an_errors: - self.print_error("Invalid archive names detected, please rename them before transfer:") - for err_msg in an_errors: - self.print_error(err_msg) - return EXIT_ERROR + an_errors.insert(0, "Invalid archive names detected, please rename them before transfer:") + raise Error("\n".join(an_errors)) ac_errors = [] for archive_name in archive_names: @@ -58,20 +54,17 @@ class TransferMixIn: try: comment_validator(archive.metadata.get("comment", "")) except argparse.ArgumentTypeError as err: - ac_errors.append((archive_name, str(err))) + ac_errors.append(f"{archive_name}: {err}") if ac_errors: - self.print_error("Invalid archive comments detected, please fix them before transfer:") - for archive_name, err_msg in ac_errors: - self.print_error(f"{archive_name}: {err_msg}") - return EXIT_ERROR + ac_errors.insert(0, "Invalid archive comments detected, please fix them before transfer:") + raise Error("\n".join(ac_errors)) from .. import upgrade as upgrade_mod try: UpgraderCls = getattr(upgrade_mod, f"Upgrader{args.upgrader}") except AttributeError: - self.print_error(f"No such upgrader: {args.upgrader}") - return EXIT_ERROR + raise Error(f"No such upgrader: {args.upgrader}") if UpgraderCls is not upgrade_mod.UpgraderFrom12To20 and other_manifest.repository.version == 1: raise Error("To transfer from a borg 1.x repo, you need to use: --upgrader=From12To20") @@ -177,7 +170,6 @@ class TransferMixIn: f"transfer_size: {format_file_size(transfer_size)} " f"present_size: {format_file_size(present_size)}" ) - return EXIT_SUCCESS def build_parser_transfer(self, subparsers, common_parser, mid_common_parser): from ._common import process_epilog diff --git a/src/borg/archiver/version_cmd.py b/src/borg/archiver/version_cmd.py index e9bef0d6..75593cbf 100644 --- a/src/borg/archiver/version_cmd.py +++ b/src/borg/archiver/version_cmd.py @@ -2,7 +2,6 @@ import argparse from .. import __version__ from ..constants import * # NOQA -from ..helpers import EXIT_SUCCESS from ..remote import RemoteRepository from ..logger import create_logger @@ -22,7 +21,6 @@ class VersionMixIn: else: server_version = client_version print(f"{format_version(client_version)} / {format_version(server_version)}") - return EXIT_SUCCESS def build_parser_version(self, subparsers, common_parser, mid_common_parser): from ._common import process_epilog diff --git a/src/borg/helpers/__init__.py b/src/borg/helpers/__init__.py index af914e67..9a2aecb6 100644 --- a/src/borg/helpers/__init__.py +++ b/src/borg/helpers/__init__.py @@ -81,8 +81,7 @@ def add_warning(msg, *args, **kwargs): """ 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 -from the archiver object. +warning or error occurred during their operation. Note: keep this in helpers/__init__.py as the code expects to be able to assign to helpers.exit_code. """ diff --git a/src/borg/helpers/fs.py b/src/borg/helpers/fs.py index 729234f9..97939f89 100644 --- a/src/borg/helpers/fs.py +++ b/src/borg/helpers/fs.py @@ -519,11 +519,14 @@ def os_stat(*, path=None, parent_fd=None, name=None, follow_symlinks=False): def umount(mountpoint): + from . import set_ec + env = prepare_subprocess_env(system=True) try: - return subprocess.call(["fusermount", "-u", mountpoint], env=env) + rc = subprocess.call(["fusermount", "-u", mountpoint], env=env) except FileNotFoundError: - return subprocess.call(["umount", mountpoint], env=env) + rc = subprocess.call(["umount", mountpoint], env=env) + set_ec(rc) # below is a slightly modified tempfile.mkstemp that has an additional mode parameter. diff --git a/src/borg/testsuite/archiver/__init__.py b/src/borg/testsuite/archiver/__init__.py index f45b34d2..bd75ec76 100644 --- a/src/borg/testsuite/archiver/__init__.py +++ b/src/borg/testsuite/archiver/__init__.py @@ -76,7 +76,6 @@ def exec_cmd(*args, archiver=None, fork=False, exe=None, input=b"", binary_outpu if archiver is None: archiver = Archiver() archiver.prerun_checks = lambda *args: None - archiver.exit_code = EXIT_SUCCESS helpers.exit_code = EXIT_SUCCESS helpers.warnings_list = [] try: diff --git a/src/borg/testsuite/archiver/config_cmd.py b/src/borg/testsuite/archiver/config_cmd.py index 2215b55a..f76234be 100644 --- a/src/borg/testsuite/archiver/config_cmd.py +++ b/src/borg/testsuite/archiver/config_cmd.py @@ -3,7 +3,7 @@ import pytest from ...constants import * # NOQA from . import RK_ENCRYPTION, create_test_files, cmd, generate_archiver_tests -from ...helpers import CommandError +from ...helpers import CommandError, Error pytest_generate_tests = lambda metafunc: generate_archiver_tests(metafunc, kinds="local,binary") # NOQA @@ -23,8 +23,13 @@ def test_config(archivers, request): assert "id" in output assert "last_segment_checked" not in output - output = cmd(archiver, "config", "last_segment_checked", exit_code=1) - assert "No option " in output + if archiver.FORK_DEFAULT: + output = cmd(archiver, "config", "last_segment_checked", exit_code=2) + assert "No option " in output + else: + with pytest.raises(Error): + cmd(archiver, "config", "last_segment_checked") + cmd(archiver, "config", "last_segment_checked", "123") output = cmd(archiver, "config", "last_segment_checked") assert output == "123" + os.linesep @@ -39,7 +44,11 @@ def test_config(archivers, request): output = cmd(archiver, "config", cfg_key) assert output == cfg_value + os.linesep cmd(archiver, "config", "--delete", cfg_key) - cmd(archiver, "config", cfg_key, exit_code=1) + if archiver.FORK_DEFAULT: + cmd(archiver, "config", cfg_key, exit_code=2) + else: + with pytest.raises(Error): + cmd(archiver, "config", cfg_key) cmd(archiver, "config", "--list", "--delete", exit_code=2) if archiver.FORK_DEFAULT: @@ -47,4 +56,8 @@ def test_config(archivers, request): else: with pytest.raises(CommandError): cmd(archiver, "config") - cmd(archiver, "config", "invalid-option", exit_code=1) + if archiver.FORK_DEFAULT: + cmd(archiver, "config", "invalid-option", exit_code=2) + else: + with pytest.raises(Error): + cmd(archiver, "config", "invalid-option") diff --git a/src/borg/testsuite/archiver/corruption.py b/src/borg/testsuite/archiver/corruption.py index 396fc4b7..a604a478 100644 --- a/src/borg/testsuite/archiver/corruption.py +++ b/src/borg/testsuite/archiver/corruption.py @@ -7,7 +7,7 @@ import pytest from ...constants import * # NOQA from ...crypto.file_integrity import FileIntegrityError -from ...helpers import bin_to_hex +from ...helpers import bin_to_hex, Error from . import cmd, create_src_archive, create_test_files, RK_ENCRYPTION @@ -22,7 +22,11 @@ def test_check_corrupted_repository(archiver): fd.seek(100) fd.write(b"XXXX") - cmd(archiver, "check", exit_code=1) + if archiver.FORK_DEFAULT: + cmd(archiver, "check", exit_code=1) + else: + with pytest.raises(Error): + cmd(archiver, "check") def corrupt_archiver(archiver): diff --git a/src/borg/testsuite/archiver/rcreate_cmd.py b/src/borg/testsuite/archiver/rcreate_cmd.py index 93f121f8..48c9dc16 100644 --- a/src/borg/testsuite/archiver/rcreate_cmd.py +++ b/src/borg/testsuite/archiver/rcreate_cmd.py @@ -3,7 +3,7 @@ from unittest.mock import patch import pytest -from ...helpers.errors import Error +from ...helpers.errors import Error, CancelledByUser from ...constants import * # NOQA from ...crypto.key import FlexiKey from ...repository import Repository @@ -37,7 +37,12 @@ def test_rcreate_interrupt(archivers, request): raise EOFError with patch.object(FlexiKey, "create", raise_eof): - cmd(archiver, "rcreate", RK_ENCRYPTION, exit_code=1) + if archiver.FORK_DEFAULT: + cmd(archiver, "rcreate", RK_ENCRYPTION, exit_code=2) + else: + with pytest.raises(CancelledByUser): + cmd(archiver, "rcreate", RK_ENCRYPTION) + assert not os.path.exists(archiver.repository_location) From b53c86cf4ca69c39005bd5e0ed6d42de7f06bcd5 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Fri, 15 Dec 2023 19:42:51 +0100 Subject: [PATCH 19/22] refactor (re-)init of exit_code and warnings_list globals stop directly accessing the variables from other modules. prefix with underscore to indicate that these shall only be used within this module and every other user shall call the respective functions. --- src/borg/helpers/__init__.py | 41 +++++++++++++++---------- src/borg/testsuite/archiver/__init__.py | 6 ++-- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/src/borg/helpers/__init__.py b/src/borg/helpers/__init__.py index 9a2aecb6..8c0bed69 100644 --- a/src/borg/helpers/__init__.py +++ b/src/borg/helpers/__init__.py @@ -64,28 +64,24 @@ 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 = [] +_warnings_list = [] def add_warning(msg, *args, **kwargs): - global warnings_list + 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)) + _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. - -Note: keep this in helpers/__init__.py as the code expects to be able to assign to helpers.exit_code. """ -exit_code = EXIT_SUCCESS +_exit_code = EXIT_SUCCESS def classify_ec(ec): @@ -128,8 +124,19 @@ def set_ec(ec): """ 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_ec(exit_code, ec) + global _exit_code + _exit_code = max_ec(_exit_code, ec) + + +def init_ec_warnings(ec=EXIT_SUCCESS, warnings=None): + """ + (Re-)Init the globals for the exit code and the warnings list. + """ + global _exit_code, _warnings_list + _exit_code = ec + warnings = [] if warnings is None else warnings + assert isinstance(warnings, list) + _warnings_list = warnings def get_ec(ec=None): @@ -139,18 +146,18 @@ def get_ec(ec=None): if ec is not None: set_ec(ec) - global exit_code - exit_code_class = classify_ec(exit_code) + 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 + return _exit_code assert exit_code_class == "success" - global warnings_list - if not warnings_list: + global _warnings_list + if not _warnings_list: # we do not have any warnings in warnings list, return success exit code - return exit_code + return _exit_code # looks like we have some warning(s) - rcs = sorted(set(w_info.wc for w_info in warnings_list)) + 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 diff --git a/src/borg/testsuite/archiver/__init__.py b/src/borg/testsuite/archiver/__init__.py index bd75ec76..bb5a7e16 100644 --- a/src/borg/testsuite/archiver/__init__.py +++ b/src/borg/testsuite/archiver/__init__.py @@ -15,7 +15,7 @@ from io import BytesIO, StringIO import pytest -from ... import xattr, helpers, platform +from ... import xattr, platform from ...archive import Archive from ...archiver import Archiver, PURE_PYTHON_MSGPACK_WARNING from ...cache import Cache @@ -23,6 +23,7 @@ from ...constants import * # NOQA from ...helpers import Location, umount from ...helpers import EXIT_SUCCESS from ...helpers import bin_to_hex +from ...helpers import init_ec_warnings from ...logger import flush_logging from ...manifest import Manifest from ...platform import get_flags @@ -76,8 +77,7 @@ def exec_cmd(*args, archiver=None, fork=False, exe=None, input=b"", binary_outpu if archiver is None: archiver = Archiver() archiver.prerun_checks = lambda *args: None - helpers.exit_code = EXIT_SUCCESS - helpers.warnings_list = [] + init_ec_warnings() try: args = archiver.parse_args(list(args)) # argparse parsing may raise SystemExit when the command line is bad or From a0a07ab46499ba032ffef225c5b16ba25a341dc1 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 16 Dec 2023 00:30:32 +0100 Subject: [PATCH 20/22] use get_reset_ec to internally re-init ec/warnings if we do multiple calls to Archiver.do_something(), we need to reset the ec / warnings after each call, otherwise they will keep growing (in severity, in length). --- src/borg/archiver/benchmark_cmd.py | 22 ++++++++++++++-------- src/borg/helpers/__init__.py | 7 +++++++ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/borg/archiver/benchmark_cmd.py b/src/borg/archiver/benchmark_cmd.py index 78bf9b39..68ad7b8e 100644 --- a/src/borg/archiver/benchmark_cmd.py +++ b/src/borg/archiver/benchmark_cmd.py @@ -9,7 +9,7 @@ from ..constants import * # NOQA from ..crypto.key import FlexiKey from ..helpers import format_file_size from ..helpers import msgpack -from ..helpers import get_ec +from ..helpers import get_reset_ec from ..item import Item from ..platform import SyncFile @@ -22,7 +22,7 @@ class BenchmarkMixIn: compression = "--compression=none" # measure create perf (without files cache to always have it chunking) t_start = time.monotonic() - rc = get_ec( + rc = get_reset_ec( self.do_create( self.parse_args( [ @@ -40,23 +40,27 @@ class BenchmarkMixIn: dt_create = t_end - t_start assert rc == 0 # now build files cache - rc1 = get_ec( + rc1 = get_reset_ec( self.do_create(self.parse_args([f"--repo={repo}", "create", compression, "borg-benchmark-crud2", path])) ) - rc2 = get_ec(self.do_delete(self.parse_args([f"--repo={repo}", "delete", "-a", "borg-benchmark-crud2"]))) + rc2 = get_reset_ec( + self.do_delete(self.parse_args([f"--repo={repo}", "delete", "-a", "borg-benchmark-crud2"])) + ) assert rc1 == rc2 == 0 # measure a no-change update (archive1 is still present) t_start = time.monotonic() - rc1 = get_ec( + rc1 = get_reset_ec( self.do_create(self.parse_args([f"--repo={repo}", "create", compression, "borg-benchmark-crud3", path])) ) t_end = time.monotonic() dt_update = t_end - t_start - rc2 = get_ec(self.do_delete(self.parse_args([f"--repo={repo}", "delete", "-a", "borg-benchmark-crud3"]))) + rc2 = get_reset_ec( + self.do_delete(self.parse_args([f"--repo={repo}", "delete", "-a", "borg-benchmark-crud3"])) + ) assert rc1 == rc2 == 0 # measure extraction (dry-run: without writing result to disk) t_start = time.monotonic() - rc = get_ec( + rc = get_reset_ec( self.do_extract(self.parse_args([f"--repo={repo}", "extract", "borg-benchmark-crud1", "--dry-run"])) ) t_end = time.monotonic() @@ -64,7 +68,9 @@ class BenchmarkMixIn: assert rc == 0 # measure archive deletion (of LAST present archive with the data) t_start = time.monotonic() - rc = get_ec(self.do_delete(self.parse_args([f"--repo={repo}", "delete", "-a", "borg-benchmark-crud1"]))) + rc = get_reset_ec( + self.do_delete(self.parse_args([f"--repo={repo}", "delete", "-a", "borg-benchmark-crud1"])) + ) t_end = time.monotonic() dt_delete = t_end - t_start assert rc == 0 diff --git a/src/borg/helpers/__init__.py b/src/borg/helpers/__init__.py index 8c0bed69..ae8259b7 100644 --- a/src/borg/helpers/__init__.py +++ b/src/borg/helpers/__init__.py @@ -164,3 +164,10 @@ def get_ec(ec=None): return rcs[0] # there were different kinds of warnings return EXIT_WARNING # generic warning rc, user has to look into the logs + + +def get_reset_ec(ec=None): + """Like get_ec, but re-initialize ec/warnings afterwards.""" + rc = get_ec(ec) + init_ec_warnings() + return rc From d7e2e2cea9ed952caceb8ca5210d60f1c60e7b66 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 17 Feb 2024 23:01:46 +0100 Subject: [PATCH 21/22] fix mypy error --- src/borg/helpers/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/borg/helpers/__init__.py b/src/borg/helpers/__init__.py index ae8259b7..0f4f98e5 100644 --- a/src/borg/helpers/__init__.py +++ b/src/borg/helpers/__init__.py @@ -6,6 +6,7 @@ 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 typing import List from collections import namedtuple from ..constants import * # NOQA @@ -65,7 +66,7 @@ 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. """ -_warnings_list = [] +_warnings_list: List[warning_info] = [] def add_warning(msg, *args, **kwargs): From ed28eb9e03da5c340e65199265c4e71bf940faa3 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 18 Feb 2024 03:46:45 +0100 Subject: [PATCH 22/22] fix: Error/CommandError have a output format for 1 argument --- src/borg/archiver/create_cmd.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/borg/archiver/create_cmd.py b/src/borg/archiver/create_cmd.py index 37226cf4..62b6fab0 100644 --- a/src/borg/archiver/create_cmd.py +++ b/src/borg/archiver/create_cmd.py @@ -80,15 +80,15 @@ class CreateMixIn: preexec_fn=None if is_win32 else ignore_sigint, ) except (FileNotFoundError, PermissionError) as e: - raise CommandError("Failed to execute command: %s", e) + raise CommandError(f"Failed to execute command: {e}") status = fso.process_pipe( path=path, cache=cache, fd=proc.stdout, mode=mode, user=user, group=group ) rc = proc.wait() if rc != 0: - raise CommandError("Command %r exited with status %d", args.paths[0], rc) + raise CommandError(f"Command {args.paths[0]!r} exited with status {rc}") except BackupError as e: - raise Error("%s: %s", path, e) + raise Error(f"{path!r}: {e}") else: status = "+" # included self.print_file_status(status, path) @@ -101,7 +101,7 @@ class CreateMixIn: args.paths, stdout=subprocess.PIPE, env=env, preexec_fn=None if is_win32 else ignore_sigint ) except (FileNotFoundError, PermissionError) as e: - raise CommandError("Failed to execute command: %s", e) + raise CommandError(f"Failed to execute command: {e}") pipe_bin = proc.stdout else: # args.paths_from_stdin == True pipe_bin = sys.stdin.buffer @@ -132,7 +132,7 @@ class CreateMixIn: if args.paths_from_command: rc = proc.wait() if rc != 0: - raise CommandError("Command %r exited with status %d", args.paths[0], rc) + raise CommandError(f"Command {args.paths[0]!r} exited with status {rc}") else: for path in args.paths: if path == "": # issue #5637