diff --git a/NEWS b/NEWS index 80385bed..64c5b2f7 100644 --- a/NEWS +++ b/NEWS @@ -31,6 +31,7 @@ character. * #1048: Fix a "no such file or directory" error in ZFS, Btrfs, and LVM hooks with nested directories that reside on separate devices/filesystems. + * #1050: Fix a failure in the "spot" check when the archive contains a symlink. 1.9.14 * #409: With the PagerDuty monitoring hook, send borgmatic logs to PagerDuty so they show up in the diff --git a/borgmatic/actions/check.py b/borgmatic/actions/check.py index afcc9262..1e65eecb 100644 --- a/borgmatic/actions/check.py +++ b/borgmatic/actions/check.py @@ -482,10 +482,12 @@ def compare_spot_check_hashes( ) source_sample_paths = tuple(random.sample(source_paths, sample_count)) working_directory = borgmatic.config.paths.get_working_directory(config) - existing_source_sample_paths = { + hashable_source_sample_path = { source_path for source_path in source_sample_paths - if os.path.exists(os.path.join(working_directory or '', source_path)) + for full_source_path in (os.path.join(working_directory or '', source_path),) + if os.path.exists(full_source_path) + if not os.path.islink(full_source_path) } logger.debug( f'Sampling {sample_count} source paths (~{spot_check_config["data_sample_percentage"]}%) for spot check' @@ -508,7 +510,7 @@ def compare_spot_check_hashes( hash_output = borgmatic.execute.execute_command_and_capture_output( (spot_check_config.get('xxh64sum_command', 'xxh64sum'),) + tuple( - path for path in source_sample_paths_subset if path in existing_source_sample_paths + path for path in source_sample_paths_subset if path in hashable_source_sample_path ), working_directory=working_directory, ) @@ -516,11 +518,13 @@ def compare_spot_check_hashes( source_hashes.update( **dict( (reversed(line.split(' ', 1)) for line in hash_output.splitlines()), - # Represent non-existent files as having empty hashes so the comparison below still works. + # 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 existing_source_sample_paths + if path not in hashable_source_sample_path }, ) ) diff --git a/tests/unit/actions/test_check.py b/tests/unit/actions/test_check.py index 069fae1e..e7eee5c4 100644 --- a/tests/unit/actions/test_check.py +++ b/tests/unit/actions/test_check.py @@ -898,6 +898,7 @@ def test_compare_spot_check_hashes_returns_paths_having_failing_hashes(): 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_return( @@ -938,6 +939,7 @@ def test_compare_spot_check_hashes_returns_relative_paths_having_failing_hashes( 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_return( @@ -978,6 +980,7 @@ def test_compare_spot_check_hashes_handles_data_sample_percentage_above_100(): 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_return( @@ -1018,6 +1021,7 @@ def test_compare_spot_check_hashes_uses_xxh64sum_command_option(): 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(('/usr/local/bin/xxh64sum', '/foo', '/bar'), working_directory=None).and_return( @@ -1055,6 +1059,7 @@ def test_compare_spot_check_hashes_considers_path_missing_from_archive_as_not_ma 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_return( @@ -1083,6 +1088,42 @@ def test_compare_spot_check_hashes_considers_path_missing_from_archive_as_not_ma ) == ('/bar',) +def test_compare_spot_check_hashes_considers_symlink_path_as_not_matching(): + flexmock(module.random).should_receive('sample').replace_with( + 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').with_args('/foo').and_return(False) + flexmock(module.os.path).should_receive('islink').with_args('/bar').and_return(True) + flexmock(module.borgmatic.execute).should_receive( + 'execute_command_and_capture_output' + ).with_args(('xxh64sum', '/foo'), working_directory=None).and_return('hash1 /foo') + flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return( + ['hash1 foo', 'hash2 bar'] + ) + + assert module.compare_spot_check_hashes( + repository={'path': 'repo'}, + archive='archive', + config={ + 'checks': [ + { + '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'), + ) == ('/bar',) + + def test_compare_spot_check_hashes_considers_non_existent_path_as_not_matching(): flexmock(module.random).should_receive('sample').replace_with( lambda population, count: population[:count] @@ -1092,6 +1133,7 @@ def test_compare_spot_check_hashes_considers_non_existent_path_as_not_matching() ) flexmock(module.os.path).should_receive('exists').with_args('/foo').and_return(True) flexmock(module.os.path).should_receive('exists').with_args('/bar').and_return(False) + 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'), working_directory=None).and_return('hash1 /foo') @@ -1127,6 +1169,7 @@ def test_compare_spot_check_hashes_with_too_many_paths_feeds_them_to_commands_in 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_return( @@ -1173,6 +1216,7 @@ def test_compare_spot_check_hashes_uses_working_directory_to_access_source_paths ) flexmock(module.os.path).should_receive('exists').with_args('/working/dir/foo').and_return(True) flexmock(module.os.path).should_receive('exists').with_args('/working/dir/bar').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='/working/dir').and_return(