Browse Source

Fix for over-aggressive deduplication of source directories that contain the borgmatic runtime directory (#1192).

Dan Helfman 1 day ago
parent
commit
2a97ac0174
4 changed files with 30 additions and 14 deletions
  1. 3 0
      NEWS
  2. 1 0
      borgmatic/actions/create.py
  3. 20 14
      borgmatic/actions/pattern.py
  4. 6 0
      tests/unit/actions/test_pattern.py

+ 3 - 0
NEWS

@@ -1,4 +1,7 @@
 2.0.13.dev0
+ * #1192: Fix for over-aggressive deduplication of source directories that contain the borgmatic
+   runtime directory, potentially resulting in data loss (data not getting backed up) when using
+   filesystem snapshots.
  * #1194: Fix for an incorrect diff command shown when running the "generate config" action with a
    source configuration file. 
  * Update the sample systemd timer with a shorter random delay when catching up on a missed run.

+ 1 - 0
borgmatic/actions/create.py

@@ -49,6 +49,7 @@ def run_create(
             working_directory,
             borgmatic_runtime_directory,
         )
+
         original_patterns = list(patterns)
         borgmatic.hooks.dispatch.call_hooks_even_if_unconfigured(
             'remove_data_source_dumps',

+ 20 - 14
borgmatic/actions/pattern.py

@@ -259,31 +259,37 @@ def deduplicate_runtime_directory_patterns(patterns, config, borgmatic_runtime_d
         return patterns
 
     deduplicated = {}  # Use just the keys as an ordered set.
+    runtime_directory_parents = set(pathlib.PurePath(borgmatic_runtime_directory).parents).union(
+        {pathlib.PurePath(borgmatic_runtime_directory)}
+    )
 
     for pattern in patterns:
         if pattern.type != borgmatic.borg.pattern.Pattern_type.ROOT:
             deduplicated[pattern] = True
             continue
 
-        parents = pathlib.PurePath(pattern.path).parents
+        pattern_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 (or one_file_system is not set), then the current
-        # directory is a duplicate.
+        # If:
+        #
+        #   1. another pattern is a parent of the current pattern (even n levels up),
+        #   2. both patterns are parents of the runtime directory (even n levels up),
+        #   3. and both patterns are on the same filesystem (or one_file_system is not set)
+        #
+        # ... then consider the current pattern as a duplicate.
         for other_pattern in patterns:
             if other_pattern.type != borgmatic.borg.pattern.Pattern_type.ROOT:
                 continue
 
-            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
-                    or config.get('one_file_system') is not True
-                )
-                for parent in parents
+            device_matches = pattern.device is not None and (
+                other_pattern.device == pattern.device or config.get('one_file_system') is not True
+            )
+
+            if (
+                pathlib.PurePath(other_pattern.path) in pattern_parents
+                and pathlib.PurePosixPath(other_pattern.path) in runtime_directory_parents
+                and pathlib.PurePosixPath(pattern.path) in runtime_directory_parents
+                and device_matches
             ):
                 break
         else:

+ 6 - 0
tests/unit/actions/test_pattern.py

@@ -402,6 +402,12 @@ def test_device_map_patterns_with_existing_device_id_does_not_overwrite_it():
             (Pattern('/', device=1),),
             False,
         ),
+        (
+            (Pattern('/', device=1), Pattern('/other', device=1)),
+            '/root',
+            (Pattern('/', device=1), Pattern('/other', device=1)),
+            False,
+        ),
         (
             (Pattern('/', device=1), Pattern('/root', device=2)),
             None,