Browse Source

Merge branch 'main' into config-command-line.

Dan Helfman 2 months ago
parent
commit
af97b95e2b
3 changed files with 54 additions and 5 deletions
  1. 1 0
      NEWS
  2. 9 5
      borgmatic/actions/check.py
  3. 44 0
      tests/unit/actions/test_check.py

+ 1 - 0
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

+ 9 - 5
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
                 },
             )
         )

+ 44 - 0
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(