diff --git a/NEWS b/NEWS index cb41b2c8..9c03f52a 100644 --- a/NEWS +++ b/NEWS @@ -9,9 +9,11 @@ * #790, #821: Deprecate all "before_*", "after_*" and "on_error" command hooks in favor of more flexible "commands:". See the documentation for more information: https://torsion.org/borgmatic/docs/how-to/add-preparation-and-cleanup-steps-to-backups/ - * #790: BREAKING: Run a configured "after" command hook even if an error occurs first. This - allows you to perform cleanup steps that correspond to "before" preparation commands—even when - something goes wrong. + * #790: BREAKING: For both new and deprecated command hooks, run a configured "after" hook even if + an error occurs first. This allows you to perform cleanup steps that correspond to "before" + preparation commands—even when something goes wrong. + * #790: BREAKING: Run all command hooks (both new and deprecated) respecting the + "working_directory" option if configured, meaning that hook commands are run in that directory. * #836: Add a custom command option for the SQLite hook. * #1010: When using Borg 2, don't pass the "--stats" flag to "borg prune". * #1020: Document a database use case involving a temporary database client container: diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 9cffbdf8..504a094c 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -36,6 +36,7 @@ import borgmatic.actions.transfer import borgmatic.commands.completion.bash import borgmatic.commands.completion.fish import borgmatic.config.load +import borgmatic.config.paths from borgmatic.borg import umount as borg_umount from borgmatic.borg import version as borg_version from borgmatic.commands.arguments import parse_arguments @@ -210,6 +211,7 @@ def run_configuration(config_filename, config, config_paths, arguments): command_hooks=config.get('commands'), before_after='configuration', umask=config.get('umask'), + working_directory=borgmatic.config.paths.get_working_directory(config), dry_run=global_arguments.dry_run, action_names=arguments.keys(), configuration_filename=config_filename, @@ -289,6 +291,7 @@ def run_configuration(config_filename, config, config_paths, arguments): config.get('commands'), after='error', action_names=arguments.keys() ), config.get('umask'), + borgmatic.config.paths.get_working_directory(config), global_arguments.dry_run, configuration_filename=config_filename, log_file=arguments['global'].log_file or '', @@ -345,6 +348,7 @@ def run_actions( command_hooks=config.get('commands'), before_after='repository', umask=config.get('umask'), + working_directory=borgmatic.config.paths.get_working_directory(config), dry_run=global_arguments.dry_run, action_names=arguments.keys(), **hook_context, @@ -357,6 +361,7 @@ def run_actions( command_hooks=config.get('commands'), before_after='action', umask=config.get('umask'), + working_directory=borgmatic.config.paths.get_working_directory(config), dry_run=global_arguments.dry_run, action_names=arguments.keys(), **hook_context, @@ -858,6 +863,7 @@ def collect_configuration_run_summary_logs(configs, config_paths, arguments): config.get('commands'), before='everything', action_names=arguments.keys() ), config.get('umask'), + borgmatic.config.paths.get_working_directory(config), arguments['global'].dry_run, configuration_filename=config_filename, log_file=arguments['global'].log_file or '', @@ -912,6 +918,7 @@ def collect_configuration_run_summary_logs(configs, config_paths, arguments): config.get('commands'), after='everything', action_names=arguments.keys() ), config.get('umask'), + borgmatic.config.paths.get_working_directory(config), arguments['global'].dry_run, configuration_filename=config_filename, log_file=arguments['global'].log_file or '', diff --git a/borgmatic/hooks/command.py b/borgmatic/hooks/command.py index c6fcc397..eff1a752 100644 --- a/borgmatic/hooks/command.py +++ b/borgmatic/hooks/command.py @@ -64,11 +64,11 @@ def filter_hooks(command_hooks, before=None, after=None, hook_name=None, action_ ) -def execute_hooks(command_hooks, umask, dry_run, **context): +def execute_hooks(command_hooks, umask, working_directory, dry_run, **context): ''' - Given a sequence of command hook dicts from configuration, a umask to execute with (or None), - and whether this is a dry run, run the commands for each hook. Or don't run them if this is a - dry run. + Given a sequence of command hook dicts from configuration, a umask to execute with (or None), a + working directory to execute with, and whether this is a dry run, run the commands for each + hook. Or don't run them if this is a dry run. The context contains optional values interpolated by name into the hook commands. @@ -121,6 +121,7 @@ def execute_hooks(command_hooks, umask, dry_run, **context): ), shell=True, environment=make_environment(os.environ), + working_directory=working_directory, ) finally: if original_umask: @@ -153,6 +154,7 @@ class Before_after_hooks: command_hooks, before_after, umask, + working_directory, dry_run, hook_name=None, action_names=None, @@ -160,12 +162,14 @@ class Before_after_hooks: ): ''' Given a sequence of command hook configuration dicts, the before/after name, a umask to run - commands with, a dry run flag, the name of the calling hook, a sequence of action names, and - any context for the executed commands, save those data points for use below. + commands with, a working directory to run commands with, a dry run flag, the name of the + calling hook, a sequence of action names, and any context for the executed commands, save + those data points for use below. ''' self.command_hooks = command_hooks self.before_after = before_after self.umask = umask + self.working_directory = working_directory self.dry_run = dry_run self.hook_name = hook_name self.action_names = action_names @@ -184,6 +188,7 @@ class Before_after_hooks: action_names=self.action_names, ), self.umask, + self.working_directory, self.dry_run, **self.context, ) @@ -210,6 +215,7 @@ class Before_after_hooks: action_names=self.action_names, ), self.umask, + self.working_directory, self.dry_run, **self.context, ) diff --git a/docs/how-to/add-preparation-and-cleanup-steps-to-backups.md b/docs/how-to/add-preparation-and-cleanup-steps-to-backups.md index 096097b8..977e87f1 100644 --- a/docs/how-to/add-preparation-and-cleanup-steps-to-backups.md +++ b/docs/how-to/add-preparation-and-cleanup-steps-to-backups.md @@ -66,10 +66,15 @@ Each command in the `commands:` list has the following options: * `when`: Only trigger the hook when borgmatic is run with particular actions (`create`, `prune`, etc.) listed here. Defaults to running for all actions. * `run`: List of one or more shell commands or scripts to run when this command hook is triggered. -An `after` command hook runs even if an error occurs in the corresponding `before` hook or between -those two hooks. This allows you to perform cleanup steps that correspond to `before` preparation -commands—even when something goes wrong. This is a departure from the way that the deprecated -`after_*` hooks worked. +An `after` command hook runs even if an error occurs in the corresponding +`before` hook or between those two hooks. This allows you to perform cleanup +steps that correspond to `before` preparation commands—even when something goes +wrong. This is a departure from the way that the deprecated `after_*` hooks +worked in borgmatic prior to version 2.0.0. + +Additionally, when command hooks run, they respect the "working_directory" +option if it is configured, meaning that the hook commands are run in that +directory. ### Order of execution @@ -104,7 +109,10 @@ configuration files. command hooks worked a little differently. In these older versions of borgmatic, you can specify `before_backup` hooks to perform preparation steps before running backups and specify `after_backup` hooks to perform cleanup steps -afterwards. Here's an example: +afterwards. These deprecated command hooks still work in version 2.0.0+, +although see below about a few semantic differences starting in that version. + +Here's an example of these deprecated hooks: ```yaml before_backup: @@ -129,6 +137,15 @@ instance, `before_prune` runs before a `prune` action for a repository, while Prior to version 1.8.0 Put these options in the `hooks:` section of your configuration. +New in version 2.0.0 An `after_*` +command hook runs even if an error occurs in the corresponding `before_*` hook +or between those two hooks. This allows you to perform cleanup steps that +correspond to `before_*` preparation commands—even when something goes wrong. + +New in version 2.0.0 When command +hooks run, they respect the "working_directory" option if it is configured, +meaning that the hook commands are run in that directory. + New in version 1.7.0 The `before_actions` and `after_actions` hooks run before/after all the actions (like `create`, `prune`, etc.) for each repository. These hooks are a good diff --git a/tests/unit/hooks/test_command.py b/tests/unit/hooks/test_command.py index f4925b36..ca6af167 100644 --- a/tests/unit/hooks/test_command.py +++ b/tests/unit/hooks/test_command.py @@ -190,11 +190,13 @@ def test_execute_hooks_invokes_each_hook_and_command(): output_log_level=LOGGING_ANSWER, shell=True, environment={}, + working_directory=None, ).once() module.execute_hooks( [{'before': 'create', 'run': ['foo']}, {'before': 'create', 'run': ['bar', 'baz']}], umask=None, + working_directory=None, dry_run=False, ) @@ -213,9 +215,35 @@ def test_execute_hooks_with_umask_sets_that_umask(): output_log_level=logging.ANSWER, shell=True, environment={}, + working_directory=None, ) - module.execute_hooks([{'before': 'create', 'run': ['foo']}], umask=77, dry_run=False) + module.execute_hooks( + [{'before': 'create', 'run': ['foo']}], umask=77, working_directory=None, dry_run=False + ) + + +def test_execute_hooks_with_working_directory_executes_command_with_it(): + flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') + flexmock(module.logging).ANSWER = LOGGING_ANSWER + flexmock(module).should_receive('interpolate_context').replace_with( + lambda hook_description, command, context: command + ) + flexmock(module).should_receive('make_environment').and_return({}) + flexmock(module.borgmatic.execute).should_receive('execute_command').with_args( + ['foo'], + output_log_level=logging.ANSWER, + shell=True, + environment={}, + working_directory='/working', + ) + + module.execute_hooks( + [{'before': 'create', 'run': ['foo']}], + umask=None, + working_directory='/working', + dry_run=False, + ) def test_execute_hooks_with_dry_run_skips_commands(): @@ -227,11 +255,13 @@ def test_execute_hooks_with_dry_run_skips_commands(): flexmock(module).should_receive('make_environment').and_return({}) flexmock(module.borgmatic.execute).should_receive('execute_command').never() - module.execute_hooks([{'before': 'create', 'run': ['foo']}], umask=None, dry_run=True) + module.execute_hooks( + [{'before': 'create', 'run': ['foo']}], umask=None, working_directory=None, dry_run=True + ) def test_execute_hooks_with_empty_commands_does_not_raise(): - module.execute_hooks([], umask=None, dry_run=True) + module.execute_hooks([], umask=None, working_directory=None, dry_run=True) def test_execute_hooks_with_error_logs_as_error(): @@ -246,9 +276,12 @@ def test_execute_hooks_with_error_logs_as_error(): output_log_level=logging.ERROR, shell=True, environment={}, + working_directory=None, ).once() - module.execute_hooks([{'after': 'error', 'run': ['foo']}], umask=None, dry_run=False) + module.execute_hooks( + [{'after': 'error', 'run': ['foo']}], umask=None, working_directory=None, dry_run=False + ) def test_execute_hooks_with_before_or_after_raises(): @@ -265,6 +298,7 @@ def test_execute_hooks_with_before_or_after_raises(): {'erstwhile': 'create', 'run': ['bar', 'baz']}, ], umask=None, + working_directory=None, dry_run=False, ) @@ -283,11 +317,13 @@ def test_execute_hooks_without_commands_to_run_does_not_raise(): output_log_level=LOGGING_ANSWER, shell=True, environment={}, + working_directory=None, ).once() module.execute_hooks( [{'before': 'create', 'run': []}, {'before': 'create', 'run': ['foo', 'bar']}], umask=None, + working_directory=None, dry_run=False, ) @@ -315,6 +351,7 @@ def test_before_after_hooks_calls_command_hooks(): command_hooks=commands, before_after='action', umask=1234, + working_directory='/working', dry_run=False, hook_name='myhook', action_names=['create'], @@ -349,6 +386,7 @@ def test_before_after_hooks_with_before_error_runs_after_hook_and_raises(): command_hooks=commands, before_after='action', umask=1234, + working_directory='/working', dry_run=False, hook_name='myhook', action_names=['create'], @@ -382,6 +420,7 @@ def test_before_after_hooks_with_before_soft_failure_does_not_raise(): command_hooks=commands, before_after='action', umask=1234, + working_directory='/working', dry_run=False, hook_name='myhook', action_names=['create'], @@ -416,6 +455,7 @@ def test_before_after_hooks_with_after_error_raises(): command_hooks=commands, before_after='action', umask=1234, + working_directory='/working', dry_run=False, hook_name='myhook', action_names=['create'], @@ -449,6 +489,7 @@ def test_before_after_hooks_with_after_soft_failure_does_not_raise(): command_hooks=commands, before_after='action', umask=1234, + working_directory='/working', dry_run=False, hook_name='myhook', action_names=['create'],