Merge branch 'main' into log-repository-everywhere

NEWS

@@ -1,8 +1,10 @@
 1.9.9.dev0
  * #635: Log the repository path or label on every relevant log message, not just some logs.
- * #981: Fix "spot" check file count delta error.
+ * #981: Fix a "spot" check file count delta error.
  * #982: Fix for borgmatic "exclude_patterns" and "exclude_from" recursing into excluded
    subdirectories.
+ * #983: Fix the Btrfs hook to support subvolumes with names like "@home", different from their
+   mount points.
 
 1.9.8
  * #979: Fix root patterns so they don't have an invalid "sh:" prefix before getting passed to Borg.

borgmatic/hooks/data_source/btrfs.py

@@ -21,9 +21,9 @@ def use_streaming(hook_config, config):  # pragma: no cover
     return False
 
 
-def get_filesystem_mount_points(findmnt_command):
+def get_subvolume_mount_points(findmnt_command):
     '''
-    Given a findmnt command to run, get all top-level Btrfs filesystem mount points.
+    Given a findmnt command to run, get all sorted Btrfs subvolume mount points.
     '''
     findmnt_output = borgmatic.execute.execute_command_and_capture_output(
         tuple(findmnt_command.split(' '))
@@ -37,7 +37,7 @@ def get_filesystem_mount_points(findmnt_command):
 
     try:
         return tuple(
-            filesystem['target'] for filesystem in json.loads(findmnt_output)['filesystems']
+            sorted(filesystem['target'] for filesystem in json.loads(findmnt_output)['filesystems'])
         )
     except json.JSONDecodeError as error:
         raise ValueError(f'Invalid {findmnt_command} JSON output: {error}')
@@ -45,34 +45,6 @@ def get_filesystem_mount_points(findmnt_command):
         raise ValueError(f'Invalid {findmnt_command} output: Missing key "{error}"')
 
 
-def get_subvolumes_for_filesystem(btrfs_command, filesystem_mount_point):
-    '''
-    Given a Btrfs command to run and a Btrfs filesystem mount point, get the sorted subvolumes for
-    that filesystem. Include the filesystem itself.
-    '''
-    btrfs_output = borgmatic.execute.execute_command_and_capture_output(
-        tuple(btrfs_command.split(' '))
-        + (
-            'subvolume',
-            'list',
-            filesystem_mount_point,
-        )
-    )
-
-    if not filesystem_mount_point.strip():
-        return ()
-
-    return (filesystem_mount_point,) + tuple(
-        sorted(
-            subvolume_path
-            for line in btrfs_output.splitlines()
-            for subvolume_subpath in (line.rstrip().split(' ')[-1],)
-            for subvolume_path in (os.path.join(filesystem_mount_point, subvolume_subpath),)
-            if subvolume_subpath.strip()
-        )
-    )
-
-
 Subvolume = collections.namedtuple('Subvolume', ('path', 'contained_patterns'), defaults=((),))
 
 
@@ -89,20 +61,16 @@ def get_subvolumes(btrfs_command, findmnt_command, patterns=None):
     candidate_patterns = set(patterns or ())
     subvolumes = []
 
-    # For each filesystem mount point, find its subvolumes and match them against the given patterns
-    # to find the subvolumes to backup. And within this loop, 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 mount_point in get_filesystem_mount_points(findmnt_command):
+    # For each subvolume mount point, 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
+    # 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 mount_point in reversed(get_subvolume_mount_points(findmnt_command)):
         subvolumes.extend(
-            Subvolume(subvolume_path, contained_patterns)
-            for subvolume_path in reversed(
-                get_subvolumes_for_filesystem(btrfs_command, mount_point)
-            )
+            Subvolume(mount_point, contained_patterns)
             for contained_patterns in (
                 borgmatic.hooks.data_source.snapshot.get_contained_patterns(
-                    subvolume_path, candidate_patterns
+                    mount_point, candidate_patterns
                 ),
             )
             if patterns is None or contained_patterns

tests/unit/hooks/data_source/test_btrfs.py

@@ -5,7 +5,7 @@ from borgmatic.borg.pattern import Pattern, Pattern_style, Pattern_type
 from borgmatic.hooks.data_source import btrfs as module
 
 
-def test_get_filesystem_mount_points_parses_findmnt_output():
+def test_get_subvolume_mount_points_parses_findmnt_output():
     flexmock(module.borgmatic.execute).should_receive(
         'execute_command_and_capture_output'
     ).and_return(
@@ -28,113 +28,60 @@ def test_get_filesystem_mount_points_parses_findmnt_output():
         '''
     )
 
-    assert module.get_filesystem_mount_points('findmnt') == ('/mnt0', '/mnt1')
+    assert module.get_subvolume_mount_points('findmnt') == ('/mnt0', '/mnt1')
 
 
-def test_get_filesystem_mount_points_with_invalid_findmnt_json_errors():
+def test_get_subvolume_mount_points_with_invalid_findmnt_json_errors():
     flexmock(module.borgmatic.execute).should_receive(
         'execute_command_and_capture_output'
     ).and_return('{')
 
     with pytest.raises(ValueError):
-        module.get_filesystem_mount_points('findmnt')
+        module.get_subvolume_mount_points('findmnt')
 
 
-def test_get_filesystem_mount_points_with_findmnt_json_missing_filesystems_errors():
+def test_get_subvolume_mount_points_with_findmnt_json_missing_filesystems_errors():
     flexmock(module.borgmatic.execute).should_receive(
         'execute_command_and_capture_output'
     ).and_return('{"wtf": "something is wrong here"}')
 
     with pytest.raises(ValueError):
-        module.get_filesystem_mount_points('findmnt')
+        module.get_subvolume_mount_points('findmnt')
 
 
-def test_get_subvolumes_for_filesystem_parses_subvolume_list_output():
-    flexmock(module.borgmatic.execute).should_receive(
-        'execute_command_and_capture_output'
-    ).and_return(
-        'ID 270 gen 107 top level 5 path subvol1\nID 272 gen 74 top level 5 path subvol2\n'
-    )
-
-    assert module.get_subvolumes_for_filesystem('btrfs', '/mnt') == (
-        '/mnt',
-        '/mnt/subvol1',
-        '/mnt/subvol2',
-    )
-
-
-def test_get_subvolumes_for_filesystem_skips_empty_subvolume_paths():
-    flexmock(module.borgmatic.execute).should_receive(
-        'execute_command_and_capture_output'
-    ).and_return('\n \nID 272 gen 74 top level 5 path subvol2\n')
-
-    assert module.get_subvolumes_for_filesystem('btrfs', '/mnt') == ('/mnt', '/mnt/subvol2')
+def test_get_subvolumes_collects_subvolumes_matching_patterns():
+    flexmock(module).should_receive('get_subvolume_mount_points').and_return(('/mnt1', '/mnt2'))
 
-
-def test_get_subvolumes_for_filesystem_skips_empty_filesystem_mount_points():
-    flexmock(module.borgmatic.execute).should_receive(
-        'execute_command_and_capture_output'
-    ).and_return(
-        'ID 270 gen 107 top level 5 path subvol1\nID 272 gen 74 top level 5 path subvol2\n'
-    )
-
-    assert module.get_subvolumes_for_filesystem('btrfs', ' ') == ()
-
-
-def test_get_subvolumes_collects_subvolumes_matching_patterns_from_all_filesystems():
-    flexmock(module).should_receive('get_filesystem_mount_points').and_return(('/mnt1', '/mnt2'))
-    flexmock(module).should_receive('get_subvolumes_for_filesystem').with_args(
-        'btrfs', '/mnt1'
-    ).and_return(('/one', '/two'))
-    flexmock(module).should_receive('get_subvolumes_for_filesystem').with_args(
-        'btrfs', '/mnt2'
-    ).and_return(('/three', '/four'))
-
-    for path in ('/one', '/four'):
-        flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive(
-            'get_contained_patterns'
-        ).with_args(path, object).and_return((Pattern(path),))
-    for path in ('/two', '/three'):
-        flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive(
-            'get_contained_patterns'
-        ).with_args(path, object).and_return(())
+    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(())
 
     assert module.get_subvolumes(
         'btrfs',
         'findmnt',
         patterns=[
-            Pattern('/one'),
-            Pattern('/four'),
-            Pattern('/five'),
-            Pattern('/six'),
-            Pattern('/mnt2'),
+            Pattern('/mnt1'),
             Pattern('/mnt3'),
         ],
-    ) == (
-        module.Subvolume('/four', contained_patterns=(Pattern('/four'),)),
-        module.Subvolume('/one', contained_patterns=(Pattern('/one'),)),
-    )
+    ) == (module.Subvolume('/mnt1', contained_patterns=(Pattern('/mnt1'),)),)
 
 
-def test_get_subvolumes_without_patterns_collects_all_subvolumes_from_all_filesystems():
-    flexmock(module).should_receive('get_filesystem_mount_points').and_return(('/mnt1', '/mnt2'))
-    flexmock(module).should_receive('get_subvolumes_for_filesystem').with_args(
-        'btrfs', '/mnt1'
-    ).and_return(('/one', '/two'))
-    flexmock(module).should_receive('get_subvolumes_for_filesystem').with_args(
-        'btrfs', '/mnt2'
-    ).and_return(('/three', '/four'))
+def test_get_subvolumes_without_patterns_collects_all_subvolumes():
+    flexmock(module).should_receive('get_subvolume_mount_points').and_return(('/mnt1', '/mnt2'))
 
-    for path in ('/one', '/two', '/three', '/four'):
-        flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive(
-            'get_contained_patterns'
-        ).with_args(path, object).and_return((Pattern(path),))
+    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('/four', contained_patterns=(Pattern('/four'),)),
-        module.Subvolume('/one', contained_patterns=(Pattern('/one'),)),
-        module.Subvolume('/three', contained_patterns=(Pattern('/three'),)),
-        module.Subvolume('/two', contained_patterns=(Pattern('/two'),)),
+        module.Subvolume('/mnt1', contained_patterns=(Pattern('/mnt1'),)),
+        module.Subvolume('/mnt2', contained_patterns=(Pattern('/mnt2'),)),
     )