From 1299aefefffac130250cb9c762f2638b7bbc57c2 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 8 Jan 2026 21:29:15 -0800 Subject: [PATCH] For the "repo-delete" action, don't pass "--log-json" to Borg when running Borg interacively (#485). --- borgmatic/borg/repo_delete.py | 25 ++++++++++++--------- tests/unit/borg/test_repo_delete.py | 35 +++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 10 deletions(-) diff --git a/borgmatic/borg/repo_delete.py b/borgmatic/borg/repo_delete.py index 5f99e920..5cb57bb9 100644 --- a/borgmatic/borg/repo_delete.py +++ b/borgmatic/borg/repo_delete.py @@ -21,11 +21,13 @@ def make_repo_delete_command( global_arguments, local_path, remote_path, + output_file, ): ''' Given a local or remote repository dict, a configuration dict, the local Borg version, the - 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. + arguments to the repo_delete action as an argparse.Namespace, and global arguments, the Borg + local path, the Borg remote path, and an optional output file, return a command as a tuple to + repo_delete the entire repository. ''' extra_borg_options = config.get('extra_borg_options', {}).get( 'repo_delete' @@ -51,7 +53,7 @@ def make_repo_delete_command( + borgmatic.borg.flags.make_flags('dry-run', global_arguments.dry_run) + borgmatic.borg.flags.make_flags('remote-path', remote_path) + borgmatic.borg.flags.make_flags('umask', config.get('umask')) - + ('--log-json',) + + (('--log-json',) if output_file is None else ()) + borgmatic.borg.flags.make_flags('lock-wait', config.get('lock_wait')) + borgmatic.borg.flags.make_flags('list', config.get('list_details')) + ( @@ -87,6 +89,14 @@ def delete_repository( ''' borgmatic.logger.add_custom_log_levels() + # Don't capture output when Borg is expected to prompt for interactive confirmation, or the + # prompt won't work. + output_file = ( + None + if repo_delete_arguments.force or repo_delete_arguments.cache_only + else borgmatic.execute.DO_NOT_CAPTURE + ) + command = make_repo_delete_command( repository, config, @@ -95,18 +105,13 @@ def delete_repository( global_arguments, local_path, remote_path, + output_file, ) borgmatic.execute.execute_command( command, output_log_level=logging.ANSWER, - # Don't capture output when Borg is expected to prompt for interactive confirmation, or the - # prompt won't work. - output_file=( - None - if repo_delete_arguments.force or repo_delete_arguments.cache_only - else borgmatic.execute.DO_NOT_CAPTURE - ), + output_file=output_file, environment=borgmatic.borg.environment.make_environment(config), working_directory=borgmatic.config.paths.get_working_directory(config), borg_local_path=local_path, diff --git a/tests/unit/borg/test_repo_delete.py b/tests/unit/borg/test_repo_delete.py index 727d97f7..6f096721 100644 --- a/tests/unit/borg/test_repo_delete.py +++ b/tests/unit/borg/test_repo_delete.py @@ -23,6 +23,7 @@ def test_make_repo_delete_command_with_feature_available_runs_borg_repo_delete() global_arguments=flexmock(dry_run=False), local_path='borg', remote_path=None, + output_file=None, ) assert command == ('borg', 'repo-delete', '--log-json', 'repo') @@ -44,6 +45,7 @@ def test_make_repo_delete_command_without_feature_available_runs_borg_delete(): global_arguments=flexmock(dry_run=False), local_path='borg', remote_path=None, + output_file=None, ) assert command == ('borg', 'delete', '--log-json', 'repo') @@ -66,6 +68,7 @@ def test_make_repo_delete_command_includes_log_info(): global_arguments=flexmock(dry_run=False), local_path='borg', remote_path=None, + output_file=None, ) assert command == ('borg', 'repo-delete', '--info', '--log-json', 'repo') @@ -88,6 +91,7 @@ def test_make_repo_delete_command_includes_log_debug(): global_arguments=flexmock(dry_run=False), local_path='borg', remote_path=None, + output_file=None, ) assert command == ('borg', 'repo-delete', '--debug', '--show-rc', '--log-json', 'repo') @@ -113,6 +117,7 @@ def test_make_repo_delete_command_includes_dry_run(): global_arguments=flexmock(dry_run=True), local_path='borg', remote_path=None, + output_file=None, ) assert command == ('borg', 'repo-delete', '--dry-run', '--log-json', 'repo') @@ -138,6 +143,7 @@ def test_make_repo_delete_command_includes_remote_path(): global_arguments=flexmock(dry_run=False), local_path='borg', remote_path='borg1', + output_file=None, ) assert command == ('borg', 'repo-delete', '--remote-path', 'borg1', '--log-json', 'repo') @@ -161,6 +167,7 @@ def test_make_repo_delete_command_includes_umask(): global_arguments=flexmock(dry_run=False), local_path='borg', remote_path=None, + output_file=None, ) assert command == ('borg', 'repo-delete', '--umask', '077', '--log-json', 'repo') @@ -186,6 +193,7 @@ def test_make_repo_delete_command_includes_lock_wait(): global_arguments=flexmock(dry_run=False), local_path='borg', remote_path=None, + output_file=None, ) assert command == ('borg', 'repo-delete', '--log-json', '--lock-wait', '5', 'repo') @@ -207,6 +215,7 @@ def test_make_repo_delete_command_without_feature_available_includes_delete_extr global_arguments=flexmock(dry_run=False), local_path='borg', remote_path=None, + output_file=None, ) assert command == ('borg', 'delete', '--log-json', '--extra', 'value with space', 'repo') @@ -228,6 +237,7 @@ def test_make_repo_delete_command_with_feature_available_includes_delete_extra_b global_arguments=flexmock(dry_run=False), local_path='borg', remote_path=None, + output_file=None, ) assert command == ('borg', 'repo-delete', '--log-json', '--extra', 'value with space', 'repo') @@ -253,6 +263,7 @@ def test_make_repo_delete_command_includes_list(): global_arguments=flexmock(dry_run=False), local_path='borg', remote_path=None, + output_file=None, ) assert command == ('borg', 'repo-delete', '--log-json', '--list', 'repo') @@ -274,6 +285,7 @@ def test_make_repo_delete_command_includes_force(): global_arguments=flexmock(dry_run=False), local_path='borg', remote_path=None, + output_file=None, ) assert command == ('borg', 'repo-delete', '--log-json', '--force', 'repo') @@ -295,11 +307,34 @@ def test_make_repo_delete_command_includes_force_twice(): global_arguments=flexmock(dry_run=False), local_path='borg', remote_path=None, + output_file=None, ) assert command == ('borg', 'repo-delete', '--log-json', '--force', '--force', 'repo') +def test_make_repo_delete_command_with_output_file_omits_log_json(): + 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(()) + flexmock(module.borgmatic.borg.flags).should_receive('make_repository_flags').and_return( + ('repo',), + ) + + command = module.make_repo_delete_command( + repository={'path': 'repo'}, + config={}, + 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, + output_file=flexmock(), + ) + + assert command == ('borg', 'repo-delete', 'repo') + + def test_delete_repository_with_defaults_does_not_capture_output(): flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER