Fix the "spot" check to support relative source directory paths (#960). Fix the "spot" check to no longer consider pipe files within an archive for file comparisons. Fix auto-excluding of special files (when databases are configured) to support relative source directory paths.

NEWS

@@ -1,8 +1,12 @@
 1.9.6.dev0
- * #959: Fix an error in the Btrfs hook when a "/" subvolume is configured in borgmatic's source
-   directories.
+ * #959: Fix an error in the Btrfs hook when a subvolume mounted at "/" is configured in borgmatic's
+   source directories.
  * #960: Fix for archives storing relative source directory paths such that they contain the working
    directory.
+ * #960: Fix the "spot" check to support relative source directory paths.
+ * Fix the "spot" check to no longer consider pipe files within an archive for file comparisons.
+ * Fix auto-excluding of special files (when databases are configured) to support relative source
+   directory paths.
  * Drop support for Python 3.8, which has been end-of-lifed.
 
 1.9.5

borgmatic/actions/check.py

@@ -409,6 +409,7 @@ def collect_spot_check_source_paths(
 
 
 BORG_DIRECTORY_FILE_TYPE = 'd'
+BORG_PIPE_FILE_TYPE = 'p'
 
 
 def collect_spot_check_archive_paths(
@@ -426,6 +427,9 @@ def collect_spot_check_archive_paths(
     local Borg version, global arguments as an argparse.Namespace instance, the local Borg path, the
     remote Borg path, and the borgmatic runtime directory, collect the paths from the given archive
     (but only include files and symlinks and exclude borgmatic runtime directories).
+
+    These paths do not have a leading slash, as that's how Borg stores them. As a result, we don't
+    know whether they came from absolute or relative source directories.
     '''
     borgmatic_source_directory = borgmatic.config.paths.get_borgmatic_source_directory(config)
 
@@ -437,15 +441,17 @@ def collect_spot_check_archive_paths(
             config,
             local_borg_version,
             global_arguments,
-            path_format='{type} /{path}{NL}',  # noqa: FS003
+            path_format='{type} {path}{NL}',  # noqa: FS003
             local_path=local_path,
             remote_path=remote_path,
         )
         for (file_type, path) in (line.split(' ', 1),)
-        if file_type != BORG_DIRECTORY_FILE_TYPE
-        if pathlib.Path('/borgmatic') not in pathlib.Path(path).parents
-        if pathlib.Path(borgmatic_source_directory) not in pathlib.Path(path).parents
-        if pathlib.Path(borgmatic_runtime_directory) not in pathlib.Path(path).parents
+        if file_type not in (BORG_DIRECTORY_FILE_TYPE, BORG_PIPE_FILE_TYPE)
+        if pathlib.Path('borgmatic') not in pathlib.Path(path).parents
+        if pathlib.Path(borgmatic_source_directory.lstrip(os.path.sep))
+        not in pathlib.Path(path).parents
+        if pathlib.Path(borgmatic_runtime_directory.lstrip(os.path.sep))
+        not in pathlib.Path(path).parents
     )
 
 
@@ -532,7 +538,7 @@ def compare_spot_check_hashes(
                     local_borg_version,
                     global_arguments,
                     list_paths=source_sample_paths_subset,
-                    path_format='{xxh64} /{path}{NL}',  # noqa: FS003
+                    path_format='{xxh64} {path}{NL}',  # noqa: FS003
                     local_path=local_path,
                     remote_path=remote_path,
                 )
@@ -544,7 +550,7 @@ def compare_spot_check_hashes(
     failing_paths = []
 
     for path, source_hash in source_hashes.items():
-        archive_hash = archive_hashes.get(path)
+        archive_hash = archive_hashes.get(path.lstrip(os.path.sep))
 
         if archive_hash is not None and archive_hash == source_hash:
             continue
@@ -626,11 +632,12 @@ def spot_check(
     count_delta_percentage = abs(len(source_paths) - len(archive_paths)) / len(source_paths) * 100
 
     if count_delta_percentage > spot_check_config['count_tolerance_percentage']:
+        rootless_source_paths = set(path.lstrip(os.path.sep) for path in source_paths)
         logger.debug(
-            f'{log_prefix}: Paths in source paths but not latest archive: {", ".join(set(source_paths) - set(archive_paths)) or "none"}'
+            f'{log_prefix}: Paths in source paths but not latest archive: {", ".join(rootless_source_paths - set(archive_paths)) or "none"}'
         )
         logger.debug(
-            f'{log_prefix}: Paths in latest archive but not source paths: {", ".join(set(archive_paths) - set(source_paths)) or "none"}'
+            f'{log_prefix}: Paths in latest archive but not source paths: {", ".join(set(archive_paths) - rootless_source_paths) or "none"}'
         )
         raise ValueError(
             f'Spot check failed: {count_delta_percentage:.2f}% file count delta between source paths and latest archive (tolerance is {spot_check_config["count_tolerance_percentage"]}%)'

borgmatic/actions/config/bootstrap.py

@@ -41,7 +41,7 @@ def get_config_paths(archive_name, bootstrap_arguments, global_arguments, local_
     config = make_bootstrap_config(bootstrap_arguments)
 
     # Probe for the manifest file in multiple locations, as the default location has moved to the
-    # borgmatic runtime directory (which get stored as just "/borgmatic" with Borg 1.4+). But we
+    # borgmatic runtime directory (which gets stored as just "/borgmatic" with Borg 1.4+). But we
     # still want to support reading the manifest from previously created archives as well.
     with borgmatic.config.paths.Runtime_directory(
         {'user_runtime_directory': bootstrap_arguments.user_runtime_directory},

borgmatic/actions/create.py

@@ -133,12 +133,15 @@ def pattern_root_directories(patterns=None):
     ]
 
 
-def process_source_directories(config, source_directories=None):
+def process_source_directories(config, source_directories=None, skip_expand_paths=None):
     '''
     Given a sequence of source directories (either in the source_directories argument or, lacking
     that, from config), expand and deduplicate the source directories, returning the result.
+
+    If any paths are given to skip, don't expand them.
     '''
     working_directory = borgmatic.config.paths.get_working_directory(config)
+    skip_paths = set(skip_expand_paths or ())
 
     if source_directories is None:
         source_directories = tuple(config.get('source_directories', ()))
@@ -146,9 +149,10 @@ def process_source_directories(config, source_directories=None):
     return deduplicate_directories(
         map_directories_to_devices(
             expand_directories(
-                tuple(source_directories),
+                tuple(source for source in source_directories if source not in skip_paths),
                 working_directory=working_directory,
             )
+            + tuple(skip_paths)
         ),
         additional_directory_devices=map_directories_to_devices(
             expand_directories(
@@ -220,7 +224,9 @@ def run_create(
         # Process source directories again in case any data source hooks updated them. Without this
         # step, we could end up with duplicate paths that cause Borg to hang when it tries to read
         # from the same named pipe twice.
-        source_directories = process_source_directories(config, source_directories)
+        source_directories = process_source_directories(
+            config, source_directories, skip_expand_paths=config_paths
+        )
         stream_processes = [process for processes in active_dumps.values() for process in processes]
 
         json_output = borgmatic.borg.create.create_archive(

borgmatic/borg/create.py

@@ -134,13 +134,14 @@ def make_list_filter_flags(local_borg_version, dry_run):
         return f'{base_flags}-'
 
 
-def special_file(path):
+def special_file(path, working_directory=None):
     '''
     Return whether the given path is a special file (character device, block device, or named pipe
-    / FIFO).
+    / FIFO). If a working directory is given, take it into account when making the full path to
+    check.
     '''
     try:
-        mode = os.stat(path).st_mode
+        mode = os.stat(os.path.join(working_directory or '', path)).st_mode
     except (FileNotFoundError, OSError):
         return False
 
@@ -208,7 +209,9 @@ def collect_special_file_paths(
                 f'The runtime directory {os.path.normpath(borgmatic_runtime_directory)} overlaps with the configured excludes. Please remove it from excludes or change the runtime directory.'
             )
 
-    return tuple(path for path in paths if special_file(path) if path not in skip_paths)
+    return tuple(
+        path for path in paths if special_file(path, working_directory) if path not in skip_paths
+    )
 
 
 def check_all_source_directories_exist(source_directories):

tests/unit/actions/test_check.py

@@ -725,15 +725,16 @@ def test_collect_spot_check_source_paths_skips_directories():
     )
 
 
-def test_collect_spot_check_archive_paths_excludes_directories():
+def test_collect_spot_check_archive_paths_excludes_directories_and_pipes():
     flexmock(module.borgmatic.config.paths).should_receive(
         'get_borgmatic_source_directory'
     ).and_return('/home/user/.borgmatic')
     flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return(
         (
-            'f /etc/path',
-            'f /etc/other',
-            'd /etc/dir',
+            'f etc/path',
+            'p var/pipe',
+            'f etc/other',
+            'd etc/dir',
         )
     )
 
@@ -746,7 +747,7 @@ def test_collect_spot_check_archive_paths_excludes_directories():
         local_path=flexmock(),
         remote_path=flexmock(),
         borgmatic_runtime_directory='/run/user/1001/borgmatic',
-    ) == ('/etc/path', '/etc/other')
+    ) == ('etc/path', 'etc/other')
 
 
 def test_collect_spot_check_archive_paths_excludes_file_in_borgmatic_runtime_directory_as_stored_with_prefix_truncation():
@@ -755,8 +756,8 @@ def test_collect_spot_check_archive_paths_excludes_file_in_borgmatic_runtime_dir
     ).and_return('/root/.borgmatic')
     flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return(
         (
-            'f /etc/path',
-            'f /borgmatic/some/thing',
+            'f etc/path',
+            'f borgmatic/some/thing',
         )
     )
 
@@ -769,7 +770,7 @@ def test_collect_spot_check_archive_paths_excludes_file_in_borgmatic_runtime_dir
         local_path=flexmock(),
         remote_path=flexmock(),
         borgmatic_runtime_directory='/run/user/0/borgmatic',
-    ) == ('/etc/path',)
+    ) == ('etc/path',)
 
 
 def test_collect_spot_check_archive_paths_excludes_file_in_borgmatic_source_directory():
@@ -778,8 +779,8 @@ def test_collect_spot_check_archive_paths_excludes_file_in_borgmatic_source_dire
     ).and_return('/root/.borgmatic')
     flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return(
         (
-            'f /etc/path',
-            'f /root/.borgmatic/some/thing',
+            'f etc/path',
+            'f root/.borgmatic/some/thing',
         )
     )
 
@@ -792,7 +793,7 @@ def test_collect_spot_check_archive_paths_excludes_file_in_borgmatic_source_dire
         local_path=flexmock(),
         remote_path=flexmock(),
         borgmatic_runtime_directory='/run/user/0/borgmatic',
-    ) == ('/etc/path',)
+    ) == ('etc/path',)
 
 
 def test_collect_spot_check_archive_paths_excludes_file_in_borgmatic_runtime_directory():
@@ -801,8 +802,8 @@ def test_collect_spot_check_archive_paths_excludes_file_in_borgmatic_runtime_dir
     ).and_return('/root.borgmatic')
     flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return(
         (
-            'f /etc/path',
-            'f /run/user/0/borgmatic/some/thing',
+            'f etc/path',
+            'f run/user/0/borgmatic/some/thing',
         )
     )
 
@@ -815,7 +816,7 @@ def test_collect_spot_check_archive_paths_excludes_file_in_borgmatic_runtime_dir
         local_path=flexmock(),
         remote_path=flexmock(),
         borgmatic_runtime_directory='/run/user/0/borgmatic',
-    ) == ('/etc/path',)
+    ) == ('etc/path',)
 
 
 def test_collect_spot_check_source_paths_uses_working_directory():
@@ -877,7 +878,7 @@ def test_compare_spot_check_hashes_returns_paths_having_failing_hashes():
         'hash1  /foo\nhash2  /bar'
     )
     flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return(
-        ['hash1 /foo', 'nothash2 /bar']
+        ['hash1 foo', 'nothash2 bar']
     )
 
     assert module.compare_spot_check_hashes(
@@ -904,6 +905,47 @@ def test_compare_spot_check_hashes_returns_paths_having_failing_hashes():
     ) == ('/bar',)
 
 
+def test_compare_spot_check_hashes_returns_relative_paths_having_failing_hashes():
+    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.borgmatic.execute).should_receive(
+        'execute_command_and_capture_output'
+    ).with_args(('xxh64sum', 'foo', 'bar'), working_directory=None).and_return(
+        'hash1  foo\nhash2  bar'
+    )
+    flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return(
+        ['hash1 foo', 'nothash2 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(),
+        log_prefix='repo',
+        source_paths=('foo', 'bar', 'baz', 'quux'),
+    ) == ('bar',)
+
+
 def test_compare_spot_check_hashes_handles_data_sample_percentage_above_100():
     flexmock(module.random).should_receive('sample').replace_with(
         lambda population, count: population[:count]
@@ -918,7 +960,7 @@ def test_compare_spot_check_hashes_handles_data_sample_percentage_above_100():
         'hash1  /foo\nhash2  /bar'
     )
     flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return(
-        ['nothash1 /foo', 'nothash2 /bar']
+        ['nothash1 foo', 'nothash2 bar']
     )
 
     assert module.compare_spot_check_hashes(
@@ -959,7 +1001,7 @@ def test_compare_spot_check_hashes_uses_xxh64sum_command_option():
         'hash1  /foo\nhash2  /bar'
     )
     flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return(
-        ['hash1 /foo', 'nothash2 /bar']
+        ['hash1 foo', 'nothash2 bar']
     )
 
     assert module.compare_spot_check_hashes(
@@ -997,7 +1039,7 @@ def test_compare_spot_check_hashes_considers_path_missing_from_archive_as_not_ma
         'hash1  /foo\nhash2  /bar'
     )
     flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return(
-        ['hash1 /foo']
+        ['hash1 foo']
     )
 
     assert module.compare_spot_check_hashes(
@@ -1033,7 +1075,7 @@ def test_compare_spot_check_hashes_considers_non_existent_path_as_not_matching()
         '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']
+        ['hash1 foo', 'hash2 bar']
     )
 
     assert module.compare_spot_check_hashes(
@@ -1076,8 +1118,8 @@ def test_compare_spot_check_hashes_with_too_many_paths_feeds_them_to_commands_in
         'hash3  /baz\nhash4  /quux'
     )
     flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return(
-        ['hash1 /foo', 'hash2 /bar']
-    ).and_return(['hash3 /baz', 'nothash4 /quux'])
+        ['hash1 foo', 'hash2 bar']
+    ).and_return(['hash3 baz', 'nothash4 quux'])
 
     assert module.compare_spot_check_hashes(
         repository={'path': 'repo'},

tests/unit/actions/test_create.py

@@ -205,6 +205,25 @@ def test_process_source_directories_prefers_source_directory_argument_to_config(
     ) == ('foo', 'bar')
 
 
+def test_process_source_directories_skips_expand_for_requested_paths():
+    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(
+        '/working'
+    )
+    flexmock(module).should_receive('deduplicate_directories').and_return(('foo', 'bar'))
+    flexmock(module).should_receive('map_directories_to_devices').and_return({})
+    flexmock(module).should_receive('expand_directories').with_args(
+        ('bar',), working_directory='/working'
+    ).and_return(()).once()
+    flexmock(module).should_receive('pattern_root_directories').and_return(())
+    flexmock(module).should_receive('expand_directories').with_args(
+        (), working_directory='/working'
+    ).and_return(())
+
+    assert module.process_source_directories(
+        config={'source_directories': ['foo', 'bar']}, skip_expand_paths=('foo',)
+    ) == ('foo', 'bar')
+
+
 def test_run_create_executes_and_calls_hooks_for_configured_repository():
     flexmock(module.logger).answer = lambda message: None
     flexmock(module.borgmatic.config.validate).should_receive('repositories_match').never()

tests/unit/borg/test_create.py

@@ -258,6 +258,17 @@ def test_special_file_treats_broken_symlink_as_non_special():
     assert module.special_file('/broken/symlink') is False
 
 
+def test_special_file_prepends_relative_path_with_working_directory():
+    flexmock(module.os).should_receive('stat').with_args('/working/dir/relative').and_return(
+        flexmock(st_mode=flexmock())
+    )
+    flexmock(module.stat).should_receive('S_ISCHR').and_return(False)
+    flexmock(module.stat).should_receive('S_ISBLK').and_return(False)
+    flexmock(module.stat).should_receive('S_ISFIFO').and_return(False)
+
+    assert module.special_file('relative', '/working/dir') is False
+
+
 def test_any_parent_directories_treats_parents_as_match():
     module.any_parent_directories('/foo/bar.txt', ('/foo', '/etc'))