Pārlūkot izejas kodu

If the exact same "everything" command hook is present in multiple configuration files, borgmatic only runs it once (#1080).

Dan Helfman 1 mēnesi atpakaļ
vecāks
revīzija
9bf316e28f

+ 2 - 0
NEWS

@@ -4,6 +4,8 @@
    differs from other hooks:
    https://torsion.org/borgmatic/docs/how-to/add-preparation-and-cleanup-steps-to-backups/
  * #1075: Fix an incorrect warning about Borg placeholders being unsupported in a command hook.
+ * #1080: If the exact same "everything" command hook is present in multiple configuration files,
+   borgmatic only runs it once.
 
 2.0.3
  * #1065: Fix a regression in monitoring hooks in which an error pinged the finish state instead of

+ 41 - 19
borgmatic/commands/borgmatic.py

@@ -893,17 +893,29 @@ def collect_configuration_run_summary_logs(configs, config_paths, arguments, log
         return
 
     try:
+        seen_command_hooks = []
+
         for config_filename, config in configs.items():
-            command.execute_hooks(
-                command.filter_hooks(
-                    config.get('commands'), before='everything', action_names=arguments.keys()
+            command_hooks = command.filter_hooks(
+                tuple(
+                    command_hook
+                    for command_hook in config.get('commands', ())
+                    if command_hook not in seen_command_hooks
                 ),
-                config.get('umask'),
-                borgmatic.config.paths.get_working_directory(config),
-                arguments['global'].dry_run,
-                configuration_filename=config_filename,
-                log_file=log_file_path or '',
+                before='everything',
+                action_names=arguments.keys(),
             )
+
+            if command_hooks:
+                command.execute_hooks(
+                    command_hooks,
+                    config.get('umask'),
+                    borgmatic.config.paths.get_working_directory(config),
+                    arguments['global'].dry_run,
+                    configuration_filename=config_filename,
+                    log_file=log_file_path or '',
+                )
+                seen_command_hooks += list(command_hooks)
     except (CalledProcessError, ValueError, OSError) as error:
         yield from log_error_records('Error running before everything hook', error)
         return
@@ -951,20 +963,30 @@ def collect_configuration_run_summary_logs(configs, config_paths, arguments, log
         sys.stdout.write(json.dumps(json_results))
 
     try:
+        seen_command_hooks = []
+
         for config_filename, config in configs.items():
-            command.execute_hooks(
-                command.filter_hooks(
-                    config.get('commands'),
-                    after='everything',
-                    action_names=arguments.keys(),
-                    state_names=['fail' if encountered_error else 'finish'],
+            command_hooks = command.filter_hooks(
+                tuple(
+                    command_hook
+                    for command_hook in config.get('commands', ())
+                    if command_hook not in seen_command_hooks
                 ),
-                config.get('umask'),
-                borgmatic.config.paths.get_working_directory(config),
-                arguments['global'].dry_run,
-                configuration_filename=config_filename,
-                log_file=log_file_path or '',
+                after='everything',
+                action_names=arguments.keys(),
+                state_names=['fail' if encountered_error else 'finish'],
             )
+
+            if command_hooks:
+                command.execute_hooks(
+                    command_hooks,
+                    config.get('umask'),
+                    borgmatic.config.paths.get_working_directory(config),
+                    arguments['global'].dry_run,
+                    configuration_filename=config_filename,
+                    log_file=log_file_path or '',
+                )
+                seen_command_hooks += list(command_hooks)
     except (CalledProcessError, ValueError, OSError) as error:
         yield from log_error_records('Error running after everything hook', error)
 

+ 4 - 0
docs/how-to/add-preparation-and-cleanup-steps-to-backups.md

@@ -91,6 +91,10 @@ Each command in the `commands:` list has the following options:
 When command hooks run, they respect the `working_directory` option if it is
 configured, meaning that the hook commands are run in that directory.
 
+<span class="minilink minilink-addedin">New in version 2.0.4</span>If the exact
+same `everything` command hook is present in multiple configuration files,
+borgmatic only runs it once.
+
 
 ### Order of execution
 

+ 58 - 12
tests/unit/commands/test_borgmatic.py

@@ -1956,14 +1956,17 @@ def test_collect_configuration_run_summary_logs_info_for_success():
 
 
 def test_collect_configuration_run_summary_executes_hooks_for_create():
+    before_everything_hook = {'before': 'everything', 'run': ['echo hi']}
+    after_everything_hook = {'after': 'everything', 'run': ['echo hi']}
+    command_hooks = (before_everything_hook, after_everything_hook)
     flexmock(module.validate).should_receive('guard_configuration_contains_repository')
     flexmock(module.command).should_receive('filter_hooks').with_args(
-        object, before='everything', action_names=object
-    )
+        command_hooks, before='everything', action_names=object
+    ).and_return([before_everything_hook])
     flexmock(module.command).should_receive('filter_hooks').with_args(
-        object, after='everything', action_names=object, state_names=['finish']
-    )
-    flexmock(module.command).should_receive('execute_hooks')
+        command_hooks, after='everything', action_names=object, state_names=['finish']
+    ).and_return([after_everything_hook])
+    flexmock(module.command).should_receive('execute_hooks').twice()
     flexmock(module).should_receive('Log_prefix').and_return(flexmock())
     flexmock(module).should_receive('run_configuration').and_return([])
     arguments = {
@@ -1973,7 +1976,7 @@ def test_collect_configuration_run_summary_executes_hooks_for_create():
 
     logs = tuple(
         module.collect_configuration_run_summary_logs(
-            {'test.yaml': {}},
+            {'test.yaml': {'commands': command_hooks}},
             config_paths=['/tmp/test.yaml'],
             arguments=arguments,
             log_file_path=None,
@@ -1983,6 +1986,43 @@ def test_collect_configuration_run_summary_executes_hooks_for_create():
     assert {log.levelno for log in logs} == {logging.INFO}
 
 
+def test_collect_configuration_run_summary_deduplicates_everything_hooks_across_config_files():
+    before_everything_hook = {'before': 'everything', 'run': ['echo hi']}
+    after_everything_hook = {'after': 'everything', 'run': ['echo hi']}
+    command_hooks = (before_everything_hook, after_everything_hook)
+    flexmock(module.validate).should_receive('guard_configuration_contains_repository')
+    flexmock(module.command).should_receive('filter_hooks').with_args(
+        command_hooks, before='everything', action_names=object
+    ).and_return([before_everything_hook]).once()
+    flexmock(module.command).should_receive('filter_hooks').with_args(
+        (after_everything_hook,), before='everything', action_names=object
+    ).and_return([]).once()
+    flexmock(module.command).should_receive('filter_hooks').with_args(
+        command_hooks, after='everything', action_names=object, state_names=['finish']
+    ).and_return([after_everything_hook]).once()
+    flexmock(module.command).should_receive('filter_hooks').with_args(
+        (before_everything_hook,), after='everything', action_names=object, state_names=['finish']
+    ).and_return([]).once()
+    flexmock(module.command).should_receive('execute_hooks').twice()
+    flexmock(module).should_receive('Log_prefix').and_return(flexmock())
+    flexmock(module).should_receive('run_configuration').and_return([])
+    arguments = {
+        'create': flexmock(),
+        'global': flexmock(monitoring_verbosity=1, dry_run=False),
+    }
+
+    logs = tuple(
+        module.collect_configuration_run_summary_logs(
+            {'test.yaml': {'commands': command_hooks}, 'other.yaml': {'commands': command_hooks}},
+            config_paths=['/tmp/test.yaml', '/tmp/other.yaml'],
+            arguments=arguments,
+            log_file_path=None,
+        )
+    )
+
+    assert {log.levelno for log in logs} == {logging.INFO}
+
+
 def test_collect_configuration_run_summary_logs_info_for_success_with_extract():
     flexmock(module.validate).should_receive('guard_configuration_contains_repository')
     flexmock(module.command).should_receive('filter_hooks').with_args(
@@ -2105,13 +2145,16 @@ def test_collect_configuration_run_summary_logs_missing_configs_error():
 
 
 def test_collect_configuration_run_summary_logs_before_hook_error():
+    before_everything_hook = {'before': 'everything', 'run': ['echo hi']}
+    after_everything_hook = {'after': 'everything', 'run': ['echo hi']}
+    command_hooks = (before_everything_hook, after_everything_hook)
     flexmock(module.validate).should_receive('guard_configuration_contains_repository')
     flexmock(module.command).should_receive('filter_hooks').with_args(
         object, before='everything', action_names=object
-    )
+    ).and_return([before_everything_hook])
     flexmock(module.command).should_receive('filter_hooks').with_args(
         object, after='everything', action_names=object, state_names=['fail']
-    )
+    ).and_return([after_everything_hook])
     flexmock(module.command).should_receive('execute_hooks').and_raise(ValueError)
     expected_logs = (flexmock(),)
     flexmock(module).should_receive('log_error_records').and_return(expected_logs)
@@ -2122,7 +2165,7 @@ def test_collect_configuration_run_summary_logs_before_hook_error():
 
     logs = tuple(
         module.collect_configuration_run_summary_logs(
-            {'test.yaml': {}},
+            {'test.yaml': {'commands': command_hooks}},
             config_paths=['/tmp/test.yaml'],
             arguments=arguments,
             log_file_path=None,
@@ -2133,13 +2176,16 @@ def test_collect_configuration_run_summary_logs_before_hook_error():
 
 
 def test_collect_configuration_run_summary_logs_after_hook_error():
+    before_everything_hook = {'before': 'everything', 'run': ['echo hi']}
+    after_everything_hook = {'after': 'everything', 'run': ['echo hi']}
+    command_hooks = (before_everything_hook, after_everything_hook)
     flexmock(module.validate).should_receive('guard_configuration_contains_repository')
     flexmock(module.command).should_receive('filter_hooks').with_args(
         object, before='everything', action_names=object
-    )
+    ).and_return([before_everything_hook])
     flexmock(module.command).should_receive('filter_hooks').with_args(
         object, after='everything', action_names=object, state_names=['finish']
-    )
+    ).and_return([after_everything_hook])
     flexmock(module.command).should_receive('execute_hooks').and_return(None).and_raise(ValueError)
     flexmock(module).should_receive('Log_prefix').and_return(flexmock())
     flexmock(module).should_receive('run_configuration').and_return([])
@@ -2152,7 +2198,7 @@ def test_collect_configuration_run_summary_logs_after_hook_error():
 
     logs = tuple(
         module.collect_configuration_run_summary_logs(
-            {'test.yaml': {}},
+            {'test.yaml': {'commands': command_hooks}},
             config_paths=['/tmp/test.yaml'],
             arguments=arguments,
             log_file_path=None,