From f5de6bf43ce661878c23096fc3cbb661aa191673 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 30 Jan 2026 22:37:12 -0800 Subject: [PATCH 01/12] Clarify command hooks documentation about YAML sequences (#1255). --- docs/reference/configuration/command-hooks.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/reference/configuration/command-hooks.md b/docs/reference/configuration/command-hooks.md index 7c7f1eb6..8c4ef3ff 100644 --- a/docs/reference/configuration/command-hooks.md +++ b/docs/reference/configuration/command-hooks.md @@ -12,11 +12,15 @@ list of `commands:` in your borgmatic configuration file. For example: ```yaml commands: - before: action - when: [create] + when: [check] # This is an inline YAML sequence. run: - echo "Before create!" + - before: action + when: [create, prune] # Also an inline YAML sequence. + run: + - echo "Before create or prune!" - after: action - when: + when: # Multi-line YAML sequence, equivalent to "[create, prune]". - create - prune run: From fd485e64a399560d8446bc11d7384813ece2af7a Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 30 Jan 2026 22:43:54 -0800 Subject: [PATCH 02/12] Fix incorrect deprecated hook option name in documentation. --- .../backup-to-a-removable-drive-or-an-intermittent-server.md | 2 +- docs/reference/configuration/command-hooks.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/how-to/backup-to-a-removable-drive-or-an-intermittent-server.md b/docs/how-to/backup-to-a-removable-drive-or-an-intermittent-server.md index 7d027141..313a7f45 100644 --- a/docs/how-to/backup-to-a-removable-drive-or-an-intermittent-server.md +++ b/docs/how-to/backup-to-a-removable-drive-or-an-intermittent-server.md @@ -82,7 +82,7 @@ before_actions: option in the `hooks:` section of your configuration. Prior to version 1.7.0 Use -`before_create` or similar instead of `before_actions`, which was introduced in +`before_backup` or similar instead of `before_actions`, which was introduced in borgmatic 1.7.0. What this does is check if the `findmnt` command errors when probing for a diff --git a/docs/reference/configuration/command-hooks.md b/docs/reference/configuration/command-hooks.md index 8c4ef3ff..e05fdad1 100644 --- a/docs/reference/configuration/command-hooks.md +++ b/docs/reference/configuration/command-hooks.md @@ -33,7 +33,7 @@ commands: Each command in the `commands:` list has the following options: * `before` or `after`: Name for the point in borgmatic's execution that the commands should be run before or after, one of: - * `action` runs before or after each action for each repository. This replaces the deprecated `before_create`, `after_prune`, etc. + * `action` runs before or after each action for each repository. This replaces the deprecated `before_backup`, `after_prune`, etc. * `repository` runs before or after all actions for each repository. This replaces the deprecated `before_actions` and `after_actions`. * `configuration` runs before or after all actions and repositories in the current configuration file. * `everything` runs before or after all configuration files. Errors here do not trigger `error` hooks or the `fail` state in monitoring hooks. This replaces the deprecated `before_everything` and `after_everything`. From 6b5390f5dd23211dd2b88e87e144769cf6a10f2a Mon Sep 17 00:00:00 2001 From: Mashrafi Rahman Date: Sun, 1 Feb 2026 15:30:09 +0800 Subject: [PATCH 03/12] Send tags as array in ntfy monitoring hook. --- borgmatic/hooks/monitoring/ntfy.py | 8 +++++++- tests/unit/hooks/monitoring/test_ntfy.py | 4 ++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/borgmatic/hooks/monitoring/ntfy.py b/borgmatic/hooks/monitoring/ntfy.py index 0429e703..50b31744 100644 --- a/borgmatic/hooks/monitoring/ntfy.py +++ b/borgmatic/hooks/monitoring/ntfy.py @@ -22,6 +22,12 @@ def initialize_monitor( ''' +def _convert_string_to_array(value): + value = str(value or '') + + return [str(item).strip() for item in value.split(',') if str(item).strip()] + + PRIORITY_NAME_TO_ID = { 'max': 5, 'urgent': 5, @@ -66,7 +72,7 @@ def ping_monitor(hook_config, config, config_filename, state, monitoring_log_lev 'title': state_config.get('title'), 'message': state_config.get('message'), 'priority': PRIORITY_NAME_TO_ID.get(state_config.get('priority'), default_priority), - 'tags': state_config.get('tags'), + 'tags': _convert_string_to_array(state_config.get('tags')), } try: diff --git a/tests/unit/hooks/monitoring/test_ntfy.py b/tests/unit/hooks/monitoring/test_ntfy.py index 7588edd6..6280785a 100644 --- a/tests/unit/hooks/monitoring/test_ntfy.py +++ b/tests/unit/hooks/monitoring/test_ntfy.py @@ -25,7 +25,7 @@ CUSTOM_MESSAGE_PAYLOAD = { 'title': CUSTOM_MESSAGE_CONFIG['title'], 'message': CUSTOM_MESSAGE_CONFIG['message'], 'priority': 1, - 'tags': CUSTOM_MESSAGE_CONFIG['tags'], + 'tags': [CUSTOM_MESSAGE_CONFIG['tags']], } @@ -35,7 +35,7 @@ def default_message_payload(state=Enum): 'title': f'A borgmatic {state.name} event happened', 'message': f'A borgmatic {state.name} event happened', 'priority': 3, - 'tags': 'borgmatic', + 'tags': ['borgmatic'], } From a27dc95c87f59ac29a9d73f0f819abdfa1dca4c7 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sun, 1 Feb 2026 20:15:03 -0800 Subject: [PATCH 04/12] Fix a "codec can't decode byte" error when running commands that output multi-byte unicode characters (#1258). --- NEWS | 2 ++ borgmatic/execute.py | 14 ++++++++------ tests/integration/test_execute.py | 13 +++++++++++++ 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/NEWS b/NEWS index f821f5c3..35ae1c86 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,7 @@ 2.1.2.dev0 * #1250: Fix a regression in which the "--stats" flag hides statistics at default verbosity. + * #1258: Fix a "codec can't decode byte" error when running commands that output multi-byte unicode + characters. 2.1.1 * #1241: For the "recreate" action, actually pass the "--dry-run" flag through to Borg instead of diff --git a/borgmatic/execute.py b/borgmatic/execute.py index 798f5226..e358f0d3 100644 --- a/borgmatic/execute.py +++ b/borgmatic/execute.py @@ -237,10 +237,12 @@ def read_lines(buffer, process, line_separator='\n'): call to know when to read more lines. Otherwise, the generator will busywait if it's called in a tight loop. ''' - data = '' + data = b'' + encoded_separator = line_separator.encode() + separator_size = len(encoded_separator) while True: - chunk = os.read(buffer.fileno(), READ_CHUNK_SIZE).decode() + chunk = os.read(buffer.fileno(), READ_CHUNK_SIZE) if not chunk: # EOF # The process is still running, so we keep running too. @@ -255,19 +257,19 @@ def read_lines(buffer, process, line_separator='\n'): # Split the data into lines, holding back anything leftover that might # be a partial line. while True: - separator_position = data.find(line_separator) + separator_position = data.find(encoded_separator) if separator_position == -1: break - lines.append(data[:separator_position].rstrip()) - data = data[separator_position + 1 :] + lines.append(data[:separator_position].decode()) + data = data[separator_position + separator_size :] yield tuple(lines) # Yield any leftover data from the end of the buffer. if data: - yield (data.rstrip(),) + yield (data.decode().rstrip(),) Buffer_reader = collections.namedtuple( diff --git a/tests/integration/test_execute.py b/tests/integration/test_execute.py index 4617fe5d..d1de7f44 100644 --- a/tests/integration/test_execute.py +++ b/tests/integration/test_execute.py @@ -26,6 +26,19 @@ def test_read_lines_yields_single_line_longer_than_chunk_size(): ) +def test_read_lines_yields_single_line_with_multibyte_unicode_character_spanning_chunk_boundary(): + # In case it's not clear, "ñ" is a multi-byte UTF-8 character. The "a" shifts it over one byte + # so it straddles the chunk boundary. + process = subprocess.Popen(['echo', 'aññññññññññññññññññññññññññññññ'], stdout=subprocess.PIPE) + + assert tuple(flexmock(module, READ_CHUNK_SIZE=16).read_lines(process.stdout, process)) == ( + (), + (), + (), + ('aññññññññññññññññññññññññññññññ',), + ) + + def test_read_lines_yields_multiple_lines(): process = subprocess.Popen(['echo', 'hi\nthere'], stdout=subprocess.PIPE) From dc89c9ec739d6c6fe6eb67804fba8525f4658bbb Mon Sep 17 00:00:00 2001 From: Mashrafi Rahman Date: Mon, 2 Feb 2026 18:26:38 +0800 Subject: [PATCH 05/12] Update and test the `convert_string_to_array` function. --- borgmatic/hooks/monitoring/ntfy.py | 14 +++++++++----- tests/unit/hooks/monitoring/test_ntfy.py | 8 ++++++++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/borgmatic/hooks/monitoring/ntfy.py b/borgmatic/hooks/monitoring/ntfy.py index 50b31744..e514d679 100644 --- a/borgmatic/hooks/monitoring/ntfy.py +++ b/borgmatic/hooks/monitoring/ntfy.py @@ -22,10 +22,14 @@ def initialize_monitor( ''' -def _convert_string_to_array(value): - value = str(value or '') - - return [str(item).strip() for item in value.split(',') if str(item).strip()] +def convert_string_to_array(value): + value = '' if value is None else str(value) + items = [] + for item in value.split(','): + stripped = item.strip() + if stripped: + items.append(stripped) + return items PRIORITY_NAME_TO_ID = { @@ -72,7 +76,7 @@ def ping_monitor(hook_config, config, config_filename, state, monitoring_log_lev 'title': state_config.get('title'), 'message': state_config.get('message'), 'priority': PRIORITY_NAME_TO_ID.get(state_config.get('priority'), default_priority), - 'tags': _convert_string_to_array(state_config.get('tags')), + 'tags': convert_string_to_array(state_config.get('tags')), } try: diff --git a/tests/unit/hooks/monitoring/test_ntfy.py b/tests/unit/hooks/monitoring/test_ntfy.py index 6280785a..17414ad4 100644 --- a/tests/unit/hooks/monitoring/test_ntfy.py +++ b/tests/unit/hooks/monitoring/test_ntfy.py @@ -380,3 +380,11 @@ def test_ping_monitor_with_other_error_logs_warning(): monitoring_log_level=1, dry_run=False, ) + + +def test_convert_string_to_array(): + assert module.convert_string_to_array(None) == [] + assert module.convert_string_to_array('') == [] + assert module.convert_string_to_array('foo') == ['foo'] + assert module.convert_string_to_array(' foo , bar ,baz ') == ['foo', 'bar', 'baz'] + assert module.convert_string_to_array('foo,,bar,') == ['foo', 'bar'] From acd1a8d1dd17c2c237680ea05089a6a59ea66660 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Mon, 2 Feb 2026 11:58:50 -0800 Subject: [PATCH 06/12] Update ntfy test to use @pytest.mark.parametrize (#1251) --- NEWS | 2 + borgmatic/hooks/monitoring/ntfy.py | 3 ++ tests/unit/hooks/monitoring/test_ntfy.py | 67 +++++++++++++++++++++--- 3 files changed, 64 insertions(+), 8 deletions(-) diff --git a/NEWS b/NEWS index 35ae1c86..000c35cb 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,7 @@ 2.1.2.dev0 * #1250: Fix a regression in which the "--stats" flag hides statistics at default verbosity. + * #1251: Fix a regression in the ntfy monitoring hook in which borgmatic sends tags incorrectly, + resulting in "400 Bad Request" from ntfy. * #1258: Fix a "codec can't decode byte" error when running commands that output multi-byte unicode characters. diff --git a/borgmatic/hooks/monitoring/ntfy.py b/borgmatic/hooks/monitoring/ntfy.py index e514d679..45d5e8f8 100644 --- a/borgmatic/hooks/monitoring/ntfy.py +++ b/borgmatic/hooks/monitoring/ntfy.py @@ -25,10 +25,13 @@ def initialize_monitor( def convert_string_to_array(value): value = '' if value is None else str(value) items = [] + for item in value.split(','): stripped = item.strip() + if stripped: items.append(stripped) + return items diff --git a/tests/unit/hooks/monitoring/test_ntfy.py b/tests/unit/hooks/monitoring/test_ntfy.py index 17414ad4..cfd966d4 100644 --- a/tests/unit/hooks/monitoring/test_ntfy.py +++ b/tests/unit/hooks/monitoring/test_ntfy.py @@ -1,10 +1,26 @@ from enum import Enum +import pytest from flexmock import flexmock import borgmatic.hooks.monitoring.monitor from borgmatic.hooks.monitoring import ntfy as module + +@pytest.mark.parametrize( + 'tags_input,expected_tags_output', + ( + (None, []), + ('', []), + ('foo', ['foo']), + (' foo , bar ,baz ', ['foo', 'bar', 'baz']), + ('foo,,bar,', ['foo', 'bar']), + ), +) +def test_convert_string_to_array(tags_input, expected_tags_output): + assert module.convert_string_to_array(tags_input) == expected_tags_output + + DEFAULT_BASE_URL = 'https://ntfy.sh' CUSTOM_BASE_URL = 'https://ntfy.example.com' TOPIC = 'borgmatic-unit-testing' @@ -41,6 +57,9 @@ def default_message_payload(state=Enum): def test_ping_monitor_minimal_config_hits_hosted_ntfy_on_fail(): hook_config = {'topic': TOPIC} + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -67,6 +86,9 @@ def test_ping_monitor_with_access_token_hits_hosted_ntfy_on_fail(): 'topic': TOPIC, 'access_token': 'abc123', } + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -95,6 +117,9 @@ def test_ping_monitor_with_username_password_and_access_token_ignores_username_p 'password': 'fakepassword', 'access_token': 'abc123', } + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -123,6 +148,9 @@ def test_ping_monitor_with_username_password_hits_hosted_ntfy_on_fail(): 'username': 'testuser', 'password': 'fakepassword', } + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -146,6 +174,9 @@ def test_ping_monitor_with_username_password_hits_hosted_ntfy_on_fail(): def test_ping_monitor_with_password_but_no_username_warns(): hook_config = {'topic': TOPIC, 'password': 'fakepassword'} + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -170,6 +201,9 @@ def test_ping_monitor_with_password_but_no_username_warns(): def test_ping_monitor_with_username_but_no_password_warns(): hook_config = {'topic': TOPIC, 'username': 'testuser'} + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -194,6 +228,9 @@ def test_ping_monitor_with_username_but_no_password_warns(): def test_ping_monitor_minimal_config_does_not_hit_hosted_ntfy_on_start(): hook_config = {'topic': TOPIC} + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -211,6 +248,9 @@ def test_ping_monitor_minimal_config_does_not_hit_hosted_ntfy_on_start(): def test_ping_monitor_minimal_config_does_not_hit_hosted_ntfy_on_finish(): hook_config = {'topic': TOPIC} + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -228,6 +268,9 @@ def test_ping_monitor_minimal_config_does_not_hit_hosted_ntfy_on_finish(): def test_ping_monitor_minimal_config_hits_selfhosted_ntfy_on_fail(): hook_config = {'topic': TOPIC, 'server': CUSTOM_BASE_URL} + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -251,6 +294,9 @@ def test_ping_monitor_minimal_config_hits_selfhosted_ntfy_on_fail(): def test_ping_monitor_minimal_config_does_not_hit_hosted_ntfy_on_fail_dry_run(): hook_config = {'topic': TOPIC} + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -268,6 +314,7 @@ def test_ping_monitor_minimal_config_does_not_hit_hosted_ntfy_on_fail_dry_run(): def test_ping_monitor_custom_message_hits_hosted_ntfy_on_fail(): hook_config = {'topic': TOPIC, 'fail': CUSTOM_MESSAGE_CONFIG} + flexmock(module).should_receive('convert_string_to_array').with_args('+1').and_return(['+1']) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -291,6 +338,9 @@ def test_ping_monitor_custom_message_hits_hosted_ntfy_on_fail(): def test_ping_monitor_custom_state_hits_hosted_ntfy_on_start(): hook_config = {'topic': TOPIC, 'states': ['start', 'fail']} + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -314,6 +364,9 @@ def test_ping_monitor_custom_state_hits_hosted_ntfy_on_start(): def test_ping_monitor_with_connection_error_logs_warning(): hook_config = {'topic': TOPIC} + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -338,6 +391,9 @@ def test_ping_monitor_with_connection_error_logs_warning(): def test_ping_monitor_with_credential_error_logs_warning(): hook_config = {'topic': TOPIC} + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).and_raise(ValueError) @@ -356,6 +412,9 @@ def test_ping_monitor_with_credential_error_logs_warning(): def test_ping_monitor_with_other_error_logs_warning(): hook_config = {'topic': TOPIC} + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -380,11 +439,3 @@ def test_ping_monitor_with_other_error_logs_warning(): monitoring_log_level=1, dry_run=False, ) - - -def test_convert_string_to_array(): - assert module.convert_string_to_array(None) == [] - assert module.convert_string_to_array('') == [] - assert module.convert_string_to_array('foo') == ['foo'] - assert module.convert_string_to_array(' foo , bar ,baz ') == ['foo', 'bar', 'baz'] - assert module.convert_string_to_array('foo,,bar,') == ['foo', 'bar'] From c64c79ad0e62681f54f51f861daa7eb465f1bcb5 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 3 Feb 2026 10:12:15 -0800 Subject: [PATCH 07/12] Fix for SSH warnings from Borg showing up as JSON logs even without the "--log-json" flag (#1260). --- NEWS | 1 + borgmatic/execute.py | 27 ++++-- tests/unit/test_execute.py | 165 ++++++++++++++++++++++++------------- 3 files changed, 128 insertions(+), 65 deletions(-) diff --git a/NEWS b/NEWS index 000c35cb..287602ec 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,7 @@ resulting in "400 Bad Request" from ntfy. * #1258: Fix a "codec can't decode byte" error when running commands that output multi-byte unicode characters. + * #1260: Fix for SSH warnings from Borg showing up as JSON logs even without the "--log-json" flag. 2.1.1 * #1241: For the "recreate" action, actually pass the "--dry-run" flag through to Borg instead of diff --git a/borgmatic/execute.py b/borgmatic/execute.py index e358f0d3..5fde85c7 100644 --- a/borgmatic/execute.py +++ b/borgmatic/execute.py @@ -29,6 +29,19 @@ class Exit_status(enum.Enum): ERROR = 4 +def command_is_borg(command, borg_local_path): + ''' + Given a command as a sequence and the Borg local path, return whether that command is a call to + Borg. + ''' + parsed_command = command.split(' ', 1) if isinstance(command, str) else command + + if not parsed_command: + return False + + return bool(borg_local_path and parsed_command[0] == borg_local_path) + + def interpret_exit_code(command, exit_code, borg_local_path=None, borg_exit_codes=None): # noqa: PLR0911 ''' Return an Exit_status value (e.g. SUCCESS, ERROR, or WARNING) based on interpreting the given @@ -42,9 +55,7 @@ def interpret_exit_code(command, exit_code, borg_local_path=None, borg_exit_code if exit_code == 0: return Exit_status.SUCCESS - parsed_command = command.split(' ', 1) if isinstance(command, str) else command - - if borg_local_path and parsed_command[0] == borg_local_path: + if command_is_borg(command, borg_local_path): # First try looking for the exit code in the borg_exit_codes configuration. for entry in borg_exit_codes or (): if entry.get('code') == exit_code: @@ -124,7 +135,7 @@ def borg_json_log_line_to_record(line, log_level): if log_type == 'log_message': borg_log_level = logging._nameToLevel.get(log_data.get('levelname')) - log_level_delta = log_level - borg_log_level + log_level_delta = 0 if log_level is None else log_level - borg_log_level if log_level_delta > 0 and log_level_delta < BORG_LOG_LEVEL_ELEVATION_THRESHOLD: return logging.makeLogRecord( @@ -188,9 +199,7 @@ def parse_log_line(line, log_level, elevate_stderr, borg_local_path, command): came from stderr and the string "warning:" appears at the start of the log line. In that case, just elevate the log level to a WARN. ''' - parsed_command = command.split(' ', 1) if isinstance(command, str) else command - - if borg_local_path and parsed_command[0] == borg_local_path: + if command_is_borg(command, borg_local_path): log_record = borg_json_log_line_to_record(line, log_level) if log_record: @@ -636,7 +645,9 @@ def execute_command_and_capture_output( command, stdin=input_file, stdout=subprocess.PIPE, - stderr=subprocess.PIPE if capture_stderr else None, + stderr=subprocess.PIPE + if capture_stderr or command_is_borg(command, borg_local_path) + else None, shell=shell, env=environment, cwd=working_directory, diff --git a/tests/unit/test_execute.py b/tests/unit/test_execute.py index afc9942c..4807002a 100644 --- a/tests/unit/test_execute.py +++ b/tests/unit/test_execute.py @@ -7,72 +7,80 @@ from borgmatic import execute as module @pytest.mark.parametrize( - 'command,exit_code,borg_local_path,borg_exit_codes,expected_result', + 'command,borg_local_path,expected_result', ( - (['grep'], 2, None, None, module.Exit_status.ERROR), - (['grep'], 2, 'borg', None, module.Exit_status.ERROR), - (['borg'], 2, 'borg', None, module.Exit_status.ERROR), - (['borg1'], 2, 'borg1', None, module.Exit_status.ERROR), - (['grep'], 1, None, None, module.Exit_status.ERROR), - (['grep'], 1, 'borg', None, module.Exit_status.ERROR), - (['borg'], 1, 'borg', None, module.Exit_status.WARNING), - (['borg1'], 1, 'borg1', None, module.Exit_status.WARNING), - (['grep'], 100, None, None, module.Exit_status.ERROR), - (['grep'], 100, 'borg', None, module.Exit_status.ERROR), - ('grep', 2, None, None, module.Exit_status.ERROR), - ('borg', 2, 'borg', None, module.Exit_status.ERROR), - (['borg'], 100, 'borg', None, module.Exit_status.WARNING), - (['borg1'], 100, 'borg1', None, module.Exit_status.WARNING), - ('borg', 100, 'borg', None, module.Exit_status.WARNING), - ('borg1', 100, 'borg1', None, module.Exit_status.WARNING), - (['grep'], 0, None, None, module.Exit_status.SUCCESS), - (['grep'], 0, 'borg', None, module.Exit_status.SUCCESS), - (['borg'], 0, 'borg', None, module.Exit_status.SUCCESS), - (['borg1'], 0, 'borg1', None, module.Exit_status.SUCCESS), - ('grep', 0, None, None, module.Exit_status.SUCCESS), - ('grep', 0, 'borg', None, module.Exit_status.SUCCESS), + (['foo', 'bar'], 'borg', False), + (['borg', 'list'], 'borg', True), + (['borg1', 'list'], 'borg', False), + (['borg', 'list'], 'borg1', False), + ([], 'borg', False), + ('foo bar', 'borg', False), + ('borg list', 'borg', True), + ('', 'borg', False), + ), +) +def test_command_is_borg_matches_local_path_to_command(command, borg_local_path, expected_result): + assert module.command_is_borg(command, borg_local_path) == expected_result + + +@pytest.mark.parametrize( + 'command_is_borg,exit_code,borg_exit_codes,expected_result', + ( + (False, 2, None, module.Exit_status.ERROR), + (True, 2, None, module.Exit_status.ERROR), + (False, 1, None, module.Exit_status.ERROR), + (True, 1, None, module.Exit_status.WARNING), + (False, 100, None, module.Exit_status.ERROR), + (False, 2, None, module.Exit_status.ERROR), + (True, 2, None, module.Exit_status.ERROR), + (True, 100, None, module.Exit_status.WARNING), + (False, 0, None, module.Exit_status.SUCCESS), + (True, 0, None, module.Exit_status.SUCCESS), # -9 exit code occurs when child process get SIGKILLed. - (['grep'], -9, None, None, module.Exit_status.ERROR), - (['grep'], -9, 'borg', None, module.Exit_status.ERROR), - (['borg'], -9, 'borg', None, module.Exit_status.ERROR), - (['borg1'], -9, 'borg1', None, module.Exit_status.ERROR), - (['borg'], None, None, None, module.Exit_status.STILL_RUNNING), - (['borg'], 1, 'borg', [], module.Exit_status.WARNING), - (['borg'], 1, 'borg', [{}], module.Exit_status.WARNING), - (['borg'], 1, 'borg', [{'code': 1}], module.Exit_status.WARNING), - (['grep'], 1, 'borg', [{'code': 100, 'treat_as': 'error'}], module.Exit_status.ERROR), - (['borg'], 1, 'borg', [{'code': 100, 'treat_as': 'error'}], module.Exit_status.WARNING), - (['borg'], 1, 'borg', [{'code': 1, 'treat_as': 'error'}], module.Exit_status.ERROR), - (['borg'], 2, 'borg', [{'code': 99, 'treat_as': 'warning'}], module.Exit_status.ERROR), - (['borg'], 2, 'borg', [{'code': 2, 'treat_as': 'warning'}], module.Exit_status.WARNING), - (['borg'], 100, 'borg', [{'code': 1, 'treat_as': 'error'}], module.Exit_status.WARNING), - (['borg'], 100, 'borg', [], module.Exit_status.WARNING), - (['borg'], 100, 'borg', [{'code': 100, 'treat_as': 'error'}], module.Exit_status.ERROR), - (['borg'], 101, 'borg', [], module.Exit_status.ERROR), - (['borg'], 101, 'borg', [{'code': 101, 'treat_as': 'warning'}], module.Exit_status.WARNING), - (['borg'], 102, 'borg', [], module.Exit_status.ERROR), - (['borg'], 102, 'borg', [{'code': 102, 'treat_as': 'warning'}], module.Exit_status.WARNING), - (['borg'], 103, 'borg', [], module.Exit_status.WARNING), - (['borg'], 103, 'borg', [{'code': 103, 'treat_as': 'error'}], module.Exit_status.ERROR), - (['borg'], 104, 'borg', [], module.Exit_status.ERROR), - (['borg'], 104, 'borg', [{'code': 104, 'treat_as': 'warning'}], module.Exit_status.WARNING), - (['borg'], 105, 'borg', [], module.Exit_status.ERROR), - (['borg'], 105, 'borg', [{'code': 105, 'treat_as': 'warning'}], module.Exit_status.WARNING), - (['borg'], 106, 'borg', [], module.Exit_status.ERROR), - (['borg'], 106, 'borg', [{'code': 106, 'treat_as': 'warning'}], module.Exit_status.WARNING), - (['borg'], 107, 'borg', [], module.Exit_status.ERROR), - (['borg'], 107, 'borg', [{'code': 107, 'treat_as': 'warning'}], module.Exit_status.WARNING), + (False, -9, None, module.Exit_status.ERROR), + (True, -9, None, module.Exit_status.ERROR), + (True, None, None, module.Exit_status.STILL_RUNNING), + (True, 1, [], module.Exit_status.WARNING), + (True, 1, [{'code': 1}], module.Exit_status.WARNING), + (False, 1, [{'code': 100, 'treat_as': 'error'}], module.Exit_status.ERROR), + (True, 1, [{'code': 100, 'treat_as': 'error'}], module.Exit_status.WARNING), + (True, 1, [{'code': 1, 'treat_as': 'error'}], module.Exit_status.ERROR), + (True, 2, [{'code': 99, 'treat_as': 'warning'}], module.Exit_status.ERROR), + (True, 2, [{'code': 2, 'treat_as': 'warning'}], module.Exit_status.WARNING), + (True, 100, [{'code': 1, 'treat_as': 'error'}], module.Exit_status.WARNING), + (True, 100, [], module.Exit_status.WARNING), + (True, 100, [{'code': 100, 'treat_as': 'error'}], module.Exit_status.ERROR), + (True, 101, [], module.Exit_status.ERROR), + (True, 101, [{'code': 101, 'treat_as': 'warning'}], module.Exit_status.WARNING), + (True, 102, [], module.Exit_status.ERROR), + (True, 102, [{'code': 102, 'treat_as': 'warning'}], module.Exit_status.WARNING), + (True, 103, [], module.Exit_status.WARNING), + (True, 103, [{'code': 103, 'treat_as': 'error'}], module.Exit_status.ERROR), + (True, 104, [], module.Exit_status.ERROR), + (True, 104, [{'code': 104, 'treat_as': 'warning'}], module.Exit_status.WARNING), + (True, 105, [], module.Exit_status.ERROR), + (True, 105, [{'code': 105, 'treat_as': 'warning'}], module.Exit_status.WARNING), + (True, 106, [], module.Exit_status.ERROR), + (True, 106, [{'code': 106, 'treat_as': 'warning'}], module.Exit_status.WARNING), + (True, 107, [], module.Exit_status.ERROR), + (True, 107, [{'code': 107, 'treat_as': 'warning'}], module.Exit_status.WARNING), ), ) def test_interpret_exit_code_respects_exit_code_and_borg_local_path( - command, + command_is_borg, exit_code, - borg_local_path, borg_exit_codes, expected_result, ): + flexmock(module).should_receive('command_is_borg').and_return(command_is_borg) + assert ( - module.interpret_exit_code(command, exit_code, borg_local_path, borg_exit_codes) + module.interpret_exit_code( + command=flexmock(), + exit_code=exit_code, + borg_local_path=flexmock(), + borg_exit_codes=borg_exit_codes, + ) is expected_result ) @@ -146,6 +154,18 @@ def test_borg_json_log_line_to_record_does_not_elevate_log_message_info_level_to assert record.name == 'borg.something' +def test_borg_json_log_line_with_none_log_level_parses_log_message_line(): + line = '{"type": "log_message", "levelname": "INFO", "time": 12345, "message": "All done", "name": "borg.something"}' + + record = module.borg_json_log_line_to_record(line, None) + + assert record.levelno == module.logging.INFO + assert record.created == 12345 + assert record.msg == 'All done' + assert record.levelname == 'INFO' + assert record.name == 'borg.something' + + def test_borg_json_log_line_to_record_parses_file_status_line(): flexmock(module.time).should_receive('time').and_return(12345) line = '{"type": "file_status", "status": "-", "path": "/foo/bar"}' @@ -1404,6 +1424,7 @@ def test_execute_command_without_run_to_completion_returns_process(): def test_execute_command_and_capture_output_returns_stdout(): full_command = ['foo', 'bar'] flexmock(module).should_receive('log_command') + flexmock(module).should_receive('command_is_borg').and_return(False) process = flexmock() flexmock(module.subprocess).should_receive('Popen').with_args( full_command, @@ -1423,8 +1444,10 @@ def test_execute_command_and_capture_output_returns_stdout(): assert output_lines == ('out',) -def test_execute_command_and_capture_output_with_capture_stderr_returns_stderr(): +def test_execute_command_and_capture_output_with_capture_stderr_popens_stderr(): full_command = ['foo', 'bar'] + flexmock(module).should_receive('log_command') + flexmock(module).should_receive('command_is_borg').and_return(False) process = flexmock() flexmock(module.subprocess).should_receive('Popen').with_args( full_command, @@ -1446,10 +1469,34 @@ def test_execute_command_and_capture_output_with_capture_stderr_returns_stderr() assert output_lines == ('out',) +def test_execute_command_and_capture_output_with_borg_command_popens_stderr(): + full_command = ['borg', 'list'] + flexmock(module).should_receive('log_command') + flexmock(module).should_receive('command_is_borg').and_return(True) + process = flexmock() + flexmock(module.subprocess).should_receive('Popen').with_args( + full_command, + stdin=None, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + shell=False, + env=None, + cwd=None, + close_fds=False, + ).and_return(process).once() + flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) + flexmock(module).should_receive('log_outputs').and_yield('out') + + output_lines = tuple(module.execute_command_and_capture_output(full_command)) + + assert output_lines == ('out',) + + def test_execute_command_and_capture_output_returns_output_when_process_error_is_not_considered_an_error(): full_command = ['foo', 'bar'] err_output = b'[]' flexmock(module).should_receive('log_command') + flexmock(module).should_receive('command_is_borg').and_return(False) flexmock(module.subprocess).should_receive('Popen').with_args( full_command, stdin=None, @@ -1472,6 +1519,7 @@ def test_execute_command_and_capture_output_returns_output_when_process_error_is def test_execute_command_and_capture_output_raises_when_command_errors(): full_command = ['foo', 'bar'] flexmock(module).should_receive('log_command') + flexmock(module).should_receive('command_is_borg').and_return(False) flexmock(module.subprocess).should_receive('Popen').with_args( full_command, stdin=None, @@ -1493,6 +1541,7 @@ def test_execute_command_and_capture_output_raises_when_command_errors(): def test_execute_command_and_capture_output_with_shell_returns_output(): full_command = ['foo', 'bar'] flexmock(module).should_receive('log_command') + flexmock(module).should_receive('command_is_borg').and_return(False) process = flexmock() flexmock(module.subprocess).should_receive('Popen').with_args( 'foo bar', @@ -1515,6 +1564,7 @@ def test_execute_command_and_capture_output_with_shell_returns_output(): def test_execute_command_and_capture_output_with_enviroment_returns_output(): full_command = ['foo', 'bar'] flexmock(module).should_receive('log_command') + flexmock(module).should_receive('command_is_borg').and_return(False) process = flexmock() flexmock(module.subprocess).should_receive('Popen').with_args( full_command, @@ -1543,6 +1593,7 @@ def test_execute_command_and_capture_output_with_enviroment_returns_output(): def test_execute_command_and_capture_output_returns_output_with_working_directory(): full_command = ['foo', 'bar'] flexmock(module).should_receive('log_command') + flexmock(module).should_receive('command_is_borg').and_return(False) process = flexmock() flexmock(module.subprocess).should_receive('Popen').with_args( full_command, From 730a4b2f18bf97ad26b81c4a791420a834773356 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 5 Feb 2026 10:19:06 -0800 Subject: [PATCH 08/12] Work around Borg returning a warning exit code when a repository/archive check fails (#1252). --- NEWS | 2 ++ borgmatic/borg/check.py | 4 +++- tests/unit/borg/test_check.py | 20 ++++++++++---------- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/NEWS b/NEWS index 287602ec..709ae6c8 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,8 @@ * #1258: Fix a "codec can't decode byte" error when running commands that output multi-byte unicode characters. * #1260: Fix for SSH warnings from Borg showing up as JSON logs even without the "--log-json" flag. + * #1252: Work around Borg returning a warning exit code when a repository/archive check fails. Now, + borgmatic interprets such failures as errors. 2.1.1 * #1241: For the "recreate" action, actually pass the "--dry-run" flag through to Borg instead of diff --git a/borgmatic/borg/check.py b/borgmatic/borg/check.py index ef160417..a74c9b5c 100644 --- a/borgmatic/borg/check.py +++ b/borgmatic/borg/check.py @@ -149,8 +149,10 @@ def check_archives( max_duration = check_arguments.max_duration or repository_check_config.get('max_duration') + # If not configured, elevate Borg's exit code 1 (an ostensible warning) to error, because Borg + # returns exit code 1 for repository check errors! + borg_exit_codes = config.get('borg_exit_codes', []) + [{'code': 1, 'treat_as': 'error'}] umask = config.get('umask') - borg_exit_codes = config.get('borg_exit_codes') working_directory = borgmatic.config.paths.get_working_directory(config) if 'data' in checks: diff --git a/tests/unit/borg/test_check.py b/tests/unit/borg/test_check.py index fb4acc32..d68c89e0 100644 --- a/tests/unit/borg/test_check.py +++ b/tests/unit/borg/test_check.py @@ -24,7 +24,7 @@ def insert_execute_command_mock( environment=None, working_directory=working_directory, borg_local_path=command[0], - borg_exit_codes=borg_exit_codes, + borg_exit_codes=(borg_exit_codes or []) + [{'code': 1, 'treat_as': 'error'}], ).once() @@ -335,7 +335,7 @@ def test_check_archives_with_progress_passes_through_to_borg(): environment=None, working_directory=None, borg_local_path='borg', - borg_exit_codes=None, + borg_exit_codes=[{'code': 1, 'treat_as': 'error'}], ).once() module.check_archives( @@ -371,7 +371,7 @@ def test_check_archives_with_log_json_and_progress_passes_through_both_to_borg() environment=None, working_directory=None, borg_local_path='borg', - borg_exit_codes=None, + borg_exit_codes=[{'code': 1, 'treat_as': 'error'}], ).once() module.check_archives( @@ -407,7 +407,7 @@ def test_check_archives_with_repair_passes_through_to_borg(): environment=None, working_directory=None, borg_local_path='borg', - borg_exit_codes=None, + borg_exit_codes=[{'code': 1, 'treat_as': 'error'}], ).once() module.check_archives( @@ -443,7 +443,7 @@ def test_check_archives_with_log_json_and_repair_passes_through_both_to_borg(): environment=None, working_directory=None, borg_local_path='borg', - borg_exit_codes=None, + borg_exit_codes=[{'code': 1, 'treat_as': 'error'}], ).once() module.check_archives( @@ -479,7 +479,7 @@ def test_check_archives_with_max_duration_flag_passes_through_to_borg(): environment=None, working_directory=None, borg_local_path='borg', - borg_exit_codes=None, + borg_exit_codes=[{'code': 1, 'treat_as': 'error'}], ).once() module.check_archives( @@ -515,7 +515,7 @@ def test_check_archives_with_max_duration_option_passes_through_to_borg(): environment=None, working_directory=None, borg_local_path='borg', - borg_exit_codes=None, + borg_exit_codes=[{'code': 1, 'treat_as': 'error'}], ).once() module.check_archives( @@ -689,7 +689,7 @@ def test_check_archives_with_max_duration_flag_overrides_max_duration_option(): environment=None, working_directory=None, borg_local_path='borg', - borg_exit_codes=None, + borg_exit_codes=[{'code': 1, 'treat_as': 'error'}], ).once() module.check_archives( @@ -854,7 +854,7 @@ def test_check_archives_with_local_path_calls_borg_via_local_path(): def test_check_archives_with_exit_codes_calls_borg_using_them(): checks = {'repository'} - borg_exit_codes = flexmock() + borg_exit_codes = [{'code': 101, 'treat_as': 'error'}] config = {'borg_exit_codes': borg_exit_codes} flexmock(module).should_receive('make_check_name_flags').with_args(checks, ()).and_return(()) flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) @@ -1026,7 +1026,7 @@ def test_check_archives_with_match_archives_passes_through_to_borg(): environment=None, working_directory=None, borg_local_path='borg', - borg_exit_codes=None, + borg_exit_codes=[{'code': 1, 'treat_as': 'error'}], ).once() module.check_archives( From 957f6be4a270510e313bed28f8deab3fdcf72bbb Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 5 Feb 2026 10:38:07 -0800 Subject: [PATCH 09/12] Switch to iterable unpacking (#1252). --- borgmatic/borg/check.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/borgmatic/borg/check.py b/borgmatic/borg/check.py index a74c9b5c..64ea23cd 100644 --- a/borgmatic/borg/check.py +++ b/borgmatic/borg/check.py @@ -151,7 +151,7 @@ def check_archives( # If not configured, elevate Borg's exit code 1 (an ostensible warning) to error, because Borg # returns exit code 1 for repository check errors! - borg_exit_codes = config.get('borg_exit_codes', []) + [{'code': 1, 'treat_as': 'error'}] + borg_exit_codes = [*config.get('borg_exit_codes', []), *[{'code': 1, 'treat_as': 'error'}]] umask = config.get('umask') working_directory = borgmatic.config.paths.get_working_directory(config) From ea05a4660cdf0e24dfd9e4f66c8737969aad7e10 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 5 Feb 2026 12:50:39 -0800 Subject: [PATCH 10/12] Deduplicate overlapping source directories and patterns so they don't throw off "spot" check file counts and cause spurious check failures. --- NEWS | 2 ++ borgmatic/actions/check.py | 8 ++++- tests/unit/actions/test_check.py | 59 ++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 709ae6c8..36aefa4d 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,8 @@ * #1260: Fix for SSH warnings from Borg showing up as JSON logs even without the "--log-json" flag. * #1252: Work around Borg returning a warning exit code when a repository/archive check fails. Now, borgmatic interprets such failures as errors. + * Deduplicate overlapping source directories and patterns so they don't throw off "spot" check file + counts and cause spurious check failures. 2.1.1 * #1241: For the "recreate" action, actually pass the "--dry-run" flag through to Borg instead of diff --git a/borgmatic/actions/check.py b/borgmatic/actions/check.py index e6390598..1cd5e38a 100644 --- a/borgmatic/actions/check.py +++ b/borgmatic/actions/check.py @@ -418,7 +418,13 @@ def collect_spot_check_source_paths( ) return tuple( - path for path in paths if os.path.isfile(os.path.join(working_directory or '', path)) + # Use dict.fromkeys() to deduplicate file paths, which are present in Borg's dry run output + # when there are overlapping source patterns. For instance, if both "/foo" and + # "/foo/file.txt" are in configured patterns, then "/foo/file.txt" will show up in Borg's + # dry run output twice. + dict.fromkeys( + path for path in paths if os.path.isfile(os.path.join(working_directory or '', path)) + ) ) diff --git a/tests/unit/actions/test_check.py b/tests/unit/actions/test_check.py index 84111ac8..7f5f8619 100644 --- a/tests/unit/actions/test_check.py +++ b/tests/unit/actions/test_check.py @@ -986,6 +986,65 @@ def test_collect_spot_check_source_paths_uses_working_directory(): ) == ('foo', 'bar') +def test_collect_spot_check_source_paths_deduplicates_borg_output_paths(): + flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hooks').and_return( + {'hook1': False, 'hook2': True}, + ) + flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return( + flexmock(), + ) + flexmock(module.borgmatic.actions.pattern).should_receive('collect_patterns').and_return( + (Pattern('collected'),), + ) + flexmock(module.borgmatic.actions.pattern).should_receive('process_patterns').with_args( + ( + Pattern('collected', source=module.borgmatic.borg.pattern.Pattern_source.HOOK), + Pattern('extra.yaml', source=module.borgmatic.borg.pattern.Pattern_source.INTERNAL), + ), + config=object, + working_directory=None, + ).and_return( + [Pattern('foo'), Pattern('bar')], + ) + flexmock(module.borgmatic.borg.create).should_receive('make_base_create_command').with_args( + dry_run=True, + repository_path='repo', + config=object, + patterns=[Pattern('foo'), Pattern('bar')], + local_borg_version=object, + global_arguments=object, + borgmatic_runtime_directory='/run/borgmatic', + local_path=object, + remote_path=object, + stream_processes=True, + ).and_return((('borg', 'create'), ('repo::archive',), flexmock())) + flexmock(module.borgmatic.borg.environment).should_receive('make_environment').and_return( + flexmock(), + ) + flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None) + flexmock(module.borgmatic.execute).should_receive( + 'execute_command_and_capture_output', + ).and_yield( + 'warning: stuff', + '- /etc/path', + '+ /etc/other', + '? /nope', + '- /etc/path', + ) + flexmock(module.os.path).should_receive('isfile').and_return(True) + + assert module.collect_spot_check_source_paths( + repository={'path': 'repo'}, + config={'working_directory': '/'}, + local_borg_version=flexmock(), + global_arguments=flexmock(), + local_path=flexmock(), + remote_path=flexmock(), + borgmatic_runtime_directory='/run/borgmatic', + bootstrap_config_paths=('extra.yaml',), + ) == ('/etc/path', '/etc/other') + + def test_compare_spot_check_hashes_returns_paths_having_failing_hashes(): flexmock(module.random).should_receive('SystemRandom').and_return( flexmock(sample=lambda population, count: population[:count]), From fc7439af3a8e247ae9cf4b4f7dfc696fbbcda617 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 5 Feb 2026 23:11:59 -0800 Subject: [PATCH 11/12] If a source file is deleted during a "spot" check, consider the file as non-matching and move on instead of immediately failing the entire (#1231). --- NEWS | 2 + borgmatic/actions/check.py | 94 +++++++++++++++++++++----------- tests/unit/actions/test_check.py | 53 ++++++++++++++++++ 3 files changed, 117 insertions(+), 32 deletions(-) diff --git a/NEWS b/NEWS index 36aefa4d..25ea8800 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ 2.1.2.dev0 + * #1231: If a source file is deleted during a "spot" check, consider the file as non-matching + and move on instead of immediately failing the entire check. * #1250: Fix a regression in which the "--stats" flag hides statistics at default verbosity. * #1251: Fix a regression in the ntfy monitoring hook in which borgmatic sends tags incorrectly, resulting in "400 Bad Request" from ntfy. diff --git a/borgmatic/actions/check.py b/borgmatic/actions/check.py index 1cd5e38a..ee0a918e 100644 --- a/borgmatic/actions/check.py +++ b/borgmatic/actions/check.py @@ -9,6 +9,7 @@ import pathlib import random import shlex import shutil +import subprocess import textwrap import borgmatic.actions.config.bootstrap @@ -530,41 +531,70 @@ def compare_spot_check_hashes( hash_paths = tuple( path for path in source_sample_paths_subset if path in hashable_source_sample_path ) - hash_lines = borgmatic.execute.execute_command_and_capture_output( - tuple( - shlex.quote(part) - for part in shlex.split(spot_check_config.get('xxh64sum_command', 'xxh64sum')) - ) - + hash_paths, - working_directory=working_directory, - ) - source_hashes.update( - **dict( - zip( - # xxh64sum rewrites/escapes the paths that it returns alongside its hashes, for - # instance if they contain special characters. When that happens, they don't - # match the original source paths and therefore hash lookups fail. So when - # building this lookup dict, use the original unaltered paths we provided as - # input to xxh64sum. - hash_paths, - ( - # For some reason, xxh64sum prefixes the hash with a backslash if the path - # contains a newline. Work around that. - line.split(' ', 1)[0].lstrip('\\') - for line in hash_lines + try: + hash_lines = borgmatic.execute.execute_command_and_capture_output( + tuple( + shlex.quote(part) + for part in shlex.split(spot_check_config.get('xxh64sum_command', 'xxh64sum')) + ) + + hash_paths, + working_directory=working_directory, + ) + source_hashes.update( + **dict( + zip( + # xxh64sum rewrites/escapes the paths that it returns alongside its hashes, for + # instance if they contain special characters. When that happens, they don't + # match the original source paths and therefore hash lookups fail. So when + # building this lookup dict, use the original unaltered paths we provided as + # input to xxh64sum. + hash_paths, + ( + # For some reason, xxh64sum prefixes the hash with a backslash if the path + # contains a newline. Work around that. + line.split(' ', 1)[0].lstrip('\\') + for line in hash_lines + ), ), + # Represent non-existent files as having empty hashes so the comparison below still + # works. Same thing for filesystem links, since Borg produces empty archive hashes + # for them. + **{ + path: '' + for path in source_sample_paths_subset + if path not in hashable_source_sample_path + }, ), - # Represent non-existent files as having empty hashes so the comparison below still - # works. Same thing for filesystem links, since Borg produces empty archive hashes - # for them. - **{ - path: '' - for path in source_sample_paths_subset - if path not in hashable_source_sample_path - }, - ), - ) + ) + except subprocess.CalledProcessError: + # This can happen if a file we planned to hash gets deleted right before we try to hash + # it. Falling back to individual file hashing allows us to find and mark just the + # file(s) with problems instead of failing the whole batch. + logger.warning( + 'Bulk source path hashing failed for this batch; falling back to individual file hashing' + ) + + for hash_path in hash_paths: + try: + hash_lines = borgmatic.execute.execute_command_and_capture_output( + ( + *( + shlex.quote(part) + for part in shlex.split( + spot_check_config.get('xxh64sum_command', 'xxh64sum') + ) + ), + hash_path, + ), + working_directory=working_directory, + ) + source_hashes[hash_path] = next(hash_lines).split(' ', 1)[0].lstrip('\\') + except (subprocess.CalledProcessError, StopIteration): # noqa: PERF203 + logger.warning( + f'Source path hashing failed for {hash_path}; treating as missing' + ) + source_hashes[hash_path] = '' # Get the hash for each file in the archive. archive_hashes.update( diff --git a/tests/unit/actions/test_check.py b/tests/unit/actions/test_check.py index 7f5f8619..8aa6552f 100644 --- a/tests/unit/actions/test_check.py +++ b/tests/unit/actions/test_check.py @@ -1174,6 +1174,59 @@ def test_compare_spot_check_hashes_handles_incorrect_path_names_from_xxh64sum(): ) == ('/bar',) +def test_compare_spot_check_hashes_with_xxh64sum_failure_falls_back_to_individual_file_hashing(): + flexmock(module.random).should_receive('SystemRandom').and_return( + flexmock(sample=lambda population, count: population[:count]), + ) + flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return( + None, + ) + flexmock(module.os.path).should_receive('exists').and_return(True) + flexmock(module.os.path).should_receive('islink').and_return(False) + flexmock(module.borgmatic.execute).should_receive( + 'execute_command_and_capture_output', + ).with_args(('xxh64sum', '/foo', '/bar'), working_directory=None).and_raise( + module.subprocess.CalledProcessError(1, 'wtf') + ) + flexmock(module.borgmatic.execute).should_receive( + 'execute_command_and_capture_output', + ).with_args(('xxh64sum', '/foo'), working_directory=None).and_raise( + module.subprocess.CalledProcessError(1, 'wtf') + ).once() + flexmock(module.borgmatic.execute).should_receive( + 'execute_command_and_capture_output', + ).with_args(('xxh64sum', '/bar'), working_directory=None).and_yield( + 'hash2 /bar', + ).once() + + flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( + {'xxh64': 'hash1', 'path': 'foo'}, + {'xxh64': 'hash2', 'path': 'bar'}, + ) + + assert module.compare_spot_check_hashes( + repository={'path': 'repo'}, + archive='archive', + config={ + 'checks': [ + { + 'name': 'archives', + 'frequency': '2 weeks', + }, + { + 'name': 'spot', + 'data_sample_percentage': 50, + }, + ], + }, + local_borg_version=flexmock(), + global_arguments=flexmock(), + local_path=flexmock(), + remote_path=flexmock(), + source_paths=('/foo', '/bar', '/baz', '/quux'), + ) == ('/foo',) + + def test_compare_spot_check_hashes_returns_relative_paths_having_failing_hashes(): flexmock(module.random).should_receive('SystemRandom').and_return( flexmock(sample=lambda population, count: population[:count]), From ad8d074effdedf6e1807a5e210b93b6b0a28ba81 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 6 Feb 2026 12:53:27 -0800 Subject: [PATCH 12/12] Bump version for release. --- NEWS | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 25ea8800..51724748 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,4 @@ -2.1.2.dev0 +2.1.2 * #1231: If a source file is deleted during a "spot" check, consider the file as non-matching and move on instead of immediately failing the entire check. * #1250: Fix a regression in which the "--stats" flag hides statistics at default verbosity. diff --git a/pyproject.toml b/pyproject.toml index 3f2596e2..8c0274a2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "borgmatic" -version = "2.1.2dev0" +version = "2.1.2" authors = [ { name="Dan Helfman", email="witten@torsion.org" }, ]