Fix a bug in which Borg hangs during database backup when different filesystems are in use (#1118).

NEWS

@@ -1,3 +1,7 @@
+2.0.8.dev0
+ * #1118: Fix a bug in which Borg hangs during database backup when different filesystems are in
+   use.
+
 2.0.7
  * #1032: Fix a bug in which a Borg archive gets created even when a database hook fails.
  * #1043: Support Btrfs subvolume paths in "source_directories" even when the subvolume is mounted

borgmatic/actions/check.py

@@ -384,6 +384,7 @@ def collect_spot_check_source_paths(
             ),
             patterns=borgmatic.actions.pattern.process_patterns(
                 borgmatic.actions.pattern.collect_patterns(config),
+                config,
                 working_directory,
             ),
             local_borg_version=local_borg_version,

borgmatic/actions/create.py

@@ -55,7 +55,9 @@ def run_create(
             borgmatic_runtime_directory,
             global_arguments.dry_run,
         )
-        patterns = pattern.process_patterns(pattern.collect_patterns(config), working_directory)
+        patterns = pattern.process_patterns(
+            pattern.collect_patterns(config), config, working_directory
+        )
         active_dumps = borgmatic.hooks.dispatch.call_hooks(
             'dump_data_sources',
             config,
@@ -70,7 +72,7 @@ def run_create(
         # we could end up with duplicate paths that cause Borg to hang when it tries to read from
         # the same named pipe twice.
         patterns = pattern.process_patterns(
-            patterns, working_directory, skip_expand_paths=config_paths
+            patterns, config, working_directory, skip_expand_paths=config_paths
         )
         stream_processes = [process for processes in active_dumps.values() for process in processes]
 

borgmatic/actions/pattern.py

@@ -225,16 +225,16 @@ def device_map_patterns(patterns, working_directory=None):
     )
 
 
-def deduplicate_patterns(patterns):
+def deduplicate_patterns(patterns, config):
     '''
-    Given a sequence of borgmatic.borg.pattern.Pattern instances, 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.
+    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.
 
-    The one exception to deduplication is two paths are on different filesystems (devices). In that
-    case, they won't get deduplicated, in case they both need to be passed to Borg (e.g. the
-    one_file_system option is true).
+    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, 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
@@ -252,7 +252,8 @@ def deduplicate_patterns(patterns):
         parents = pathlib.PurePath(pattern.path).parents
 
         # If another directory in the given list is a parent of current directory (even n levels up)
-        # and both are on the same filesystem, then the current directory is a duplicate.
+        # and both are on the same filesystem (or one_file_system is not set), then the current
+        # directory is a duplicate.
         for other_pattern in patterns:
             if other_pattern.type != borgmatic.borg.pattern.Pattern_type.ROOT:
                 continue
@@ -260,7 +261,10 @@ def deduplicate_patterns(patterns):
             if any(
                 pathlib.PurePath(other_pattern.path) == parent
                 and pattern.device is not None
-                and other_pattern.device == pattern.device
+                and (
+                    other_pattern.device == pattern.device
+                    or config.get('one_file_system') is not True
+                )
                 for parent in parents
             ):
                 break
@@ -270,12 +274,11 @@ def deduplicate_patterns(patterns):
     return tuple(deduplicated.keys())
 
 
-def process_patterns(patterns, working_directory, skip_expand_paths=None):
+def process_patterns(patterns, config, working_directory, skip_expand_paths=None):
     '''
-    Given a sequence of Borg patterns and a configured working directory, expand and deduplicate any
-    "root" patterns, returning the resulting root and non-root patterns as a list.
-
-    If any paths are given to skip, don't expand them.
+    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 ())
 
@@ -287,6 +290,7 @@ def process_patterns(patterns, working_directory, skip_expand_paths=None):
                     working_directory=working_directory,
                     skip_paths=skip_paths,
                 )
-            )
+            ),
+            config,
         )
     )

borgmatic/actions/recreate.py

