From 802ca5aef305ae79afc0aa94ae32424101d7350a Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 26 Dec 2025 09:46:39 -0800 Subject: [PATCH] Fix an error when running the "spot" check or "extract" action with the "progress" option or "--progress" flag (#1210). --- NEWS | 2 + borgmatic/actions/check.py | 5 +-- borgmatic/actions/config/bootstrap.py | 2 - borgmatic/actions/restore.py | 2 +- borgmatic/borg/extract.py | 27 ++++++------- tests/unit/actions/test_check.py | 4 +- tests/unit/actions/test_restore.py | 4 +- tests/unit/borg/test_extract.py | 56 +++++++++++++++++++-------- 8 files changed, 59 insertions(+), 43 deletions(-) diff --git a/NEWS b/NEWS index c9562283..15caa986 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ 2.0.14.dev0 + * #1210: Fix an error when running the "spot" check or "extract" action with the "progress" option + or "--progress" flag. * #1212: Fix an error when restoring multiple directory-format database dumps at once. 2.0.13 diff --git a/borgmatic/actions/check.py b/borgmatic/actions/check.py index 7186e3da..631feaca 100644 --- a/borgmatic/actions/check.py +++ b/borgmatic/actions/check.py @@ -380,10 +380,7 @@ def collect_spot_check_source_paths( dry_run=True, repository_path=repository['path'], # Omit "progress" because it interferes with "list_details". - config=dict( - {option: value for option, value in config.items() if option != 'progress'}, - list_details=True, - ), + config=dict(config, progress=False, list_details=True), patterns=borgmatic.actions.pattern.process_patterns( borgmatic.actions.pattern.collect_patterns(config) + tuple( diff --git a/borgmatic/actions/config/bootstrap.py b/borgmatic/actions/config/bootstrap.py index 20558a0c..5e035a11 100644 --- a/borgmatic/actions/config/bootstrap.py +++ b/borgmatic/actions/config/bootstrap.py @@ -127,8 +127,6 @@ def run_bootstrap(bootstrap_arguments, global_arguments, local_borg_version): bootstrap_arguments.repository, archive_name, [config_path.lstrip(os.path.sep) for config_path in manifest_config_paths], - # Only add progress here and not the extract_archive() call above, because progress - # conflicts with extract_to_stdout. dict(config, progress=bootstrap_arguments.progress or False), local_borg_version, global_arguments, diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index 0503f77f..f34c5446 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -496,7 +496,7 @@ def ensure_requested_dumps_restored(dumps_to_restore, dumps_actually_restored): if any requested dumps to restore weren't restored, indicating that they were missing from the configuration. ''' - if not dumps_actually_restored: + if not dumps_to_restore: raise ValueError('No data source dumps were found to restore') missing_dumps = sorted( diff --git a/borgmatic/borg/extract.py b/borgmatic/borg/extract.py index 3004a47e..c4113762 100644 --- a/borgmatic/borg/extract.py +++ b/borgmatic/borg/extract.py @@ -97,9 +97,6 @@ def extract_archive( lock_wait = config.get('lock_wait', None) extra_borg_options = config.get('extra_borg_options', {}).get('extract', '') - if config.get('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 config.get('numeric_ids') else () else: @@ -133,7 +130,7 @@ def extract_archive( + (('--debug', '--list', '--show-rc') if logger.isEnabledFor(logging.DEBUG) else ()) + (('--dry-run',) if dry_run else ()) + (('--strip-components', str(strip_components)) if strip_components else ()) - + (('--progress',) if config.get('progress') else ()) + + (('--progress',) if config.get('progress') and not extract_to_stdout else ()) + (('--stdout',) if extract_to_stdout else ()) + (tuple(shlex.split(extra_borg_options)) if extra_borg_options else ()) + flags.make_repository_archive_flags( @@ -152,6 +149,17 @@ def extract_archive( os.path.join(working_directory or '', destination_path) if destination_path else None ) + if extract_to_stdout: + return execute_command( + full_command, + output_file=subprocess.PIPE, + run_to_completion=False, + environment=environment.make_environment(config), + working_directory=full_destination_path, + borg_local_path=local_path, + borg_exit_codes=borg_exit_codes, + ) + # The progress output isn't compatible with captured and logged output, as progress messes with # the terminal directly. if config.get('progress'): @@ -165,17 +173,6 @@ def extract_archive( ) return None - if extract_to_stdout: - return execute_command( - full_command, - output_file=subprocess.PIPE, - run_to_completion=False, - environment=environment.make_environment(config), - working_directory=full_destination_path, - borg_local_path=local_path, - borg_exit_codes=borg_exit_codes, - ) - # Don't give Borg local path so as to error on warnings, as "borg extract" only gives a warning # if the restore paths don't exist in the archive. execute_command( diff --git a/tests/unit/actions/test_check.py b/tests/unit/actions/test_check.py index d262a5da..3420848b 100644 --- a/tests/unit/actions/test_check.py +++ b/tests/unit/actions/test_check.py @@ -651,7 +651,7 @@ def test_collect_spot_check_source_paths_omits_progress_from_create_dry_run_comm flexmock(module.borgmatic.borg.create).should_receive('make_base_create_command').with_args( dry_run=True, repository_path='repo', - config={'working_directory': '/', 'list_details': True}, + config={'working_directory': '/', 'progress': False, 'list_details': True}, patterns=[Pattern('foo'), Pattern('bar')], local_borg_version=object, global_arguments=object, @@ -941,7 +941,7 @@ def test_collect_spot_check_source_paths_uses_working_directory(): flexmock(module.borgmatic.borg.create).should_receive('make_base_create_command').with_args( dry_run=True, repository_path='repo', - config={'working_directory': '/working/dir', 'list_details': True}, + config={'working_directory': '/working/dir', 'progress': False, 'list_details': True}, patterns=[Pattern('foo'), Pattern('bar')], local_borg_version=object, global_arguments=object, diff --git a/tests/unit/actions/test_restore.py b/tests/unit/actions/test_restore.py index fa64e9c4..081725ad 100644 --- a/tests/unit/actions/test_restore.py +++ b/tests/unit/actions/test_restore.py @@ -1144,8 +1144,8 @@ def test_ensure_requested_dumps_restored_with_all_dumps_restored_does_not_raise( def test_ensure_requested_dumps_restored_with_no_dumps_raises(): with pytest.raises(ValueError): module.ensure_requested_dumps_restored( - dumps_to_restore={}, - dumps_actually_restored={}, + dumps_to_restore=set(), + dumps_actually_restored=set(), ) diff --git a/tests/unit/borg/test_extract.py b/tests/unit/borg/test_extract.py index e293e687..de0fa5c7 100644 --- a/tests/unit/borg/test_extract.py +++ b/tests/unit/borg/test_extract.py @@ -654,23 +654,7 @@ def test_extract_archive_calls_borg_with_progress_flag(): ) -def test_extract_archive_with_progress_and_extract_to_stdout_raises(): - flexmock(module).should_receive('execute_command').never() - - with pytest.raises(ValueError): - module.extract_archive( - dry_run=False, - repository='repo', - archive='archive', - paths=None, - config={'progress': True}, - local_borg_version='1.2.3', - global_arguments=flexmock(), - extract_to_stdout=True, - ) - - -def test_extract_archive_calls_borg_with_stdout_parameter_and_returns_process(): +def test_extract_archive_calls_borg_with_extract_to_stdout_returns_process(): flexmock(module.os.path).should_receive('abspath').and_return('repo') process = flexmock() flexmock(module.environment).should_receive('make_environment') @@ -708,6 +692,44 @@ def test_extract_archive_calls_borg_with_stdout_parameter_and_returns_process(): ) +def test_extract_archive_with_progress_and_extract_to_stdout_ignores_progress(): + flexmock(module.os.path).should_receive('abspath').and_return('repo') + process = 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('execute_command').with_args( + ('borg', 'extract', '--stdout', 'repo::archive'), + output_file=module.subprocess.PIPE, + run_to_completion=False, + environment=None, + working_directory=None, + borg_local_path='borg', + borg_exit_codes=None, + ).and_return(process).once() + flexmock(module.feature).should_receive('available').and_return(True) + flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None) + flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( + ('repo::archive',), + ) + flexmock(module.borgmatic.config.validate).should_receive( + 'normalize_repository_path', + ).and_return('repo') + + assert ( + module.extract_archive( + dry_run=False, + repository='repo', + archive='archive', + paths=None, + config={'progress': True}, + local_borg_version='1.2.3', + global_arguments=flexmock(), + extract_to_stdout=True, + ) + == process + ) + + def test_extract_archive_skips_abspath_for_remote_repository(): flexmock(module.os.path).should_receive('abspath').never() flexmock(module.environment).should_receive('make_environment')