Browse Source

Merge branch 'fix-borg-2-latest-archive'

Dan Helfman 19 hours ago
parent
commit
22b77a4262
5 changed files with 257 additions and 24 deletions
  1. 2 0
      NEWS
  2. 8 1
      borgmatic/actions/create.py
  3. 14 4
      borgmatic/borg/repo_list.py
  4. 30 1
      tests/unit/actions/test_create.py
  5. 203 18
      tests/unit/borg/test_repo_list.py

+ 2 - 0
NEWS

@@ -17,6 +17,8 @@
    support the "bootstrap" action.
  * #1136: For all database hooks, record metadata about the dumps contained within an archive.
  * #1139: Set "borgmatic" as the user agent when connecting to monitoring services.
+ * #1146: Fix a broken "create" action and "--archive latest" flag when multiple archives share the
+   same name with Borg 2.
  * Treat configuration file permissions issues as errors instead of warnings.
  * When running tests, use Ruff for faster and more comprehensive code linting and formatting,
    replacing Flake8, Black, isort, etc.

+ 8 - 1
borgmatic/actions/create.py

@@ -2,6 +2,7 @@ import logging
 
 import borgmatic.actions.json
 import borgmatic.borg.create
+import borgmatic.borg.feature
 import borgmatic.borg.rename
 import borgmatic.borg.repo_list
 import borgmatic.config.paths
