diff --git a/NEWS b/NEWS index 30ecdb7a..8f8d4bc0 100644 --- a/NEWS +++ b/NEWS @@ -2,8 +2,9 @@ * #1035: Document potential performance issues and workarounds with the ZFS, Btrfs, and LVM hooks: https://torsion.org/borgmatic/docs/how-to/snapshot-your-filesystems/ * #1059: Fix a regression in which soft failure exit codes in command hooks were not respected. - * #1060: Fix command hooks getting run too many times when multiple borgmatic actions are executed - (implicitly or explicitly). + * #1060: Fix action command hooks getting run too many times when multiple borgmatic actions are + executed (implicitly or explicitly). + * #1060: Don't run action command hooks for actions listed in the "skip_actions" option. 2.0.1 * #1057: Fix argument parsing to avoid using Python 3.12+ string features. Now borgmatic will diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 2613646d..6a9cd309 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -356,7 +356,7 @@ def run_actions( **hook_context, ): for action_name, action_arguments in arguments.items(): - if action_name == 'global': + if action_name == 'global' or action_name in skip_actions: continue with borgmatic.hooks.command.Before_after_hooks( @@ -368,7 +368,7 @@ def run_actions( action_names=(action_name,), **hook_context, ): - if action_name == 'repo-create' and action_name not in skip_actions: + if action_name == 'repo-create': borgmatic.actions.repo_create.run_repo_create( repository, config, @@ -378,7 +378,7 @@ def run_actions( local_path, remote_path, ) - elif action_name == 'transfer' and action_name not in skip_actions: + elif action_name == 'transfer': borgmatic.actions.transfer.run_transfer( repository, config, @@ -388,7 +388,7 @@ def run_actions( local_path, remote_path, ) - elif action_name == 'create' and action_name not in skip_actions: + elif action_name == 'create': yield from borgmatic.actions.create.run_create( config_filename, repository, @@ -401,7 +401,7 @@ def run_actions( local_path, remote_path, ) - elif action_name == 'recreate' and action_name not in skip_actions: + elif action_name == 'recreate': borgmatic.actions.recreate.run_recreate( repository, config, @@ -411,7 +411,7 @@ def run_actions( local_path, remote_path, ) - elif action_name == 'prune' and action_name not in skip_actions: + elif action_name == 'prune': borgmatic.actions.prune.run_prune( config_filename, repository, @@ -423,7 +423,7 @@ def run_actions( local_path, remote_path, ) - elif action_name == 'compact' and action_name not in skip_actions: + elif action_name == 'compact': borgmatic.actions.compact.run_compact( config_filename, repository, @@ -435,7 +435,7 @@ def run_actions( local_path, remote_path, ) - elif action_name == 'check' and action_name not in skip_actions: + elif action_name == 'check': if checks.repository_enabled_for_checks(repository, config): borgmatic.actions.check.run_check( config_filename, @@ -447,7 +447,7 @@ def run_actions( local_path, remote_path, ) - elif action_name == 'extract' and action_name not in skip_actions: + elif action_name == 'extract': borgmatic.actions.extract.run_extract( config_filename, repository, @@ -458,7 +458,7 @@ def run_actions( local_path, remote_path, ) - elif action_name == 'export-tar' and action_name not in skip_actions: + elif action_name == 'export-tar': borgmatic.actions.export_tar.run_export_tar( repository, config, @@ -468,7 +468,7 @@ def run_actions( local_path, remote_path, ) - elif action_name == 'mount' and action_name not in skip_actions: + elif action_name == 'mount': borgmatic.actions.mount.run_mount( repository, config, @@ -478,7 +478,7 @@ def run_actions( local_path, remote_path, ) - elif action_name == 'restore' and action_name not in skip_actions: + elif action_name == 'restore': borgmatic.actions.restore.run_restore( repository, config, @@ -488,7 +488,7 @@ def run_actions( local_path, remote_path, ) - elif action_name == 'repo-list' and action_name not in skip_actions: + elif action_name == 'repo-list': yield from borgmatic.actions.repo_list.run_repo_list( repository, config, @@ -498,7 +498,7 @@ def run_actions( local_path, remote_path, ) - elif action_name == 'list' and action_name not in skip_actions: + elif action_name == 'list': yield from borgmatic.actions.list.run_list( repository, config, @@ -508,7 +508,7 @@ def run_actions( local_path, remote_path, ) - elif action_name == 'repo-info' and action_name not in skip_actions: + elif action_name == 'repo-info': yield from borgmatic.actions.repo_info.run_repo_info( repository, config, @@ -518,7 +518,7 @@ def run_actions( local_path, remote_path, ) - elif action_name == 'info' and action_name not in skip_actions: + elif action_name == 'info': yield from borgmatic.actions.info.run_info( repository, config, @@ -528,7 +528,7 @@ def run_actions( local_path, remote_path, ) - elif action_name == 'break-lock' and action_name not in skip_actions: + elif action_name == 'break-lock': borgmatic.actions.break_lock.run_break_lock( repository, config, @@ -538,7 +538,7 @@ def run_actions( local_path, remote_path, ) - elif action_name == 'export' and action_name not in skip_actions: + elif action_name == 'export': borgmatic.actions.export_key.run_export_key( repository, config, @@ -548,7 +548,7 @@ def run_actions( local_path, remote_path, ) - elif action_name == 'import' and action_name not in skip_actions: + elif action_name == 'import': borgmatic.actions.import_key.run_import_key( repository, config, @@ -558,7 +558,7 @@ def run_actions( local_path, remote_path, ) - elif action_name == 'change-passphrase' and action_name not in skip_actions: + elif action_name == 'change-passphrase': borgmatic.actions.change_passphrase.run_change_passphrase( repository, config, @@ -568,7 +568,7 @@ def run_actions( local_path, remote_path, ) - elif action_name == 'delete' and action_name not in skip_actions: + elif action_name == 'delete': borgmatic.actions.delete.run_delete( repository, config, @@ -578,7 +578,7 @@ def run_actions( local_path, remote_path, ) - elif action_name == 'repo-delete' and action_name not in skip_actions: + elif action_name == 'repo-delete': borgmatic.actions.repo_delete.run_repo_delete( repository, config, @@ -588,7 +588,7 @@ def run_actions( local_path, remote_path, ) - elif action_name == 'borg' and action_name not in skip_actions: + elif action_name == 'borg': borgmatic.actions.borg.run_borg( repository, config, diff --git a/tests/unit/commands/test_borgmatic.py b/tests/unit/commands/test_borgmatic.py index 433b533e..b5ab13d4 100644 --- a/tests/unit/commands/test_borgmatic.py +++ b/tests/unit/commands/test_borgmatic.py @@ -1034,13 +1034,26 @@ def test_run_actions_runs_create(): assert result == (expected,) -def test_run_actions_with_skip_actions_skips_create(): +def test_run_actions_with_skip_actions_does_not_run_action_or_action_command_hooks(): flexmock(module).should_receive('add_custom_log_levels') flexmock(module).should_receive('get_skip_actions').and_return(['create']) flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return( flexmock() ) flexmock(module.command).should_receive('Before_after_hooks').and_return(flexmock()) + flexmock(module.command).should_receive('Before_after_hooks').with_args( + command_hooks=object, + before_after='action', + umask=object, + working_directory=object, + dry_run=object, + action_names=object, + configuration_filename=object, + repository_label=object, + log_file=object, + repositories=object, + repository=object, + ).never() flexmock(borgmatic.actions.create).should_receive('run_create').never() tuple( @@ -1081,29 +1094,6 @@ def test_run_actions_runs_recreate(): ) -def test_run_actions_with_skip_actions_skips_recreate(): - flexmock(module).should_receive('add_custom_log_levels') - flexmock(module).should_receive('get_skip_actions').and_return(['recreate']) - flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return( - flexmock() - ) - flexmock(module.command).should_receive('Before_after_hooks').and_return(flexmock()) - flexmock(borgmatic.actions.recreate).should_receive('run_recreate').never() - - tuple( - module.run_actions( - arguments={'global': flexmock(dry_run=False), 'recreate': flexmock()}, - config_filename=flexmock(), - config={'repositories': [], 'skip_actions': ['recreate']}, - config_paths=[], - local_path=flexmock(), - remote_path=flexmock(), - local_borg_version=flexmock(), - repository={'path': 'repo'}, - ) - ) - - def test_run_actions_runs_prune(): flexmock(module).should_receive('add_custom_log_levels') flexmock(module).should_receive('get_skip_actions').and_return([]) @@ -1127,29 +1117,6 @@ def test_run_actions_runs_prune(): ) -def test_run_actions_with_skip_actions_skips_prune(): - flexmock(module).should_receive('add_custom_log_levels') - flexmock(module).should_receive('get_skip_actions').and_return(['prune']) - flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return( - flexmock() - ) - flexmock(module.command).should_receive('Before_after_hooks').and_return(flexmock()) - flexmock(borgmatic.actions.prune).should_receive('run_prune').never() - - tuple( - module.run_actions( - arguments={'global': flexmock(dry_run=False), 'prune': flexmock()}, - config_filename=flexmock(), - config={'repositories': [], 'skip_actions': ['prune']}, - config_paths=[], - local_path=flexmock(), - remote_path=flexmock(), - local_borg_version=flexmock(), - repository={'path': 'repo'}, - ) - ) - - def test_run_actions_runs_compact(): flexmock(module).should_receive('add_custom_log_levels') flexmock(module).should_receive('get_skip_actions').and_return([]) @@ -1173,29 +1140,6 @@ def test_run_actions_runs_compact(): ) -def test_run_actions_with_skip_actions_skips_compact(): - flexmock(module).should_receive('add_custom_log_levels') - flexmock(module).should_receive('get_skip_actions').and_return(['compact']) - flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return( - flexmock() - ) - flexmock(module.command).should_receive('Before_after_hooks').and_return(flexmock()) - flexmock(borgmatic.actions.compact).should_receive('run_compact').never() - - tuple( - module.run_actions( - arguments={'global': flexmock(dry_run=False), 'compact': flexmock()}, - config_filename=flexmock(), - config={'repositories': [], 'skip_actions': ['compact']}, - config_paths=[], - local_path=flexmock(), - remote_path=flexmock(), - local_borg_version=flexmock(), - repository={'path': 'repo'}, - ) - ) - - def test_run_actions_runs_check_when_repository_enabled_for_checks(): flexmock(module).should_receive('add_custom_log_levels') flexmock(module).should_receive('get_skip_actions').and_return([]) @@ -1244,30 +1188,6 @@ def test_run_actions_skips_check_when_repository_not_enabled_for_checks(): ) -def test_run_actions_with_skip_actions_skips_check(): - flexmock(module).should_receive('add_custom_log_levels') - flexmock(module).should_receive('get_skip_actions').and_return(['check']) - flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return( - flexmock() - ) - flexmock(module.command).should_receive('Before_after_hooks').and_return(flexmock()) - flexmock(module.checks).should_receive('repository_enabled_for_checks').and_return(True) - flexmock(borgmatic.actions.check).should_receive('run_check').never() - - tuple( - module.run_actions( - arguments={'global': flexmock(dry_run=False), 'check': flexmock()}, - config_filename=flexmock(), - config={'repositories': [], 'skip_actions': ['check']}, - config_paths=[], - local_path=flexmock(), - remote_path=flexmock(), - local_borg_version=flexmock(), - repository={'path': 'repo'}, - ) - ) - - def test_run_actions_runs_extract(): flexmock(module).should_receive('add_custom_log_levels') flexmock(module).should_receive('get_skip_actions').and_return([])