More accurately collect Btrfs subvolumes to snapshot. As part of this, the Btrfs hook no longer uses "findmnt" (#1105).

Reviewed-on: https://projects.torsion.org/borgmatic-collective/borgmatic/pulls/1154

NEWS

@@ -1,4 +1,6 @@
 2.0.9.dev0
+ * #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.
  * #1123: Add loading of systemd credentials even when running borgmatic outside of a systemd
    service.
  * #1149: Add support for Python 3.14.

borgmatic/actions/create.py

@@ -50,18 +50,19 @@ 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.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,
@@ -138,6 +139,7 @@ def run_create(
             config,
             borgmatic.hooks.dispatch.Hook_type.DATA_SOURCE,
             borgmatic_runtime_directory,
+            patterns,
             global_arguments.dry_run,
         )
 

borgmatic/actions/restore.py

@@ -5,6 +5,7 @@ import pathlib
 import shutil
 import tempfile
 
+import borgmatic.actions.pattern
 import borgmatic.borg.extract
 import borgmatic.borg.list
 import borgmatic.borg.mount
@@ -536,13 +537,20 @@ 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,
         )
 
@@ -625,6 +633,7 @@ def run_restore(
             config,
             borgmatic.hooks.dispatch.Hook_type.DATA_SOURCE,
             borgmatic_runtime_directory,
+            patterns,
             global_arguments.dry_run,
         )
 

borgmatic/config/schema.yaml

@@ -2951,7 +2951,8 @@ properties:
             findmnt_command:
                 type: string
                 description: |
-                    Command to use instead of "findmnt".
+                    Deprecated and unused. Was the command to use instead of
+                    "findmnt".
                 example: /usr/local/bin/findmnt
         description: |
             Configuration for integration with the Btrfs filesystem.

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, dry_run):
+def remove_data_source_dumps(hook_config, config, borgmatic_runtime_directory, patterns, dry_run):
     '''
-    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.
+    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.
     '''
     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,93 +22,44 @@ def use_streaming(hook_config, config):  # pragma: no cover
     return False
 
 
-def get_contained_subvolume_paths(btrfs_command, subvolume_path):
+@functools.cache
+def path_is_a_subvolume(btrfs_command, path):
     '''
-    Given the path of a Btrfs subvolume, return it in a sequence along with the paths of its
-    contained subvolumes.
+    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.
 
-    If the btrfs command errors, log that error and return an empty sequence.
+    As a performance optimization, multiple calls to this function with the same arguments are
+    cached.
     '''
     try:
-        btrfs_output = borgmatic.execute.execute_command_and_capture_output(
+        borgmatic.execute.execute_command(
             (
                 *btrfs_command.split(' '),
                 'subvolume',
-                'list',
-                subvolume_path,
+                'show',
+                path,
             ),
+            output_log_level=None,
             close_fds=True,
         )
-    except subprocess.CalledProcessError as error:
-        logger.debug(
-            f'Ignoring Btrfs subvolume {subvolume_path} because of error listing its subvolumes: {error}',
-        )
-
-        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}"')
+    # An error from the command (probably) indicates that the path is not actually a subvolume.
+    except subprocess.CalledProcessError:
+        return False
 
-
-Subvolume = collections.namedtuple('Subvolume', ('path', 'contained_patterns'), defaults=((),))
+    return True
 
 
+@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(
         (
@@ -134,37 +85,71 @@ def get_subvolume_property(btrfs_command, subvolume_path, property_name):
     }.get(value, value)
 
 
-def omit_read_only_subvolume_paths(btrfs_command, subvolume_paths):
+def get_containing_subvolume_path(btrfs_command, path):
     '''
-    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.
+    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.
     '''
-    retained_subvolume_paths = []
+    # 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
 
-    for subvolume_path in subvolume_paths:
         try:
-            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
+            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:
             logger.debug(
-                f'Error determining read-only status of Btrfs subvolume {subvolume_path}: {error}',
+                f'Error determining read-only status of Btrfs subvolume {candidate_path}: {error}',
             )
 
-    return tuple(retained_subvolume_paths)
+            return None
 
+    return None
 
-def get_subvolumes(btrfs_command, findmnt_command, patterns=None):
+
+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):
     '''
     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). But if patterns is None, then
-    return all subvolumes instead, sorted by path.
+    configuration (as opposed to generated elsewhere in borgmatic).
 
     Return the result as a sequence of matching Subvolume instances.
     '''
@@ -172,15 +157,10 @@ def get_subvolumes(btrfs_command, findmnt_command, patterns=None):
     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 mount points get
