From 948c86f62ca79cee8788c3f0761e91d6748dbc3a Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 10 Feb 2022 10:09:18 -0800 Subject: [PATCH] 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 | 15 ++++++++---- borgmatic/commands/borgmatic.py | 2 ++ tests/unit/borg/test_create.py | 2 +- tests/unit/borg/test_extract.py | 41 +++++++++++++++++++++++++++++++-- 4 files changed, 53 insertions(+), 7 deletions(-) diff --git a/borgmatic/borg/extract.py b/borgmatic/borg/extract.py index d558715..2621fb8 100644 --- a/borgmatic/borg/extract.py +++ b/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 ()) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 361ee27..43ead04 100644 --- a/borgmatic/commands/borgmatic.py +++ b/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='/', diff --git a/tests/unit/borg/test_create.py b/tests/unit/borg/test_create.py index 79f958b..2a470d9 100644 --- a/tests/unit/borg/test_create.py +++ b/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', diff --git a/tests/unit/borg/test_extract.py b/tests/unit/borg/test_extract.py index 6ebd458..7881fce 100644 --- a/tests/unit/borg/test_extract.py +++ b/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', )