Run all command hooks respecting the "working_directory" option if configured (#790).

NEWS

@@ -2,9 +2,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:

borgmatic/commands/borgmatic.py

@@ -33,6 +33,7 @@ import borgmatic.actions.restore
 import borgmatic.actions.transfer
 import borgmatic.commands.completion.bash
 import borgmatic.commands.completion.fish
+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
@@ -207,6 +208,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,
@@ -286,6 +288,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 '',
@@ -342,6 +345,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,
@@ -354,6 +358,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,
@@ -854,6 +859,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 '',
@@ -908,6 +914,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 '',

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,
             )

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
 <span class="minilink minilink-addedin">Prior to version 1.8.0</span> Put
 these options in the `hooks:` section of your configuration.
 
+<span class="minilink minilink-addedin">New in version 2.0.0</span> 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.
+
+<span class="minilink minilink-addedin">New in version 2.0.0</span> 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 1.7.0</span> 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

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'],