Merge branch 'main' into command-hook-steps

NEWS

@@ -1,18 +1,12 @@
-2.0.9
- * #1105: More accurately collect Btrfs subvolumes to snapshot. As part of this, the Btrfs hook no
-   longer uses "findmnt" and the "findmnt_command" option is deprecated.
+2.0.9.dev0
  * #1123: Add loading of systemd credentials even when running borgmatic outside of a systemd
    service.
+ * #1134: Add a "steps" option to run command hooks around particular sub-action steps like
+   individual checks.
  * #1149: Add support for Python 3.14.
  * #1149: Include automated tests in the source dist tarball uploaded to PyPI.
  * #1151: Fix snapshotting in the ZFS, Btrfs, and LVM hooks to play nicely with the Borg 1.4+
    "slashdot" hack within source directory paths.
- * #1152: Fix a regression in the Loki monitoring hook in which log messages weren't sending.
- * #1156: Fix snapshotting in the ZFS, Btrfs, and LVM hooks to snapshot both parent and child
-   volumes/filesystems instead of just the parent. As part of this fix, borgmatic no longer
-   deduplicates patterns except for those containing the borgmatic runtime directory.
- * Fix a traceback (TypeError) regression in the "spot" check when the "local_path" option isn't
-   set.
 
 2.0.8
  * #1114: Document systemd configuration changes for the ZFS filesystem hook.

borgmatic/actions/check.py

@@ -744,6 +744,7 @@ def run_check(
     global_arguments,
     local_path,
     remote_path,
+    hook_context,
 ):
     '''
     Run the "check" action for the given repository.
@@ -783,44 +784,75 @@ def run_check(
         archives_check_id,
     )
     borg_specific_checks = set(checks).intersection({'repository', 'archives', 'data'})
+    working_directory = borgmatic.config.paths.get_working_directory(config)
 
     if borg_specific_checks:
-        borgmatic.borg.check.check_archives(
-            repository['path'],
-            config,
-            local_borg_version,
-            check_arguments,
-            global_arguments,
-            borg_specific_checks,
-            archive_filter_flags,
-            local_path=local_path,
-            remote_path=remote_path,
-        )
-        for check in borg_specific_checks:
-            write_check_time(make_check_time_path(config, repository_id, check, archives_check_id))
+        with borgmatic.hooks.command.Before_after_hooks(
+            command_hooks=config.get('commands'),
+            before_after='step',
+            umask=config.get('umask'),
+            working_directory=working_directory,
+            dry_run=global_arguments.dry_run,
+            action_names=('check',),
+            step_names=('archives_repository_data',),
+            **hook_context,
+        ):
+            borgmatic.borg.check.check_archives(
+                repository['path'],
+                config,
+                local_borg_version,
+                check_arguments,
+                global_arguments,
+                borg_specific_checks,
+                archive_filter_flags,
+                local_path=local_path,
+                remote_path=remote_path,
+            )
+            for check in borg_specific_checks:
+                write_check_time(make_check_time_path(config, repository_id, check, archives_check_id))
 
     if 'extract' in checks:
-        borgmatic.borg.extract.extract_last_archive_dry_run(
-            config,
-            local_borg_version,
-            global_arguments,
-            repository['path'],
-            config.get('lock_wait'),
-            local_path,
-            remote_path,
-        )
-        write_check_time(make_check_time_path(config, repository_id, 'extract'))
-
-    if 'spot' in checks:
-        with borgmatic.config.paths.Runtime_directory(config) as borgmatic_runtime_directory:
-            spot_check(
-                repository,
+        with borgmatic.hooks.command.Before_after_hooks(
+            command_hooks=config.get('commands'),
+            before_after='step',
+            umask=config.get('umask'),
+            working_directory=working_directory,
+            dry_run=global_arguments.dry_run,
+            action_names=('check',),
+            step_names=('extract',),
+            **hook_context,
+        ):
+            borgmatic.borg.extract.extract_last_archive_dry_run(
                 config,
                 local_borg_version,
                 global_arguments,
+                repository['path'],
+                config.get('lock_wait'),
                 local_path,
                 remote_path,
-                borgmatic_runtime_directory,
             )
+            write_check_time(make_check_time_path(config, repository_id, 'extract'))
 
-        write_check_time(make_check_time_path(config, repository_id, 'spot'))
+    if 'spot' in checks:
+        with borgmatic.hooks.command.Before_after_hooks(
+            command_hooks=config.get('commands'),
+            before_after='step',
+            umask=config.get('umask'),
+            working_directory=working_directory,
+            dry_run=global_arguments.dry_run,
+            action_names=('check',),
+            step_names=('spot',),
+            **hook_context,
+        ):
+            with borgmatic.config.paths.Runtime_directory(config) as borgmatic_runtime_directory:
+                spot_check(
+                    repository,
+                    config,
+                    local_borg_version,
+                    global_arguments,
+                    local_path,
+                    remote_path,
+                    borgmatic_runtime_directory,
+                )
+
+            write_check_time(make_check_time_path(config, repository_id, 'spot'))

borgmatic/actions/config/bootstrap.py

@@ -66,7 +66,7 @@ def load_config_paths_from_archive(
             config,
             local_borg_version,
             global_arguments,
-            local_path=config.get('local_path', 'borg'),
+            local_path=config.get('local_path'),
             remote_path=config.get('remote_path'),
             extract_to_stdout=True,
         )

borgmatic/actions/create.py

@@ -50,20 +50,18 @@ def run_create(
     working_directory = borgmatic.config.paths.get_working_directory(config)
 
     with borgmatic.config.paths.Runtime_directory(config) as borgmatic_runtime_directory:
-        patterns = pattern.process_patterns(
-            pattern.collect_patterns(config),
-            config,
-            working_directory,
-            borgmatic_runtime_directory,
-        )
         borgmatic.hooks.dispatch.call_hooks_even_if_unconfigured(
             'remove_data_source_dumps',
             config,
             borgmatic.hooks.dispatch.Hook_type.DATA_SOURCE,
             borgmatic_runtime_directory,
-            patterns,
             global_arguments.dry_run,
         )
+        patterns = pattern.process_patterns(
+            pattern.collect_patterns(config),
+            config,
+            working_directory,
+        )
         active_dumps = borgmatic.hooks.dispatch.call_hooks(
             'dump_data_sources',
             config,
@@ -81,7 +79,6 @@ def run_create(
             patterns,
             config,
             working_directory,
-            borgmatic_runtime_directory,
             skip_expand_paths=config_paths,
         )
         stream_processes = [process for processes in active_dumps.values() for process in processes]
@@ -141,7 +138,6 @@ def run_create(
             config,
             borgmatic.hooks.dispatch.Hook_type.DATA_SOURCE,
             borgmatic_runtime_directory,
-            patterns,
             global_arguments.dry_run,
         )
 

borgmatic/actions/pattern.py

@@ -227,37 +227,23 @@ def device_map_patterns(patterns, working_directory=None):
     )
 
 
-def deduplicate_runtime_directory_patterns(patterns, config, borgmatic_runtime_directory=None):
+def deduplicate_patterns(patterns, config):
     '''
-    Given a sequence of borgmatic.borg.pattern.Pattern instances, the borgmatic runtime directory,
-    and a configuration dict, return them without any duplicate root child patterns that contain the
-    runtime directory. For instance, if two root patterns are given with paths "/foo" and
-    "/foo/bar", and the runtime directory is "/foo/bar", return just the "/foo" pattern. Non-root
-    patterns and patterns not containing the runtime directory are passed through without
-    modification.
+    Given a sequence of borgmatic.borg.pattern.Pattern instances and a configuration dict, return
+    them with all duplicate root child patterns removed. For instance, if two root patterns are
+    given with paths "/foo" and "/foo/bar", return just the one with "/foo". Non-root patterns are
+    passed through without modification.
 
-    One exception to deduplication is if two paths are on different filesystems (devices) and
+    The one exception to deduplication is if two paths are on different filesystems (devices) and
     "one_file_system" is True in the given configuration. In that case, the paths won't get
     deduplicated, because Borg won't cross filesystem boundaries when "one_file_system" is True.
 
-    The idea is that if Borg is given a root parent pattern containing the borgmatic runtime
-    directory, then Borg doesn't also need to be given child patterns, because it will naturally
-    spider the contents of the parent pattern's path. Additionally, there are cases where Borg
-    coming across the same file twice will result in duplicate reads and even hangs, e.g. when a
-    database hook in the borgmatic runtime directory is using a named pipe for streaming database
-    dumps to Borg.
-
-    This deduplication is limited to the borgmatic runtime directory (where borgmatic's named pipes
-    exist), because there are other legitimate use cases for parent and child patterns to both exist
-    in patterns. For instance, with some snapshotted filesystems, snapshots don't traverse from a
-    parent filesystem to a child and therefore both need to remain in patterns.
-
-    And for the case of named pipes outside of the borgmatic runtime directory, there is code
-    elsewhere (in the "create" action) that auto-excludes special files to prevent Borg hangs.
+    The idea is that if Borg is given a root parent pattern, then it doesn't also need to be given
+    child patterns, because it will naturally spider the contents of the parent pattern's path. And
+    there are cases where Borg coming across the same file twice will result in duplicate reads and
+    even hangs, e.g. when a database hook is using a named pipe for streaming database dumps to
+    Borg.
     '''
-    if borgmatic_runtime_directory is None:
-        return patterns
-
     deduplicated = {}  # Use just the keys as an ordered set.
 
     for pattern in patterns:
@@ -276,8 +262,6 @@ def deduplicate_runtime_directory_patterns(patterns, config, borgmatic_runtime_d
 
             if any(
                 pathlib.PurePath(other_pattern.path) == parent
-                and pathlib.PurePosixPath(other_pattern.path)
-                in pathlib.PurePath(borgmatic_runtime_directory).parents
                 and pattern.device is not None
                 and (
                     other_pattern.device == pattern.device
@@ -292,22 +276,16 @@ def deduplicate_runtime_directory_patterns(patterns, config, borgmatic_runtime_d
     return tuple(deduplicated.keys())
 
 
-def process_patterns(
-    patterns, config, working_directory, borgmatic_runtime_directory=None, skip_expand_paths=None
-):
+def process_patterns(patterns, config, working_directory, skip_expand_paths=None):
     '''
-    Given a sequence of Borg patterns, a configuration dict, a configured working directory, the
-    borgmatic runtime directory, and a sequence of paths to skip path expansion for, expand and
-    deduplicate any "root" patterns, returning the resulting root and non-root patterns as a list.
-
-    If the borgmatic runtime directory is None, then don't deduplicate patterns. Deduplication is
-    really only necessary for the "create" action when the runtime directory might contain named
-    pipes for database dumps.
+    Given a sequence of Borg patterns, a configuration dict, a configured working directory, and a
+    sequence of paths to skip path expansion for, expand and deduplicate any "root" patterns,
+    returning the resulting root and non-root patterns as a list.
     '''
     skip_paths = set(skip_expand_paths or ())
 
     return list(
-        deduplicate_runtime_directory_patterns(
+        deduplicate_patterns(
             device_map_patterns(
                 expand_patterns(
                     patterns,
@@ -316,6 +294,5 @@ def process_patterns(
                 ),
             ),
             config,
-            borgmatic_runtime_directory,
         ),
     )

borgmatic/actions/restore.py

@@ -5,7 +5,6 @@ import pathlib
 import shutil
 import tempfile
 
-import borgmatic.actions.pattern
 import borgmatic.borg.extract
 import borgmatic.borg.list
 import borgmatic.borg.mount
@@ -537,20 +536,13 @@ def run_restore(
         return
 
     logger.info(f'Restoring data sources from archive {restore_arguments.archive}')
-    working_directory = borgmatic.config.paths.get_working_directory(config)
 
     with borgmatic.config.paths.Runtime_directory(config) as borgmatic_runtime_directory:
-        patterns = borgmatic.actions.pattern.process_patterns(
-            borgmatic.actions.pattern.collect_patterns(config),
-            config,
-            working_directory,
-        )
         borgmatic.hooks.dispatch.call_hooks_even_if_unconfigured(
             'remove_data_source_dumps',
             config,
             borgmatic.hooks.dispatch.Hook_type.DATA_SOURCE,
             borgmatic_runtime_directory,
-            patterns,
             global_arguments.dry_run,
         )
 
@@ -633,7 +625,6 @@ def run_restore(
             config,
             borgmatic.hooks.dispatch.Hook_type.DATA_SOURCE,
             borgmatic_runtime_directory,
-            patterns,
             global_arguments.dry_run,
         )
 

borgmatic/borg/create.py

@@ -62,7 +62,7 @@ def validate_planned_backup_paths(
     excluded.
 
     Raise ValueError if the runtime directory has been excluded via "exclude_patterns" or similar,
-    because any features that rely on the runtime directory getting backed up will break. For
+    because any features that rely on the runtime directory getting backed up will break.  For
     instance, without the runtime directory, Borg can't consume any database dumps and borgmatic may
     hang waiting for them to be consumed.
     '''