@@ -35,7 +35,7 @@ def run_recreate(
 
         # Collect and process patterns.
         processed_patterns = process_patterns(
-            collect_patterns(config), borgmatic.config.paths.get_working_directory(config)
+            collect_patterns(config), config, borgmatic.config.paths.get_working_directory(config)
         )
 
         archive = borgmatic.borg.repo_list.resolve_archive_name(

pyproject.toml

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

tests/unit/actions/test_pattern.py

@@ -350,38 +350,127 @@ def test_device_map_patterns_with_existing_device_id_does_not_overwrite_it():
 
 
 @pytest.mark.parametrize(
-    'patterns,expected_patterns',
+    'patterns,expected_patterns,one_file_system',
     (
-        ((Pattern('/', device=1), Pattern('/root', device=1)), (Pattern('/', device=1),)),
-        ((Pattern('/', device=1), Pattern('/root/', device=1)), (Pattern('/', device=1),)),
+        ((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)),
+            (Pattern('/', device=1),),
+            False,
+        ),
+        ((Pattern('/root', device=1), Pattern('/', device=1)), (Pattern('/', device=1),), False),
+        (
+            (Pattern('/root', device=1), Pattern('/root/foo', device=1)),
+            (Pattern('/root', device=1),),
+            False,
+        ),
+        (
+            (Pattern('/root/', device=1), Pattern('/root/foo', device=1)),
+            (Pattern('/root/', device=1),),
+            False,
+        ),
+        (
+            (Pattern('/root', device=1), Pattern('/root/foo/', device=1)),
+            (Pattern('/root', device=1),),
+            False,
+        ),
+        (
+            (Pattern('/root', device=1), Pattern('/root/foo', device=2)),
+            (Pattern('/root', device=1),),
+            False,
+        ),
+        (
+            (Pattern('/root/foo', device=1), Pattern('/root', device=1)),
+            (Pattern('/root', device=1),),
+            False,
+        ),
+        (
+            (Pattern('/root', device=None), Pattern('/root/foo', device=None)),
+            (Pattern('/root'), Pattern('/root/foo')),
+            False,
+        ),
+        (
+            (
+                Pattern('/root', device=1),
+                Pattern('/etc', device=1),
+                Pattern('/root/foo/bar', device=1),
+            ),
+            (Pattern('/root', device=1), Pattern('/etc', device=1)),
+            False,
+        ),
+        (
+            (
+                Pattern('/root', device=1),
+                Pattern('/root/foo', device=1),
+                Pattern('/root/foo/bar', device=1),
+            ),
+            (Pattern('/root', device=1),),
+            False,
+        ),
+        (
+            (Pattern('/dup', device=1), Pattern('/dup', device=1)),
+            (Pattern('/dup', device=1),),
+            False,
+        ),
+        (
+            (Pattern('/foo', device=1), Pattern('/bar', device=1)),
+            (Pattern('/foo', device=1), Pattern('/bar', device=1)),
+            False,
+        ),
+        (
+            (Pattern('/foo', device=1), Pattern('/bar', device=2)),
+            (Pattern('/foo', device=1), Pattern('/bar', device=2)),
+            False,
+        ),
+        ((Pattern('/root/foo', device=1),), (Pattern('/root/foo', device=1),), False),
+        (
+            (Pattern('/', device=1), Pattern('/root', Pattern_type.INCLUDE, device=1)),
+            (Pattern('/', device=1), Pattern('/root', Pattern_type.INCLUDE, device=1)),
+            False,
+        ),
+        (
+            (Pattern('/root', Pattern_type.INCLUDE, device=1), Pattern('/', device=1)),
+            (Pattern('/root', Pattern_type.INCLUDE, device=1), Pattern('/', device=1)),
+            False,
+        ),
+        ((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)),
+            (Pattern('/', device=1), Pattern('/root', device=2)),
+            True,
         ),
-        ((Pattern('/root', device=1), Pattern('/', device=1)), (Pattern('/', device=1),)),
+        ((Pattern('/root', device=1), Pattern('/', device=1)), (Pattern('/', device=1),), True),
         (
             (Pattern('/root', device=1), Pattern('/root/foo', device=1)),
             (Pattern('/root', device=1),),
+            True,
         ),
         (
             (Pattern('/root/', device=1), Pattern('/root/foo', device=1)),
             (Pattern('/root/', device=1),),
+            True,
         ),
         (
             (Pattern('/root', device=1), Pattern('/root/foo/', device=1)),
             (Pattern('/root', device=1),),
+            True,
         ),
         (
             (Pattern('/root', device=1), Pattern('/root/foo', device=2)),
             (Pattern('/root', device=1), Pattern('/root/foo', device=2)),
+            True,
         ),
         (
             (Pattern('/root/foo', device=1), Pattern('/root', device=1)),
             (Pattern('/root', device=1),),
+            True,
         ),
         (
             (Pattern('/root', device=None), Pattern('/root/foo', device=None)),
             (Pattern('/root'), Pattern('/root/foo')),
+            True,
         ),
         (
             (
@@ -390,6 +479,7 @@ def test_device_map_patterns_with_existing_device_id_does_not_overwrite_it():
                 Pattern('/root/foo/bar', device=1),
             ),
             (Pattern('/root', device=1), Pattern('/etc', device=1)),
+            True,
         ),
         (
             (
@@ -398,29 +488,43 @@ def test_device_map_patterns_with_existing_device_id_does_not_overwrite_it():
                 Pattern('/root/foo/bar', device=1),
             ),
             (Pattern('/root', device=1),),
+            True,
+        ),
+        (
+            (Pattern('/dup', device=1), Pattern('/dup', device=1)),
+            (Pattern('/dup', device=1),),
+            True,
         ),
-        ((Pattern('/dup', device=1), Pattern('/dup', device=1)), (Pattern('/dup', device=1),)),
         (
             (Pattern('/foo', device=1), Pattern('/bar', device=1)),
             (Pattern('/foo', device=1), Pattern('/bar', device=1)),
+            True,
         ),
         (
             (Pattern('/foo', device=1), Pattern('/bar', device=2)),
             (Pattern('/foo', device=1), Pattern('/bar', device=2)),
+            True,
         ),
-        ((Pattern('/root/foo', device=1),), (Pattern('/root/foo', device=1),)),
+        ((Pattern('/root/foo', device=1),), (Pattern('/root/foo', device=1),), True),
         (
             (Pattern('/', device=1), Pattern('/root', Pattern_type.INCLUDE, device=1)),
             (Pattern('/', device=1), Pattern('/root', Pattern_type.INCLUDE, device=1)),
+            True,
         ),
         (
             (Pattern('/root', Pattern_type.INCLUDE, device=1), Pattern('/', device=1)),
             (Pattern('/root', Pattern_type.INCLUDE, device=1), Pattern('/', device=1)),
+            True,
         ),
     ),
 )
-def test_deduplicate_patterns_omits_child_paths_on_the_same_filesystem(patterns, expected_patterns):
-    assert module.deduplicate_patterns(patterns) == expected_patterns
+def test_deduplicate_patterns_omits_child_paths_based_on_device_and_one_file_system(
+    patterns, expected_patterns, one_file_system
+):
+    assert (
+        module.deduplicate_patterns(patterns, {'one_file_system': one_file_system})
+        == expected_patterns
+    )
 
 
 def test_process_patterns_includes_patterns():
@@ -436,6 +540,7 @@ def test_process_patterns_includes_patterns():
 
     assert module.process_patterns(
         (Pattern('foo'), Pattern('bar')),
+        config={},
         working_directory='/working',
     ) == [Pattern('foo'), Pattern('bar')]
 
@@ -454,6 +559,7 @@ def test_process_patterns_skips_expand_for_requested_paths():
 
     assert module.process_patterns(
         (Pattern('foo'), Pattern('bar')),
+        config={},
         working_directory='/working',
         skip_expand_paths=skip_paths,
     ) == [Pattern('foo'), Pattern('bar')]