Browse Source

Fix for the "list", "info", and "delete" options in "extra_borg_options" being ignored when "--archive" is omitted with Borg 1.x (#1168).

Dan Helfman 2 weeks ago
parent
commit
0c4222037d

+ 3 - 1
NEWS

@@ -1,9 +1,11 @@
 2.0.11.dev0
  * #1165: Fix for when the systemd service directories (RuntimeDirectory and StateDirectory) each
    contain multiple paths.
+ * #1168: Fix for the "list", "info", and "delete" options in "extra_borg_options" being ignored
+   when "--archive" is omitted with Borg 1.x.
 
 2.0.10
- * #427: Expand the "borg_extra_options" option to support passing arbitrary Borg flags to every
+ * #427: Expand the "extra_borg_options" option to support passing arbitrary Borg flags to every
    Borg sub-command that borgmatic uses. As part of this, deprecate the "init" option under
    "borg_extra_options" in favor of "repo_create".
  * #942: Factor reference material out of the documentation how-to guides. This means there's now a

+ 8 - 1
borgmatic/borg/repo_delete.py

@@ -27,7 +27,14 @@ def make_repo_delete_command(
     arguments to the repo_delete action as an argparse.Namespace, and global arguments, return a command
     as a tuple to repo_delete the entire repository.
     '''
-    extra_borg_options = config.get('extra_borg_options', {}).get('repo_delete', '')
+    extra_borg_options = config.get('extra_borg_options', {}).get(
+        'repo_delete'
+        if borgmatic.borg.feature.available(
+            borgmatic.borg.feature.Feature.REPO_DELETE, local_borg_version
+        )
+        else 'delete',
+        '',
+    )
 
     return (
         (local_path,)

+ 4 - 1
borgmatic/borg/repo_info.py

@@ -25,7 +25,10 @@ def display_repository_info(
     '''
     borgmatic.logger.add_custom_log_levels()
     lock_wait = config.get('lock_wait', None)
-    extra_borg_options = config.get('extra_borg_options', {}).get('repo_info', '')
+    extra_borg_options = config.get('extra_borg_options', {}).get(
+        'repo_info' if feature.available(feature.Feature.REPO_INFO, local_borg_version) else 'info',
+        '',
+    )
 
     full_command = (
         (local_path,)

+ 8 - 2
borgmatic/borg/repo_list.py

@@ -63,7 +63,10 @@ def get_latest_archive(
 
     Raises ValueError if there are no archives in the repository.
     '''
-    extra_borg_options = config.get('extra_borg_options', {}).get('repo_list', '')
+    extra_borg_options = config.get('extra_borg_options', {}).get(
+        'repo_list' if feature.available(feature.Feature.REPO_LIST, local_borg_version) else 'list',
+        '',
+    )
 
     full_command = (
         local_path,
@@ -124,7 +127,10 @@ def make_repo_list_command(
     arguments to the repo_list action, global arguments as an argparse.Namespace instance, and local and
     remote Borg paths, return a command as a tuple to list archives with a repository.
     '''
-    extra_borg_options = config.get('extra_borg_options', {}).get('repo_list', '')
+    extra_borg_options = config.get('extra_borg_options', {}).get(
+        'repo_list' if feature.available(feature.Feature.REPO_LIST, local_borg_version) else 'list',
+        '',
+    )
 
     return (
         (

+ 22 - 1
tests/unit/borg/test_repo_delete.py

@@ -216,7 +216,28 @@ def test_make_repo_delete_command_includes_lock_wait():
     assert command == ('borg', 'repo-delete', '--lock-wait', '5', 'repo')
 
 
-def test_make_repo_delete_command_includes_extra_borg_options():
+def test_make_repo_delete_command_without_feature_available_includes_delete_extra_borg_options():
+    flexmock(module.borgmatic.borg.feature).should_receive('available').and_return(False)
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags_from_arguments').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_repository_flags').and_return(
+        ('repo',),
+    )
+
+    command = module.make_repo_delete_command(
+        repository={'path': 'repo'},
+        config={'extra_borg_options': {'delete': '--extra "value with space"'}},
+        local_borg_version='1.2.3',
+        repo_delete_arguments=flexmock(list_details=False, force=0),
+        global_arguments=flexmock(dry_run=False),
+        local_path='borg',
+        remote_path=None,
+    )
+
+    assert command == ('borg', 'delete', '--extra', 'value with space', 'repo')
+
+
+def test_make_repo_delete_command_with_feature_available_includes_delete_extra_borg_options():
     flexmock(module.borgmatic.borg.feature).should_receive('available').and_return(True)
     flexmock(module.borgmatic.borg.flags).should_receive('make_flags').and_return(())
     flexmock(module.borgmatic.borg.flags).should_receive('make_flags_from_arguments').and_return(())

+ 43 - 1
tests/unit/borg/test_repo_info.py

@@ -528,7 +528,49 @@ def test_display_repository_info_with_lock_wait_calls_borg_with_lock_wait_flags(
     )
 
 
-def test_display_repository_info_calls_borg_with_extra_borg_options():
+def test_display_repository_info_without_feature_available_calls_borg_with_info_extra_borg_options():
+    flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels')
+    flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER
+    config = {'extra_borg_options': {'info': '--extra "value with space"'}}
+    flexmock(module.feature).should_receive('available').and_return(False)
+    flexmock(module.flags).should_receive('make_flags').replace_with(
+        lambda name, value: (f'--{name}', str(value)) if value else (),
+    )
+    flexmock(module.flags).should_receive('make_repository_flags').and_return(
+        (
+            '--repo',
+            'repo',
+        ),
+    )
+    flexmock(module.environment).should_receive('make_environment')
+    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None)
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
+        ('borg', 'info', '--extra', 'value with space', '--json', '--repo', 'repo'),
+        environment=None,
+        working_directory=None,
+        borg_local_path='borg',
+        borg_exit_codes=None,
+    ).and_return('[]')
+    flexmock(module.flags).should_receive('warn_for_aggressive_archive_flags')
+    flexmock(module).should_receive('execute_command').with_args(
+        ('borg', 'info', '--extra', 'value with space', '--repo', 'repo'),
+        output_log_level=module.borgmatic.logger.ANSWER,
+        environment=None,
+        working_directory=None,
+        borg_local_path='borg',
+        borg_exit_codes=None,
+    )
+
+    module.display_repository_info(
+        repository_path='repo',
+        config=config,
+        local_borg_version='2.3.4',
+        repo_info_arguments=flexmock(json=False),
+        global_arguments=flexmock(),
+    )
+
+
+def test_display_repository_info_with_feature_available_calls_borg_with_repo_info_extra_borg_options():
     flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels')
     flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER
     config = {'extra_borg_options': {'repo_info': '--extra "value with space"'}}

+ 85 - 3
tests/unit/borg/test_repo_list.py

@@ -412,7 +412,7 @@ def test_get_latest_archive_with_lock_wait_calls_borg_with_lock_wait_flags():
     )
 
 
-def test_get_latest_archive_calls_borg_with_extra_borg_options():
+def test_get_latest_archive_calls_borg_with_list_extra_borg_options():
     expected_archive = {'name': 'archive-name', 'id': 'd34db33f'}
     flexmock(module.feature).should_receive('available').and_return(False)
     flexmock(module.flags).should_receive('make_flags').and_return(())
@@ -437,6 +437,42 @@ def test_get_latest_archive_calls_borg_with_extra_borg_options():
         borg_exit_codes=None,
     ).and_return(json.dumps({'archives': [expected_archive]}))
 
+    assert (
+        module.get_latest_archive(
+            'repo',
+            config={'extra_borg_options': {'list': '--extra "value with space"'}},
+            local_borg_version='1.2.3',
+            global_arguments=flexmock(),
+        )
+        == expected_archive
+    )
+
+
+def test_get_latest_archive_with_feature_available_calls_borg_with_repo_list_extra_borg_options():
+    expected_archive = {'name': 'archive-name', 'id': 'd34db33f'}
+    flexmock(module.feature).should_receive('available').and_return(True)
+    flexmock(module.flags).should_receive('make_flags').and_return(())
+    flexmock(module.flags).should_receive('make_flags').with_args('last', 1).and_return(
+        ('--last', '1')
+    )
+    flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',))
+    flexmock(module.environment).should_receive('make_environment')
+    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None)
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
+        (
+            'borg',
+            'repo-list',
+            *BORG_LIST_LATEST_ARGUMENTS[:-1],
+            '--extra',
+            'value with space',
+            *BORG_LIST_LATEST_ARGUMENTS[-1:],
+        ),
+        environment=None,
+        working_directory=None,
+        borg_local_path='borg',
+        borg_exit_codes=None,
+    ).and_return(json.dumps({'archives': [expected_archive]}))
+
     assert (
         module.get_latest_archive(
             'repo',
@@ -546,6 +582,7 @@ def test_get_latest_archive_calls_borg_with_working_directory():
 
 
 def test_make_repo_list_command_includes_log_info():
+    flexmock(module.feature).should_receive('available').and_return(False)
     insert_logging_mock(logging.INFO)
     flexmock(module.flags).should_receive('make_flags').and_return(())
     flexmock(module.flags).should_receive('make_match_archives_flags').with_args(
@@ -574,6 +611,7 @@ def test_make_repo_list_command_includes_log_info():
 
 
 def test_make_repo_list_command_includes_json_but_not_info():
+    flexmock(module.feature).should_receive('available').and_return(False)
     insert_logging_mock(logging.INFO)
     flexmock(module.flags).should_receive('make_flags').and_return(())
     flexmock(module.flags).should_receive('make_match_archives_flags').with_args(
@@ -602,6 +640,7 @@ def test_make_repo_list_command_includes_json_but_not_info():
 
 
 def test_make_repo_list_command_includes_log_debug():
+    flexmock(module.feature).should_receive('available').and_return(False)
     insert_logging_mock(logging.DEBUG)
     flexmock(module.flags).should_receive('make_flags').and_return(())
     flexmock(module.flags).should_receive('make_match_archives_flags').with_args(
@@ -630,6 +669,7 @@ def test_make_repo_list_command_includes_log_debug():
 
 
 def test_make_repo_list_command_includes_json_but_not_debug():
+    flexmock(module.feature).should_receive('available').and_return(False)
     insert_logging_mock(logging.DEBUG)
     flexmock(module.flags).should_receive('make_flags').and_return(())
     flexmock(module.flags).should_receive('make_match_archives_flags').with_args(
@@ -658,6 +698,7 @@ def test_make_repo_list_command_includes_json_but_not_debug():
 
 
 def test_make_repo_list_command_includes_json():
+    flexmock(module.feature).should_receive('available').and_return(False)
     flexmock(module.flags).should_receive('make_flags').and_return(())
     flexmock(module.flags).should_receive('make_match_archives_flags').with_args(
         None,
@@ -685,6 +726,7 @@ def test_make_repo_list_command_includes_json():
 
 
 def test_make_repo_list_command_includes_log_json():
+    flexmock(module.feature).should_receive('available').and_return(False)
     flexmock(module.flags).should_receive('make_flags').and_return(()).and_return(
         ('--log-json',),
     ).and_return(())
@@ -714,6 +756,7 @@ def test_make_repo_list_command_includes_log_json():
 
 
 def test_make_repo_list_command_includes_lock_wait():
+    flexmock(module.feature).should_receive('available').and_return(False)
     flexmock(module.flags).should_receive('make_flags').and_return(()).and_return(
         ('--lock-wait', '5'),
     ).and_return(())
@@ -742,7 +785,8 @@ def test_make_repo_list_command_includes_lock_wait():
     assert command == ('borg', 'list', '--lock-wait', '5', 'repo')
 
 
-def test_make_repo_list_command_includes_extra_borg_options():
+def test_make_repo_list_command_includes_list_extra_borg_options():
+    flexmock(module.feature).should_receive('available').and_return(False)
     flexmock(module.flags).should_receive('make_flags').and_return(())
     flexmock(module.flags).should_receive('make_match_archives_flags').with_args(
         None,
@@ -754,7 +798,7 @@ def test_make_repo_list_command_includes_extra_borg_options():
 
     command = module.make_repo_list_command(
         repository_path='repo',
-        config={'extra_borg_options': {'repo_list': '--extra "value with space"'}},
+        config={'extra_borg_options': {'list': '--extra "value with space"'}},
         local_borg_version='1.2.3',
         repo_list_arguments=flexmock(
             archive=None,
@@ -769,7 +813,36 @@ def test_make_repo_list_command_includes_extra_borg_options():
     assert command == ('borg', 'list', '--extra', 'value with space', 'repo')
 
 
+def test_make_repo_list_command_with_feature_available_includes_repo_list_extra_borg_options():
+    flexmock(module.feature).should_receive('available').and_return(True)
+    flexmock(module.flags).should_receive('make_flags').and_return(())
+    flexmock(module.flags).should_receive('make_match_archives_flags').with_args(
+        None,
+        None,
+        '1.2.3',
+    ).and_return(())
+    flexmock(module.flags).should_receive('make_flags_from_arguments').and_return(())
+    flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',))
+
+    command = module.make_repo_list_command(
+        repository_path='repo',
+        config={'extra_borg_options': {'repo_list': '--extra "value with space"'}},
+        local_borg_version='1.2.3',
+        repo_list_arguments=flexmock(
+            archive=None,
+            paths=None,
+            json=False,
+            prefix=None,
+            match_archives=None,
+        ),
+        global_arguments=flexmock(),
+    )
+
+    assert command == ('borg', 'repo-list', '--extra', 'value with space', 'repo')
+
+
 def test_make_repo_list_command_includes_local_path():
+    flexmock(module.feature).should_receive('available').and_return(False)
     flexmock(module.flags).should_receive('make_flags').and_return(())
     flexmock(module.flags).should_receive('make_match_archives_flags').with_args(
         None,
@@ -798,6 +871,7 @@ def test_make_repo_list_command_includes_local_path():
 
 
 def test_make_repo_list_command_includes_remote_path():
+    flexmock(module.feature).should_receive('available').and_return(False)
     flexmock(module.flags).should_receive('make_flags').replace_with(
         lambda name, value: (f'--{name}', value) if value else (),
     )
@@ -828,6 +902,7 @@ def test_make_repo_list_command_includes_remote_path():
 
 
 def test_make_repo_list_command_includes_umask():
+    flexmock(module.feature).should_receive('available').and_return(False)
     flexmock(module.flags).should_receive('make_flags').replace_with(
         lambda name, value: (f'--{name}', value) if value else (),
     )
@@ -857,6 +932,7 @@ def test_make_repo_list_command_includes_umask():
 
 
 def test_make_repo_list_command_transforms_prefix_into_match_archives():
+    flexmock(module.feature).should_receive('available').and_return(False)
     flexmock(module.flags).should_receive('make_flags').and_return(()).and_return(()).and_return(
         ('--match-archives', 'sh:foo*'),
     )
@@ -880,6 +956,7 @@ def test_make_repo_list_command_transforms_prefix_into_match_archives():
 
 
 def test_make_repo_list_command_prefers_prefix_over_archive_name_format():
+    flexmock(module.feature).should_receive('available').and_return(False)
     flexmock(module.flags).should_receive('make_flags').and_return(()).and_return(()).and_return(
         ('--match-archives', 'sh:foo*'),
     )
@@ -899,6 +976,7 @@ def test_make_repo_list_command_prefers_prefix_over_archive_name_format():
 
 
 def test_make_repo_list_command_transforms_archive_name_format_into_match_archives():
+    flexmock(module.feature).should_receive('available').and_return(False)
     flexmock(module.flags).should_receive('make_flags').and_return(())
     flexmock(module.flags).should_receive('make_match_archives_flags').with_args(
         None,
@@ -926,6 +1004,7 @@ def test_make_repo_list_command_transforms_archive_name_format_into_match_archiv
 
 
 def test_make_repo_list_command_includes_short():
+    flexmock(module.feature).should_receive('available').and_return(False)
     flexmock(module.flags).should_receive('make_flags').and_return(())
     flexmock(module.flags).should_receive('make_match_archives_flags').with_args(
         None,
@@ -966,6 +1045,7 @@ def test_make_repo_list_command_includes_short():
     ),
 )
 def test_make_repo_list_command_includes_additional_flags(argument_name):
+    flexmock(module.feature).should_receive('available').and_return(False)
     flexmock(module.flags).should_receive('make_flags').and_return(())
     flexmock(module.flags).should_receive('make_match_archives_flags').with_args(
         None,
@@ -998,6 +1078,7 @@ def test_make_repo_list_command_includes_additional_flags(argument_name):
 
 
 def test_make_repo_list_command_with_match_archives_calls_borg_with_match_archives_flags():
+    flexmock(module.feature).should_receive('available').and_return(False)
     flexmock(module.flags).should_receive('make_flags').and_return(())
     flexmock(module.flags).should_receive('make_match_archives_flags').with_args(
         None,
@@ -1073,6 +1154,7 @@ def test_list_repository_with_json_calls_json_command_only():
 
 
 def test_make_repo_list_command_with_date_based_matching_calls_borg_with_date_based_flags():
+    flexmock(module.feature).should_receive('available').and_return(False)
     flexmock(module.flags).should_receive('make_flags').and_return(())
     flexmock(module.flags).should_receive('make_match_archives_flags').with_args(
         None,