borgmatic/commands/borgmatic.py

@@ -463,6 +463,7 @@ def run_actions(  # noqa: PLR0912, PLR0915
                             global_arguments,
                             local_path,
                             remote_path,
+                            hook_context,
                         )
                 elif action_name == 'extract':
                     borgmatic.actions.extract.run_extract(

borgmatic/config/schema.yaml

@@ -1140,6 +1140,7 @@ properties:
                       before:
                           type: string
                           enum:
+                              - step
                               - action
                               - repository
                               - configuration
@@ -1148,6 +1149,8 @@ properties:
                               Name for the point in borgmatic's execution that
                               the commands should be run before (required if
                               "after" isn't set):
+                               * "step" runs before a sub-action step for each
+                              repository, e.g. for an individual check.
                                * "action" runs before each action for each
                               repository.
                                * "repository" runs before all actions for each
@@ -1188,6 +1191,18 @@ properties:
                               List of actions for which the commands will be
                               run. Defaults to running for all actions.
                           example: [create, prune, compact, check]
+                      steps:
+                          type: array
+                          items:
+                              type: string
+                              enum:
+                                  - archives_repository_data
+                                  - extract
+                                  - spot
+                          description: |
+                              List of sub-action steps for which the commands
+                              will be run. Defaults to running for all steps.
+                          example: [extract, spot]
                       run:
                           type: array
                           items:
@@ -1203,6 +1218,7 @@ properties:
                       after:
                           type: string
                           enum:
+                              - step
                               - action
                               - repository
                               - configuration
@@ -1212,6 +1228,8 @@ properties:
                               Name for the point in borgmatic's execution that
                               the commands should be run after (required if
                               "before" isn't set):
+                               * "step" runs before a sub-action step for each
+                              repository, e.g. for an individual check.
                                * "action" runs after each action for each
                               repository.
                                * "repository" runs after all actions for each
@@ -1254,6 +1272,18 @@ properties:
                               particular actions listed here. Defaults to
                               running for all actions.
                           example: [create, prune, compact, check]
+                      steps:
+                          type: array
+                          items:
+                              type: string
+                              enum:
+                                  - archives_repository_data
+                                  - extract
+                                  - spot
+                          description: |
+                              List of sub-action steps for which the commands
+                              will be run. Defaults to running for all steps.
+                          example: [extract, spot]
                       states:
                           type: array
                           items:
@@ -2951,8 +2981,7 @@ properties:
             findmnt_command:
                 type: string
                 description: |
-                    Deprecated and unused. Was the command to use instead of
-                    "findmnt".
+                    Command to use instead of "findmnt".
                 example: /usr/local/bin/findmnt
         description: |
             Configuration for integration with the Btrfs filesystem.

borgmatic/hooks/command.py

@@ -64,22 +64,27 @@ def make_environment(current_environment, sys_module=sys):
     return environment
 
 
-def filter_hooks(command_hooks, before=None, after=None, action_names=None, state_names=None):
+def filter_hooks(command_hooks, before=None, after=None, action_names=None, step_names=None, state_names=None):
     '''
     Given a sequence of command hook dicts from configuration and one or more filters (before name,
-    after name, a sequence of action names, and/or a sequence of execution result state names),
-    filter down the command hooks to just the ones that match the given filters.
+    after name, a sequence of action names, a sequence of sub-action steps, and/or a sequence of
+    execution result state names), filter down the command hooks to just the ones that match the
+    given filters.
     '''
     return tuple(
         hook_config
         for hook_config in command_hooks or ()
         for config_action_names in (hook_config.get('when'),)
+        for config_step_names in (hook_config.get('steps'),)
         for config_state_names in (hook_config.get('states'),)
         if before is None or hook_config.get('before') == before
         if after is None or hook_config.get('after') == after
         if action_names is None
         or config_action_names is None
         or set(config_action_names or ()).intersection(set(action_names))
+        if step_names is None
+        or config_step_names is None
+        or set(config_step_names or ()).intersection(set(step_names))
         if state_names is None
         or config_state_names is None
         or set(config_state_names or ()).intersection(set(state_names))
@@ -164,7 +169,8 @@ class Before_after_hooks:
            before_after='do_stuff',
            umask=config.get('umask'),
            dry_run=dry_run,
-           action_names=['create'],
+           action_names=['check'],
+           step_names=['spot'],
        ):
             do()
             some()
@@ -182,13 +188,14 @@ class Before_after_hooks:
         working_directory,
         dry_run,
         action_names=None,
+        step_names=None,
         **context,
     ):
         '''
         Given a sequence of command hook configuration dicts, the before/after name, a umask to run
         commands with, a working directory to run commands with, a dry run flag, a sequence of
-        action names, and any context for the executed commands, save those data points for use
-        below.
+        action names, a sequence of sub-action step names, and any context for the executed
+        commands, save those data points for use below.
         '''
         self.command_hooks = command_hooks
         self.before_after = before_after
@@ -196,6 +203,7 @@ class Before_after_hooks:
         self.working_directory = working_directory
         self.dry_run = dry_run
         self.action_names = action_names
+        self.step_names = step_names
         self.context = context
 
     def __enter__(self):
@@ -208,6 +216,7 @@ class Before_after_hooks:
                     self.command_hooks,
                     before=self.before_after,
                     action_names=self.action_names,
+                    step_names=self.step_names,
                 ),
                 self.umask,
                 self.working_directory,
@@ -234,6 +243,7 @@ class Before_after_hooks:
                     self.command_hooks,
                     after=self.before_after,
                     action_names=self.action_names,
+                    step_names=self.step_names,
                     state_names=['fail' if exception_type else 'finish'],
                 ),
                 self.umask,

borgmatic/hooks/data_source/bootstrap.py

