When using the "numeric_owner" option with the "extract" action, tailor the flags passed to Borg depending on the Borg version (#394).

borgmatic/borg/extract.py

@@ -2,6 +2,7 @@ import logging
 import os
 import subprocess
 
+from borgmatic.borg import feature
 from borgmatic.execute import DO_NOT_CAPTURE, execute_command
 
 logger = logging.getLogger(__name__)
@@ -61,6 +62,7 @@ def extract_archive(
     paths,
     location_config,
     storage_config,
+    local_borg_version,
     local_path='borg',
     remote_path=None,
     destination_path=None,
@@ -70,9 +72,9 @@ def extract_archive(
 ):
     '''
     Given a dry-run flag, a local or remote repository path, an archive name, zero or more paths to
-    restore from the archive, location/storage configuration dicts, optional local and remote Borg
-    paths, and an optional destination path to extract to, extract the archive into the current
-    directory.
+    restore from the archive, the local Borg version string, location/storage configuration dicts,
+    optional local and remote Borg paths, and an optional destination path to extract to, extract
+    the archive into the current directory.
 
     If extract to stdout is True, then start the extraction streaming to stdout, and return that
     extract process as an instance of subprocess.Popen.
@@ -83,10 +85,15 @@ def extract_archive(
     if progress and extract_to_stdout:
         raise ValueError('progress and extract_to_stdout cannot both be set')
 
+    if feature.available(feature.Feature.NUMERIC_IDS, local_borg_version):
+        numeric_ids_flags = ('--numeric-ids',) if location_config.get('numeric_owner') else ()
+    else:
+        numeric_ids_flags = ('--numeric-owner',) if location_config.get('numeric_owner') else ()
+
     full_command = (
         (local_path, 'extract')
         + (('--remote-path', remote_path) if remote_path else ())
-        + (('--numeric-owner',) if location_config.get('numeric_owner') else ())
+        + numeric_ids_flags
         + (('--umask', str(umask)) if umask else ())
         + (('--lock-wait', str(lock_wait)) if lock_wait else ())
         + (('--info',) if logger.getEffectiveLevel() == logging.INFO else ())

borgmatic/commands/borgmatic.py

@@ -425,6 +425,7 @@ def run_actions(
                 arguments['extract'].paths,
                 location,
                 storage,
+                local_borg_version,
                 local_path=local_path,
                 remote_path=remote_path,
                 destination_path=arguments['extract'].destination,
@@ -533,6 +534,7 @@ def run_actions(
                         paths=dump.convert_glob_patterns_to_borg_patterns([dump_pattern]),
                         location_config=location,
                         storage_config=storage,
+                        local_borg_version=local_borg_version,
                         local_path=local_path,
                         remote_path=remote_path,
                         destination_path='/',

tests/unit/borg/test_create.py

@@ -715,7 +715,7 @@ def test_create_archive_with_numeric_owner_calls_borg_with_numeric_ids_parameter
     flexmock(module).should_receive('_make_pattern_flags').and_return(())
     flexmock(module).should_receive('_make_exclude_flags').and_return(())
     flexmock(module).should_receive('execute_command').with_args(
-        ('borg', 'create') + ((option_flag,) if option_flag else ()) + ARCHIVE_WITH_PATHS,
+        ('borg', 'create', option_flag) + ARCHIVE_WITH_PATHS,
         output_log_level=logging.INFO,
         output_file=None,
         borg_local_path='borg',

tests/unit/borg/test_extract.py

@@ -25,12 +25,14 @@ def test_extract_last_archive_dry_run_calls_borg_with_last_archive():
         ('borg', 'list', '--short', 'repo'), result='archive1\narchive2\n'
     )
     insert_execute_command_mock(('borg', 'extract', '--dry-run', 'repo::archive2'))
+    flexmock(module.feature).should_receive('available').and_return(True)
 
     module.extract_last_archive_dry_run(repository='repo', lock_wait=None)
 
 
 def test_extract_last_archive_dry_run_without_any_archives_should_not_raise():
     insert_execute_command_output_mock(('borg', 'list', '--short', 'repo'), result='\n')
+    flexmock(module.feature).should_receive('available').and_return(True)
 
     module.extract_last_archive_dry_run(repository='repo', lock_wait=None)
 
@@ -41,6 +43,7 @@ def test_extract_last_archive_dry_run_with_log_info_calls_borg_with_info_paramet
     )
     insert_execute_command_mock(('borg', 'extract', '--dry-run', '--info', 'repo::archive2'))
     insert_logging_mock(logging.INFO)
+    flexmock(module.feature).should_receive('available').and_return(True)
 
     module.extract_last_archive_dry_run(repository='repo', lock_wait=None)
 
@@ -53,6 +56,7 @@ def test_extract_last_archive_dry_run_with_log_debug_calls_borg_with_debug_param
         ('borg', 'extract', '--dry-run', '--debug', '--show-rc', '--list', 'repo::archive2')
     )
     insert_logging_mock(logging.DEBUG)
+    flexmock(module.feature).should_receive('available').and_return(True)
 
     module.extract_last_archive_dry_run(repository='repo', lock_wait=None)
 
@@ -62,6 +66,7 @@ def test_extract_last_archive_dry_run_calls_borg_via_local_path():
         ('borg1', 'list', '--short', 'repo'), result='archive1\narchive2\n'
     )
     insert_execute_command_mock(('borg1', 'extract', '--dry-run', 'repo::archive2'))
+    flexmock(module.feature).should_receive('available').and_return(True)
 
     module.extract_last_archive_dry_run(repository='repo', lock_wait=None, local_path='borg1')
 
@@ -73,6 +78,7 @@ def test_extract_last_archive_dry_run_calls_borg_with_remote_path_parameters():
     insert_execute_command_mock(
         ('borg', 'extract', '--dry-run', '--remote-path', 'borg1', 'repo::archive2')
     )
+    flexmock(module.feature).should_receive('available').and_return(True)
 
     module.extract_last_archive_dry_run(repository='repo', lock_wait=None, remote_path='borg1')
 
@@ -84,6 +90,7 @@ def test_extract_last_archive_dry_run_calls_borg_with_lock_wait_parameters():
     insert_execute_command_mock(
         ('borg', 'extract', '--dry-run', '--lock-wait', '5', 'repo::archive2')
     )
+    flexmock(module.feature).should_receive('available').and_return(True)
 
     module.extract_last_archive_dry_run(repository='repo', lock_wait=5)
 
@@ -91,6 +98,7 @@ def test_extract_last_archive_dry_run_calls_borg_with_lock_wait_parameters():
 def test_extract_archive_calls_borg_with_path_parameters():
     flexmock(module.os.path).should_receive('abspath').and_return('repo')
     insert_execute_command_mock(('borg', 'extract', 'repo::archive', 'path1', 'path2'))
+    flexmock(module.feature).should_receive('available').and_return(True)
 
     module.extract_archive(
         dry_run=False,
@@ -99,12 +107,14 @@ def test_extract_archive_calls_borg_with_path_parameters():
         paths=['path1', 'path2'],
         location_config={},
         storage_config={},
+        local_borg_version='1.2.3',
     )
 
 
 def test_extract_archive_calls_borg_with_remote_path_parameters():
     flexmock(module.os.path).should_receive('abspath').and_return('repo')
     insert_execute_command_mock(('borg', 'extract', '--remote-path', 'borg1', 'repo::archive'))
+    flexmock(module.feature).should_receive('available').and_return(True)
 
     module.extract_archive(
         dry_run=False,
@@ -113,13 +123,18 @@ def test_extract_archive_calls_borg_with_remote_path_parameters():
         paths=None,
         location_config={},
         storage_config={},
+        local_borg_version='1.2.3',
         remote_path='borg1',
     )
 
 
-def test_extract_archive_calls_borg_with_numeric_owner_parameter():
+@pytest.mark.parametrize(
+    'feature_available,option_flag', ((True, '--numeric-ids'), (False, '--numeric-owner'),),
+)
+def test_extract_archive_calls_borg_with_numeric_ids_parameter(feature_available, option_flag):
     flexmock(module.os.path).should_receive('abspath').and_return('repo')
-    insert_execute_command_mock(('borg', 'extract', '--numeric-owner', 'repo::archive'))
+    insert_execute_command_mock(('borg', 'extract', option_flag, 'repo::archive'))
+    flexmock(module.feature).should_receive('available').and_return(feature_available)
 
     module.extract_archive(
         dry_run=False,
@@ -128,12 +143,14 @@ def test_extract_archive_calls_borg_with_numeric_owner_parameter():
         paths=None,
         location_config={'numeric_owner': True},
         storage_config={},
+        local_borg_version='1.2.3',
     )
 
 
 def test_extract_archive_calls_borg_with_umask_parameters():
     flexmock(module.os.path).should_receive('abspath').and_return('repo')
     insert_execute_command_mock(('borg', 'extract', '--umask', '0770', 'repo::archive'))
+    flexmock(module.feature).should_receive('available').and_return(True)
 
     module.extract_archive(
         dry_run=False,
@@ -142,12 +159,14 @@ def test_extract_archive_calls_borg_with_umask_parameters():
         paths=None,
         location_config={},
         storage_config={'umask': '0770'},
+        local_borg_version='1.2.3',
     )
 
 
 def test_extract_archive_calls_borg_with_lock_wait_parameters():
     flexmock(module.os.path).should_receive('abspath').and_return('repo')
     insert_execute_command_mock(('borg', 'extract', '--lock-wait', '5', 'repo::archive'))
+    flexmock(module.feature).should_receive('available').and_return(True)
 
     module.extract_archive(
         dry_run=False,
@@ -156,6 +175,7 @@ def test_extract_archive_calls_borg_with_lock_wait_parameters():
         paths=None,
         location_config={},
         storage_config={'lock_wait': '5'},
+        local_borg_version='1.2.3',
     )
 
 
@@ -163,6 +183,7 @@ def test_extract_archive_with_log_info_calls_borg_with_info_parameter():
     flexmock(module.os.path).should_receive('abspath').and_return('repo')
     insert_execute_command_mock(('borg', 'extract', '--info', 'repo::archive'))
     insert_logging_mock(logging.INFO)
+    flexmock(module.feature).should_receive('available').and_return(True)
 
     module.extract_archive(
         dry_run=False,
@@ -171,6 +192,7 @@ def test_extract_archive_with_log_info_calls_borg_with_info_parameter():
         paths=None,
         location_config={},
         storage_config={},
+        local_borg_version='1.2.3',
     )
 
 
@@ -180,6 +202,7 @@ def test_extract_archive_with_log_debug_calls_borg_with_debug_parameters():
         ('borg', 'extract', '--debug', '--list', '--show-rc', 'repo::archive')
     )
     insert_logging_mock(logging.DEBUG)
+    flexmock(module.feature).should_receive('available').and_return(True)
 
     module.extract_archive(
         dry_run=False,
@@ -188,12 +211,14 @@ def test_extract_archive_with_log_debug_calls_borg_with_debug_parameters():
         paths=None,
         location_config={},
         storage_config={},
+        local_borg_version='1.2.3',
     )
 
 
 def test_extract_archive_calls_borg_with_dry_run_parameter():
     flexmock(module.os.path).should_receive('abspath').and_return('repo')
     insert_execute_command_mock(('borg', 'extract', '--dry-run', 'repo::archive'))
+    flexmock(module.feature).should_receive('available').and_return(True)
 
     module.extract_archive(
         dry_run=True,
@@ -202,12 +227,14 @@ def test_extract_archive_calls_borg_with_dry_run_parameter():
         paths=None,
         location_config={},
         storage_config={},
+        local_borg_version='1.2.3',
     )
 
 
 def test_extract_archive_calls_borg_with_destination_path():
     flexmock(module.os.path).should_receive('abspath').and_return('repo')
     insert_execute_command_mock(('borg', 'extract', 'repo::archive'), working_directory='/dest')
+    flexmock(module.feature).should_receive('available').and_return(True)
 
     module.extract_archive(
         dry_run=False,
@@ -216,6 +243,7 @@ def test_extract_archive_calls_borg_with_destination_path():
         paths=None,
         location_config={},
         storage_config={},
+        local_borg_version='1.2.3',
         destination_path='/dest',
     )
 
@@ -223,6 +251,7 @@ def test_extract_archive_calls_borg_with_destination_path():
 def test_extract_archive_calls_borg_with_strip_components():
     flexmock(module.os.path).should_receive('abspath').and_return('repo')
     insert_execute_command_mock(('borg', 'extract', '--strip-components', '5', 'repo::archive'))
+    flexmock(module.feature).should_receive('available').and_return(True)
 
     module.extract_archive(
         dry_run=False,
@@ -231,6 +260,7 @@ def test_extract_archive_calls_borg_with_strip_components():
         paths=None,
         location_config={},
         storage_config={},
+        local_borg_version='1.2.3',
         strip_components=5,
     )
 
@@ -242,6 +272,7 @@ def test_extract_archive_calls_borg_with_progress_parameter():
         output_file=module.DO_NOT_CAPTURE,
         working_directory=None,
     ).once()
+    flexmock(module.feature).should_receive('available').and_return(True)
 
     module.extract_archive(
         dry_run=False,
@@ -250,6 +281,7 @@ def test_extract_archive_calls_borg_with_progress_parameter():
         paths=None,
         location_config={},
         storage_config={},
+        local_borg_version='1.2.3',
         progress=True,
     )
 
@@ -265,6 +297,7 @@ def test_extract_archive_with_progress_and_extract_to_stdout_raises():
             paths=None,
             location_config={},
             storage_config={},
+            local_borg_version='1.2.3',
             progress=True,
             extract_to_stdout=True,
         )
@@ -279,6 +312,7 @@ def test_extract_archive_calls_borg_with_stdout_parameter_and_returns_process():
         working_directory=None,
         run_to_completion=False,
     ).and_return(process).once()
+    flexmock(module.feature).should_receive('available').and_return(True)
 
     assert (
         module.extract_archive(
@@ -288,6 +322,7 @@ def test_extract_archive_calls_borg_with_stdout_parameter_and_returns_process():
             paths=None,
             location_config={},
             storage_config={},
+            local_borg_version='1.2.3',
             extract_to_stdout=True,
         )
         == process
@@ -299,6 +334,7 @@ def test_extract_archive_skips_abspath_for_remote_repository():
     flexmock(module).should_receive('execute_command').with_args(
         ('borg', 'extract', 'server:repo::archive'), working_directory=None
     ).once()
+    flexmock(module.feature).should_receive('available').and_return(True)
 
     module.extract_archive(
         dry_run=False,
@@ -307,4 +343,5 @@ def test_extract_archive_skips_abspath_for_remote_repository():
         paths=None,
         location_config={},
         storage_config={},
+        local_borg_version='1.2.3',
     )