@@ -182,7 +183,13 @@ def rename_checkpoint_archive(
 
     borgmatic.borg.rename.rename_archive(
         repository_path,
-        archive_name,
+        (
+            archive['id']
+            if borgmatic.borg.feature.available(
+                borgmatic.borg.feature.Feature.ARCHIVE_SERIES, local_borg_version
+            )
+            else archive['name']
+        ),
         new_archive_name,
         global_arguments.dry_run,
         config,

+ 14 - 4
borgmatic/borg/repo_list.py

@@ -23,7 +23,9 @@ def resolve_archive_name(
     Given a local or remote repository path, an archive name, a configuration dict, the local Borg
     version, global arguments as an argparse.Namespace, a local Borg path, and a remote Borg path,
     return the archive name. But if the archive name is "latest", then instead introspect the
-    repository for the latest archive and return its name.
+    repository for the latest archive and return its name or ID, depending on whether the version of
+    Borg in use supports archive series—different archives that share the same name but have unique
+    IDs.
 
     Raise ValueError if "latest" is given but there are no archives in the repository.
     '''
@@ -39,7 +41,11 @@ def resolve_archive_name(
         remote_path=remote_path,
     )
 
-    return latest_archive['name']
+    return (
+        latest_archive['id']
+        if feature.available(feature.Feature.ARCHIVE_SERIES, local_borg_version)
+        else latest_archive['name']
+    )
 
 
 def get_latest_archive(
@@ -68,7 +74,11 @@ def get_latest_archive(
         *flags.make_flags('umask', config.get('umask')),
         *flags.make_flags('log-json', config.get('log_json')),
         *flags.make_flags('lock-wait', config.get('lock_wait')),
-        *flags.make_flags('consider-checkpoints', consider_checkpoints),
+        *(
+            flags.make_flags('consider-checkpoints', consider_checkpoints)
+            if not feature.available(feature.Feature.REPO_LIST, local_borg_version)
+            else ()
+        ),
         *flags.make_flags('last', 1),
         '--json',
         *flags.make_repository_flags(repository_path, local_borg_version),
@@ -89,7 +99,7 @@ def get_latest_archive(
     except IndexError:
         raise ValueError('No archives found in the repository')
 
-    logger.debug(f'Latest archive is {latest_archive["name"]}')
+    logger.debug(f'Latest archive is {latest_archive["name"]} ({latest_archive["id"]})')
 
     return latest_archive
 

+ 30 - 1
tests/unit/actions/test_create.py

@@ -379,7 +379,7 @@ def test_run_create_with_active_dumps_json_updates_archive_info():
     ) == [expected_create_result]
 
 
-def test_rename_checkpoint_archive_renames_archive():
+def test_rename_checkpoint_archive_renames_archive_using_name():
     global_arguments = flexmock(monitoring_verbosity=1, dry_run=False)
     flexmock(module.borgmatic.borg.repo_list).should_receive('get_latest_archive').and_return(
         {'id': 'id1', 'name': 'archive.checkpoint'},
@@ -395,6 +395,35 @@ def test_rename_checkpoint_archive_renames_archive():
         local_path=None,
         remote_path=None,
     )
+    flexmock(module.borgmatic.borg.feature).should_receive('available').and_return(False)
+
+    module.rename_checkpoint_archive(
+        repository_path='path',
+        global_arguments=global_arguments,
+        config={},
+        local_borg_version=None,
+        local_path=None,
+        remote_path=None,
+    )
+
+
+def test_rename_checkpoint_with_feature_available_archive_renames_archive_using_id():
+    global_arguments = flexmock(monitoring_verbosity=1, dry_run=False)
+    flexmock(module.borgmatic.borg.repo_list).should_receive('get_latest_archive').and_return(
+        {'id': 'id1', 'name': 'archive.checkpoint'},
+    )
+
+    flexmock(module.borgmatic.borg.rename).should_receive('rename_archive').with_args(
+        repository_name='path',
+        old_archive_name='id1',
+        new_archive_name='archive',
+        dry_run=False,
+        config={},
+        local_borg_version=None,
+        local_path=None,
+        remote_path=None,
+    )
+    flexmock(module.borgmatic.borg.feature).should_receive('available').and_return(True)
 
     module.rename_checkpoint_archive(
         repository_path='path',

+ 203 - 18
tests/unit/borg/test_repo_list.py

@@ -16,6 +16,14 @@ BORG_LIST_LATEST_ARGUMENTS = (
     'repo',
 )
 
+BORG_REPO_LIST_LATEST_ARGUMENTS = (
+    '--last',
+    '1',
+    '--json',
+    '--repo',
+    'repo',
+)
+
 
 def test_resolve_archive_name_passes_through_non_latest_archive_name():
     archive = 'myhost-2030-01-01T14:41:17.647620'
@@ -32,16 +40,48 @@ def test_resolve_archive_name_passes_through_non_latest_archive_name():
     )
 
 
-def test_resolve_archive_name_calls_get_latest_archive():
-    expected_archive = 'archive-name'
-
+def test_resolve_archive_looks_up_latest_archive_name():
+    expected_name = 'archive-name'
     repository_path = flexmock()
     config = flexmock()
     local_borg_version = flexmock()
     global_arguments = flexmock()
     local_path = flexmock()
     remote_path = flexmock()
+    flexmock(module.environment).should_receive('make_environment')
+    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None)
+    flexmock(module).should_receive('get_latest_archive').with_args(
+        repository_path,
+        config,
+        local_borg_version,
+        global_arguments,
+        local_path,
+        remote_path,
+    ).and_return({'name': expected_name, 'id': 'd34db33f'})
+    flexmock(module.feature).should_receive('available').and_return(False)
 
+    assert (
+        module.resolve_archive_name(
+            repository_path,
+            'latest',
+            config,
+            local_borg_version,
+            global_arguments,
+            local_path,
+            remote_path,
+        )
+        == expected_name
+    )
+
+
+def test_resolve_archive_with_feature_available_looks_up_latest_archive_id():
+    expected_id = 'd34db33f'
+    repository_path = flexmock()
+    config = flexmock()
+    local_borg_version = flexmock()
+    global_arguments = flexmock()
+    local_path = flexmock()
+    remote_path = flexmock()
     flexmock(module.environment).should_receive('make_environment')
     flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None)
     flexmock(module).should_receive('get_latest_archive').with_args(
@@ -51,7 +91,8 @@ def test_resolve_archive_name_calls_get_latest_archive():
         global_arguments,
         local_path,
         remote_path,
-    ).and_return({'name': expected_archive})
+    ).and_return({'name': 'archive-name', 'id': expected_id})
+    flexmock(module.feature).should_receive('available').and_return(True)
 
     assert (
         module.resolve_archive_name(
@@ -63,12 +104,18 @@ def test_resolve_archive_name_calls_get_latest_archive():
             local_path,
             remote_path,
         )
-        == expected_archive
+        == expected_id
     )
 
 
 def test_get_latest_archive_calls_borg_with_flags():
-    expected_archive = {'name': 'archive-name'}
+    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(())
+    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(
@@ -91,7 +138,13 @@ def test_get_latest_archive_calls_borg_with_flags():
 
 
 def test_get_latest_archive_with_log_info_calls_borg_without_info_flag():
-    expected_archive = {'name': 'archive-name'}
+    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(())
+    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(
@@ -115,7 +168,13 @@ def test_get_latest_archive_with_log_info_calls_borg_without_info_flag():
 
 
 def test_get_latest_archive_with_log_debug_calls_borg_without_debug_flag():
-    expected_archive = {'name': 'archive-name'}
+    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(())
+    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(
@@ -139,7 +198,13 @@ def test_get_latest_archive_with_log_debug_calls_borg_without_debug_flag():
 
 
 def test_get_latest_archive_with_local_path_calls_borg_via_local_path():
-    expected_archive = {'name': 'archive-name'}
+    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(())
+    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(
@@ -163,7 +228,13 @@ def test_get_latest_archive_with_local_path_calls_borg_via_local_path():
 
 
 def test_get_latest_archive_with_exit_codes_calls_borg_using_them():
-    expected_archive = {'name': 'archive-name'}
+    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(())
+    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)
     borg_exit_codes = flexmock()
@@ -187,7 +258,16 @@ def test_get_latest_archive_with_exit_codes_calls_borg_using_them():
 
 
 def test_get_latest_archive_with_remote_path_calls_borg_with_remote_path_flags():
-    expected_archive = {'name': 'archive-name'}
+    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(())
+    flexmock(module.flags).should_receive('make_flags').with_args(
+        'remote-path', 'borg1'
+    ).and_return(('--remote-path', 'borg1'))
+    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(
@@ -211,7 +291,16 @@ def test_get_latest_archive_with_remote_path_calls_borg_with_remote_path_flags()
 
 
 def test_get_latest_archive_with_umask_calls_borg_with_umask_flags():
-    expected_archive = {'name': 'archive-name'}
+    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(())
+    flexmock(module.flags).should_receive('make_flags').with_args('umask', '077').and_return(
+        ('--umask', '077')
+    )
+    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(
@@ -235,6 +324,12 @@ def test_get_latest_archive_with_umask_calls_borg_with_umask_flags():
 
 def test_get_latest_archive_without_archives_raises():
     flexmock(module.environment).should_receive('make_environment')
+    flexmock(module.feature).should_receive('available').and_return(False)
+    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.borgmatic.config.paths).should_receive('get_working_directory').and_return(None)
     flexmock(module).should_receive('execute_command_and_capture_output').with_args(
         ('borg', 'list', *BORG_LIST_LATEST_ARGUMENTS),
@@ -253,9 +348,17 @@ def test_get_latest_archive_without_archives_raises():
         )
 
 
-def test_get_latest_archive_with_log_json_calls_borg_with_log_json_flags():
-    expected_archive = {'name': 'archive-name'}
-
+def test_get_latest_archive_with_log_json_calls_borg_with_log_json_flag():
+    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(())
+    flexmock(module.flags).should_receive('make_flags').with_args('log-json', True).and_return(
+        ('--log-json',)
+    )
+    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(
@@ -278,8 +381,16 @@ def test_get_latest_archive_with_log_json_calls_borg_with_log_json_flags():
 
 
 def test_get_latest_archive_with_lock_wait_calls_borg_with_lock_wait_flags():
-    expected_archive = {'name': 'archive-name'}
-
+    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(())
+    flexmock(module.flags).should_receive('make_flags').with_args('lock-wait', 'okay').and_return(
+        ('--lock-wait', 'okay')
+    )
+    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(
@@ -301,8 +412,80 @@ def test_get_latest_archive_with_lock_wait_calls_borg_with_lock_wait_flags():
     )
 
 
+def test_get_latest_archive_with_consider_checkpoints_calls_borg_with_consider_checkpoints_flag():
+    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(())
+    flexmock(module.flags).should_receive('make_flags').with_args(
+        'consider-checkpoints', True
+    ).and_return(('--consider-checkpoints',))
+    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', 'list', '--consider-checkpoints', *BORG_LIST_LATEST_ARGUMENTS),
+        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',
+            config={},
+            local_borg_version='1.2.3',
+            global_arguments=flexmock(),
+            consider_checkpoints=True,
+        )
+        == expected_archive
+    )
+
+
+def test_get_latest_archive_with_consider_checkpoints_and_feature_available_calls_borg_without_consider_checkpoints_flag():
+    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(
+        'consider-checkpoints', object
+    ).never()
+    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', '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_REPO_LIST_LATEST_ARGUMENTS),
+        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',
+            config={},
+            local_borg_version='1.2.3',
+            global_arguments=flexmock(),
+            consider_checkpoints=True,
+        )
+        == expected_archive
+    )
+
+
 def test_get_latest_archive_calls_borg_with_working_directory():
-    expected_archive = {'name': 'archive-name'}
+    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(())
+    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(
         '/working/dir',
@@ -787,6 +970,7 @@ def test_make_repo_list_command_with_match_archives_calls_borg_with_match_archiv
 
 def test_list_repository_calls_two_commands():
     flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels')
+    flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER
     flexmock(module).should_receive('make_repo_list_command')
     flexmock(module.environment).should_receive('make_environment')
     flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None)
@@ -872,6 +1056,7 @@ def test_make_repo_list_command_with_date_based_matching_calls_borg_with_date_ba
 
 def test_list_repository_calls_borg_with_working_directory():
     flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels')
+    flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER
     flexmock(module).should_receive('make_repo_list_command')
     flexmock(module.environment).should_receive('make_environment')
     flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(