Initial work on unified command hooks (#790).

NEWS

@@ -2,6 +2,9 @@
  * #409: With the PagerDuty monitoring hook, send borgmatic logs to PagerDuty so they show up in the
    incident UI. See the documentation for more information:
    https://torsion.org/borgmatic/docs/how-to/monitor-your-backups/#pagerduty-hook
+ * #790: 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/
  * #936: Clarify Zabbix monitoring hook documentation about creating items:
    https://torsion.org/borgmatic/docs/how-to/monitor-your-backups/#zabbix-hook
  * #1017: Fix a regression in which some MariaDB/MySQL passwords were not escaped correctly.

borgmatic/actions/check.py

@@ -682,7 +682,6 @@ def run_check(
     config_filename,
     repository,
     config,
-    hook_context,
     local_borg_version,
     check_arguments,
     global_arguments,
@@ -699,15 +698,6 @@ def run_check(
     ):
         return
 
-    borgmatic.hooks.command.execute_hook(
-        config.get('before_check'),
-        config.get('umask'),
-        config_filename,
-        'pre-check',
-        global_arguments.dry_run,
-        **hook_context,
-    )
-
     logger.info('Running consistency checks')
 
     repository_id = borgmatic.borg.check.get_repository_id(
@@ -772,12 +762,3 @@ def run_check(
                 borgmatic_runtime_directory,
             )
         write_check_time(make_check_time_path(config, repository_id, 'spot'))
-
-    borgmatic.hooks.command.execute_hook(
-        config.get('after_check'),
-        config.get('umask'),
-        config_filename,
-        'post-check',
-        global_arguments.dry_run,
-        **hook_context,
-    )

borgmatic/actions/compact.py

@@ -12,7 +12,6 @@ def run_compact(
     config_filename,
     repository,
     config,
-    hook_context,
     local_borg_version,
     compact_arguments,
     global_arguments,
@@ -28,14 +27,6 @@ def run_compact(
     ):
         return
 
-    borgmatic.hooks.command.execute_hook(
-        config.get('before_compact'),
-        config.get('umask'),
-        config_filename,
-        'pre-compact',
-        global_arguments.dry_run,
-        **hook_context,
-    )
     if borgmatic.borg.feature.available(borgmatic.borg.feature.Feature.COMPACT, local_borg_version):
         logger.info(f'Compacting segments{dry_run_label}')
         borgmatic.borg.compact.compact_segments(
@@ -52,12 +43,3 @@ def run_compact(
         )
     else:  # pragma: nocover
         logger.info('Skipping compact (only available/needed in Borg 1.2+)')
-
-    borgmatic.hooks.command.execute_hook(
-        config.get('after_compact'),
-        config.get('umask'),
-        config_filename,
-        'post-compact',
-        global_arguments.dry_run,
-        **hook_context,
-    )

borgmatic/actions/create.py

@@ -261,7 +261,6 @@ def run_create(
     repository,
     config,
     config_paths,
-    hook_context,
     local_borg_version,
     create_arguments,
     global_arguments,
@@ -279,15 +278,6 @@ def run_create(
     ):
         return
 
-    borgmatic.hooks.command.execute_hook(
-        config.get('before_backup'),
-        config.get('umask'),
-        config_filename,
-        'pre-backup',
-        global_arguments.dry_run,
-        **hook_context,
-    )
-
     logger.info(f'Creating archive{dry_run_label}')
     working_directory = borgmatic.config.paths.get_working_directory(config)
 
@@ -343,12 +333,3 @@ def run_create(
             borgmatic_runtime_directory,
             global_arguments.dry_run,
         )
-
-    borgmatic.hooks.command.execute_hook(
-        config.get('after_backup'),
-        config.get('umask'),
-        config_filename,
-        'post-backup',
-        global_arguments.dry_run,
-        **hook_context,
-    )

borgmatic/actions/extract.py

@@ -12,7 +12,6 @@ def run_extract(
     config_filename,
     repository,
     config,
-    hook_context,
     local_borg_version,
     extract_arguments,
     global_arguments,
@@ -22,14 +21,6 @@ def run_extract(
     '''
     Run the "extract" action for the given repository.
     '''
-    borgmatic.hooks.command.execute_hook(
-        config.get('before_extract'),
-        config.get('umask'),
-        config_filename,
-        'pre-extract',
-        global_arguments.dry_run,
-        **hook_context,
-    )
     if extract_arguments.repository is None or borgmatic.config.validate.repositories_match(
         repository, extract_arguments.repository
     ):
@@ -56,11 +47,3 @@ def run_extract(
             strip_components=extract_arguments.strip_components,
             progress=extract_arguments.progress,
         )
-    borgmatic.hooks.command.execute_hook(
-        config.get('after_extract'),
-        config.get('umask'),
-        config_filename,
-        'post-extract',
-        global_arguments.dry_run,
-        **hook_context,
-    )

borgmatic/actions/prune.py

@@ -11,7 +11,6 @@ def run_prune(
     config_filename,
     repository,
     config,
-    hook_context,
     local_borg_version,
     prune_arguments,
     global_arguments,
@@ -27,14 +26,6 @@ def run_prune(
     ):
         return
 
-    borgmatic.hooks.command.execute_hook(
-        config.get('before_prune'),
-        config.get('umask'),
-        config_filename,
-        'pre-prune',
-        global_arguments.dry_run,
-        **hook_context,
-    )
     logger.info(f'Pruning archives{dry_run_label}')
     borgmatic.borg.prune.prune_archives(
         global_arguments.dry_run,
@@ -46,11 +37,3 @@ def run_prune(
         local_path=local_path,
         remote_path=remote_path,
     )
-    borgmatic.hooks.command.execute_hook(
-        config.get('after_prune'),
-        config.get('umask'),
-        config_filename,
-        'post-prune',
-        global_arguments.dry_run,
-        **hook_context,
-    )

borgmatic/commands/borgmatic.py

@@ -96,6 +96,13 @@ def run_configuration(config_filename, config, config_paths, arguments):
             f"Skipping {'/'.join(skip_actions)} action{'s' if len(skip_actions) > 1 else ''} due to configured skip_actions"
         )
 
+    command.execute_hooks(
+        command.filter_hooks(config.get('commands'), before='configuration', action_names=arguments.keys()),
+        config.get('umask'),
+        global_arguments.dry_run,
+        configuration_filename=config_filename,
+    )
+
     try:
         local_borg_version = borg_version.local_borg_version(config, local_path)
         logger.debug(f'Borg {local_borg_version}')
@@ -226,18 +233,26 @@ def run_configuration(config_filename, config, config_paths, arguments):
             encountered_error = error
             yield from log_error_records(f'{config_filename}: Error pinging monitor', error)
 
+    if encountered_error:
+        command.execute_hooks(
+            command.filter_hooks(config.get('commands'), after='error', action_names=arguments.keys()),
+            config.get('umask'),
+            global_arguments.dry_run,
+            configuration_filename=config_filename,
+            repository=error_repository,
+            error=encountered_error,
+            output=getattr(encountered_error, 'output', ''),
+        )
+
+    command.execute_hooks(
+        command.filter_hooks(config.get('commands'), after='configuration', action_names=arguments.keys()),
+        config.get('umask'),
+        global_arguments.dry_run,
+        configuration_filename=config_filename,
+    )
+
     if encountered_error and using_primary_action:
         try:
-            command.execute_hook(
-                config.get('on_error'),
-                config.get('umask'),
-                config_filename,
-                'on-error',
-                global_arguments.dry_run,
-                repository=error_repository,
-                error=encountered_error,
-                output=getattr(encountered_error, 'output', ''),
-            )
             dispatch.call_hooks(
                 'ping_monitor',
                 config,
@@ -289,6 +304,7 @@ def run_actions(
     global_arguments = arguments['global']
     dry_run_label = ' (dry run; not making any changes)' if global_arguments.dry_run else ''
     hook_context = {
+        'configuration_filename': config_filename,
         'repository_label': repository.get('label', ''),
         'log_file': global_arguments.log_file if global_arguments.log_file else '',
         # Deprecated: For backwards compatibility with borgmatic < 1.6.0.
@@ -297,16 +313,24 @@ def run_actions(
     }
     skip_actions = set(get_skip_actions(config, arguments))
 
-    command.execute_hook(
-        config.get('before_actions'),
+    command.execute_hooks(
+        command.filter_hooks(config.get('commands'), before='repository', action_names=arguments.keys()),
         config.get('umask'),
-        config_filename,
-        'pre-actions',
         global_arguments.dry_run,
         **hook_context,
     )
 
     for action_name, action_arguments in arguments.items():
+        if action_name == 'global':
+            continue
+
+        command.execute_hooks(
+            command.filter_hooks(config.get('commands'), before='action', action_names=arguments.keys()),
+            config.get('umask'),
+            global_arguments.dry_run,
+            **hook_context,
+        )
+
         if action_name == 'repo-create' and action_name not in skip_actions:
             borgmatic.actions.repo_create.run_repo_create(
                 repository,
@@ -333,7 +357,6 @@ def run_actions(
                 repository,
                 config,
                 config_paths,
-                hook_context,
                 local_borg_version,
                 action_arguments,
                 global_arguments,
@@ -346,7 +369,6 @@ def run_actions(
                 config_filename,
                 repository,
                 config,
-                hook_context,
                 local_borg_version,
                 action_arguments,
                 global_arguments,
@@ -359,7 +381,6 @@ def run_actions(
                 config_filename,
                 repository,
                 config,
-                hook_context,
                 local_borg_version,
                 action_arguments,
                 global_arguments,
@@ -373,7 +394,6 @@ def run_actions(
                     config_filename,
                     repository,
                     config,
-                    hook_context,
                     local_borg_version,
                     action_arguments,
                     global_arguments,
@@ -385,7 +405,6 @@ def run_actions(
                 config_filename,
                 repository,
                 config,
-                hook_context,
                 local_borg_version,
                 action_arguments,
                 global_arguments,
@@ -523,11 +542,16 @@ def run_actions(
                 remote_path,
             )
 
-    command.execute_hook(
-        config.get('after_actions'),
+        command.execute_hooks(
+            command.filter_hooks(config.get('commands'), after='action', action_names=arguments.keys()),
+            config.get('umask'),
+            global_arguments.dry_run,
+            **hook_context,
+        )
+
+    command.execute_hooks(
+        command.filter_hooks(config.get('commands'), after='repository', action_names=arguments.keys()),
         config.get('umask'),
-        config_filename,
-        'post-actions',
         global_arguments.dry_run,
         **hook_context,
     )
@@ -813,12 +837,11 @@ def collect_configuration_run_summary_logs(configs, config_paths, arguments):
     if 'create' in arguments:
         try:
             for config_filename, config in configs.items():
-                command.execute_hook(
-                    config.get('before_everything'),
+                command.execute_hooks(
+                    command.filter_hooks(config.get('commands'), before='everything', action_names=arguments.keys()),
                     config.get('umask'),
-                    config_filename,
-                    'pre-everything',
                     arguments['global'].dry_run,
+                    configuration_filename=config_filename,
                 )
         except (CalledProcessError, ValueError, OSError) as error:
             yield from log_error_records('Error running pre-everything hook', error)
@@ -865,12 +888,11 @@ def collect_configuration_run_summary_logs(configs, config_paths, arguments):
     if 'create' in arguments:
         try:
             for config_filename, config in configs.items():
-                command.execute_hook(
-                    config.get('after_everything'),
+                command.execute_hooks(
+                    command.filter_hooks(config.get('commands'), after='everything', action_names=arguments.keys()),
                     config.get('umask'),
-                    config_filename,
-                    'post-everything',
                     arguments['global'].dry_run,
+                    configuration_filename=config_filename,
                 )
         except (CalledProcessError, ValueError, OSError) as error:
             yield from log_error_records('Error running post-everything hook', error)

borgmatic/config/generate.py

@@ -1,5 +1,6 @@
 import collections
 import io
+import itertools
 import os
 import re
 
@@ -24,20 +25,28 @@ def insert_newline_before_comment(config, field_name):
 def get_properties(schema):
     '''
     Given a schema dict, return its properties. But if it's got sub-schemas with multiple different
-    potential properties, returned their merged properties instead.
+    potential properties, returned their merged properties instead (interleaved so the first
+    properties of each sub-schema come first). The idea is that the user should see all possible
+    options even if they're not all possible together.
     '''
     if 'oneOf' in schema:
         return dict(
-            collections.ChainMap(*[sub_schema['properties'] for sub_schema in schema['oneOf']])
+            item
+            for item in itertools.chain(
+                *itertools.zip_longest(
+                    *[sub_schema['properties'].items() for sub_schema in schema['oneOf']]
+                )
+            )
+            if item is not None
         )
 
     return schema['properties']
 
 
-def schema_to_sample_configuration(schema, level=0, parent_is_sequence=False):
+def schema_to_sample_configuration(schema, source_config, level=0, parent_is_sequence=False):
     '''
-    Given a loaded configuration schema, generate and return sample config for it. Include comments
-    for each option based on the schema "description".
+    Given a loaded configuration schema and a source configuration, generate and return sample
+    config for the schema. Include comments for each option based on the schema "description".
     '''
     schema_type = schema.get('type')
     example = schema.get('example')
@@ -47,19 +56,22 @@ def schema_to_sample_configuration(schema, level=0, parent_is_sequence=False):
 
     if schema_type == 'array' or (isinstance(schema_type, list) and 'array' in schema_type):
         config = ruamel.yaml.comments.CommentedSeq(
-            [schema_to_sample_configuration(schema['items'], level, parent_is_sequence=True)]
+            [schema_to_sample_configuration(schema['items'], source_config, level, parent_is_sequence=True)]
         )
         add_comments_to_configuration_sequence(config, schema, indent=(level * INDENT))
     elif schema_type == 'object' or (isinstance(schema_type, list) and 'object' in schema_type):
+        if source_config and isinstance(source_config, list) and isinstance(source_config[0], dict):
+            source_config = dict(collections.ChainMap(*source_config))
+
         config = ruamel.yaml.comments.CommentedMap(
             [
-                (field_name, schema_to_sample_configuration(sub_schema, level + 1))
+                (field_name, schema_to_sample_configuration(sub_schema, source_config.get(field_name, {}), level + 1))
                 for field_name, sub_schema in get_properties(schema).items()
             ]
         )
         indent = (level * INDENT) + (SEQUENCE_INDENT if parent_is_sequence else 0)
         add_comments_to_configuration_object(
-            config, schema, indent=indent, skip_first=parent_is_sequence
+            config, schema, source_config, indent=indent, skip_first=parent_is_sequence
         )
     else:
         raise ValueError(f'Schema at level {level} is unsupported: {schema}')
@@ -179,14 +191,19 @@ def add_comments_to_configuration_sequence(config, schema, indent=0):
         return
 
 
-REQUIRED_KEYS = {'source_directories', 'repositories', 'keep_daily'}
+DEFAULT_KEYS = {'source_directories', 'repositories', 'keep_daily'}
 COMMENTED_OUT_SENTINEL = 'COMMENT_OUT'
 
 
-def add_comments_to_configuration_object(config, schema, indent=0, skip_first=False):
+def add_comments_to_configuration_object(config, schema, source_config, indent=0, skip_first=False):
     '''
     Using descriptions from a schema as a source, add those descriptions as comments to the given
-    config mapping, before each field. Indent the comment the given number of characters.
+    configuration dict, putting them before each field. Indent the comment the given number of
+    characters.
+
+    And a sentinel for commenting out options that are neither in DEFAULT_KEYS nor the the given
+    source configuration dict. The idea is that any options used in the source configuration should
+    stay active in the generated configuration.
     '''
     for index, field_name in enumerate(config.keys()):
         if skip_first and index == 0:
@@ -195,10 +212,10 @@ def add_comments_to_configuration_object(config, schema, indent=0, skip_first=Fa
         field_schema = get_properties(schema).get(field_name, {})
         description = field_schema.get('description', '').strip()
 
-        # If this is an optional key, add an indicator to the comment flagging it to be commented
+        # If this isn't a default key, add an indicator to the comment flagging it to be commented
         # out from the sample configuration. This sentinel is consumed by downstream processing that
         # does the actual commenting out.
-        if field_name not in REQUIRED_KEYS:
+        if field_name not in DEFAULT_KEYS and field_name not in source_config:
             description = (
                 '\n'.join((description, COMMENTED_OUT_SENTINEL))
                 if description
@@ -218,21 +235,6 @@ def add_comments_to_configuration_object(config, schema, indent=0, skip_first=Fa
 RUAMEL_YAML_COMMENTS_INDEX = 1
 
 
-def remove_commented_out_sentinel(config, field_name):
-    '''
-    Given a configuration CommentedMap and a top-level field name in it, remove any "commented out"
-    sentinel found at the end of its YAML comments. This prevents the given field name from getting
-    commented out by downstream processing that consumes the sentinel.
-    '''
-    try:
-        last_comment_value = config.ca.items[field_name][RUAMEL_YAML_COMMENTS_INDEX][-1].value
-    except KeyError:
-        return
-
-    if last_comment_value == f'# {COMMENTED_OUT_SENTINEL}\n':
-        config.ca.items[field_name][RUAMEL_YAML_COMMENTS_INDEX].pop()
-
-
 def merge_source_configuration_into_destination(destination_config, source_config):
     '''
     Deep merge the given source configuration dict into the destination configuration CommentedMap,
@@ -247,12 +249,6 @@ def merge_source_configuration_into_destination(destination_config, source_confi
         return source_config
 
     for field_name, source_value in source_config.items():
-        # Since this key/value is from the source configuration, leave it uncommented and remove any
-        # sentinel that would cause it to get commented out.
-        remove_commented_out_sentinel(
-            ruamel.yaml.comments.CommentedMap(destination_config), field_name
-        )
-
         # This is a mapping. Recurse for this key/value.
         if isinstance(source_value, collections.abc.Mapping):
             destination_config[field_name] = merge_source_configuration_into_destination(
@@ -298,7 +294,7 @@ def generate_sample_configuration(
         normalize.normalize(source_filename, source_config)
 
     destination_config = merge_source_configuration_into_destination(
-        schema_to_sample_configuration(schema), source_config
+        schema_to_sample_configuration(schema, source_config), source_config
     )
 
     if dry_run:

borgmatic/config/normalize.py

@@ -58,6 +58,89 @@ def normalize_sections(config_filename, config):
     return []
 
 
+def make_command_hook_deprecation_log(config_filename, option_name):
+    '''
+    Given a configuration filename and the name of a configuration option, return a deprecation
+    warning log for it.
+    '''
+    return logging.makeLogRecord(
+        dict(
+            levelno=logging.WARNING,
+            levelname='WARNING',
+            msg=f'{config_filename}: {option_name} is deprecated and support will be removed from a future release. Use commands: instead.',
+        )
+    )
+
+
+def normalize_commands(config_filename, config):
+    '''
+    Given a configuration filename and a configuration dict, transform any "before_*"- and
+    "after_*"-style command hooks into "commands:".
+    '''
+    logs = []
+
+    # Normalize "before_actions" and "after_actions".
+    for preposition in ('before', 'after'):
+        option_name = f'{preposition}_actions'
+        commands = config.pop(option_name, None)
+
+        if commands:
+            logs.append(make_command_hook_deprecation_log(config_filename, option_name))
+            config.setdefault('commands', []).append(
+                {
+                    preposition: 'repository',
+                    'run': commands,
+                }
+            )
+
+    # Normalize "before_backup", "before_prune", "after_backup", "after_prune", etc.
+    for action_name in ('create', 'prune', 'compact', 'check', 'extract'):
+        for preposition in ('before', 'after'):
+            option_name = f'{preposition}_{"backup" if action_name == "create" else action_name}'
+            commands = config.pop(option_name, None)
+            
+            if not commands:
+                continue
+
+            logs.append(make_command_hook_deprecation_log(config_filename, option_name))
+            config.setdefault('commands', []).append(
+                {
+                    preposition: 'action',
+                    'when': [action_name],
+                    'run': commands,
+                }
+            )
+
+    # Normalize "on_error".
+    commands = config.pop('on_error', None)
+
+    if commands:
+        logs.append(make_command_hook_deprecation_log(config_filename, 'on_error'))
+        config.setdefault('commands', []).append(
+            {
+                'after': 'error',
+                'when': ['create', 'prune', 'compact', 'check'],
+                'run': commands,
+            }
+        )
+
+    # Normalize "before_everything" and "after_everything".
+    for preposition in ('before', 'after'):
+        option_name = f'{preposition}_everything'
+        commands = config.pop(option_name, None)
+
+        if commands:
+            logs.append(make_command_hook_deprecation_log(config_filename, option_name))
+            config.setdefault('commands', []).append(
+                {
+                    preposition: 'everything',
+                    'run': commands,
+                }
+            )
+
+    return logs
+
+
 def normalize(config_filename, config):
     '''
     Given a configuration filename and a configuration dict of its loaded contents, apply particular
@@ -67,6 +150,7 @@ def normalize(config_filename, config):
     Raise ValueError the configuration cannot be normalized.
     '''
     logs = normalize_sections(config_filename, config)
+    logs += normalize_commands(config_filename, config)
 
     if config.get('borgmatic_source_directory'):
         logs.append(

borgmatic/config/schema.yaml

@@ -796,8 +796,9 @@ properties:
         items:
             type: string
         description: |
-            List of one or more shell commands or scripts to execute before all
-            the actions for each repository.
+            Deprecated. Use "commands:" instead. List of one or more shell
+            commands or scripts to execute before all the actions for each
+            repository.
         example:
             - "echo Starting actions."
     before_backup:
@@ -805,8 +806,9 @@ properties:
         items:
             type: string
         description: |
-            List of one or more shell commands or scripts to execute before
-            creating a backup, run once per repository.
+            Deprecated. Use "commands:" instead. List of one or more shell
+            commands or scripts to execute before creating a backup, run once
+            per repository.
         example:
             - "echo Starting a backup."
     before_prune:
@@ -814,8 +816,9 @@ properties:
         items:
             type: string
         description: |
-            List of one or more shell commands or scripts to execute before
-            pruning, run once per repository.
+            Deprecated. Use "commands:" instead. List of one or more shell
+            commands or scripts to execute before pruning, run once per
+            repository.
         example:
             - "echo Starting pruning."
     before_compact:
@@ -823,8 +826,9 @@ properties:
         items:
             type: string
         description: |
-            List of one or more shell commands or scripts to execute before
-            compaction, run once per repository.
+            Deprecated. Use "commands:" instead. List of one or more shell
+            commands or scripts to execute before compaction, run once per
+            repository.
         example:
             - "echo Starting compaction."
     before_check:
@@ -832,8 +836,9 @@ properties:
         items:
             type: string
         description: |
-            List of one or more shell commands or scripts to execute before
-            consistency checks, run once per repository.
+            Deprecated. Use "commands:" instead. List of one or more shell
+            commands or scripts to execute before consistency checks, run once
+            per repository.
         example:
             - "echo Starting checks."
     before_extract:
@@ -841,8 +846,9 @@ properties:
         items:
             type: string
         description: |
-            List of one or more shell commands or scripts to execute before
-            extracting a backup, run once per repository.
+            Deprecated. Use "commands:" instead. List of one or more shell
+            commands or scripts to execute before extracting a backup, run once
+            per repository.
         example:
             - "echo Starting extracting."
     after_backup:
@@ -850,8 +856,9 @@ properties:
         items:
             type: string
         description: |
-            List of one or more shell commands or scripts to execute after
-            creating a backup, run once per repository.
+            Deprecated. Use "commands:" instead. List of one or more shell
+            commands or scripts to execute after creating a backup, run once per
+            repository.
         example:
             - "echo Finished a backup."
     after_compact:
@@ -859,8 +866,9 @@ properties:
         items:
             type: string
         description: |
-            List of one or more shell commands or scripts to execute after
-            compaction, run once per repository.
+            Deprecated. Use "commands:" instead. List of one or more shell
+            commands or scripts to execute after compaction, run once per
+            repository.
         example:
             - "echo Finished compaction."
     after_prune:
@@ -868,8 +876,9 @@ properties:
         items:
             type: string
         description: |
-            List of one or more shell commands or scripts to execute after
-            pruning, run once per repository.
+            Deprecated. Use "commands:" instead. List of one or more shell
+            commands or scripts to execute after pruning, run once per
+            repository.
         example:
             - "echo Finished pruning."
     after_check:
@@ -877,8 +886,9 @@ properties:
         items:
             type: string
         description: |
-            List of one or more shell commands or scripts to execute after
-            consistency checks, run once per repository.
+            Deprecated. Use "commands:" instead. List of one or more shell
+            commands or scripts to execute after consistency checks, run once
+            per repository.
         example:
             - "echo Finished checks."
     after_extract:
@@ -886,8 +896,9 @@ properties:
         items:
             type: string
         description: |
-            List of one or more shell commands or scripts to execute after
-            extracting a backup, run once per repository.
+            Deprecated. Use "commands:" instead. List of one or more shell
+            commands or scripts to execute after extracting a backup, run once
+            per repository.
         example:
             - "echo Finished extracting."
     after_actions:
@@ -895,8 +906,9 @@ properties:
         items:
             type: string
         description: |
-            List of one or more shell commands or scripts to execute after all
-            actions for each repository.
+            Deprecated. Use "commands:" instead. List of one or more shell
+            commands or scripts to execute after all actions for each
+            repository.
         example:
             - "echo Finished actions."
     on_error:
@@ -904,9 +916,10 @@ properties:
         items:
             type: string
         description: |
-            List of one or more shell commands or scripts to execute when an
-            exception occurs during a "create", "prune", "compact", or "check"
-            action or an associated before/after hook.
+            Deprecated. Use "commands:" instead. List of one or more shell
+            commands or scripts to execute when an exception occurs during a
+            "create", "prune", "compact", or "check" action or an associated
+            before/after hook.
         example:
             - "echo Error during create/prune/compact/check."
     before_everything:
@@ -914,10 +927,10 @@ properties:
         items:
             type: string
         description: |
-            List of one or more shell commands or scripts to execute before
-            running all actions (if one of them is "create"). These are
-            collected from all configuration files and then run once before all
-            of them (prior to all actions).
+            Deprecated. Use "commands:" instead. List of one or more shell
+            commands or scripts to execute before running all actions (if one of
+            them is "create"). These are collected from all configuration files
+            and then run once before all of them (prior to all actions).
         example:
             - "echo Starting actions."
     after_everything:
@@ -925,12 +938,183 @@ properties:
         items:
             type: string
         description: |
-            List of one or more shell commands or scripts to execute after
-            running all actions (if one of them is "create"). These are
-            collected from all configuration files and then run once after all
-            of them (after any action).
+            Deprecated. Use "commands:" instead. List of one or more shell
+            commands or scripts to execute after running all actions (if one of
+            them is "create"). These are collected from all configuration files
+            and then run once after all of them (after any action).
         example:
             - "echo Completed actions."
+    commands:
+        type: array
+        items:
+            type: object
+            oneOf:
+                - required: [before, run]
+                  additionalProperties: false
+                  properties:
+                      before:
+                          type: string
+                          enum:
+                              - action
+                              - repository
+                              - configuration
+                              - everything
+                              - dump_data_sources
+                              - restore_data_source_dump
+                          description: |
+                              Name for the point in borgmatic's execution before
+                              which the commands should be run (required if
+                              "after" isn't set):
+                               * "action" runs before each action for each
+                              repository.
+                               * "repository" runs before all actions for each
+                              repository.
+                               * "configuration" runs before all actions and
+                              repositories in the current configuration file.
+                               * "everything" runs before all configuration
+                              files.
+                               * "dump_data_sources" runs before each data
+                              source is dumped.
+                               * "restore_data_source_dumps" runs before each
+                              data source is restored.
+                          example: action
+                      hooks:
+                          type: array
+                          items:
+                              type: string
+                          description: |
+                              List of names of other hooks that this command
+                              hook applies to. Defaults to all hooks of the
+                              relevant type. Only supported for
+                              "dump_data_sources" and
+                              "restore_data_source_dumps" hooks.
+                          example: postgresql
+                      when:
+                          type: array
+                          items:
+                              type: string
+                              enum:
+                                  - repo-create
+                                  - transfer
+                                  - prune
+                                  - compact
+                                  - create
+                                  - check
+                                  - delete
+                                  - extract
+                                  - config
+                                  - export-tar
+                                  - mount
+                                  - umount
+                                  - repo-delete
+                                  - restore
+                                  - repo-list
+                                  - list
+                                  - repo-info
+                                  - info
+                                  - break-lock
+                                  - key
+                                  - borg
+                          description: |
+                              List of actions for which the commands will be run. Defaults to
+                              running for all actions. Ignored for "dump_data_sources" and
+                              "restore_data_source_dump", which by their nature only run for
+                              particular actions.
+                          example: [create, prune, compact, check]
+                      run:
+                          type: array
+                          items:
+                              type: string
+                          description: |
+                              List of one or more shell commands or scripts to
+                              run when this command hook is triggered. Required.
+                          example:
+                              - "echo Doing stuff."
+                - required: [after, run]
+                  additionalProperties: false
+                  properties:
+                      after:
+                          type: string
+                          enum:
+                              - action
+                              - repository
+                              - configuration
+                              - everything
+                              - error
+                              - dump_data_sources
+                              - restore_data_source_dump
+                          description: |
+                              Name for the point in borgmatic's execution after
+                              which the commands should be run (required if
+                              "before" isn't set):
+                               * "action" runs after each action for each
+                              repository.
+                               * "repository" runs after all actions for each
+                              repository.
+                               * "configuration" runs after all actions and
+                              repositories in the current configuration file.
+                               * "everything" runs after all configuration
+                              files.
+                               * "error" runs after an error occurs.
+                               * "dump_data_sources" runs after each data
+                              source is dumped.
+                               * "restore_data_source_dumps" runs after each
+                              data source is restored.
+                          example: action
+                      hooks:
+                          type: array
+                          items:
+                              type: string
+                          description: |
+                              List of names of other hooks that this command
+                              hook applies to. Defaults to all hooks of the
+                              relevant type. Only supported for
+                              "dump_data_sources" and
+                              "restore_data_source_dumps" hooks.
+                          example: postgresql
+                      when:
+                          type: array
+                          items:
+                              type: string
+                              enum:
+                                  - repo-create
+                                  - transfer
+                                  - prune
+                                  - compact
+                                  - create
+                                  - check
+                                  - delete
+                                  - extract
+                                  - config
+                                  - export-tar
+                                  - mount
+                                  - umount
+                                  - repo-delete
+                                  - restore
+                                  - repo-list
+                                  - list
+                                  - repo-info
+                                  - info
+                                  - break-lock
+                                  - key
+                                  - borg
+                          description: |
+                              List of actions for which the commands will be run.
+                              Defaults to running for all actions.
+                          example: [create, prune, compact, check]
+                      run:
+                          type: array
+                          items:
+                              type: string
+                          description: |
+                              List of one or more shell commands or scripts to
+                              run when this command hook is triggered. Required.
+                          example:
+                              - "echo Doing stuff."
+        description: |
+            List of one or more command hooks to execute, triggered at
+            particular points during borgmatic's execution. For each command
+            hook, specify one of "before" or "after", not both.
     bootstrap:
         type: object
         properties:

borgmatic/hooks/command.py

@@ -5,6 +5,7 @@ import shlex
 import sys
 
 import borgmatic.execute
+import borgmatic.logger
 
 logger = logging.getLogger(__name__)
 
@@ -44,54 +45,83 @@ def make_environment(current_environment, sys_module=sys):
     return environment
 
 
-def execute_hook(commands, umask, config_filename, description, dry_run, **context):
+def filter_hooks(command_hooks, before=None, after=None, hook_name=None, action_names=None):
     '''
-    Given a list of hook commands to execute, a umask to execute with (or None), a config filename,
-    a hook description, and whether this is a dry run, run the given commands. Or, don't run them
-    if this is a dry run.
+    Given a sequence of command hook dicts from configuration and one or more filters (before name,
+    after name, calling hook name, or a sequence of action names), filter down the command hooks to
+    just the ones that match the given filters.
+    '''
+    return tuple(
+        hook_config
+        for hook_config in command_hooks
+        for config_hook_names in (hook_config.get('hooks'),)
+        for config_action_names in (hook_config.get('when'),)
+        if before is None or hook_config.get('before') == before
+        if after is None or hook_config.get('after') == after
+        if hook_name is None or config_hook_names is None or hook_name in config_hook_names
+        if action_names is None or config_action_names is None or set(config_action_names or ()).intersection(set(action_names))
+    )
+
+
+def execute_hooks(command_hooks, umask, 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.
 
     The context contains optional values interpolated by name into the hook commands.
 
-    Raise ValueError if the umask cannot be parsed.
+    Raise ValueError if the umask cannot be parsed or a hook is invalid.
     Raise subprocesses.CalledProcessError if an error occurs in a hook.
     '''
-    if not commands:
-        logger.debug(f'No commands to run for {description} hook')
-        return
+    borgmatic.logger.add_custom_log_levels()
 
     dry_run_label = ' (dry run; not actually running hooks)' if dry_run else ''
 
-    context['configuration_filename'] = config_filename
-    commands = [interpolate_context(description, command, context) for command in commands]
+    for hook_config in command_hooks:
+        commands = hook_config.get('run')
 
-    if len(commands) == 1:
-        logger.info(f'Running command for {description} hook{dry_run_label}')
-    else:
-        logger.info(
-            f'Running {len(commands)} commands for {description} hook{dry_run_label}',
-        )
+        if 'before' in hook_config:
+            description = f'before {hook_config.get("before")}'
+        elif 'after' in hook_config:
+            description = f'after {hook_config.get("after")}'
+        else:
+            raise ValueError('Invalid hook configuration: {hook_config}')
 
-    if umask:
-        parsed_umask = int(str(umask), 8)
-        logger.debug(f'Set hook umask to {oct(parsed_umask)}')
-        original_umask = os.umask(parsed_umask)
-    else:
-        original_umask = None
-
-    try:
-        for command in commands:
-            if dry_run:
-                continue
-
-            borgmatic.execute.execute_command(
-                [command],
-                output_log_level=(logging.ERROR if description == 'on-error' else logging.WARNING),
-                shell=True,
-                environment=make_environment(os.environ),
+        if not commands:
+            logger.debug(f'No commands to run for {description} hook')
+            return
+
+        commands = [interpolate_context(description, command, context) for command in commands]
+
+        if len(commands) == 1:
+            logger.info(f'Running command for {description} hook{dry_run_label}')
+        else:
+            logger.info(
+                f'Running {len(commands)} commands for {description} hook{dry_run_label}',
             )
-    finally:
-        if original_umask:
-            os.umask(original_umask)
+
+        if umask:
+            parsed_umask = int(str(umask), 8)
+            logger.debug(f'Setting hook umask to {oct(parsed_umask)}')
+            original_umask = os.umask(parsed_umask)
+        else:
+            original_umask = None
+
+        try:
+            for command in commands:
+                if dry_run:
+                    continue
+
+                borgmatic.execute.execute_command(
+                    [command],
+                    output_log_level=(logging.ERROR if hook_config.get('after') == 'error' else logging.ANSWER),
+                    shell=True,
+                    environment=make_environment(os.environ),
+                )
+        finally:
+            if original_umask:
+                os.umask(original_umask)
 
 
 def considered_soft_failure(error):

borgmatic/hooks/dispatch.py

@@ -3,6 +3,7 @@ import importlib
 import logging
 import pkgutil
 
+import borgmatic.hooks.command
 import borgmatic.hooks.credential
 import borgmatic.hooks.data_source
 import borgmatic.hooks.monitoring
@@ -62,7 +63,20 @@ def call_hook(function_name, config, hook_name, *args, **kwargs):
 
     logger.debug(f'Calling {hook_name} hook function {function_name}')
 
-    return getattr(module, function_name)(hook_config, config, *args, **kwargs)
+    borgmatic.hooks.command.execute_hooks(
+        borgmatic.hooks.command.filter_hooks(config.get('commands'), before=function_name, hook_name=hook_name),
+        config.get('umask'),
+        dry_run=False,  # FIXME: Need to get this from somewhere.
+    )
+
+    try:
+        return getattr(module, function_name)(hook_config, config, *args, **kwargs)
+    finally:
+        borgmatic.hooks.command.execute_hooks(
+            borgmatic.hooks.command.filter_hooks(config.get('commands'), after=function_name, hook_name=hook_name),
+            config.get('umask'),
+            dry_run=False,  # FIXME: Need to get this from somewhere.
+        )
 
 
 def call_hooks(function_name, config, hook_type, *args, **kwargs):

tests/integration/config/test_generate.py

@@ -170,38 +170,6 @@ def test_add_comments_to_configuration_object_with_skip_first_does_not_raise():
     module.add_comments_to_configuration_object(config, schema, skip_first=True)
 
 
-def test_remove_commented_out_sentinel_keeps_other_comments():
-    field_name = 'foo'
-    config = module.ruamel.yaml.comments.CommentedMap([(field_name, 33)])
-    config.yaml_set_comment_before_after_key(key=field_name, before='Actual comment.\nCOMMENT_OUT')
-
-    module.remove_commented_out_sentinel(config, field_name)
-
-    comments = config.ca.items[field_name][module.RUAMEL_YAML_COMMENTS_INDEX]
-    assert len(comments) == 1
-    assert comments[0].value == '# Actual comment.\n'
-
-
-def test_remove_commented_out_sentinel_without_sentinel_keeps_other_comments():
-    field_name = 'foo'
-    config = module.ruamel.yaml.comments.CommentedMap([(field_name, 33)])
-    config.yaml_set_comment_before_after_key(key=field_name, before='Actual comment.')
-
-    module.remove_commented_out_sentinel(config, field_name)
-
-    comments = config.ca.items[field_name][module.RUAMEL_YAML_COMMENTS_INDEX]
-    assert len(comments) == 1
-    assert comments[0].value == '# Actual comment.\n'
-
-
-def test_remove_commented_out_sentinel_on_unknown_field_does_not_raise():
-    field_name = 'foo'
-    config = module.ruamel.yaml.comments.CommentedMap([(field_name, 33)])
-    config.yaml_set_comment_before_after_key(key=field_name, before='Actual comment.')
-
-    module.remove_commented_out_sentinel(config, 'unknown')
-
-
 def test_generate_sample_configuration_does_not_raise():
     builtins = flexmock(sys.modules['builtins'])
     builtins.should_receive('open').with_args('schema.yaml').and_return('')