@@ -75,11 +75,11 @@ def dump_data_sources(
     return []
 
 
-def remove_data_source_dumps(hook_config, config, borgmatic_runtime_directory, patterns, dry_run):
+def remove_data_source_dumps(hook_config, config, borgmatic_runtime_directory, dry_run):
     '''
-    Given a bootstrap configuration dict, a configuration dict, the borgmatic runtime directory, the
-    configured patterns, and whether this is a dry run, then remove the manifest file created above.
-    If this is a dry run, then don't actually remove anything.
+    Given a bootstrap configuration dict, a configuration dict, the borgmatic runtime directory, and
+    whether this is a dry run, then remove the manifest file created above. If this is a dry run,
+    then don't actually remove anything.
     '''
     dry_run_label = ' (dry run; not actually removing anything)' if dry_run else ''
 

borgmatic/hooks/data_source/btrfs.py

@@ -1,9 +1,9 @@
 import collections
-import functools
 import glob
+import itertools
+import json
 import logging
 import os
-import pathlib
 import shutil
 import subprocess
 
@@ -22,44 +22,93 @@ def use_streaming(hook_config, config):  # pragma: no cover
     return False
 
 
-@functools.cache
-def path_is_a_subvolume(btrfs_command, path):
+def get_contained_subvolume_paths(btrfs_command, subvolume_path):
     '''
-    Given a btrfs command and a path, return whether the path is a Btrfs subvolume. Return False if
-    the btrfs command errors, which probably indicates there isn't a containing Btrfs subvolume for
-    the given path.
+    Given the path of a Btrfs subvolume, return it in a sequence along with the paths of its
+    contained subvolumes.
 
-    As a performance optimization, multiple calls to this function with the same arguments are
-    cached.
+    If the btrfs command errors, log that error and return an empty sequence.
     '''
     try:
-        borgmatic.execute.execute_command(
+        btrfs_output = borgmatic.execute.execute_command_and_capture_output(
             (
                 *btrfs_command.split(' '),
                 'subvolume',
-                'show',
-                path,
+                'list',
+                subvolume_path,
             ),
-            output_log_level=None,
             close_fds=True,
         )
-    # An error from the command (probably) indicates that the path is not actually a subvolume.
-    except subprocess.CalledProcessError:
-        return False
+    except subprocess.CalledProcessError as error:
+        logger.debug(
+            f'Ignoring Btrfs subvolume {subvolume_path} because of error listing its subvolumes: {error}',
+        )
 
-    return True
+        return ()
+
+    return (
+        subvolume_path,
+        *tuple(
+            os.path.join(subvolume_path, line.split(' ')[-1])
+            for line in btrfs_output.splitlines()
+            if line.strip()
+        ),
+    )
+
+
+FINDMNT_BTRFS_ROOT_SUBVOLUME_OPTION = 'subvolid=5'
+
+
+def get_all_subvolume_paths(btrfs_command, findmnt_command):
+    '''
+    Given btrfs and findmnt commands to run, get the sorted paths for all Btrfs subvolumes on the
+    system.
+    '''
+    findmnt_output = borgmatic.execute.execute_command_and_capture_output(
+        (
+            *findmnt_command.split(' '),
+            '-t',  # Filesystem type.
+            'btrfs',
+            '--json',
+            '--list',  # Request a flat list instead of a nested subvolume hierarchy.
+        ),
+        close_fds=True,
+    )
+
+    try:
+        return tuple(
+            sorted(
+                itertools.chain.from_iterable(
+                    # If findmnt gave us a Btrfs root filesystem, list the subvolumes within it.
+                    # This is necessary because findmnt only returns a subvolume's mount point
+                    # rather than its original subvolume path (which can differ). For instance,
+                    # a subvolume might exist at /mnt/subvolume but be mounted at /home/myuser.
+                    # findmnt is still useful though because it's a global way to discover all
+                    # Btrfs subvolumes—even if we have to do some additional legwork ourselves.
+                    (
+                        get_contained_subvolume_paths(btrfs_command, filesystem['target'])
+                        if FINDMNT_BTRFS_ROOT_SUBVOLUME_OPTION in filesystem['options'].split(',')
+                        else (filesystem['target'],)
+                    )
+                    for filesystem in json.loads(findmnt_output)['filesystems']
+                ),
+            ),
+        )
+    except json.JSONDecodeError as error:
+        raise ValueError(f'Invalid {findmnt_command} JSON output: {error}')
+    except KeyError as error:
+        raise ValueError(f'Invalid {findmnt_command} output: Missing key "{error}"')
+
+
+Subvolume = collections.namedtuple('Subvolume', ('path', 'contained_patterns'), defaults=((),))
 
 
-@functools.cache
 def get_subvolume_property(btrfs_command, subvolume_path, property_name):
     '''
     Given a btrfs command, a subvolume path, and a property name to lookup, return the value of the
     corresponding property.
 
     Raise subprocess.CalledProcessError if the btrfs command errors.
-
-    As a performance optimization, multiple calls to this function with the same arguments are
-    cached.
     '''
     output = borgmatic.execute.execute_command_and_capture_output(
         (
@@ -85,71 +134,37 @@ def get_subvolume_property(btrfs_command, subvolume_path, property_name):
     }.get(value, value)
 
 
-def get_containing_subvolume_path(btrfs_command, path):
+def omit_read_only_subvolume_paths(btrfs_command, subvolume_paths):
     '''
-    Given a btrfs command and a path, return the subvolume path that contains the given path (or is
-    the same as the path).
-
-    If there is no such subvolume path or the containing subvolume is read-only, return None.
+    Given a Btrfs command to run and a sequence of Btrfs subvolume paths, filter them down to just
+    those that are read-write. The idea is that Btrfs can't actually snapshot a read-only subvolume,
+    so we should just ignore them.
     '''
-    # Probe the given pattern's path and all of its parents, grandparents, etc. to try to find a
-    # Btrfs subvolume.
-    for candidate_path in (
-        path,
-        *tuple(str(ancestor) for ancestor in pathlib.PurePath(path).parents),
-    ):
-        if not path_is_a_subvolume(btrfs_command, candidate_path):
-            continue
+    retained_subvolume_paths = []
 
+    for subvolume_path in subvolume_paths:
         try:
-            if get_subvolume_property(btrfs_command, candidate_path, 'ro'):
-                logger.debug(f'Ignoring Btrfs subvolume {candidate_path} because it is read-only')
-
-                return None
-
-            logger.debug(f'Path {candidate_path} is a Btrfs subvolume')
-
-            return candidate_path
-        except subprocess.CalledProcessError as error:
+            if get_subvolume_property(btrfs_command, subvolume_path, 'ro'):
+                logger.debug(f'Ignoring Btrfs subvolume {subvolume_path} because it is read-only')
+            else:
+                retained_subvolume_paths.append(subvolume_path)
+        except subprocess.CalledProcessError as error:  # noqa: PERF203
             logger.debug(
-                f'Error determining read-only status of Btrfs subvolume {candidate_path}: {error}',
+                f'Error determining read-only status of Btrfs subvolume {subvolume_path}: {error}',
             )
 
-            return None
-
-    return None
+    return tuple(retained_subvolume_paths)
 
 
-def get_all_subvolume_paths(btrfs_command, patterns):
-    '''
-    Given a btrfs command and a sequence of patterns, get the sorted paths for all Btrfs subvolumes
-    containing those patterns.
-    '''
-    return tuple(
-        sorted(
-            {
-                subvolume_path
-                for pattern in patterns
-                if pattern.type == borgmatic.borg.pattern.Pattern_type.ROOT
-                if pattern.source == borgmatic.borg.pattern.Pattern_source.CONFIG
-                for subvolume_path in (get_containing_subvolume_path(btrfs_command, pattern.path),)
-                if subvolume_path
-            }
-        ),
-    )
-
-
-Subvolume = collections.namedtuple('Subvolume', ('path', 'contained_patterns'), defaults=((),))
-
-
-def get_subvolumes(btrfs_command, patterns):
+def get_subvolumes(btrfs_command, findmnt_command, patterns=None):
     '''
     Given a Btrfs command to run and a sequence of configured patterns, find the intersection
-    between the current Btrfs filesystem and subvolume paths and the paths of any patterns. The
+    between the current Btrfs filesystem and subvolume paths and the paths of any patterns.  The
     idea is that these pattern paths represent the requested subvolumes to snapshot.
 
     Only include subvolumes that contain at least one root pattern sourced from borgmatic
-    configuration (as opposed to generated elsewhere in borgmatic).
+    configuration (as opposed to generated elsewhere in borgmatic). But if patterns is None, then
+    return all subvolumes instead, sorted by path.
 
     Return the result as a sequence of matching Subvolume instances.
     '''
@@ -157,10 +172,15 @@ def get_subvolumes(btrfs_command, patterns):
     subvolumes = []
 
     # For each subvolume path, match it against the given patterns to find the subvolumes to
-    # backup. Sort the subvolumes from longest to shortest mount points, so longer subvolumes get
+    # backup. Sort the subvolumes from longest to shortest mount points, so longer mount points get
     # a whack at the candidate pattern piñata before their parents do. (Patterns are consumed during
     # this process, so no two subvolumes end up with the same contained patterns.)
-    for subvolume_path in reversed(get_all_subvolume_paths(btrfs_command, patterns)):
+    for subvolume_path in reversed(
+        omit_read_only_subvolume_paths(
+            btrfs_command,
+            get_all_subvolume_paths(btrfs_command, findmnt_command),
+        ),
+    ):
         subvolumes.extend(
             Subvolume(subvolume_path, contained_patterns)
             for contained_patterns in (
@@ -169,7 +189,8 @@ def get_subvolumes(btrfs_command, patterns):
                     candidate_patterns,
                 ),
             )
-            if any(
+            if patterns is None
+            or any(
                 pattern.type == borgmatic.borg.pattern.Pattern_type.ROOT
                 and pattern.source == borgmatic.borg.pattern.Pattern_source.CONFIG
                 for pattern in contained_patterns
@@ -304,15 +325,11 @@ def dump_data_sources(
     dry_run_label = ' (dry run; not actually snapshotting anything)' if dry_run else ''
     logger.info(f'Snapshotting Btrfs subvolumes{dry_run_label}')
 
-    if 'findmnt_command' in hook_config:
-        logger.warning(
-            'The Btrfs "findmnt_command" option is deprecated and will be removed from a future release; findmnt is no longer used',
-        )
-
     # Based on the configured patterns, determine Btrfs subvolumes to backup. Only consider those
     # patterns that came from actual user configuration (as opposed to, say, other hooks).
     btrfs_command = hook_config.get('btrfs_command', 'btrfs')
-    subvolumes = get_subvolumes(btrfs_command, patterns)
+    findmnt_command = hook_config.get('findmnt_command', 'findmnt')
+    subvolumes = get_subvolumes(btrfs_command, findmnt_command, patterns)
 
     if not subvolumes:
         logger.warning(f'No Btrfs subvolumes found to snapshot{dry_run_label}')
@@ -358,12 +375,11 @@ def delete_snapshot(btrfs_command, snapshot_path):  # pragma: no cover
     )
 
 
-def remove_data_source_dumps(hook_config, config, borgmatic_runtime_directory, patterns, dry_run):
+def remove_data_source_dumps(hook_config, config, borgmatic_runtime_directory, dry_run):
     '''
-    Given a Btrfs configuration dict, a configuration dict, the borgmatic runtime directory, the
-    configured patterns, and whether this is a dry run, delete any Btrfs snapshots created by
-    borgmatic. If this is a dry run or Btrfs isn't configured in borgmatic's configuration, then
-    don't actually remove anything.
+    Given a Btrfs configuration dict, a configuration dict, the borgmatic runtime directory, and
+    whether this is a dry run, delete any Btrfs snapshots created by borgmatic. If this is a dry run
+    or Btrfs isn't configured in borgmatic's configuration, then don't actually remove anything.
     '''
     if hook_config is None:
         return
@@ -371,9 +387,10 @@ def remove_data_source_dumps(hook_config, config, borgmatic_runtime_directory, p
     dry_run_label = ' (dry run; not actually removing anything)' if dry_run else ''
 
     btrfs_command = hook_config.get('btrfs_command', 'btrfs')
+    findmnt_command = hook_config.get('findmnt_command', 'findmnt')
 
     try:
-        all_subvolumes = get_subvolumes(btrfs_command, patterns)
+        all_subvolumes = get_subvolumes(btrfs_command, findmnt_command)
     except FileNotFoundError as error:
         logger.debug(f'Could not find "{error.filename}" command')
         return

borgmatic/hooks/data_source/lvm.py

@@ -353,12 +353,12 @@ def get_snapshots(lvs_command, snapshot_name=None):
         raise ValueError(f'Invalid {lvs_command} output: Missing key "{error}"')
 
 
-def remove_data_source_dumps(hook_config, config, borgmatic_runtime_directory, patterns, dry_run):  # noqa: PLR0912
+def remove_data_source_dumps(hook_config, config, borgmatic_runtime_directory, dry_run):  # noqa: PLR0912
     '''
-    Given an LVM configuration dict, a configuration dict, the borgmatic runtime directory, the
-    configured patterns, and whether this is a dry run, unmount and delete any LVM snapshots created
-    by borgmatic. If this is a dry run or LVM isn't configured in borgmatic's configuration, then
-    don't actually remove anything.
+    Given an LVM configuration dict, a configuration dict, the borgmatic runtime directory, and
+    whether this is a dry run, unmount and delete any LVM snapshots created by borgmatic. If this is
+    a dry run or LVM isn't configured in borgmatic's configuration, then don't actually remove
+    anything.
     '''
     if hook_config is None:
         return

borgmatic/hooks/data_source/mariadb.py

@@ -369,7 +369,6 @@ def remove_data_source_dumps(
     databases,
     config,
     borgmatic_runtime_directory,
-    patterns,
     dry_run,
 ):  # pragma: no cover
     '''

borgmatic/hooks/data_source/mongodb.py

@@ -185,7 +185,6 @@ def remove_data_source_dumps(
     databases,
     config,
     borgmatic_runtime_directory,
-    patterns,
     dry_run,
 ):  # pragma: no cover
     '''

borgmatic/hooks/data_source/mysql.py

@@ -300,7 +300,6 @@ def remove_data_source_dumps(
     databases,
     config,
     borgmatic_runtime_directory,
-    patterns,
     dry_run,
 ):  # pragma: no cover
     '''

borgmatic/hooks/data_source/postgresql.py

@@ -274,7 +274,6 @@ def remove_data_source_dumps(
     databases,
     config,
     borgmatic_runtime_directory,
-    patterns,
     dry_run,
 ):  # pragma: no cover
     '''

borgmatic/hooks/data_source/sqlite.py

@@ -120,7 +120,6 @@ def remove_data_source_dumps(
     databases,
     config,
     borgmatic_runtime_directory,
-    patterns,
     dry_run,
 ):  # pragma: no cover
     '''

borgmatic/hooks/data_source/zfs.py

@@ -363,12 +363,12 @@ def get_all_snapshots(zfs_command):
     return tuple(line.rstrip() for line in list_output.splitlines())
 
 
-def remove_data_source_dumps(hook_config, config, borgmatic_runtime_directory, patterns, dry_run):  # noqa: PLR0912
+def remove_data_source_dumps(hook_config, config, borgmatic_runtime_directory, dry_run):  # noqa: PLR0912
     '''
-    Given a ZFS configuration dict, a configuration dict, the borgmatic runtime directory, the
-    configured patterns, and whether this is a dry run, unmount and destroy any ZFS snapshots
-    created by borgmatic. If this is a dry run or ZFS isn't configured in borgmatic's configuration,
-    then don't actually remove anything.
+    Given a ZFS configuration dict, a configuration dict, the borgmatic runtime directory, and
+    whether this is a dry run, unmount and destroy any ZFS snapshots created by borgmatic. If this
+    is a dry run or ZFS isn't configured in borgmatic's configuration, then don't actually remove
+    anything.
     '''
     if hook_config is None:
         return

borgmatic/hooks/monitoring/loki.py

@@ -65,13 +65,12 @@ class Loki_log_buffer:
             # Skip as there are not logs to send yet
             return
 
-        request_body = self.to_request()
         self.root['streams'][0]['values'] = []
 
         try:
             result = requests.post(
                 self.url,
-                data=request_body,
+                data=self.to_request(),
                 timeout=TIMEOUT_SECONDS,
                 headers={
                     'Content-Type': 'application/json',

docs/how-to/add-preparation-and-cleanup-steps-to-backups.md

@@ -77,12 +77,14 @@ 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:
+    * <span class="minilink minilink-addedin">New in version 2.0.9</span> `step` runs before or after each sub-action step for each repository, e.g. for an individual check.
     * `action` runs before or after each action for each repository. This replaces the deprecated `before_create`, `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`.
     * `error` runs after an error occurs—and it's only available for `after`. This replaces the deprecated `on_error` hook.
  * `when`: Only trigger the hook when borgmatic is run with particular actions (`create`, `prune`, etc.) listed here. Defaults to running for all actions.
+ * `steps`: <span class="minilink minilink-addedin">New in version 2.0.9</span> Only trigger the hook when borgmatic runs particular sub-action steps (`extract`, `spot`, etc.) listed here. Defaults to running for all steps.
  * `states`: <span class="minilink minilink-addedin">New in version 2.0.3</span> Only trigger the hook if borgmatic encounters one of the states (execution results) listed here. This state is evaluated only for the scope of the configured `action`, `repository`, etc., rather than for the entire borgmatic run. Only available for `after` hooks. Defaults to running the hook for all states. One or more of:
     * `finish`: No errors occurred.
     * `fail`: An error occurred.
@@ -107,17 +109,22 @@ execution.
 
 Let's say you've got a borgmatic configuration file with a configured
 repository. And suppose you configure several command hooks and then run
-borgmatic for the `create` and `prune` actions. Here's the order of execution:
+borgmatic for the `create` and `check` actions. Here's the order of execution:
 
  * Run `before: everything` hooks (from all configuration files).
     * Run `before: configuration` hooks (from the first configuration file).
         * Run `before: repository` hooks (for the first repository).
             * Run `before: action` hooks for `create`.
-            * Actually run the `create` action (e.g. `borg create`).
+                * Run the `create` action including `borg create`.
             * Run `after: action` hooks for `create`.
-            * Run `before: action` hooks for `prune`.
-            * Actually run the `prune` action (e.g. `borg prune`).
-            * Run `after: action` hooks for `prune`.
+            * Run `before: action` hooks for `check`.
+                * Run `before: step` hooks for the `archives_repository_data` step.
+                    * Run the `borg check` portion of the `check` action.
+                * Run `after: step` hooks for the `archives_repository_data` step.
+                * Run `before: step` hooks for the `spot` step.
+                    * Run the `spot` check portion of the `check` action.
+                * Run `after: step` hooks for the `spot` step.
+            * Run `after: action` hooks for `check`.
         * Run `after: repository` hooks (for the first repository).
     * Run `after: configuration` hooks (from the first configuration file).
     * Run `after: error` hooks (if an error occurs).
@@ -134,9 +141,9 @@ have a chance to run. Whereas the `after: error` hook doesn't run until all
 actions for—and repositories in—a configuration file have had a chance to
 execute.
 
-And if there are multiple hooks defined for a particular step (e.g. `before:
-action` for `create`), then those hooks are run in the order they're defined in
-configuration.
+And if there are multiple hooks defined for a particular combination (e.g.
+`before: action` for `create`), then those hooks are run in the order they're
+defined in configuration.
 
 
 ### Deprecated command hooks

docs/how-to/snapshot-your-filesystems.md

@@ -162,11 +162,12 @@ btrfs:
 ```
 
 No other options are necessary to enable Btrfs support, but if desired you can
-override the `btrfs` command used by the Btrfs hook. For instance:
+override some of the commands used by the Btrfs hook. For instance:
 
 ```yaml
 btrfs:
     btrfs_command: /usr/local/bin/btrfs
+    findmnt_command: /usr/local/bin/findmnt
 ```
 
 If you're using systemd to run borgmatic, you may need to modify the [sample systemd service
@@ -182,8 +183,8 @@ feedback](https://torsion.org/borgmatic/#issues) you have on this feature.
 #### Subvolume discovery
 
 For any read-write subvolume you'd like backed up, add its subvolume path to
-borgmatic's `source_directories` option. borgmatic does not currently support
-snapshotting read-only subvolumes.
+borgmatic's `source_directories` option. Btrfs does not support snapshotting
+read-only subvolumes.
 
 <span class="minilink minilink-addedin">New in version 2.0.7</span> The path can
 be either the path of the subvolume itself or the mount point where the

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "borgmatic"
-version = "2.0.9"
+version = "2.0.9.dev0"
 authors = [
   { name="Dan Helfman", email="witten@torsion.org" },
 ]

tests/end-to-end/commands/fake_btrfs.py

@@ -12,8 +12,9 @@ def parse_arguments(*unparsed_arguments):
     subvolume_parser = action_parsers.add_parser('subvolume')
     subvolume_subparser = subvolume_parser.add_subparsers(dest='subaction')
 
-    show_parser = subvolume_subparser.add_parser('show')
-    show_parser.add_argument('subvolume_path')
+    list_parser = subvolume_subparser.add_parser('list')
+    list_parser.add_argument('-s', dest='snapshots_only', action='store_true')
+    list_parser.add_argument('subvolume_path')
 
     snapshot_parser = subvolume_subparser.add_parser('snapshot')
     snapshot_parser.add_argument('-r', dest='read_only', action='store_true')
@@ -23,9 +24,6 @@ def parse_arguments(*unparsed_arguments):
     delete_parser = subvolume_subparser.add_parser('delete')
     delete_parser.add_argument('snapshot_path')
 
-    ensure_deleted_parser = subvolume_subparser.add_parser('ensure_deleted')
-    ensure_deleted_parser.add_argument('snapshot_path')
-
     property_parser = action_parsers.add_parser('property')
     property_subparser = property_parser.add_subparsers(dest='subaction')
     get_parser = property_subparser.add_parser('get')
@@ -36,6 +34,13 @@ def parse_arguments(*unparsed_arguments):
     return (global_parser, global_parser.parse_args(unparsed_arguments))
 
 
+BUILTIN_SUBVOLUME_LIST_LINES = (
+    '261 gen 29 top level 5 path sub',
+    '262 gen 29 top level 5 path other',
+)
+SUBVOLUME_LIST_LINE_PREFIX = '263 gen 29 top level 5 path '
+
+
 def load_snapshots():
     try:
         return json.load(open('/tmp/fake_btrfs.json'))
@@ -47,12 +52,18 @@ def save_snapshots(snapshot_paths):
     json.dump(snapshot_paths, open('/tmp/fake_btrfs.json', 'w'))
 
 
-def print_subvolume_show(arguments):
+def print_subvolume_list(arguments, snapshot_paths):
     assert arguments.subvolume_path == '/e2e/mnt/subvolume'
 
-    # borgmatic doesn't currently parse the output of "btrfs subvolume show"—it's just checking the
-    # exit code—so what we print in response doesn't matter in this test.
-    print('Totally legit btrfs subvolume!')
+    if not arguments.snapshots_only:
+        for line in BUILTIN_SUBVOLUME_LIST_LINES:
+            print(line)
+
+    for snapshot_path in snapshot_paths:
+        print(
+            SUBVOLUME_LIST_LINE_PREFIX
+            + snapshot_path[snapshot_path.index('.borgmatic-snapshot-') :],
+        )
 
 
 def main():
@@ -63,8 +74,8 @@ def main():
         global_parser.print_help()
         sys.exit(1)
 
-    if arguments.subaction == 'show':
-        print_subvolume_show(arguments)
+    if arguments.subaction == 'list':
+        print_subvolume_list(arguments, snapshot_paths)
     elif arguments.subaction == 'snapshot':
         snapshot_paths.append(arguments.snapshot_path)
         save_snapshots(snapshot_paths)
@@ -84,9 +95,6 @@ def main():
             if snapshot_path.endswith('/' + arguments.snapshot_path)
         ]
         save_snapshots(snapshot_paths)
-    # Not a real btrfs subcommand.
-    elif arguments.subaction == 'ensure_deleted':
-        assert arguments.snapshot_path not in snapshot_paths
     elif arguments.action == 'property' and arguments.subaction == 'get':
         print(f'{arguments.property_name}=false')
 

tests/end-to-end/commands/fake_findmnt.py

@@ -0,0 +1,42 @@
+import argparse
+import sys
+
+
+def parse_arguments(*unparsed_arguments):
+    parser = argparse.ArgumentParser(add_help=False)
+    parser.add_argument('-t', dest='type')
+    parser.add_argument('--json', action='store_true')
+    parser.add_argument('--list', action='store_true')
+
+    return parser.parse_args(unparsed_arguments)
+
+
+BUILTIN_FILESYSTEM_MOUNT_OUTPUT = '''{
+       "filesystems": [
+          {
+             "target": "/e2e/mnt/subvolume",
+             "source": "/dev/loop0",
+             "fstype": "btrfs",
+             "options": "rw,relatime,ssd,space_cache=v2,subvolid=5,subvol=/"
+          }
+       ]
+    }
+    '''
+
+
+def print_filesystem_mounts():
+    print(BUILTIN_FILESYSTEM_MOUNT_OUTPUT)
+
+
+def main():
+    arguments = parse_arguments(*sys.argv[1:])
+
+    assert arguments.type == 'btrfs'
+    assert arguments.json
+    assert arguments.list
+
+    print_filesystem_mounts()
+
+
+if __name__ == '__main__':
+    main()

tests/end-to-end/hooks/data_source/test_btrfs.py

@@ -24,6 +24,7 @@ def generate_configuration(config_path, repository_path):
         + 'encryption_passphrase: "test"\n'
         + 'btrfs:\n'
         + '    btrfs_command: python3 /app/tests/end-to-end/commands/fake_btrfs.py\n'
+        + '    findmnt_command: python3 /app/tests/end-to-end/commands/fake_findmnt.py\n'
     )
     config_file = open(config_path, 'w')
     config_file.write(config)
@@ -54,7 +55,7 @@ def test_btrfs_create_and_list():
 
         # Assert that the snapshot has been deleted.
         assert not subprocess.check_output(
-            'python3 /app/tests/end-to-end/commands/fake_btrfs.py subvolume ensure_deleted /e2e/mnt/subvolume'.split(
+            'python3 /app/tests/end-to-end/commands/fake_btrfs.py subvolume list -s /e2e/mnt/subvolume'.split(
                 ' ',
             ),
         )

tests/integration/hooks/monitoring/test_loki.py

@@ -15,7 +15,8 @@ def test_initialize_monitor_replaces_labels():
         'labels': {'hostname': '__hostname', 'config': '__config', 'config_full': '__config_path'},
     }
     config_filename = '/mock/path/test.yaml'
-    module.initialize_monitor(hook_config, flexmock(), config_filename, flexmock(), dry_run=False)
+    dry_run = True
+    module.initialize_monitor(hook_config, flexmock(), config_filename, flexmock(), dry_run)
 
     for handler in tuple(logging.getLogger().handlers):
         if isinstance(handler, module.Loki_log_handler):
@@ -47,39 +48,32 @@ def test_initialize_monitor_adds_log_handler():
     raise AssertionError()
 
 
-def test_ping_monitor_sends_log_message():
+def test_ping_monitor_adds_log_message():
     '''
-    Assert that calling ping_monitor sends a message to Loki via our logger.
+    Assert that calling ping_monitor adds a message to our logger.
     '''
     hook_config = {'url': 'http://localhost:3100/loki/api/v1/push', 'labels': {'app': 'borgmatic'}}
     config_filename = 'test.yaml'
-    post_called = False
-
-    def post(url, data, timeout, headers):
-        nonlocal post_called
-        post_called = True
-
-        assert any(
-            value[1] == f'{module.MONITOR_STATE_TO_LOKI[module.monitor.State.FINISH]} backup'
-            for value in module.json.loads(data)['streams'][0]['values']
-        )
-
-        return flexmock(raise_for_status=lambda: None)
-
-    flexmock(module.requests).should_receive('post').replace_with(post)
-
-    module.initialize_monitor(hook_config, flexmock(), config_filename, flexmock(), dry_run=False)
+    dry_run = True
+    module.initialize_monitor(hook_config, flexmock(), config_filename, flexmock(), dry_run)
     module.ping_monitor(
         hook_config,
         flexmock(),
         config_filename,
         module.monitor.State.FINISH,
         flexmock(),
-        dry_run=False,
+        dry_run,
     )
-    module.destroy_monitor(hook_config, flexmock(), flexmock(), dry_run=False)
 
-    assert post_called
+    for handler in tuple(logging.getLogger().handlers):
+        if isinstance(handler, module.Loki_log_handler):
+            assert any(
+                value[1] == f'{module.MONITOR_STATE_TO_LOKI[module.monitor.State.FINISH]} backup'
+                for value in handler.buffer.root['streams'][0]['values']
+            )
+            return
+
+    raise AssertionError()
 
 
 def test_destroy_monitor_removes_log_handler():
@@ -88,10 +82,9 @@ def test_destroy_monitor_removes_log_handler():
     '''
     hook_config = {'url': 'http://localhost:3100/loki/api/v1/push', 'labels': {'app': 'borgmatic'}}
     config_filename = 'test.yaml'
-    flexmock(module.requests).should_receive('post').never()
-
-    module.initialize_monitor(hook_config, flexmock(), config_filename, flexmock(), dry_run=False)
-    module.destroy_monitor(hook_config, flexmock(), flexmock(), dry_run=False)
+    dry_run = True
+    module.initialize_monitor(hook_config, flexmock(), config_filename, flexmock(), dry_run)
+    module.destroy_monitor(hook_config, flexmock(), flexmock(), dry_run)
 
     for handler in tuple(logging.getLogger().handlers):
         if isinstance(handler, module.Loki_log_handler):

tests/unit/actions/test_create.py

@@ -18,7 +18,6 @@ def test_run_create_executes_and_calls_hooks_for_configured_repository():
     flexmock(module.borgmatic.hooks.dispatch).should_receive(
         'call_hooks_even_if_unconfigured',
     ).and_return({})
-    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None)
     flexmock(module.borgmatic.actions.pattern).should_receive('collect_patterns').and_return(())
     flexmock(module.borgmatic.actions.pattern).should_receive('process_patterns').and_return([])
     flexmock(os.path).should_receive('join').and_return('/run/borgmatic/bootstrap')
@@ -61,7 +60,6 @@ def test_run_create_runs_with_selected_repository():
     flexmock(module.borgmatic.hooks.dispatch).should_receive(
         'call_hooks_even_if_unconfigured',
     ).and_return({})
-    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None)
     flexmock(module.borgmatic.actions.pattern).should_receive('collect_patterns').and_return(())
     flexmock(module.borgmatic.actions.pattern).should_receive('process_patterns').and_return([])
     flexmock(os.path).should_receive('join').and_return('/run/borgmatic/bootstrap')
@@ -209,7 +207,6 @@ def test_run_create_produces_json():
     flexmock(module.borgmatic.hooks.dispatch).should_receive(
         'call_hooks_even_if_unconfigured',
     ).and_return({})
-    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None)
     flexmock(module.borgmatic.actions.pattern).should_receive('collect_patterns').and_return(())
     flexmock(module.borgmatic.actions.pattern).should_receive('process_patterns').and_return([])
     flexmock(os.path).should_receive('join').and_return('/run/borgmatic/bootstrap')
@@ -255,7 +252,6 @@ def test_run_create_with_active_dumps_roundtrips_via_checkpoint_archive():
     flexmock(module.borgmatic.hooks.dispatch).should_receive(
         'call_hooks_even_if_unconfigured',
     ).and_return({})
-    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None)
     flexmock(module.borgmatic.actions.pattern).should_receive('collect_patterns').and_return(())
     flexmock(module.borgmatic.actions.pattern).should_receive('process_patterns').and_return([])
     flexmock(os.path).should_receive('join').and_return('/run/borgmatic/bootstrap')
@@ -339,7 +335,6 @@ def test_run_create_with_active_dumps_json_updates_archive_info():
     flexmock(module.borgmatic.hooks.dispatch).should_receive(
         'call_hooks_even_if_unconfigured',
     ).and_return({})
-    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None)
     flexmock(module.borgmatic.actions.pattern).should_receive('collect_patterns').and_return(())
     flexmock(module.borgmatic.actions.pattern).should_receive('process_patterns').and_return([])
     flexmock(os.path).should_receive('join').and_return('/run/borgmatic/bootstrap')

tests/unit/actions/test_pattern.py

@@ -375,97 +375,43 @@ def test_device_map_patterns_with_existing_device_id_does_not_overwrite_it():
 
 
 @pytest.mark.parametrize(
-    'patterns,borgmatic_runtime_directory,expected_patterns,one_file_system',
+    'patterns,expected_patterns,one_file_system',
     (
-        (
-            (Pattern('/', device=1), Pattern('/root', device=1)),
-            '/root',
-            (Pattern('/', device=1),),
-            False,
-        ),
-        # No deduplication is expected when borgmatic runtime directory is None.
-        (
-            (Pattern('/', device=1), Pattern('/root', device=1)),
-            None,
-            (Pattern('/', device=1), Pattern('/root', device=1)),
-            False,
-        ),
-        (
-            (Pattern('/', device=1), Pattern('/root/', device=1)),
-            '/root',
-            (Pattern('/', device=1),),
-            False,
-        ),
+        ((Pattern('/', device=1), Pattern('/root', device=1)), (Pattern('/', device=1),), False),
+        ((Pattern('/', device=1), Pattern('/root/', device=1)), (Pattern('/', device=1),), False),
         (
             (Pattern('/', device=1), Pattern('/root', device=2)),
-            '/root',
             (Pattern('/', device=1),),
             False,
         ),
-        (
-            (Pattern('/', device=1), Pattern('/root', device=2)),
-            None,
-            (Pattern('/', device=1), Pattern('/root', device=2)),
-            False,
-        ),
-        (
-            (Pattern('/root', device=1), Pattern('/', device=1)),
-            '/root',
-            (Pattern('/', device=1),),
-            False,
-        ),
-        (
-            (Pattern('/root', device=1), Pattern('/', device=1)),
-            None,
-            (Pattern('/root', device=1), Pattern('/', device=1)),
-            False,
-        ),
+        ((Pattern('/root', device=1), Pattern('/', device=1)), (Pattern('/', device=1),), False),
         (
             (Pattern('/root', device=1), Pattern('/root/foo', device=1)),
-            '/root/foo',
             (Pattern('/root', device=1),),
             False,
         ),
-        (
-            (Pattern('/root', device=1), Pattern('/root/foo', device=1)),
-            None,
-            (Pattern('/root', device=1), Pattern('/root/foo', device=1)),
-            False,
-        ),
-        # No deduplication is expected when the runtime directory doesn't match the patterns.
-        (
-            (Pattern('/root', device=1), Pattern('/root/foo', device=1)),
-            '/other',
-            (Pattern('/root', device=1), Pattern('/root/foo', device=1)),
-            False,
-        ),
         (
             (Pattern('/root/', device=1), Pattern('/root/foo', device=1)),
-            '/root/foo',
             (Pattern('/root/', device=1),),
             False,
         ),
         (
             (Pattern('/root', device=1), Pattern('/root/foo/', device=1)),
-            '/root/foo',
             (Pattern('/root', device=1),),
             False,
         ),
         (
             (Pattern('/root', device=1), Pattern('/root/foo', device=2)),
-            '/root/foo',
             (Pattern('/root', device=1),),
             False,
         ),
         (
             (Pattern('/root/foo', device=1), Pattern('/root', device=1)),
-            '/root/foo',
             (Pattern('/root', device=1),),
             False,
         ),
         (
             (Pattern('/root', device=None), Pattern('/root/foo', device=None)),
-            '/root/foo',
             (Pattern('/root'), Pattern('/root/foo')),
             False,
         ),
@@ -475,7 +421,6 @@ def test_device_map_patterns_with_existing_device_id_does_not_overwrite_it():
                 Pattern('/etc', device=1),
                 Pattern('/root/foo/bar', device=1),
             ),
-            '/root/foo/bar',
             (Pattern('/root', device=1), Pattern('/etc', device=1)),
             False,
         ),
@@ -485,126 +430,70 @@ def test_device_map_patterns_with_existing_device_id_does_not_overwrite_it():
                 Pattern('/root/foo', device=1),
                 Pattern('/root/foo/bar', device=1),
             ),