+    # backup. Sort the subvolumes from longest to shortest mount points, so longer subvolumes 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(
-        omit_read_only_subvolume_paths(
-            btrfs_command,
-            get_all_subvolume_paths(btrfs_command, findmnt_command),
-        ),
-    ):
+    for subvolume_path in reversed(get_all_subvolume_paths(btrfs_command, patterns)):
         subvolumes.extend(
             Subvolume(subvolume_path, contained_patterns)
             for contained_patterns in (
@@ -189,8 +169,7 @@ def get_subvolumes(btrfs_command, findmnt_command, patterns=None):
                     candidate_patterns,
                 ),
             )
-            if patterns is None
-            or any(
+            if any(
                 pattern.type == borgmatic.borg.pattern.Pattern_type.ROOT
                 and pattern.source == borgmatic.borg.pattern.Pattern_source.CONFIG
                 for pattern in contained_patterns
@@ -325,11 +304,15 @@ 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')
-    findmnt_command = hook_config.get('findmnt_command', 'findmnt')
-    subvolumes = get_subvolumes(btrfs_command, findmnt_command, patterns)
+    subvolumes = get_subvolumes(btrfs_command, patterns)
 
     if not subvolumes:
         logger.warning(f'No Btrfs subvolumes found to snapshot{dry_run_label}')
@@ -375,11 +358,12 @@ def delete_snapshot(btrfs_command, snapshot_path):  # pragma: no cover
     )
 
 
-def remove_data_source_dumps(hook_config, config, borgmatic_runtime_directory, dry_run):
+def remove_data_source_dumps(hook_config, config, borgmatic_runtime_directory, patterns, dry_run):
     '''
-    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.
+    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.
     '''
     if hook_config is None:
         return
@@ -387,10 +371,9 @@ def remove_data_source_dumps(hook_config, config, borgmatic_runtime_directory, d
     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, findmnt_command)
+        all_subvolumes = get_subvolumes(btrfs_command, patterns)
     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, dry_run):  # noqa: PLR0912
+def remove_data_source_dumps(hook_config, config, borgmatic_runtime_directory, patterns, dry_run):  # noqa: PLR0912
     '''
-    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.
+    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.
     '''
     if hook_config is None:
         return

borgmatic/hooks/data_source/mariadb.py

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

borgmatic/hooks/data_source/mongodb.py

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

borgmatic/hooks/data_source/mysql.py

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

borgmatic/hooks/data_source/postgresql.py

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

borgmatic/hooks/data_source/sqlite.py

@@ -120,6 +120,7 @@ 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, dry_run):  # noqa: PLR0912
+def remove_data_source_dumps(hook_config, config, borgmatic_runtime_directory, patterns, dry_run):  # noqa: PLR0912
     '''
-    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.
+    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.
     '''
     if hook_config is None:
         return

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

