Browse Source

Deduplicate directories again after hooks have their way with them (#261).

Dan Helfman 6 months ago
parent
commit
f4a736bdfe
3 changed files with 39 additions and 6 deletions
  1. 3 1
      borgmatic/actions/check.py
  2. 15 5
      borgmatic/actions/create.py
  3. 21 0
      tests/unit/actions/test_create.py

+ 3 - 1
borgmatic/actions/check.py

@@ -373,7 +373,9 @@ def collect_spot_check_source_paths(
             dry_run=True,
             dry_run=True,
             repository_path=repository['path'],
             repository_path=repository['path'],
             config=config,
             config=config,
-            source_directories=borgmatic.actions.create.process_source_directories(config, ()),
+            source_directories=borgmatic.actions.create.process_source_directories(
+                config, config_paths=()
+            ),
             local_borg_version=local_borg_version,
             local_borg_version=local_borg_version,
             global_arguments=global_arguments,
             global_arguments=global_arguments,
             borgmatic_runtime_directory=borgmatic_runtime_directory,
             borgmatic_runtime_directory=borgmatic_runtime_directory,

+ 15 - 5
borgmatic/actions/create.py

@@ -146,18 +146,23 @@ def pattern_root_directories(patterns=None):
     ]
     ]
 
 
 
 
-def process_source_directories(config, config_paths):
+def process_source_directories(config, config_paths, source_directories=None):
     '''
     '''
-    Given a configuration dict and a sequence of configuration paths, expand and deduplicate the
-    source directories from them.
+    Given a sequence of source directories (either in the source_directories argument or, lacking
+    that, from config) and a sequence of config paths to append, expand and deduplicate the source
+    directories, returning the result.
     '''
     '''
     working_directory = borgmatic.config.paths.get_working_directory(config)
     working_directory = borgmatic.config.paths.get_working_directory(config)
 
 
+    if source_directories is None:
+        source_directories = tuple(config.get('source_directories', ())) + (
+            tuple(config_paths) if config.get('store_config_files', True) else ()
+        )
+
     return deduplicate_directories(
     return deduplicate_directories(
         map_directories_to_devices(
         map_directories_to_devices(
             expand_directories(
             expand_directories(
-                tuple(config.get('source_directories', ()))
-                + tuple(config_paths if config.get('store_config_files', True) else ()),
+                tuple(source_directories),
                 working_directory=working_directory,
                 working_directory=working_directory,
             )
             )
         ),
         ),
@@ -226,6 +231,11 @@ def run_create(
             source_directories,
             source_directories,
             global_arguments.dry_run,
             global_arguments.dry_run,
         )
         )
+
+        # Process source directories again in case any data source hooks updated them. Without this
+        # step, we could end up with duplicate paths that cause Borg to hang when it tries to read
+        # from the same named pipe twice.
+        source_directories = process_source_directories(config, config_paths, source_directories)
         stream_processes = [process for processes in active_dumps.values() for process in processes]
         stream_processes = [process for processes in active_dumps.values() for process in processes]
 
 
         if config.get('store_config_files', True):
         if config.get('store_config_files', True):

+ 21 - 0
tests/unit/actions/test_create.py

@@ -248,6 +248,27 @@ def test_process_source_directories_does_not_include_config_paths_when_store_con
     ) == ('foo', 'bar')
     ) == ('foo', 'bar')
 
 
 
 
+def test_process_source_directories_prefers_source_directory_argument_to_config():
+    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(
+        '/working'
+    )
+    flexmock(module).should_receive('deduplicate_directories').and_return(('foo', 'bar'))
+    flexmock(module).should_receive('map_directories_to_devices').and_return({})
+    flexmock(module).should_receive('expand_directories').with_args(
+        ('foo', 'bar'), working_directory='/working'
+    ).and_return(()).once()
+    flexmock(module).should_receive('pattern_root_directories').and_return(())
+    flexmock(module).should_receive('expand_directories').with_args(
+        (), working_directory='/working'
+    ).and_return(())
+
+    assert module.process_source_directories(
+        config={'source_directories': ['nope']},
+        config_paths=('test.yaml',),
+        source_directories=['foo', 'bar'],
+    ) == ('foo', 'bar')
+
+
 def test_run_create_executes_and_calls_hooks_for_configured_repository():
 def test_run_create_executes_and_calls_hooks_for_configured_repository():
     flexmock(module.logger).answer = lambda message: None
     flexmock(module.logger).answer = lambda message: None
     flexmock(module.borgmatic.config.validate).should_receive('repositories_match').never()
     flexmock(module.borgmatic.config.validate).should_receive('repositories_match').never()