-            '/root/foo/bar',
             (Pattern('/root', device=1),),
             False,
         ),
-        (
-            (
-                Pattern('/root', device=1),
-                Pattern('/root/foo', device=1),
-                Pattern('/root/foo/bar', device=1),
-            ),
-            None,
-            (
-                Pattern('/root', device=1),
-                Pattern('/root/foo', device=1),
-                Pattern('/root/foo/bar', device=1),
-            ),
-            False,
-        ),
-        (
-            (
-                Pattern('/root', device=1),
-                Pattern('/root/foo', device=1),
-                Pattern('/root/foo/bar', device=1),
-            ),
-            '/other',
-            (
-                Pattern('/root', device=1),
-                Pattern('/root/foo', device=1),
-                Pattern('/root/foo/bar', device=1),
-            ),
-            False,
-        ),
         (
             (Pattern('/dup', device=1), Pattern('/dup', device=1)),
-            '/dup',
             (Pattern('/dup', device=1),),
             False,
         ),
         (
             (Pattern('/foo', device=1), Pattern('/bar', device=1)),
-            '/bar',
             (Pattern('/foo', device=1), Pattern('/bar', device=1)),
             False,
         ),
         (
             (Pattern('/foo', device=1), Pattern('/bar', device=2)),
-            '/bar',
             (Pattern('/foo', device=1), Pattern('/bar', device=2)),
             False,
         ),
