From fc7439af3a8e247ae9cf4b4f7dfc696fbbcda617 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 5 Feb 2026 23:11:59 -0800 Subject: [PATCH] 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]),