@@ -162,12 +162,11 @@ btrfs:
 ```
 
 No other options are necessary to enable Btrfs support, but if desired you can
-override some of the commands used by the Btrfs hook. For instance:
+override the `btrfs` command 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
@@ -183,8 +182,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. Btrfs does not support snapshotting
-read-only subvolumes.
+borgmatic's `source_directories` option. borgmatic does not currently 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

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

@@ -12,9 +12,8 @@ def parse_arguments(*unparsed_arguments):
     subvolume_parser = action_parsers.add_parser('subvolume')
     subvolume_subparser = subvolume_parser.add_subparsers(dest='subaction')
 
-    list_parser = subvolume_subparser.add_parser('list')
-    list_parser.add_argument('-s', dest='snapshots_only', action='store_true')
-    list_parser.add_argument('subvolume_path')
+    show_parser = subvolume_subparser.add_parser('show')
+    show_parser.add_argument('subvolume_path')
 
     snapshot_parser = subvolume_subparser.add_parser('snapshot')
     snapshot_parser.add_argument('-r', dest='read_only', action='store_true')
@@ -24,6 +23,9 @@ 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')
@@ -34,13 +36,6 @@ 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'))
@@ -52,18 +47,12 @@ def save_snapshots(snapshot_paths):
     json.dump(snapshot_paths, open('/tmp/fake_btrfs.json', 'w'))
 
 
-def print_subvolume_list(arguments, snapshot_paths):
+def print_subvolume_show(arguments):
     assert arguments.subvolume_path == '/e2e/mnt/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-') :],
-        )
+    # 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!')
 
 
 def main():
@@ -74,8 +63,8 @@ def main():
         global_parser.print_help()
         sys.exit(1)
 
-    if arguments.subaction == 'list':
-        print_subvolume_list(arguments, snapshot_paths)
+    if arguments.subaction == 'show':
+        print_subvolume_show(arguments)
     elif arguments.subaction == 'snapshot':
         snapshot_paths.append(arguments.snapshot_path)
         save_snapshots(snapshot_paths)
@@ -95,6 +84,9 @@ 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

@@ -1,42 +0,0 @@
-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,7 +24,6 @@ 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)
@@ -55,7 +54,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 list -s /e2e/mnt/subvolume'.split(
+            'python3 /app/tests/end-to-end/commands/fake_btrfs.py subvolume ensure_deleted /e2e/mnt/subvolume'.split(
                 ' ',
             ),
         )

tests/unit/actions/test_create.py

@@ -18,6 +18,7 @@ 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')
@@ -60,6 +61,7 @@ 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')
@@ -207,6 +209,7 @@ 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')
@@ -252,6 +255,7 @@ 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')
@@ -335,6 +339,7 @@ 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_restore.py

@@ -1176,6 +1176,9 @@ 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(),
@@ -1245,6 +1248,9 @@ 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()
@@ -1274,6 +1280,9 @@ 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(),
@@ -1334,6 +1343,9 @@ 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(),
@@ -1416,6 +1428,9 @@ 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(),
@@ -1498,6 +1513,9 @@ 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,6 +76,7 @@ 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,
     )
 
@@ -92,6 +93,7 @@ def test_remove_data_source_dumps_with_dry_run_bails():
         hook_config=None,
         config={},
         borgmatic_runtime_directory='/run/borgmatic',
+        patterns=flexmock(),
         dry_run=True,
     )
 
@@ -110,6 +112,7 @@ 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,
     )
 
@@ -130,5 +133,6 @@ 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,100 +5,40 @@ from borgmatic.borg.pattern import Pattern, Pattern_source, Pattern_style, Patte
 from borgmatic.hooks.data_source import btrfs as module
 
 
-def test_get_contained_subvolume_paths_parses_btrfs_output():
+def test_path_is_a_subvolume_with_btrfs_success_call_returns_true():
+    module.path_is_a_subvolume.cache_clear()
     flexmock(module.borgmatic.execute).should_receive(
-        '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',
-    )
+        'execute_command',
+    ).with_args(('btrfs', 'subvolume', 'show', '/mnt0'), output_log_level=None, close_fds=True)
 
-    assert module.get_contained_subvolume_paths('btrfs', '/mnt0') == (
-        '/mnt0',
-        '/mnt0/@sub',
-        '/mnt0/snap',
-    )
+    assert module.path_is_a_subvolume('btrfs', '/mnt0') is True
 
 
-def test_get_contained_subvolume_paths_swallows_called_process_error():
+def test_path_is_a_subvolume_with_btrfs_error_returns_false():
+    module.path_is_a_subvolume.cache_clear()
     flexmock(module.borgmatic.execute).should_receive(
-        'execute_command_and_capture_output',
-    ).with_args(('btrfs', 'subvolume', 'list', '/mnt0'), close_fds=True).and_raise(
+        'execute_command',
+    ).with_args(
+        ('btrfs', 'subvolume', 'show', '/mnt0'), output_log_level=None, close_fds=True
+    ).and_raise(
         module.subprocess.CalledProcessError(1, 'btrfs'),
     )
 
-    assert module.get_contained_subvolume_paths('btrfs', '/mnt0') == ()
-
-
-def test_get_all_subvolume_paths_parses_findmnt_output():
-    flexmock(module.borgmatic.execute).should_receive(
-        '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.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')
+    assert module.path_is_a_subvolume('btrfs', '/mnt0') is False
 
 
-def test_get_all_subvolume_paths_with_findmnt_json_missing_filesystems_errors():
+def test_path_is_a_subvolume_caches_result_after_first_call():
+    module.path_is_a_subvolume.cache_clear()
     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()
+        'execute_command',
+    ).once()
 
-    with pytest.raises(ValueError):
-        module.get_all_subvolume_paths('btrfs', 'findmnt')
+    assert module.path_is_a_subvolume('btrfs', '/mnt0') is True
+    assert module.path_is_a_subvolume('btrfs', '/mnt0') is True
 
 
 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')
@@ -108,6 +48,7 @@ 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')
@@ -116,6 +57,7 @@ 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')
@@ -124,6 +66,7 @@ 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')
@@ -131,52 +74,187 @@ def test_get_subvolume_property_passes_through_general_value():
     assert module.get_subvolume_property('btrfs', '/foo', 'thing') == 'value'
 
 
-def test_omit_read_only_subvolume_paths_filters_out_read_only_subvolumes():
-    flexmock(module).should_receive('get_subvolume_property').with_args(
-        'btrfs',
-        '/foo',
-        'ro',
+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'
+    ).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'
     ).and_return(False)
-    flexmock(module).should_receive('get_subvolume_property').with_args(
-        'btrfs',
-        '/bar',
-        'ro',
+    flexmock(module).should_receive('path_is_a_subvolume').with_args(
+        'btrfs', '/foo/bar'
     ).and_return(True)
-    flexmock(module).should_receive('get_subvolume_property').with_args(
-        'btrfs',
-        '/baz',
-        'ro',
+    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
+
 
-    assert module.omit_read_only_subvolume_paths('btrfs', ('/foo', '/bar', '/baz')) == (
-        '/foo',
-        '/baz',
+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.get_containing_subvolume_path('btrfs', '/foo/bar/baz') is None
 
-def test_omit_read_only_subvolume_paths_filters_out_erroring_subvolumes():
-    flexmock(module).should_receive('get_subvolume_property').with_args(
-        'btrfs',
-        '/foo',
-        'ro',
-    ).and_raise(module.subprocess.CalledProcessError(1, 'btrfs'))
-    flexmock(module).should_receive('get_subvolume_property').with_args(
+
+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(
         'btrfs',
-        '/bar',
-        'ro',
-    ).and_return(True)
-    flexmock(module).should_receive('get_subvolume_property').with_args(
+        (
+            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(
         'btrfs',
-        '/baz',
-        'ro',
-    ).and_return(False)
+        (
+            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',)
 
-    assert module.omit_read_only_subvolume_paths('btrfs', ('/foo', '/bar', '/baz')) == ('/baz',)
+
+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(
+        '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')
 
 
 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',
@@ -192,7 +270,6 @@ def test_get_subvolumes_collects_subvolumes_matching_patterns():
 
     assert module.get_subvolumes(
         'btrfs',
-        'findmnt',
         patterns=[
             Pattern('/mnt1'),
             Pattern('/mnt3'),
@@ -202,7 +279,6 @@ 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',
@@ -222,7 +298,6 @@ def test_get_subvolumes_skips_non_root_patterns():
     assert (
         module.get_subvolumes(
             'btrfs',
-            'findmnt',
             patterns=[
                 Pattern('/mnt1'),
                 Pattern('/mnt3'),
@@ -234,7 +309,6 @@ 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',
@@ -254,7 +328,6 @@ def test_get_subvolumes_skips_non_config_patterns():
     assert (
         module.get_subvolumes(
             'btrfs',
-            'findmnt',
             patterns=[
                 Pattern('/mnt1'),
                 Pattern('/mnt3'),
@@ -264,23 +337,6 @@ 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',
     (
@@ -483,12 +539,12 @@ def test_dump_data_sources_uses_custom_btrfs_command_in_commands():
     }
 
 
-def test_dump_data_sources_uses_custom_findmnt_command_in_commands():
+def test_dump_data_sources_with_findmnt_command_warns():
     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'),)),),
@@ -773,6 +829,7 @@ def test_remove_data_source_dumps_deletes_snapshots():
         hook_config=config['btrfs'],
         config=config,
         borgmatic_runtime_directory='/run/borgmatic',
+        patterns=flexmock(),
         dry_run=False,
     )
 
@@ -790,6 +847,7 @@ 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,
     )
 
@@ -808,6 +866,7 @@ 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,
     )
 
@@ -828,6 +887,7 @@ 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,
     )
 
@@ -887,6 +947,7 @@ 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,
     )
 
@@ -905,6 +966,7 @@ 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,
     )
 
@@ -944,6 +1006,7 @@ 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,
     )
 
@@ -1003,6 +1066,7 @@ 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,
     )
 
@@ -1064,6 +1128,7 @@ 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,
     )
 
@@ -1107,5 +1172,6 @@ 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,6 +912,7 @@ 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,
     )
 
@@ -928,6 +929,7 @@ 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,
     )
 
@@ -945,6 +947,7 @@ 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,
     )
 
@@ -964,6 +967,7 @@ 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,
     )
 
@@ -1010,6 +1014,7 @@ 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,
     )
 
@@ -1070,6 +1075,7 @@ 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,
     )
 
@@ -1135,6 +1141,7 @@ 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,
     )
 
@@ -1195,6 +1202,7 @@ 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,
     )
 
@@ -1241,6 +1249,7 @@ 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,
     )
 
@@ -1293,6 +1302,7 @@ 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,
     )
 
@@ -1339,6 +1349,7 @@ 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,
     )
 
@@ -1387,6 +1398,7 @@ 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,
     )
 
@@ -1430,5 +1442,6 @@ 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,6 +524,7 @@ 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,
     )
 
@@ -556,6 +557,7 @@ 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,
     )
 
@@ -570,6 +572,7 @@ 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,
     )
 
@@ -585,6 +588,7 @@ 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,
     )
 
@@ -602,6 +606,7 @@ 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,
     )
 
@@ -629,6 +634,7 @@ 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,
     )
 
@@ -661,6 +667,7 @@ 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,
     )
 
@@ -688,6 +695,7 @@ 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,
     )
 
@@ -721,6 +729,7 @@ 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,
     )
 
@@ -756,6 +765,7 @@ 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,
     )
 
@@ -789,6 +799,7 @@ 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,
     )
 
@@ -814,5 +825,6 @@ 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,
     )