-        ((Pattern('/root/foo', device=1),), '/root/foo', (Pattern('/root/foo', device=1),), False),
+        ((Pattern('/root/foo', device=1),), (Pattern('/root/foo', device=1),), False),
         (
             (Pattern('/', device=1), Pattern('/root', Pattern_type.INCLUDE, device=1)),
-            '/root',
             (Pattern('/', device=1), Pattern('/root', Pattern_type.INCLUDE, device=1)),
             False,
         ),
         (
             (Pattern('/root', Pattern_type.INCLUDE, device=1), Pattern('/', device=1)),
-            '/root',
             (Pattern('/root', Pattern_type.INCLUDE, device=1), Pattern('/', device=1)),
             False,
         ),
-        (
-            (Pattern('/', device=1), Pattern('/root', device=1)),
-            '/root',
-            (Pattern('/', device=1),),
-            True,
-        ),
-        (
-            (Pattern('/', device=1), Pattern('/root/', device=1)),
-            '/root',
-            (Pattern('/', device=1),),
-            True,
-        ),
+        ((Pattern('/', device=1), Pattern('/root', device=1)), (Pattern('/', device=1),), True),
+        ((Pattern('/', device=1), Pattern('/root/', device=1)), (Pattern('/', device=1),), True),
         (
             (Pattern('/', device=1), Pattern('/root', device=2)),
-            '/root',
             (Pattern('/', device=1), Pattern('/root', device=2)),
             True,
         ),
-        (
-            (Pattern('/root', device=1), Pattern('/', device=1)),
-            '/root',
-            (Pattern('/', device=1),),
-            True,
-        ),
+        ((Pattern('/root', device=1), Pattern('/', device=1)), (Pattern('/', device=1),), True),
         (
             (Pattern('/root', device=1), Pattern('/root/foo', device=1)),
-            '/root/foo',
             (Pattern('/root', device=1),),
             True,
         ),
         (
             (Pattern('/root/', device=1), Pattern('/root/foo', device=1)),
-            '/root/foo',
             (Pattern('/root/', device=1),),
             True,
         ),
         (
             (Pattern('/root', device=1), Pattern('/root/foo/', device=1)),
-            '/root/foo',
             (Pattern('/root', device=1),),
             True,
         ),
         (
             (Pattern('/root', device=1), Pattern('/root/foo', device=2)),
-            '/root/foo',
             (Pattern('/root', device=1), Pattern('/root/foo', device=2)),
             True,
         ),
         (
             (Pattern('/root/foo', device=1), Pattern('/root', device=1)),
-            '/root/foo',
             (Pattern('/root', device=1),),
             True,
         ),
         (
             (Pattern('/root', device=None), Pattern('/root/foo', device=None)),
-            '/root/foo',
             (Pattern('/root'), Pattern('/root/foo')),
             True,
         ),
@@ -614,7 +503,6 @@ def test_device_map_patterns_with_existing_device_id_does_not_overwrite_it():
                 Pattern('/etc', device=1),
                 Pattern('/root/foo/bar', device=1),
             ),
-            '/root/foo/bar',
             (Pattern('/root', device=1), Pattern('/etc', device=1)),
             True,
         ),
@@ -624,59 +512,50 @@ def test_device_map_patterns_with_existing_device_id_does_not_overwrite_it():
                 Pattern('/root/foo', device=1),
                 Pattern('/root/foo/bar', device=1),
             ),
-            '/root/foo/bar',
             (Pattern('/root', device=1),),
             True,
         ),
         (
             (Pattern('/dup', device=1), Pattern('/dup', device=1)),
-            '/dup',
             (Pattern('/dup', device=1),),
             True,
         ),
         (
             (Pattern('/foo', device=1), Pattern('/bar', device=1)),
-            '/bar',
             (Pattern('/foo', device=1), Pattern('/bar', device=1)),
             True,
         ),
         (
             (Pattern('/foo', device=1), Pattern('/bar', device=2)),
-            '/bar',
             (Pattern('/foo', device=1), Pattern('/bar', device=2)),
             True,
         ),
-        ((Pattern('/root/foo', device=1),), '/root/foo', (Pattern('/root/foo', device=1),), True),
+        ((Pattern('/root/foo', device=1),), (Pattern('/root/foo', device=1),), True),
         (
             (Pattern('/', device=1), Pattern('/root', Pattern_type.INCLUDE, device=1)),
-            '/root',
             (Pattern('/', device=1), Pattern('/root', Pattern_type.INCLUDE, device=1)),
             True,
         ),
         (
             (Pattern('/root', Pattern_type.INCLUDE, device=1), Pattern('/', device=1)),
-            '/root',
             (Pattern('/root', Pattern_type.INCLUDE, device=1), Pattern('/', device=1)),
             True,
         ),
     ),
 )
-def test_deduplicate_runtime_directory_patterns_omits_child_paths_based_on_device_and_one_file_system(
+def test_deduplicate_patterns_omits_child_paths_based_on_device_and_one_file_system(
     patterns,
-    borgmatic_runtime_directory,
     expected_patterns,
     one_file_system,
 ):
     assert (
-        module.deduplicate_runtime_directory_patterns(
-            patterns, {'one_file_system': one_file_system}, borgmatic_runtime_directory
-        )
+        module.deduplicate_patterns(patterns, {'one_file_system': one_file_system})
         == expected_patterns
     )
 
 
 def test_process_patterns_includes_patterns():
-    flexmock(module).should_receive('deduplicate_runtime_directory_patterns').and_return(
+    flexmock(module).should_receive('deduplicate_patterns').and_return(
         (Pattern('foo'), Pattern('bar')),
     )
     flexmock(module).should_receive('device_map_patterns').and_return({})
@@ -695,7 +574,7 @@ def test_process_patterns_includes_patterns():
 
 def test_process_patterns_skips_expand_for_requested_paths():
     skip_paths = {flexmock()}
-    flexmock(module).should_receive('deduplicate_runtime_directory_patterns').and_return(
+    flexmock(module).should_receive('deduplicate_patterns').and_return(
         (Pattern('foo'), Pattern('bar')),
     )
     flexmock(module).should_receive('device_map_patterns').and_return({})

tests/unit/actions/test_restore.py

@@ -1176,9 +1176,6 @@ def test_run_restore_restores_each_data_source():
     flexmock(module.borgmatic.config.paths).should_receive(
         'make_runtime_directory_glob',
     ).replace_with(lambda path: path)
-    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None)
-    flexmock(module.borgmatic.actions.pattern).should_receive('collect_patterns').and_return(())
-    flexmock(module.borgmatic.actions.pattern).should_receive('process_patterns').and_return([])
     flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hooks_even_if_unconfigured')
     flexmock(module.borgmatic.borg.repo_list).should_receive('resolve_archive_name').and_return(
         flexmock(),
@@ -1248,9 +1245,6 @@ def test_run_restore_bails_for_non_matching_repository():
     flexmock(module.borgmatic.config.paths).should_receive(
         'make_runtime_directory_glob',
     ).replace_with(lambda path: path)
-    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None)
-    flexmock(module.borgmatic.actions.pattern).should_receive('collect_patterns').and_return(())
-    flexmock(module.borgmatic.actions.pattern).should_receive('process_patterns').and_return([])
     flexmock(module.borgmatic.hooks.dispatch).should_receive(
         'call_hooks_even_if_unconfigured',
     ).never()
@@ -1280,9 +1274,6 @@ def test_run_restore_restores_data_source_by_falling_back_to_all_name():
     flexmock(module.borgmatic.config.paths).should_receive(
         'make_runtime_directory_glob',
     ).replace_with(lambda path: path)
-    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None)
-    flexmock(module.borgmatic.actions.pattern).should_receive('collect_patterns').and_return(())
-    flexmock(module.borgmatic.actions.pattern).should_receive('process_patterns').and_return([])
     flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hooks_even_if_unconfigured')
     flexmock(module.borgmatic.borg.repo_list).should_receive('resolve_archive_name').and_return(
         flexmock(),
@@ -1343,9 +1334,6 @@ def test_run_restore_restores_data_source_configured_with_all_name():
     flexmock(module.borgmatic.config.paths).should_receive(
         'make_runtime_directory_glob',
     ).replace_with(lambda path: path)
-    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None)
-    flexmock(module.borgmatic.actions.pattern).should_receive('collect_patterns').and_return(())
-    flexmock(module.borgmatic.actions.pattern).should_receive('process_patterns').and_return([])
     flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hooks_even_if_unconfigured')
     flexmock(module.borgmatic.borg.repo_list).should_receive('resolve_archive_name').and_return(
         flexmock(),
@@ -1428,9 +1416,6 @@ def test_run_restore_skips_missing_data_source():
     flexmock(module.borgmatic.config.paths).should_receive(
         'make_runtime_directory_glob',
     ).replace_with(lambda path: path)
-    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None)
-    flexmock(module.borgmatic.actions.pattern).should_receive('collect_patterns').and_return(())
-    flexmock(module.borgmatic.actions.pattern).should_receive('process_patterns').and_return([])
     flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hooks_even_if_unconfigured')
     flexmock(module.borgmatic.borg.repo_list).should_receive('resolve_archive_name').and_return(
         flexmock(),
@@ -1513,9 +1498,6 @@ def test_run_restore_restores_data_sources_from_different_hooks():
     flexmock(module.borgmatic.config.paths).should_receive(
         'make_runtime_directory_glob',
     ).replace_with(lambda path: path)
-    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None)
-    flexmock(module.borgmatic.actions.pattern).should_receive('collect_patterns').and_return(())
-    flexmock(module.borgmatic.actions.pattern).should_receive('process_patterns').and_return([])
     flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hooks_even_if_unconfigured')
     flexmock(module.borgmatic.borg.repo_list).should_receive('resolve_archive_name').and_return(
         flexmock(),

tests/unit/hooks/data_source/test_bootstrap.py

@@ -76,7 +76,6 @@ def test_remove_data_source_dumps_deletes_manifest_and_parent_directory():
         hook_config=None,
         config={},
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -93,7 +92,6 @@ def test_remove_data_source_dumps_with_dry_run_bails():
         hook_config=None,
         config={},
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=True,
     )
 
@@ -112,7 +110,6 @@ def test_remove_data_source_dumps_swallows_manifest_file_not_found_error():
         hook_config=None,
         config={},
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -133,6 +130,5 @@ def test_remove_data_source_dumps_swallows_manifest_parent_directory_not_found_e
         hook_config=None,
         config={},
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )

tests/unit/hooks/data_source/test_btrfs.py

@@ -5,40 +5,100 @@ from borgmatic.borg.pattern import Pattern, Pattern_source, Pattern_style, Patte
 from borgmatic.hooks.data_source import btrfs as module
 
 
-def test_path_is_a_subvolume_with_btrfs_success_call_returns_true():
-    module.path_is_a_subvolume.cache_clear()
+def test_get_contained_subvolume_paths_parses_btrfs_output():
     flexmock(module.borgmatic.execute).should_receive(
-        'execute_command',
-    ).with_args(('btrfs', 'subvolume', 'show', '/mnt0'), output_log_level=None, close_fds=True)
+        'execute_command_and_capture_output',
+    ).with_args(('btrfs', 'subvolume', 'list', '/mnt0'), close_fds=True).and_return(
+        'ID 256 gen 28 top level 5 path @sub\nID 258 gen 17 top level 5 path snap\n\n',
+    )
 
-    assert module.path_is_a_subvolume('btrfs', '/mnt0') is True
+    assert module.get_contained_subvolume_paths('btrfs', '/mnt0') == (
+        '/mnt0',
+        '/mnt0/@sub',
+        '/mnt0/snap',
+    )
 
 
-def test_path_is_a_subvolume_with_btrfs_error_returns_false():
-    module.path_is_a_subvolume.cache_clear()
+def test_get_contained_subvolume_paths_swallows_called_process_error():
     flexmock(module.borgmatic.execute).should_receive(
-        'execute_command',
-    ).with_args(
-        ('btrfs', 'subvolume', 'show', '/mnt0'), output_log_level=None, close_fds=True
-    ).and_raise(
+        'execute_command_and_capture_output',
+    ).with_args(('btrfs', 'subvolume', 'list', '/mnt0'), close_fds=True).and_raise(
         module.subprocess.CalledProcessError(1, 'btrfs'),
     )
 
-    assert module.path_is_a_subvolume('btrfs', '/mnt0') is False
+    assert module.get_contained_subvolume_paths('btrfs', '/mnt0') == ()
 
 
-def test_path_is_a_subvolume_caches_result_after_first_call():
-    module.path_is_a_subvolume.cache_clear()
+def test_get_all_subvolume_paths_parses_findmnt_output():
     flexmock(module.borgmatic.execute).should_receive(
-        'execute_command',
-    ).once()
+        'execute_command_and_capture_output',
+    ).and_return(
+        '''{
+           "filesystems": [
+              {
+                 "target": "/mnt0",
+                 "source": "/dev/loop0",
+                 "fstype": "btrfs",
+                 "options": "rw,relatime,ssd,space_cache=v2,subvolid=5,subvol=/"
+              },
+              {
+                 "target": "/mnt1",
+                 "source": "/dev/loop0",
+                 "fstype": "btrfs",
+                 "options": "rw,relatime,ssd,space_cache=v2,subvolid=5,subvol=/"
+              },
+              {
+                 "target": "/mnt2",
+                 "source": "/dev/loop0",
+                 "fstype": "btrfs",
+                 "options": "rw,relatime,ssd,space_cache=v2,subvolid=256,subvol=/"
+              }
+           ]
+        }
+        ''',
+    )
+    flexmock(module).should_receive('get_contained_subvolume_paths').with_args(
+        'btrfs',
+        '/mnt0',
+    ).and_return(('/mnt0',))
+    flexmock(module).should_receive('get_contained_subvolume_paths').with_args(
+        'btrfs',
+        '/mnt1',
+    ).and_return(('/mnt1', '/mnt1/sub'))
+    flexmock(module).should_receive('get_contained_subvolume_paths').with_args(
+        'btrfs',
+        '/mnt2',
+    ).never()
 
-    assert module.path_is_a_subvolume('btrfs', '/mnt0') is True
-    assert module.path_is_a_subvolume('btrfs', '/mnt0') is True
+    assert module.get_all_subvolume_paths('btrfs', 'findmnt') == (
+        '/mnt0',
+        '/mnt1',
+        '/mnt1/sub',
+        '/mnt2',
+    )
+
+
+def test_get_all_subvolume_paths_with_invalid_findmnt_json_errors():
+    flexmock(module.borgmatic.execute).should_receive(
+        'execute_command_and_capture_output',
+    ).and_return('{')
+    flexmock(module).should_receive('get_contained_subvolume_paths').never()
+
+    with pytest.raises(ValueError):
+        module.get_all_subvolume_paths('btrfs', 'findmnt')
+
+
+def test_get_all_subvolume_paths_with_findmnt_json_missing_filesystems_errors():
+    flexmock(module.borgmatic.execute).should_receive(
+        'execute_command_and_capture_output',
+    ).and_return('{"wtf": "something is wrong here"}')
+    flexmock(module).should_receive('get_contained_subvolume_paths').never()
+
+    with pytest.raises(ValueError):
+        module.get_all_subvolume_paths('btrfs', 'findmnt')
 
 
 def test_get_subvolume_property_with_invalid_btrfs_output_errors():
-    module.get_subvolume_property.cache_clear()
     flexmock(module.borgmatic.execute).should_receive(
         'execute_command_and_capture_output',
     ).and_return('invalid')
@@ -48,7 +108,6 @@ def test_get_subvolume_property_with_invalid_btrfs_output_errors():
 
 
 def test_get_subvolume_property_with_true_output_returns_true_bool():
-    module.get_subvolume_property.cache_clear()
     flexmock(module.borgmatic.execute).should_receive(
         'execute_command_and_capture_output',
     ).and_return('ro=true')
@@ -57,7 +116,6 @@ def test_get_subvolume_property_with_true_output_returns_true_bool():
 
 
 def test_get_subvolume_property_with_false_output_returns_false_bool():
-    module.get_subvolume_property.cache_clear()
     flexmock(module.borgmatic.execute).should_receive(
         'execute_command_and_capture_output',
     ).and_return('ro=false')
@@ -66,7 +124,6 @@ def test_get_subvolume_property_with_false_output_returns_false_bool():
 
 
 def test_get_subvolume_property_passes_through_general_value():
-    module.get_subvolume_property.cache_clear()
     flexmock(module.borgmatic.execute).should_receive(
         'execute_command_and_capture_output',
     ).and_return('thing=value')
@@ -74,187 +131,52 @@ def test_get_subvolume_property_passes_through_general_value():
     assert module.get_subvolume_property('btrfs', '/foo', 'thing') == 'value'
 
 
-def test_get_subvolume_property_caches_result_after_first_call():
-    module.get_subvolume_property.cache_clear()
-    flexmock(module.borgmatic.execute).should_receive(
-        'execute_command_and_capture_output',
-    ).and_return('thing=value').once()
-
-    assert module.get_subvolume_property('btrfs', '/foo', 'thing') == 'value'
-    assert module.get_subvolume_property('btrfs', '/foo', 'thing') == 'value'
-
-
-def test_get_containing_subvolume_path_with_subvolume_self_returns_it():
-    flexmock(module).should_receive('path_is_a_subvolume').with_args(
-        'btrfs', '/foo/bar/baz'
+def test_omit_read_only_subvolume_paths_filters_out_read_only_subvolumes():
+    flexmock(module).should_receive('get_subvolume_property').with_args(
+        'btrfs',
+        '/foo',
+        'ro',
+    ).and_return(False)
+    flexmock(module).should_receive('get_subvolume_property').with_args(
+        'btrfs',
+        '/bar',
+        'ro',
     ).and_return(True)
-    flexmock(module).should_receive('path_is_a_subvolume').with_args('btrfs', '/foo/bar').never()
-    flexmock(module).should_receive('path_is_a_subvolume').with_args('btrfs', '/foo').never()
-    flexmock(module).should_receive('path_is_a_subvolume').with_args('btrfs', '/').never()
-    flexmock(module).should_receive('get_subvolume_property').and_return(False)
-
-    assert module.get_containing_subvolume_path('btrfs', '/foo/bar/baz') == '/foo/bar/baz'
-
-
-def test_get_containing_subvolume_path_with_subvolume_parent_returns_it():
-    flexmock(module).should_receive('path_is_a_subvolume').with_args(
-        'btrfs', '/foo/bar/baz'
+    flexmock(module).should_receive('get_subvolume_property').with_args(
+        'btrfs',
+        '/baz',
+        'ro',
     ).and_return(False)
-    flexmock(module).should_receive('path_is_a_subvolume').with_args(
-        'btrfs', '/foo/bar'
-    ).and_return(True)
-    flexmock(module).should_receive('path_is_a_subvolume').with_args('btrfs', '/foo').never()
-    flexmock(module).should_receive('path_is_a_subvolume').with_args('btrfs', '/').never()
-    flexmock(module).should_receive('get_subvolume_property').and_return(False)
 
-    assert module.get_containing_subvolume_path('btrfs', '/foo/bar/baz') == '/foo/bar'
-
-
-def test_get_containing_subvolume_path_with_subvolume_grandparent_returns_it():
-    flexmock(module).should_receive('path_is_a_subvolume').with_args(
-        'btrfs', '/foo/bar/baz'
-    ).and_return(False)
-    flexmock(module).should_receive('path_is_a_subvolume').with_args(
-        'btrfs', '/foo/bar'
-    ).and_return(False)
-    flexmock(module).should_receive('path_is_a_subvolume').with_args('btrfs', '/foo').and_return(
-        True
-    )
-    flexmock(module).should_receive('path_is_a_subvolume').with_args('btrfs', '/').never()
-    flexmock(module).should_receive('get_subvolume_property').and_return(False)
-
-    assert module.get_containing_subvolume_path('btrfs', '/foo/bar/baz') == '/foo'
-
-
-def test_get_containing_subvolume_path_without_subvolume_ancestor_returns_none():
-    flexmock(module).should_receive('path_is_a_subvolume').with_args(
-        'btrfs', '/foo/bar/baz'
-    ).and_return(False)
-    flexmock(module).should_receive('path_is_a_subvolume').with_args(
-        'btrfs', '/foo/bar'
-    ).and_return(False)
-    flexmock(module).should_receive('path_is_a_subvolume').with_args('btrfs', '/foo').and_return(
-        False
-    )
-    flexmock(module).should_receive('path_is_a_subvolume').with_args('btrfs', '/').and_return(False)
-    flexmock(module).should_receive('get_subvolume_property').and_return(False)
-
-    assert module.get_containing_subvolume_path('btrfs', '/foo/bar/baz') is None
-
-
-def test_get_containing_subvolume_path_with_read_only_subvolume_returns_none():
-    flexmock(module).should_receive('path_is_a_subvolume').with_args(
-        'btrfs', '/foo/bar/baz'
-    ).and_return(True)
-    flexmock(module).should_receive('get_subvolume_property').and_return(True)
-
-    assert module.get_containing_subvolume_path('btrfs', '/foo/bar/baz') is None
-
-
-def test_get_containing_subvolume_path_with_read_only_error_returns_none():
-    flexmock(module).should_receive('path_is_a_subvolume').with_args(
-        'btrfs', '/foo/bar/baz'
-    ).and_return(True)
-    flexmock(module).should_receive('get_subvolume_property').and_raise(
-        module.subprocess.CalledProcessError(1, 'wtf')
+    assert module.omit_read_only_subvolume_paths('btrfs', ('/foo', '/bar', '/baz')) == (
+        '/foo',
+        '/baz',
     )
 
-    assert module.get_containing_subvolume_path('btrfs', '/foo/bar/baz') is None
 
-
-def test_get_all_subvolume_paths_skips_non_root_and_non_config_patterns():
-    flexmock(module).should_receive('get_containing_subvolume_path').with_args(
-        'btrfs', '/foo'
-    ).never()
-    flexmock(module).should_receive('get_containing_subvolume_path').with_args(
-        'btrfs', '/bar'
-    ).and_return('/bar').once()
-    flexmock(module).should_receive('get_containing_subvolume_path').with_args(
-        'btrfs', '/baz'
-    ).never()
-
-    assert module.get_all_subvolume_paths(
+def test_omit_read_only_subvolume_paths_filters_out_erroring_subvolumes():
+    flexmock(module).should_receive('get_subvolume_property').with_args(
         'btrfs',
-        (
-            module.borgmatic.borg.pattern.Pattern(
-                '/foo',
-                type=module.borgmatic.borg.pattern.Pattern_type.ROOT,
-                source=module.borgmatic.borg.pattern.Pattern_source.HOOK,
-            ),
-            module.borgmatic.borg.pattern.Pattern(
-                '/bar',
-                type=module.borgmatic.borg.pattern.Pattern_type.ROOT,
-                source=module.borgmatic.borg.pattern.Pattern_source.CONFIG,
-            ),
-            module.borgmatic.borg.pattern.Pattern(
-                '/baz',
-                type=module.borgmatic.borg.pattern.Pattern_type.INCLUDE,
-                source=module.borgmatic.borg.pattern.Pattern_source.CONFIG,
-            ),
-        ),
-    ) == ('/bar',)
-
-
-def test_get_all_subvolume_paths_skips_non_btrfs_patterns():
-    flexmock(module).should_receive('get_containing_subvolume_path').with_args(
-        'btrfs', '/foo'
-    ).and_return(None).once()
-    flexmock(module).should_receive('get_containing_subvolume_path').with_args(
-        'btrfs', '/bar'
-    ).and_return('/bar').once()
-
-    assert module.get_all_subvolume_paths(
+        '/foo',
+        'ro',
+    ).and_raise(module.subprocess.CalledProcessError(1, 'btrfs'))
+    flexmock(module).should_receive('get_subvolume_property').with_args(
         'btrfs',
-        (
-            module.borgmatic.borg.pattern.Pattern(
-                '/foo',
-                type=module.borgmatic.borg.pattern.Pattern_type.ROOT,
-                source=module.borgmatic.borg.pattern.Pattern_source.CONFIG,
-            ),
-            module.borgmatic.borg.pattern.Pattern(
-                '/bar',
-                type=module.borgmatic.borg.pattern.Pattern_type.ROOT,
-                source=module.borgmatic.borg.pattern.Pattern_source.CONFIG,
-            ),
-        ),
-    ) == ('/bar',)
-
-
-def test_get_all_subvolume_paths_sorts_subvolume_paths():
-    flexmock(module).should_receive('get_containing_subvolume_path').with_args(
-        'btrfs', '/foo'
-    ).and_return('/foo').once()
-    flexmock(module).should_receive('get_containing_subvolume_path').with_args(
-        'btrfs', '/bar'
-    ).and_return('/bar').once()
-    flexmock(module).should_receive('get_containing_subvolume_path').with_args(
-        'btrfs', '/baz'
-    ).and_return('/baz').once()
-
-    assert module.get_all_subvolume_paths(
+        '/bar',
+        'ro',
+    ).and_return(True)
+    flexmock(module).should_receive('get_subvolume_property').with_args(
         'btrfs',
-        (
-            module.borgmatic.borg.pattern.Pattern(
-                '/foo',
-                type=module.borgmatic.borg.pattern.Pattern_type.ROOT,
-                source=module.borgmatic.borg.pattern.Pattern_source.CONFIG,
-            ),
-            module.borgmatic.borg.pattern.Pattern(
-                '/bar',
-                type=module.borgmatic.borg.pattern.Pattern_type.ROOT,
-                source=module.borgmatic.borg.pattern.Pattern_source.CONFIG,
-            ),
-            module.borgmatic.borg.pattern.Pattern(
-                '/baz',
-                type=module.borgmatic.borg.pattern.Pattern_type.ROOT,
-                source=module.borgmatic.borg.pattern.Pattern_source.CONFIG,
-            ),
-        ),
-    ) == ('/bar', '/baz', '/foo')
+        '/baz',
+        'ro',
+    ).and_return(False)
+
+    assert module.omit_read_only_subvolume_paths('btrfs', ('/foo', '/bar', '/baz')) == ('/baz',)
 
 
 def test_get_subvolumes_collects_subvolumes_matching_patterns():
     flexmock(module).should_receive('get_all_subvolume_paths').and_return(('/mnt1', '/mnt2'))
+    flexmock(module).should_receive('omit_read_only_subvolume_paths').and_return(('/mnt1', '/mnt2'))
 
     contained_pattern = Pattern(
         '/mnt1',
@@ -270,6 +192,7 @@ def test_get_subvolumes_collects_subvolumes_matching_patterns():
 
     assert module.get_subvolumes(
         'btrfs',
+        'findmnt',
         patterns=[
             Pattern('/mnt1'),
             Pattern('/mnt3'),
@@ -279,6 +202,7 @@ def test_get_subvolumes_collects_subvolumes_matching_patterns():
 
 def test_get_subvolumes_skips_non_root_patterns():
     flexmock(module).should_receive('get_all_subvolume_paths').and_return(('/mnt1', '/mnt2'))
+    flexmock(module).should_receive('omit_read_only_subvolume_paths').and_return(('/mnt1', '/mnt2'))
 
     flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive(
         'get_contained_patterns',
@@ -298,6 +222,7 @@ def test_get_subvolumes_skips_non_root_patterns():
     assert (
         module.get_subvolumes(
             'btrfs',
+            'findmnt',
             patterns=[
                 Pattern('/mnt1'),
                 Pattern('/mnt3'),
@@ -309,6 +234,7 @@ def test_get_subvolumes_skips_non_root_patterns():
 
 def test_get_subvolumes_skips_non_config_patterns():
     flexmock(module).should_receive('get_all_subvolume_paths').and_return(('/mnt1', '/mnt2'))
+    flexmock(module).should_receive('omit_read_only_subvolume_paths').and_return(('/mnt1', '/mnt2'))
 
     flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive(
         'get_contained_patterns',
@@ -328,6 +254,7 @@ def test_get_subvolumes_skips_non_config_patterns():
     assert (
         module.get_subvolumes(
             'btrfs',
+            'findmnt',
             patterns=[
                 Pattern('/mnt1'),
                 Pattern('/mnt3'),
@@ -337,6 +264,23 @@ def test_get_subvolumes_skips_non_config_patterns():
     )
 
 
+def test_get_subvolumes_without_patterns_collects_all_subvolumes():
+    flexmock(module).should_receive('get_all_subvolume_paths').and_return(('/mnt1', '/mnt2'))
+    flexmock(module).should_receive('omit_read_only_subvolume_paths').and_return(('/mnt1', '/mnt2'))
+
+    flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive(
+        'get_contained_patterns',
+    ).with_args('/mnt1', object).and_return((Pattern('/mnt1'),))
+    flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive(
+        'get_contained_patterns',
+    ).with_args('/mnt2', object).and_return((Pattern('/mnt2'),))
+
+    assert module.get_subvolumes('btrfs', 'findmnt') == (
+        module.Subvolume('/mnt1', contained_patterns=(Pattern('/mnt1'),)),
+        module.Subvolume('/mnt2', contained_patterns=(Pattern('/mnt2'),)),
+    )
+
+
 @pytest.mark.parametrize(
     'subvolume_path,expected_snapshot_path',
     (
@@ -539,12 +483,12 @@ def test_dump_data_sources_uses_custom_btrfs_command_in_commands():
     }
 
 
-def test_dump_data_sources_with_findmnt_command_warns():
+def test_dump_data_sources_uses_custom_findmnt_command_in_commands():
     patterns = [Pattern('/foo'), Pattern('/mnt/subvol1')]
     config = {'btrfs': {'findmnt_command': '/usr/local/bin/findmnt'}}
-    flexmock(module.logger).should_receive('warning').once()
     flexmock(module).should_receive('get_subvolumes').with_args(
         'btrfs',
+        '/usr/local/bin/findmnt',
         patterns,
     ).and_return(
         (module.Subvolume('/mnt/subvol1', contained_patterns=(Pattern('/mnt/subvol1'),)),),
@@ -829,7 +773,6 @@ def test_remove_data_source_dumps_deletes_snapshots():
         hook_config=config['btrfs'],
         config=config,
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -847,7 +790,6 @@ def test_remove_data_source_dumps_without_hook_configuration_bails():
         hook_config=None,
         config={'source_directories': '/mnt/subvolume'},
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -866,7 +808,6 @@ def test_remove_data_source_dumps_with_get_subvolumes_file_not_found_error_bails
         hook_config=config['btrfs'],
         config=config,
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -887,7 +828,6 @@ def test_remove_data_source_dumps_with_get_subvolumes_called_process_error_bails
         hook_config=config['btrfs'],
         config=config,
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -947,7 +887,6 @@ def test_remove_data_source_dumps_with_dry_run_skips_deletes():
         hook_config=config['btrfs'],
         config=config,
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=True,
     )
 
@@ -966,7 +905,6 @@ def test_remove_data_source_dumps_without_subvolumes_skips_deletes():
         hook_config=config['btrfs'],
         config=config,
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -1006,7 +944,6 @@ def test_remove_data_source_without_snapshots_skips_deletes():
         hook_config=config['btrfs'],
         config=config,
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -1066,7 +1003,6 @@ def test_remove_data_source_dumps_with_delete_snapshot_file_not_found_error_bail
         hook_config=config['btrfs'],
         config=config,
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -1128,7 +1064,6 @@ def test_remove_data_source_dumps_with_delete_snapshot_called_process_error_bail
         hook_config=config['btrfs'],
         config=config,
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -1172,6 +1107,5 @@ def test_remove_data_source_dumps_with_root_subvolume_skips_duplicate_removal():
         hook_config=config['btrfs'],
         config=config,
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )

tests/unit/hooks/data_source/test_lvm.py

@@ -912,7 +912,6 @@ def test_remove_data_source_dumps_unmounts_and_remove_snapshots():
         hook_config=config['lvm'],
         config=config,
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -929,7 +928,6 @@ def test_remove_data_source_dumps_bails_for_missing_lvm_configuration():
         hook_config=None,
         config={'source_directories': '/mnt/lvolume'},
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -947,7 +945,6 @@ def test_remove_data_source_dumps_bails_for_missing_lsblk_command():
         hook_config=config['lvm'],
         config=config,
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -967,7 +964,6 @@ def test_remove_data_source_dumps_bails_for_lsblk_command_error():
         hook_config=config['lvm'],
         config=config,
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -1014,7 +1010,6 @@ def test_remove_data_source_dumps_with_missing_snapshot_directory_skips_unmount(
         hook_config=config['lvm'],
         config=config,
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -1075,7 +1070,6 @@ def test_remove_data_source_dumps_with_missing_snapshot_mount_path_skips_unmount
         hook_config=config['lvm'],
         config=config,
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -1141,7 +1135,6 @@ def test_remove_data_source_dumps_with_empty_snapshot_mount_path_skips_unmount()
         hook_config=config['lvm'],
         config=config,
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -1202,7 +1195,6 @@ def test_remove_data_source_dumps_with_successful_mount_point_removal_skips_unmo
         hook_config=config['lvm'],
         config=config,
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -1249,7 +1241,6 @@ def test_remove_data_source_dumps_bails_for_missing_umount_command():
         hook_config=config['lvm'],
         config=config,
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -1302,7 +1293,6 @@ def test_remove_data_source_dumps_swallows_umount_command_error():
         hook_config=config['lvm'],
         config=config,
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -1349,7 +1339,6 @@ def test_remove_data_source_dumps_bails_for_missing_lvs_command():
         hook_config=config['lvm'],
         config=config,
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -1398,7 +1387,6 @@ def test_remove_data_source_dumps_bails_for_lvs_command_error():
         hook_config=config['lvm'],
         config=config,
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -1442,6 +1430,5 @@ def test_remove_data_source_with_dry_run_skips_snapshot_unmount_and_delete():
         hook_config=config['lvm'],
         config=config,
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=True,
     )

tests/unit/hooks/data_source/test_zfs.py

@@ -524,7 +524,6 @@ def test_remove_data_source_dumps_unmounts_and_destroys_snapshots():
         hook_config={},
         config={'source_directories': '/mnt/dataset', 'zfs': {}},
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -557,7 +556,6 @@ def test_remove_data_source_dumps_use_custom_commands():
         hook_config=hook_config,
         config={'source_directories': '/mnt/dataset', 'zfs': hook_config},
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -572,7 +570,6 @@ def test_remove_data_source_dumps_bails_for_missing_hook_configuration():
         hook_config=None,
         config={'source_directories': '/mnt/dataset'},
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -588,7 +585,6 @@ def test_remove_data_source_dumps_bails_for_missing_zfs_command():
         hook_config=hook_config,
         config={'source_directories': '/mnt/dataset', 'zfs': hook_config},
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -606,7 +602,6 @@ def test_remove_data_source_dumps_bails_for_zfs_command_error():
         hook_config=hook_config,
         config={'source_directories': '/mnt/dataset', 'zfs': hook_config},
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -634,7 +629,6 @@ def test_remove_data_source_dumps_bails_for_missing_umount_command():
         hook_config=hook_config,
         config={'source_directories': '/mnt/dataset', 'zfs': hook_config},
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -667,7 +661,6 @@ def test_remove_data_source_dumps_swallows_umount_command_error():
         hook_config=hook_config,
         config={'source_directories': '/mnt/dataset', 'zfs': hook_config},
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -695,7 +688,6 @@ def test_remove_data_source_dumps_skips_unmount_snapshot_directories_that_are_no
         hook_config={},
         config={'source_directories': '/mnt/dataset', 'zfs': {}},
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -729,7 +721,6 @@ def test_remove_data_source_dumps_skips_unmount_snapshot_mount_paths_that_are_no
         hook_config={},
         config={'source_directories': '/mnt/dataset', 'zfs': {}},
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -765,7 +756,6 @@ def test_remove_data_source_dumps_skips_unmount_snapshot_mount_paths_that_are_em
         hook_config={},
         config={'source_directories': '/mnt/dataset', 'zfs': {}},
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -799,7 +789,6 @@ def test_remove_data_source_dumps_skips_unmount_snapshot_mount_paths_after_rmtre
         hook_config={},
         config={'source_directories': '/mnt/dataset', 'zfs': {}},
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -825,6 +814,5 @@ def test_remove_data_source_dumps_with_dry_run_skips_unmount_and_destroy():
         hook_config={},
         config={'source_directories': '/mnt/dataset', 'zfs': {}},
         borgmatic_runtime_directory='/run/borgmatic',
-        patterns=flexmock(),
         dry_run=True,
     )