From b2d61ade4e671ca6df6659883336cbb9c983272b Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Mon, 3 Feb 2025 11:17:21 -0800 Subject: [PATCH 1/8] Change the default value for the "--original-hostname" flag from "localhost" to no host specified (#985). --- NEWS | 3 +++ borgmatic/actions/restore.py | 11 +++++++---- tests/unit/actions/test_restore.py | 22 +++++++++++----------- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/NEWS b/NEWS index 7dae7e7b..0be7d27a 100644 --- a/NEWS +++ b/NEWS @@ -8,6 +8,9 @@ subdirectories. * #983: Fix the Btrfs hook to support subvolumes with names like "@home", different from their mount points. + * #985: Change the default value for the "--original-hostname" flag from "localhost" to no host + specified. This way, the "restore" action works without a hostname if there's a single matching + dump. 1.9.8 * #979: Fix root patterns so they don't have an invalid "sh:" prefix before getting passed to Borg. diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index 595b66ae..6562393b 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -57,7 +57,7 @@ def render_dump_metadata(dump): Given a Dump instance, make a display string describing it for use in log messages. ''' name = 'unspecified' if dump.data_source_name is UNSPECIFIED else dump.data_source_name - hostname = dump.hostname or 'localhost' + hostname = dump.hostname or UNSPECIFIED port = None if dump.port is UNSPECIFIED else dump.port if port: @@ -343,12 +343,15 @@ def get_dumps_to_restore(restore_arguments, dumps_from_archive): else UNSPECIFIED ), data_source_name=name, - hostname=restore_arguments.original_hostname or 'localhost', + hostname=restore_arguments.original_hostname or UNSPECIFIED, port=restore_arguments.original_port, ) - for name in restore_arguments.data_sources + for name in restore_arguments.data_sources or (UNSPECIFIED,) } - if restore_arguments.data_sources + if restore_arguments.hook + or restore_arguments.data_sources + or restore_arguments.original_hostname + or restore_arguments.original_port else { Dump( hook_name=UNSPECIFIED, diff --git a/tests/unit/actions/test_restore.py b/tests/unit/actions/test_restore.py index d930d670..4881e715 100644 --- a/tests/unit/actions/test_restore.py +++ b/tests/unit/actions/test_restore.py @@ -575,11 +575,11 @@ def test_get_dumps_to_restore_gets_requested_dumps_found_in_archive(): } flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( - module.Dump(module.UNSPECIFIED, 'foo'), + module.Dump(module.UNSPECIFIED, 'foo', hostname=module.UNSPECIFIED), module.Dump('postgresql_databases', 'foo'), ).and_return(True) flexmock(module).should_receive('dumps_match').with_args( - module.Dump(module.UNSPECIFIED, 'bar'), + module.Dump(module.UNSPECIFIED, 'bar', hostname=module.UNSPECIFIED), module.Dump('postgresql_databases', 'bar'), ).and_return(True) @@ -644,11 +644,11 @@ def test_get_dumps_to_restore_with_all_in_requested_dumps_finds_all_archive_dump } flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( - module.Dump(module.UNSPECIFIED, 'foo'), + module.Dump(module.UNSPECIFIED, 'foo', hostname=module.UNSPECIFIED), module.Dump('postgresql_databases', 'foo'), ).and_return(True) flexmock(module).should_receive('dumps_match').with_args( - module.Dump(module.UNSPECIFIED, 'bar'), + module.Dump(module.UNSPECIFIED, 'bar', hostname=module.UNSPECIFIED), module.Dump('postgresql_databases', 'bar'), ).and_return(True) @@ -673,11 +673,11 @@ def test_get_dumps_to_restore_with_all_in_requested_dumps_plus_additional_reques } flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( - module.Dump(module.UNSPECIFIED, 'foo'), + module.Dump(module.UNSPECIFIED, 'foo', hostname=module.UNSPECIFIED), module.Dump('postgresql_databases', 'foo'), ).and_return(True) flexmock(module).should_receive('dumps_match').with_args( - module.Dump(module.UNSPECIFIED, 'bar'), + module.Dump(module.UNSPECIFIED, 'bar', hostname=module.UNSPECIFIED), module.Dump('postgresql_databases', 'bar'), ).and_return(True) @@ -698,11 +698,11 @@ def test_get_dumps_to_restore_with_all_in_requested_dumps_plus_additional_reques def test_get_dumps_to_restore_raises_for_multiple_matching_dumps_in_archive(): flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( - module.Dump(module.UNSPECIFIED, 'foo'), + module.Dump(module.UNSPECIFIED, 'foo', hostname=module.UNSPECIFIED), module.Dump('postgresql_databases', 'foo'), ).and_return(True) flexmock(module).should_receive('dumps_match').with_args( - module.Dump(module.UNSPECIFIED, 'foo'), + module.Dump(module.UNSPECIFIED, 'foo', hostname=module.UNSPECIFIED), module.Dump('mariadb_databases', 'foo'), ).and_return(True) flexmock(module).should_receive('render_dump_metadata').and_return('test') @@ -725,7 +725,7 @@ def test_get_dumps_to_restore_raises_for_multiple_matching_dumps_in_archive(): def test_get_dumps_to_restore_raises_for_all_in_requested_dumps_and_requested_dumps_missing_from_archive(): flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( - module.Dump(module.UNSPECIFIED, 'foo'), + module.Dump(module.UNSPECIFIED, 'foo', hostname=module.UNSPECIFIED), module.Dump('postgresql_databases', 'foo'), ).and_return(True) flexmock(module).should_receive('render_dump_metadata').and_return('test') @@ -750,7 +750,7 @@ def test_get_dumps_to_restore_with_requested_hook_name_filters_dumps_found_in_ar } flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( - module.Dump('postgresql_databases', 'foo'), + module.Dump('postgresql_databases', 'foo', hostname=module.UNSPECIFIED), module.Dump('postgresql_databases', 'foo'), ).and_return(True) @@ -775,7 +775,7 @@ def test_get_dumps_to_restore_with_requested_shortened_hook_name_filters_dumps_f } flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( - module.Dump('postgresql_databases', 'foo'), + module.Dump('postgresql_databases', 'foo', hostname=module.UNSPECIFIED), module.Dump('postgresql_databases', 'foo'), ).and_return(True) From c3f4f9419019326312e8f7adc1b0bafd9e3ca508 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Mon, 3 Feb 2025 11:20:13 -0800 Subject: [PATCH 2/8] Bump version for release. --- NEWS | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 0be7d27a..ed4f1401 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,4 @@ -1.9.9.dev0 +1.9.9 * #635: Log the repository path or label on every relevant log message, not just some logs. * #961: When the "encryption_passcommand" option is set, call the command once from borgmatic to collect the encryption passphrase and pass it to Borg multiple times. See the documentation for diff --git a/pyproject.toml b/pyproject.toml index 34f9c0d9..34c7f1ba 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "borgmatic" -version = "1.9.9.dev0" +version = "1.9.9" authors = [ { name="Dan Helfman", email="witten@torsion.org" }, ] From d29d0bc1c62928e54e3fac4492f8f64676e90f15 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Mon, 3 Feb 2025 11:22:54 -0800 Subject: [PATCH 3/8] NEWS wording tweaks for clarity. --- NEWS | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index ed4f1401..4c99236c 100644 --- a/NEWS +++ b/NEWS @@ -1,16 +1,16 @@ 1.9.9 * #635: Log the repository path or label on every relevant log message, not just some logs. * #961: When the "encryption_passcommand" option is set, call the command once from borgmatic to - collect the encryption passphrase and pass it to Borg multiple times. See the documentation for - more information: https://torsion.org/borgmatic/docs/how-to/provide-your-passwords/ + collect the encryption passphrase and then pass it to Borg multiple times. See the documentation + for more information: https://torsion.org/borgmatic/docs/how-to/provide-your-passwords/ * #981: Fix a "spot" check file count delta error. * #982: Fix for borgmatic "exclude_patterns" and "exclude_from" recursing into excluded subdirectories. - * #983: Fix the Btrfs hook to support subvolumes with names like "@home", different from their + * #983: Fix the Btrfs hook to support subvolumes with names like "@home" different from their mount points. * #985: Change the default value for the "--original-hostname" flag from "localhost" to no host specified. This way, the "restore" action works without a hostname if there's a single matching - dump. + database dump. 1.9.8 * #979: Fix root patterns so they don't have an invalid "sh:" prefix before getting passed to Borg. From 6cfa10fb7e8172c44726ed2d5d51c86099887916 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Mon, 3 Feb 2025 23:11:59 -0800 Subject: [PATCH 4/8] Fix a "list" action error when the "encryption_passcommand" option is set (#987). --- NEWS | 3 +++ borgmatic/actions/check.py | 3 +-- borgmatic/borg/break_lock.py | 3 +-- borgmatic/borg/check.py | 5 ++--- borgmatic/borg/create.py | 21 ++++++++------------- borgmatic/borg/environment.py | 3 +++ borgmatic/borg/extract.py | 10 ++++------ borgmatic/borg/list.py | 9 +++------ borgmatic/borg/mount.py | 5 ++--- borgmatic/borg/repo_info.py | 5 ++--- borgmatic/borg/repo_list.py | 5 ++--- pyproject.toml | 2 +- tests/unit/borg/test_create.py | 12 ++++++------ 13 files changed, 38 insertions(+), 48 deletions(-) diff --git a/NEWS b/NEWS index 4c99236c..9e81eae9 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +1.9.10.dev0 + * #987: Fix a "list" action error when the "encryption_passcommand" option is set. + 1.9.9 * #635: Log the repository path or label on every relevant log message, not just some logs. * #961: When the "encryption_passcommand" option is set, call the command once from borgmatic to diff --git a/borgmatic/actions/check.py b/borgmatic/actions/check.py index d2bcbc25..d3e347e3 100644 --- a/borgmatic/actions/check.py +++ b/borgmatic/actions/check.py @@ -386,13 +386,12 @@ def collect_spot_check_source_paths( stream_processes=stream_processes, ) ) - borg_environment = borgmatic.borg.environment.make_environment(config) working_directory = borgmatic.config.paths.get_working_directory(config) paths_output = borgmatic.execute.execute_command_and_capture_output( create_flags + create_positional_arguments, capture_stderr=True, - extra_environment=borg_environment, + extra_environment=borgmatic.borg.environment.make_environment(config), working_directory=working_directory, borg_local_path=local_path, borg_exit_codes=config.get('borg_exit_codes'), diff --git a/borgmatic/borg/break_lock.py b/borgmatic/borg/break_lock.py index f7c19f7e..478221f0 100644 --- a/borgmatic/borg/break_lock.py +++ b/borgmatic/borg/break_lock.py @@ -34,10 +34,9 @@ def break_lock( + flags.make_repository_flags(repository_path, local_borg_version) ) - borg_environment = environment.make_environment(config) execute_command( full_command, - extra_environment=borg_environment, + extra_environment=environment.make_environment(config), working_directory=borgmatic.config.paths.get_working_directory(config), borg_local_path=local_path, borg_exit_codes=config.get('borg_exit_codes'), diff --git a/borgmatic/borg/check.py b/borgmatic/borg/check.py index 6fb203db..ee26c923 100644 --- a/borgmatic/borg/check.py +++ b/borgmatic/borg/check.py @@ -152,7 +152,6 @@ def check_archives( max_duration = check_arguments.max_duration or repository_check_config.get('max_duration') umask = config.get('umask') - borg_environment = environment.make_environment(config) borg_exit_codes = config.get('borg_exit_codes') full_command = ( @@ -178,7 +177,7 @@ def check_archives( execute_command( full_command, output_file=DO_NOT_CAPTURE, - extra_environment=borg_environment, + extra_environment=environment.make_environment(config), working_directory=working_directory, borg_local_path=local_path, borg_exit_codes=borg_exit_codes, @@ -186,7 +185,7 @@ def check_archives( else: execute_command( full_command, - extra_environment=borg_environment, + extra_environment=environment.make_environment(config), working_directory=working_directory, borg_local_path=local_path, borg_exit_codes=borg_exit_codes, diff --git a/borgmatic/borg/create.py b/borgmatic/borg/create.py index 3961b511..924c50e7 100644 --- a/borgmatic/borg/create.py +++ b/borgmatic/borg/create.py @@ -122,15 +122,14 @@ def collect_special_file_paths( config, local_path, working_directory, - borg_environment, borgmatic_runtime_directory, ): ''' Given a dry-run flag, a Borg create command as a tuple, a configuration dict, a local Borg path, - a working directory, a dict of environment variables to pass to Borg, and the borgmatic runtime - directory, collect the paths for any special files (character devices, block devices, and named - pipes / FIFOs) that Borg would encounter during a create. These are all paths that could cause - Borg to hang if its --read-special flag is used. + a working directory, and the borgmatic runtime directory, collect the paths for any special + files (character devices, block devices, and named pipes / FIFOs) that Borg would encounter + during a create. These are all paths that could cause Borg to hang if its --read-special flag is + used. Skip looking for special files in the given borgmatic runtime directory, as borgmatic creates its own special files there for database dumps. And if the borgmatic runtime directory is @@ -144,7 +143,7 @@ def collect_special_file_paths( + ('--dry-run', '--list'), capture_stderr=True, working_directory=working_directory, - extra_environment=borg_environment, + extra_environment=environment.make_environment(config), borg_local_path=local_path, borg_exit_codes=config.get('borg_exit_codes'), ) @@ -299,7 +298,6 @@ def make_base_create_command( logger.warning( 'Ignoring configured "read_special" value of false, as true is needed for database hooks.' ) - borg_environment = environment.make_environment(config) working_directory = borgmatic.config.paths.get_working_directory(config) logger.debug('Collecting special file paths') @@ -309,7 +307,6 @@ def make_base_create_command( config, local_path, working_directory, - borg_environment, borgmatic_runtime_directory=borgmatic_runtime_directory, ) @@ -396,8 +393,6 @@ def create_archive( # the terminal directly. output_file = DO_NOT_CAPTURE if progress else None - borg_environment = environment.make_environment(config) - create_flags += ( (('--info',) if logger.getEffectiveLevel() == logging.INFO and not json else ()) + (('--stats',) if stats and not json and not dry_run else ()) @@ -414,7 +409,7 @@ def create_archive( output_log_level, output_file, working_directory=working_directory, - extra_environment=borg_environment, + extra_environment=environment.make_environment(config), borg_local_path=local_path, borg_exit_codes=borg_exit_codes, ) @@ -422,7 +417,7 @@ def create_archive( return execute_command_and_capture_output( create_flags + create_positional_arguments, working_directory=working_directory, - extra_environment=borg_environment, + extra_environment=environment.make_environment(config), borg_local_path=local_path, borg_exit_codes=borg_exit_codes, ) @@ -432,7 +427,7 @@ def create_archive( output_log_level, output_file, working_directory=working_directory, - extra_environment=borg_environment, + extra_environment=environment.make_environment(config), borg_local_path=local_path, borg_exit_codes=borg_exit_codes, ) diff --git a/borgmatic/borg/environment.py b/borgmatic/borg/environment.py index 0e4a04e2..839d0887 100644 --- a/borgmatic/borg/environment.py +++ b/borgmatic/borg/environment.py @@ -28,6 +28,9 @@ def make_environment(config): ''' Given a borgmatic configuration dict, return its options converted to a Borg environment variable dict. + + Do not reuse this environment across multiple Borg invocations, because it can include + references to resources like anonymous pipes for passphrases—which can only be consumed once. ''' environment = {} diff --git a/borgmatic/borg/extract.py b/borgmatic/borg/extract.py index 67a6a63e..d599b5b6 100644 --- a/borgmatic/borg/extract.py +++ b/borgmatic/borg/extract.py @@ -44,7 +44,6 @@ def extract_last_archive_dry_run( return list_flag = ('--list',) if logger.isEnabledFor(logging.DEBUG) else () - borg_environment = environment.make_environment(config) full_extract_command = ( (local_path, 'extract', '--dry-run') + (('--remote-path', remote_path) if remote_path else ()) @@ -59,7 +58,7 @@ def extract_last_archive_dry_run( execute_command( full_extract_command, - extra_environment=borg_environment, + extra_environment=environment.make_environment(config), working_directory=borgmatic.config.paths.get_working_directory(config), borg_local_path=local_path, borg_exit_codes=config.get('borg_exit_codes'), @@ -144,7 +143,6 @@ def extract_archive( + (tuple(paths) if paths else ()) ) - borg_environment = environment.make_environment(config) borg_exit_codes = config.get('borg_exit_codes') full_destination_path = ( os.path.join(working_directory or '', destination_path) if destination_path else None @@ -156,7 +154,7 @@ def extract_archive( return execute_command( full_command, output_file=DO_NOT_CAPTURE, - extra_environment=borg_environment, + extra_environment=environment.make_environment(config), working_directory=full_destination_path, borg_local_path=local_path, borg_exit_codes=borg_exit_codes, @@ -168,7 +166,7 @@ def extract_archive( full_command, output_file=subprocess.PIPE, run_to_completion=False, - extra_environment=borg_environment, + extra_environment=environment.make_environment(config), working_directory=full_destination_path, borg_local_path=local_path, borg_exit_codes=borg_exit_codes, @@ -178,7 +176,7 @@ def extract_archive( # if the restore paths don't exist in the archive. execute_command( full_command, - extra_environment=borg_environment, + extra_environment=environment.make_environment(config), working_directory=full_destination_path, borg_local_path=local_path, borg_exit_codes=borg_exit_codes, diff --git a/borgmatic/borg/list.py b/borgmatic/borg/list.py index 7862d884..755b05c9 100644 --- a/borgmatic/borg/list.py +++ b/borgmatic/borg/list.py @@ -106,8 +106,6 @@ def capture_archive_listing( format to use for the output, and local and remote Borg paths, capture the output of listing that archive and return it as a list of file paths. ''' - borg_environment = environment.make_environment(config) - return tuple( execute_command_and_capture_output( make_list_command( @@ -126,7 +124,7 @@ def capture_archive_listing( local_path, remote_path, ), - extra_environment=borg_environment, + extra_environment=environment.make_environment(config), working_directory=borgmatic.config.paths.get_working_directory(config), borg_local_path=local_path, borg_exit_codes=config.get('borg_exit_codes'), @@ -194,7 +192,6 @@ def list_archive( 'The --json flag on the list action is not supported when using the --archive/--find flags.' ) - borg_environment = environment.make_environment(config) borg_exit_codes = config.get('borg_exit_codes') # If there are any paths to find (and there's not a single archive already selected), start by @@ -224,7 +221,7 @@ def list_archive( local_path, remote_path, ), - extra_environment=borg_environment, + extra_environment=environment.make_environment(config), working_directory=borgmatic.config.paths.get_working_directory(config), borg_local_path=local_path, borg_exit_codes=borg_exit_codes, @@ -260,7 +257,7 @@ def list_archive( execute_command( main_command, output_log_level=logging.ANSWER, - extra_environment=borg_environment, + extra_environment=environment.make_environment(config), working_directory=borgmatic.config.paths.get_working_directory(config), borg_local_path=local_path, borg_exit_codes=borg_exit_codes, diff --git a/borgmatic/borg/mount.py b/borgmatic/borg/mount.py index 378fd5eb..06c6422c 100644 --- a/borgmatic/borg/mount.py +++ b/borgmatic/borg/mount.py @@ -59,7 +59,6 @@ def mount_archive( + (tuple(mount_arguments.paths) if mount_arguments.paths else ()) ) - borg_environment = environment.make_environment(config) working_directory = borgmatic.config.paths.get_working_directory(config) # Don't capture the output when foreground mode is used so that ctrl-C can work properly. @@ -67,7 +66,7 @@ def mount_archive( execute_command( full_command, output_file=DO_NOT_CAPTURE, - extra_environment=borg_environment, + extra_environment=environment.make_environment(config), working_directory=working_directory, borg_local_path=local_path, borg_exit_codes=config.get('borg_exit_codes'), @@ -76,7 +75,7 @@ def mount_archive( execute_command( full_command, - extra_environment=borg_environment, + extra_environment=environment.make_environment(config), working_directory=working_directory, borg_local_path=local_path, borg_exit_codes=config.get('borg_exit_codes'), diff --git a/borgmatic/borg/repo_info.py b/borgmatic/borg/repo_info.py index 839a6889..0c37a052 100644 --- a/borgmatic/borg/repo_info.py +++ b/borgmatic/borg/repo_info.py @@ -50,14 +50,13 @@ def display_repository_info( + flags.make_repository_flags(repository_path, local_borg_version) ) - extra_environment = environment.make_environment(config) working_directory = borgmatic.config.paths.get_working_directory(config) borg_exit_codes = config.get('borg_exit_codes') if repo_info_arguments.json: return execute_command_and_capture_output( full_command, - extra_environment=extra_environment, + extra_environment=environment.make_environment(config), working_directory=working_directory, borg_local_path=local_path, borg_exit_codes=borg_exit_codes, @@ -66,7 +65,7 @@ def display_repository_info( execute_command( full_command, output_log_level=logging.ANSWER, - extra_environment=extra_environment, + extra_environment=environment.make_environment(config), working_directory=working_directory, borg_local_path=local_path, borg_exit_codes=borg_exit_codes, diff --git a/borgmatic/borg/repo_list.py b/borgmatic/borg/repo_list.py index cfe914d4..f8e46289 100644 --- a/borgmatic/borg/repo_list.py +++ b/borgmatic/borg/repo_list.py @@ -140,7 +140,6 @@ def list_repository( return JSON output). ''' borgmatic.logger.add_custom_log_levels() - borg_environment = environment.make_environment(config) main_command = make_repo_list_command( repository_path, @@ -165,7 +164,7 @@ def list_repository( json_listing = execute_command_and_capture_output( json_command, - extra_environment=borg_environment, + extra_environment=environment.make_environment(config), working_directory=working_directory, borg_local_path=local_path, borg_exit_codes=borg_exit_codes, @@ -179,7 +178,7 @@ def list_repository( execute_command( main_command, output_log_level=logging.ANSWER, - extra_environment=borg_environment, + extra_environment=environment.make_environment(config), working_directory=working_directory, borg_local_path=local_path, borg_exit_codes=borg_exit_codes, diff --git a/pyproject.toml b/pyproject.toml index 34c7f1ba..92102e3f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "borgmatic" -version = "1.9.9" +version = "1.9.10.dev0" authors = [ { name="Dan Helfman", email="witten@torsion.org" }, ] diff --git a/tests/unit/borg/test_create.py b/tests/unit/borg/test_create.py index e0a3d1cc..6015004a 100644 --- a/tests/unit/borg/test_create.py +++ b/tests/unit/borg/test_create.py @@ -185,6 +185,7 @@ def test_any_parent_directories_treats_unrelated_paths_as_non_match(): def test_collect_special_file_paths_parses_special_files_from_borg_dry_run_file_list(): + flexmock(module.environment).should_receive('make_environment').and_return(None) flexmock(module).should_receive('execute_command_and_capture_output').and_return( 'Processing files ...\n- /foo\n+ /bar\n- /baz' ) @@ -198,12 +199,12 @@ def test_collect_special_file_paths_parses_special_files_from_borg_dry_run_file_ config={}, local_path=None, working_directory=None, - borg_environment=None, borgmatic_runtime_directory='/run/borgmatic', ) == ('/foo', '/bar', '/baz') def test_collect_special_file_paths_skips_borgmatic_runtime_directory(): + flexmock(module.environment).should_receive('make_environment').and_return(None) flexmock(module).should_receive('execute_command_and_capture_output').and_return( '+ /foo\n- /run/borgmatic/bar\n- /baz' ) @@ -225,12 +226,12 @@ def test_collect_special_file_paths_skips_borgmatic_runtime_directory(): config={}, local_path=None, working_directory=None, - borg_environment=None, borgmatic_runtime_directory='/run/borgmatic', ) == ('/foo', '/baz') def test_collect_special_file_paths_with_borgmatic_runtime_directory_missing_from_paths_output_errors(): + flexmock(module.environment).should_receive('make_environment').and_return(None) flexmock(module).should_receive('execute_command_and_capture_output').and_return( '+ /foo\n- /bar\n- /baz' ) @@ -245,12 +246,12 @@ def test_collect_special_file_paths_with_borgmatic_runtime_directory_missing_fro config={}, local_path=None, working_directory=None, - borg_environment=None, borgmatic_runtime_directory='/run/borgmatic', ) def test_collect_special_file_paths_with_dry_run_and_borgmatic_runtime_directory_missing_from_paths_output_does_not_raise(): + flexmock(module.environment).should_receive('make_environment').and_return(None) flexmock(module).should_receive('execute_command_and_capture_output').and_return( '+ /foo\n- /bar\n- /baz' ) @@ -264,12 +265,12 @@ def test_collect_special_file_paths_with_dry_run_and_borgmatic_runtime_directory config={}, local_path=None, working_directory=None, - borg_environment=None, borgmatic_runtime_directory='/run/borgmatic', ) == ('/foo', '/bar', '/baz') def test_collect_special_file_paths_excludes_non_special_files(): + flexmock(module.environment).should_receive('make_environment').and_return(None) flexmock(module).should_receive('execute_command_and_capture_output').and_return( '+ /foo\n+ /bar\n+ /baz' ) @@ -285,12 +286,12 @@ def test_collect_special_file_paths_excludes_non_special_files(): config={}, local_path=None, working_directory=None, - borg_environment=None, borgmatic_runtime_directory='/run/borgmatic', ) == ('/foo', '/baz') def test_collect_special_file_paths_omits_exclude_no_dump_flag_from_command(): + flexmock(module.environment).should_receive('make_environment').and_return(None) flexmock(module).should_receive('execute_command_and_capture_output').with_args( ('borg', 'create', '--dry-run', '--list'), capture_stderr=True, @@ -309,7 +310,6 @@ def test_collect_special_file_paths_omits_exclude_no_dump_flag_from_command(): config={}, local_path='borg', working_directory=None, - borg_environment=None, borgmatic_runtime_directory='/run/borgmatic', ) From 23009e22aa3f9f96cbf836435e01a6ccf2605269 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Mon, 3 Feb 2025 23:20:31 -0800 Subject: [PATCH 5/8] When both "encryption_passcommand" and "encryption_passphrase" are configured, prefer "encryption_passphrase" even if it's an empty value (#987). --- NEWS | 2 ++ borgmatic/borg/passcommand.py | 2 +- tests/unit/borg/test_passcommand.py | 16 ++++++++++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 9e81eae9..25b6a918 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,7 @@ 1.9.10.dev0 * #987: Fix a "list" action error when the "encryption_passcommand" option is set. + * #987: When both "encryption_passcommand" and "encryption_passphrase" are configured, prefer + "encryption_passphrase" even if it's an empty value. 1.9.9 * #635: Log the repository path or label on every relevant log message, not just some logs. diff --git a/borgmatic/borg/passcommand.py b/borgmatic/borg/passcommand.py index 2574ce23..f70a1392 100644 --- a/borgmatic/borg/passcommand.py +++ b/borgmatic/borg/passcommand.py @@ -47,4 +47,4 @@ def get_passphrase_from_passcommand(config): passphrase = config.get('encryption_passphrase') working_directory = borgmatic.config.paths.get_working_directory(config) - return run_passcommand(passcommand, bool(passphrase), working_directory) + return run_passcommand(passcommand, bool(passphrase is not None), working_directory) diff --git a/tests/unit/borg/test_passcommand.py b/tests/unit/borg/test_passcommand.py index 2b6161c8..198cfd15 100644 --- a/tests/unit/borg/test_passcommand.py +++ b/tests/unit/borg/test_passcommand.py @@ -53,3 +53,19 @@ def test_get_passphrase_from_passcommand_with_configured_passphrase_and_passcomm ) is None ) + + +def test_get_passphrase_from_passcommand_with_configured_blank_passphrase_and_passcommand_detects_passphrase(): + flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return( + '/working' + ) + flexmock(module).should_receive('run_passcommand').with_args( + 'command', True, '/working' + ).and_return(None).once() + + assert ( + module.get_passphrase_from_passcommand( + {'encryption_passphrase': '', 'encryption_passcommand': 'command'}, + ) + is None + ) From 5dc0b08f22308584b5371379e4d8572b95261e59 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 4 Feb 2025 11:51:39 -0800 Subject: [PATCH 6/8] Fix the log message code to avoid using Python 3.10+ logging features (#989). --- NEWS | 2 ++ borgmatic/logger.py | 55 ++++++++++++++++++++++----------------- tests/unit/test_logger.py | 52 ++++++++++++++---------------------- 3 files changed, 52 insertions(+), 57 deletions(-) diff --git a/NEWS b/NEWS index 25b6a918..65032279 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,8 @@ * #987: Fix a "list" action error when the "encryption_passcommand" option is set. * #987: When both "encryption_passcommand" and "encryption_passphrase" are configured, prefer "encryption_passphrase" even if it's an empty value. + * #989: Fix the log message code to avoid using Python 3.10+ logging features. Now borgmatic will + work with Python 3.9 again. 1.9.9 * #635: Log the repository path or label on every relevant log message, not just some logs. diff --git a/borgmatic/logger.py b/borgmatic/logger.py index a81541d1..4fb03a58 100644 --- a/borgmatic/logger.py +++ b/borgmatic/logger.py @@ -87,11 +87,16 @@ class Multi_stream_handler(logging.Handler): handler.setLevel(level) -class Console_no_color_formatter(logging.Formatter): - def __init__(self, *args, **kwargs): # pragma: no cover - super(Console_no_color_formatter, self).__init__( - '{prefix}{message}', style='{', defaults={'prefix': ''}, *args, **kwargs - ) +class Log_prefix_formatter(logging.Formatter): + def __init__(self, fmt='{prefix}{message}', style='{', *args, **kwargs): # pragma: no cover + self.prefix = None + + super(Log_prefix_formatter, self).__init__(fmt=fmt, style=style, *args, **kwargs) + + def format(self, record): # pragma: no cover + record.prefix = f'{self.prefix}: ' if self.prefix else '' + + return super(Log_prefix_formatter, self).format(record) class Color(enum.Enum): @@ -105,8 +110,9 @@ class Color(enum.Enum): class Console_color_formatter(logging.Formatter): def __init__(self, *args, **kwargs): + self.prefix = None super(Console_color_formatter, self).__init__( - '{prefix}{message}', style='{', defaults={'prefix': ''}, *args, **kwargs + '{prefix}{message}', style='{', *args, **kwargs ) def format(self, record): @@ -124,6 +130,7 @@ class Console_color_formatter(logging.Formatter): .get(record.levelno) .value ) + record.prefix = f'{self.prefix}: ' if self.prefix else '' return color_text(color, super(Console_color_formatter, self).format(record)) @@ -188,28 +195,32 @@ def add_custom_log_levels(): # pragma: no cover def get_log_prefix(): ''' - Return the current log prefix from the defaults for the formatter on the first logging handler, - set by set_log_prefix(). Return None if no such prefix exists. + Return the current log prefix set by set_log_prefix(). Return None if no such prefix exists. + + It would be a whole lot easier to use logger.Formatter(defaults=...) instead, but that argument + doesn't exist until Python 3.10+. ''' try: - return next( - handler.formatter._style._defaults.get('prefix').rstrip().rstrip(':') + formatter = next( + handler.formatter for handler in logging.getLogger().handlers + if handler.formatter + if hasattr(handler.formatter, 'prefix') ) - except (StopIteration, AttributeError): + except StopIteration: return None + return formatter.prefix + def set_log_prefix(prefix): ''' - Given a log prefix as a string, set it into the defaults for the formatters on all logging - handlers. Note that this overwrites any existing defaults. + Given a log prefix as a string, set it into the each handler's formatter so that it can inject + the prefix into each logged record. ''' for handler in logging.getLogger().handlers: - try: - handler.formatter._style._defaults = {'prefix': f'{prefix}: ' if prefix else ''} - except AttributeError: - pass + if handler.formatter and hasattr(handler.formatter, 'prefix'): + handler.formatter.prefix = prefix class Log_prefix: @@ -296,7 +307,7 @@ def configure_logging( if color_enabled: console_handler.setFormatter(Console_color_formatter()) else: - console_handler.setFormatter(Console_no_color_formatter()) + console_handler.setFormatter(Log_prefix_formatter()) console_handler.setLevel(console_log_level) @@ -315,10 +326,8 @@ def configure_logging( if syslog_path: syslog_handler = logging.handlers.SysLogHandler(address=syslog_path) syslog_handler.setFormatter( - logging.Formatter( + Log_prefix_formatter( 'borgmatic: {levelname} {prefix}{message}', # noqa: FS003 - style='{', - defaults={'prefix': ''}, ) ) syslog_handler.setLevel(syslog_log_level) @@ -327,10 +336,8 @@ def configure_logging( if log_file and log_file_log_level != logging.DISABLED: file_handler = logging.handlers.WatchedFileHandler(log_file) file_handler.setFormatter( - logging.Formatter( + Log_prefix_formatter( log_file_format or '[{asctime}] {levelname}: {prefix}{message}', # noqa: FS003 - style='{', - defaults={'prefix': ''}, ) ) file_handler.setLevel(log_file_log_level) diff --git a/tests/unit/test_logger.py b/tests/unit/test_logger.py index ef694413..5f0fae12 100644 --- a/tests/unit/test_logger.py +++ b/tests/unit/test_logger.py @@ -228,16 +228,12 @@ def test_add_logging_level_skips_global_setting_if_already_set(): module.add_logging_level('PLAID', 99) -def test_get_log_prefix_gets_prefix_from_first_handler(): +def test_get_log_prefix_gets_prefix_from_first_handler_formatter_with_prefix(): flexmock(module.logging).should_receive('getLogger').and_return( flexmock( handlers=[ - flexmock( - formatter=flexmock( - _style=flexmock(_defaults=flexmock(get=lambda name: 'myprefix: ')) - ) - ), - flexmock(), + flexmock(formatter=flexmock()), + flexmock(formatter=flexmock(prefix='myprefix')), ], removeHandler=lambda handler: None, ) @@ -261,8 +257,8 @@ def test_get_log_prefix_with_no_formatters_does_not_raise(): flexmock(module.logging).should_receive('getLogger').and_return( flexmock( handlers=[ - flexmock(), - flexmock(), + flexmock(formatter=None), + flexmock(formatter=None), ], removeHandler=lambda handler: None, ) @@ -276,9 +272,8 @@ def test_get_log_prefix_with_no_prefix_does_not_raise(): flexmock( handlers=[ flexmock( - formatter=flexmock(_style=flexmock(_defaults=flexmock(get=lambda name: None))) + formatter=flexmock(), ), - flexmock(), ], removeHandler=lambda handler: None, ) @@ -287,24 +282,20 @@ def test_get_log_prefix_with_no_prefix_does_not_raise(): assert module.get_log_prefix() is None -def test_set_log_prefix_updates_all_handlers(): - styles = ( - flexmock(_defaults=None), - flexmock(_defaults=None), +def test_set_log_prefix_updates_all_handler_formatters(): + formatters = ( + flexmock(prefix=None), + flexmock(prefix=None), ) flexmock(module.logging).should_receive('getLogger').and_return( flexmock( handlers=[ flexmock( - formatter=flexmock( - _style=styles[0], - ) + formatter=formatters[0], ), flexmock( - formatter=flexmock( - _style=styles[1], - ) + formatter=formatters[1], ), ], removeHandler=lambda handler: None, @@ -313,12 +304,12 @@ def test_set_log_prefix_updates_all_handlers(): module.set_log_prefix('myprefix') - for style in styles: - assert style._defaults == {'prefix': 'myprefix: '} + for formatter in formatters: + assert formatter.prefix == 'myprefix' def test_set_log_prefix_skips_handlers_without_a_formatter(): - style = flexmock(_defaults=None) + formatter = flexmock(prefix=None) flexmock(module.logging).should_receive('getLogger').and_return( flexmock( @@ -326,11 +317,8 @@ def test_set_log_prefix_skips_handlers_without_a_formatter(): flexmock( formatter=None, ), - flexmock(), flexmock( - formatter=flexmock( - _style=style, - ) + formatter=formatter, ), ], removeHandler=lambda handler: None, @@ -339,7 +327,7 @@ def test_set_log_prefix_skips_handlers_without_a_formatter(): module.set_log_prefix('myprefix') - assert style._defaults == {'prefix': 'myprefix: '} + assert formatter.prefix == 'myprefix' def test_log_prefix_sets_prefix_and_then_restores_no_prefix_after(): @@ -541,10 +529,8 @@ def test_configure_logging_to_both_log_file_and_syslog(): def test_configure_logging_to_log_file_formats_with_custom_log_format(): flexmock(module).should_receive('add_custom_log_levels') flexmock(module.logging).ANSWER = module.ANSWER - flexmock(module.logging).should_receive('Formatter').with_args( + flexmock(module).should_receive('Log_prefix_formatter').with_args( '{message}', # noqa: FS003 - style='{', - defaults={'prefix': ''}, ).once() fake_formatter = flexmock() flexmock(module).should_receive('Console_color_formatter').and_return(fake_formatter) @@ -594,7 +580,7 @@ def test_configure_logging_uses_console_no_color_formatter_if_color_disabled(): flexmock(module.logging).ANSWER = module.ANSWER fake_formatter = flexmock() flexmock(module).should_receive('Console_color_formatter').never() - flexmock(module).should_receive('Console_no_color_formatter').and_return(fake_formatter) + flexmock(module).should_receive('Log_prefix_formatter').and_return(fake_formatter) multi_stream_handler = flexmock(setLevel=lambda level: None, level=logging.INFO) multi_stream_handler.should_receive('setFormatter').with_args(fake_formatter).once() flexmock(module).should_receive('Multi_stream_handler').and_return(multi_stream_handler) From ffaa99ba15418caf5add29b2ce4cf4e8bf105bbf Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 6 Feb 2025 11:52:16 -0800 Subject: [PATCH 7/8] With the "max_duration" option or the "--max-duration" flag, run the archives and repository checks separately so they don't interfere with one another (#988). --- NEWS | 3 + borgmatic/borg/check.py | 83 +++++----- borgmatic/config/schema.yaml | 4 +- tests/unit/borg/test_check.py | 275 +++++++++++++++++++++++++--------- 4 files changed, 246 insertions(+), 119 deletions(-) diff --git a/NEWS b/NEWS index 65032279..4d25f9dc 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,9 @@ * #987: Fix a "list" action error when the "encryption_passcommand" option is set. * #987: When both "encryption_passcommand" and "encryption_passphrase" are configured, prefer "encryption_passphrase" even if it's an empty value. + * #988: With the "max_duration" option or the "--max-duration" flag, run the archives and + repository checks separately so they don't interfere with one another. Previously, borgmatic + refused to run checks in this situation. * #989: Fix the log message code to avoid using Python 3.10+ logging features. Now borgmatic will work with Python 3.9 again. diff --git a/borgmatic/borg/check.py b/borgmatic/borg/check.py index ee26c923..4350b8e4 100644 --- a/borgmatic/borg/check.py +++ b/borgmatic/borg/check.py @@ -64,15 +64,11 @@ def make_check_name_flags(checks, archive_filter_flags): ('--repository-only',) - However, if both "repository" and "archives" are in checks, then omit them from the returned - flags because Borg does both checks by default. If "data" is in checks, that implies "archives". + However, if both "repository" and "archives" are in checks, then omit the "only" flags from the + returned flags because Borg does both checks by default. Note that a "data" check only works + along with an "archives" check. ''' - if 'data' in checks: - data_flags = ('--verify-data',) - checks.update({'archives'}) - else: - data_flags = () - + data_flags = ('--verify-data',) if 'data' in checks else () common_flags = (archive_filter_flags if 'archives' in checks else ()) + data_flags if {'repository', 'archives'}.issubset(checks): @@ -142,49 +138,50 @@ def check_archives( except StopIteration: repository_check_config = {} - if check_arguments.max_duration and 'archives' in checks: - raise ValueError('The archives check cannot run when the --max-duration flag is used') - if repository_check_config.get('max_duration') and 'archives' in checks: - raise ValueError( - 'The archives check cannot run when the repository check has the max_duration option set' - ) - max_duration = check_arguments.max_duration or repository_check_config.get('max_duration') + umask = config.get('umask') - borg_exit_codes = config.get('borg_exit_codes') - - full_command = ( - (local_path, 'check') - + (('--repair',) if check_arguments.repair else ()) - + (('--max-duration', str(max_duration)) if max_duration else ()) - + make_check_name_flags(checks, archive_filter_flags) - + (('--remote-path', remote_path) if remote_path else ()) - + (('--umask', str(umask)) if umask else ()) - + (('--log-json',) if global_arguments.log_json else ()) - + (('--lock-wait', str(lock_wait)) if lock_wait else ()) - + verbosity_flags - + (('--progress',) if check_arguments.progress else ()) - + (tuple(extra_borg_options.split(' ')) if extra_borg_options else ()) - + flags.make_repository_flags(repository_path, local_borg_version) - ) - working_directory = borgmatic.config.paths.get_working_directory(config) - # The Borg repair option triggers an interactive prompt, which won't work when output is - # captured. And progress messes with the terminal directly. - if check_arguments.repair or check_arguments.progress: - execute_command( - full_command, - output_file=DO_NOT_CAPTURE, - extra_environment=environment.make_environment(config), - working_directory=working_directory, - borg_local_path=local_path, - borg_exit_codes=borg_exit_codes, + if 'data' in checks: + checks.add('archives') + + grouped_checks = (checks,) + + # If max_duration is set, then archives and repository checks need to be run separately, as Borg + # doesn't support --max-duration along with an archives checks. + if max_duration and 'archives' in checks and 'repository' in checks: + checks.remove('repository') + grouped_checks = (checks, {'repository'}) + + for checks_subset in grouped_checks: + full_command = ( + (local_path, 'check') + + (('--repair',) if check_arguments.repair else ()) + + ( + ('--max-duration', str(max_duration)) + if max_duration and 'repository' in checks_subset + else () + ) + + make_check_name_flags(checks_subset, archive_filter_flags) + + (('--remote-path', remote_path) if remote_path else ()) + + (('--umask', str(umask)) if umask else ()) + + (('--log-json',) if global_arguments.log_json else ()) + + (('--lock-wait', str(lock_wait)) if lock_wait else ()) + + verbosity_flags + + (('--progress',) if check_arguments.progress else ()) + + (tuple(extra_borg_options.split(' ')) if extra_borg_options else ()) + + flags.make_repository_flags(repository_path, local_borg_version) ) - else: + execute_command( full_command, + # The Borg repair option triggers an interactive prompt, which won't work when output is + # captured. And progress messes with the terminal directly. + output_file=( + DO_NOT_CAPTURE if check_arguments.repair or check_arguments.progress else None + ), extra_environment=environment.make_environment(config), working_directory=working_directory, borg_local_path=local_path, diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index c7bce20e..33a6bbe2 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -632,8 +632,8 @@ properties: long-running repository check into multiple partial checks. Defaults to no interruption. Only applies to the "repository" check, does not check - the repository index, and is not compatible with a - simultaneous "archives" check or "--repair" flag. + the repository index and is not compatible with + the "--repair" flag. example: 3600 - required: - name diff --git a/tests/unit/borg/test_check.py b/tests/unit/borg/test_check.py index 1ff43313..3fadc2fd 100644 --- a/tests/unit/borg/test_check.py +++ b/tests/unit/borg/test_check.py @@ -8,13 +8,16 @@ from borgmatic.borg import check as module from ..test_verbosity import insert_logging_mock -def insert_execute_command_mock(command, working_directory=None, borg_exit_codes=None): +def insert_execute_command_mock( + command, output_file=None, working_directory=None, borg_exit_codes=None +): flexmock(module.environment).should_receive('make_environment') flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return( working_directory, ) flexmock(module).should_receive('execute_command').with_args( command, + output_file=output_file, extra_environment=None, working_directory=working_directory, borg_local_path=command[0], @@ -250,11 +253,11 @@ def test_make_check_name_flags_without_archives_check_and_with_archive_filter_fl assert flags == ('--repository-only',) -def test_make_check_name_flags_with_data_check_returns_flag_and_implies_archives(): +def test_make_check_name_flags_with_archives_and_data_check_returns_verify_data_flag(): flexmock(module.feature).should_receive('available').and_return(True) flexmock(module.flags).should_receive('make_match_archives_flags').and_return(()) - flags = module.make_check_name_flags({'data'}, ()) + flags = module.make_check_name_flags({'archives', 'data'}, ()) assert flags == ( '--archives-only', @@ -262,6 +265,15 @@ def test_make_check_name_flags_with_data_check_returns_flag_and_implies_archives ) +def test_make_check_name_flags_with_repository_and_data_check_returns_verify_data_flag(): + flexmock(module.feature).should_receive('available').and_return(True) + flexmock(module.flags).should_receive('make_match_archives_flags').and_return(()) + + flags = module.make_check_name_flags({'archives', 'data', 'repository'}, ()) + + assert flags == ('--verify-data',) + + def test_make_check_name_flags_with_extract_omits_extract_flag(): flexmock(module.feature).should_receive('available').and_return(True) flexmock(module.flags).should_receive('make_match_archives_flags').and_return(()) @@ -271,21 +283,6 @@ def test_make_check_name_flags_with_extract_omits_extract_flag(): assert flags == () -def test_make_check_name_flags_with_repository_and_data_checks_does_not_return_repository_only(): - flexmock(module.feature).should_receive('available').and_return(True) - flexmock(module.flags).should_receive('make_match_archives_flags').and_return(()) - - flags = module.make_check_name_flags( - { - 'repository', - 'data', - }, - (), - ) - - assert flags == ('--verify-data',) - - def test_get_repository_id_with_valid_json_does_not_raise(): config = {} flexmock(module.repo_info).should_receive('display_repository_info').and_return( @@ -336,7 +333,9 @@ def test_get_repository_id_with_missing_json_keys_raises(): def test_check_archives_with_progress_passes_through_to_borg(): config = {} - flexmock(module).should_receive('make_check_name_flags').and_return(()) + flexmock(module).should_receive('make_check_name_flags').with_args( + {'repository'}, () + ).and_return(()) 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) @@ -369,7 +368,9 @@ def test_check_archives_with_progress_passes_through_to_borg(): def test_check_archives_with_repair_passes_through_to_borg(): config = {} - flexmock(module).should_receive('make_check_name_flags').and_return(()) + flexmock(module).should_receive('make_check_name_flags').with_args( + {'repository'}, () + ).and_return(()) 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) @@ -402,12 +403,15 @@ def test_check_archives_with_repair_passes_through_to_borg(): def test_check_archives_with_max_duration_flag_passes_through_to_borg(): config = {} - flexmock(module).should_receive('make_check_name_flags').and_return(()) + flexmock(module).should_receive('make_check_name_flags').with_args( + {'repository'}, () + ).and_return(()) 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').with_args( ('borg', 'check', '--max-duration', '33', 'repo'), + output_file=None, extra_environment=None, working_directory=None, borg_local_path='borg', @@ -432,37 +436,17 @@ def test_check_archives_with_max_duration_flag_passes_through_to_borg(): ) -def test_check_archives_with_max_duration_flag_and_archives_check_errors(): - config = {} - flexmock(module).should_receive('execute_command').never() - - with pytest.raises(ValueError): - module.check_archives( - repository_path='repo', - config=config, - local_borg_version='1.2.3', - check_arguments=flexmock( - progress=None, - repair=None, - only_checks=None, - force=None, - match_archives=None, - max_duration=33, - ), - global_arguments=flexmock(log_json=False), - checks={'repository', 'archives'}, - archive_filter_flags=(), - ) - - def test_check_archives_with_max_duration_option_passes_through_to_borg(): config = {'checks': [{'name': 'repository', 'max_duration': 33}]} - flexmock(module).should_receive('make_check_name_flags').and_return(()) + flexmock(module).should_receive('make_check_name_flags').with_args( + {'repository'}, () + ).and_return(()) 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').with_args( ('borg', 'check', '--max-duration', '33', 'repo'), + output_file=None, extra_environment=None, working_directory=None, borg_local_path='borg', @@ -487,37 +471,145 @@ def test_check_archives_with_max_duration_option_passes_through_to_borg(): ) -def test_check_archives_with_max_duration_option_and_archives_check_errors(): - config = {'checks': [{'name': 'repository', 'max_duration': 33}]} - flexmock(module).should_receive('execute_command').never() +def test_check_archives_with_max_duration_option_and_archives_check_runs_repository_check_separately(): + config = {'checks': [{'name': 'repository', 'max_duration': 33}, {'name': 'archives'}]} + flexmock(module).should_receive('make_check_name_flags').with_args({'archives'}, ()).and_return( + ('--archives-only',) + ) + flexmock(module).should_receive('make_check_name_flags').with_args( + {'repository'}, () + ).and_return(('--repository-only',)) + flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) + insert_execute_command_mock(('borg', 'check', '--archives-only', 'repo')) + insert_execute_command_mock( + ('borg', 'check', '--max-duration', '33', '--repository-only', 'repo') + ) - with pytest.raises(ValueError): - module.check_archives( - repository_path='repo', - config=config, - local_borg_version='1.2.3', - check_arguments=flexmock( - progress=None, - repair=None, - only_checks=None, - force=None, - match_archives=None, - max_duration=None, - ), - global_arguments=flexmock(log_json=False), - checks={'repository', 'archives'}, - archive_filter_flags=(), - ) + module.check_archives( + repository_path='repo', + config=config, + local_borg_version='1.2.3', + check_arguments=flexmock( + progress=None, + repair=None, + only_checks=None, + force=None, + match_archives=None, + max_duration=None, + ), + global_arguments=flexmock(log_json=False), + checks={'repository', 'archives'}, + archive_filter_flags=(), + ) + + +def test_check_archives_with_max_duration_flag_and_archives_check_runs_repository_check_separately(): + config = {'checks': [{'name': 'repository'}, {'name': 'archives'}]} + flexmock(module).should_receive('make_check_name_flags').with_args({'archives'}, ()).and_return( + ('--archives-only',) + ) + flexmock(module).should_receive('make_check_name_flags').with_args( + {'repository'}, () + ).and_return(('--repository-only',)) + flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) + insert_execute_command_mock(('borg', 'check', '--archives-only', 'repo')) + insert_execute_command_mock( + ('borg', 'check', '--max-duration', '33', '--repository-only', 'repo') + ) + + module.check_archives( + repository_path='repo', + config=config, + local_borg_version='1.2.3', + check_arguments=flexmock( + progress=None, + repair=None, + only_checks=None, + force=None, + match_archives=None, + max_duration=33, + ), + global_arguments=flexmock(log_json=False), + checks={'repository', 'archives'}, + archive_filter_flags=(), + ) + + +def test_check_archives_with_max_duration_option_and_data_check_runs_repository_check_separately(): + config = {'checks': [{'name': 'repository', 'max_duration': 33}, {'name': 'data'}]} + flexmock(module).should_receive('make_check_name_flags').with_args( + {'data', 'archives'}, () + ).and_return(('--archives-only', '--verify-data')) + flexmock(module).should_receive('make_check_name_flags').with_args( + {'repository'}, () + ).and_return(('--repository-only',)) + flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) + insert_execute_command_mock(('borg', 'check', '--archives-only', '--verify-data', 'repo')) + insert_execute_command_mock( + ('borg', 'check', '--max-duration', '33', '--repository-only', 'repo') + ) + + module.check_archives( + repository_path='repo', + config=config, + local_borg_version='1.2.3', + check_arguments=flexmock( + progress=None, + repair=None, + only_checks=None, + force=None, + match_archives=None, + max_duration=None, + ), + global_arguments=flexmock(log_json=False), + checks={'repository', 'data'}, + archive_filter_flags=(), + ) + + +def test_check_archives_with_max_duration_flag_and_data_check_runs_repository_check_separately(): + config = {'checks': [{'name': 'repository'}, {'name': 'data'}]} + flexmock(module).should_receive('make_check_name_flags').with_args( + {'data', 'archives'}, () + ).and_return(('--archives-only', '--verify-data')) + flexmock(module).should_receive('make_check_name_flags').with_args( + {'repository'}, () + ).and_return(('--repository-only',)) + flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) + insert_execute_command_mock(('borg', 'check', '--archives-only', '--verify-data', 'repo')) + insert_execute_command_mock( + ('borg', 'check', '--max-duration', '33', '--repository-only', 'repo') + ) + + module.check_archives( + repository_path='repo', + config=config, + local_borg_version='1.2.3', + check_arguments=flexmock( + progress=None, + repair=None, + only_checks=None, + force=None, + match_archives=None, + max_duration=33, + ), + global_arguments=flexmock(log_json=False), + checks={'repository', 'data'}, + archive_filter_flags=(), + ) def test_check_archives_with_max_duration_flag_overrides_max_duration_option(): config = {'checks': [{'name': 'repository', 'max_duration': 33}]} - flexmock(module).should_receive('make_check_name_flags').and_return(()) + flexmock(module).should_receive('make_check_name_flags').with_args( + {'repository'}, () + ).and_return(()) 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').with_args( ('borg', 'check', '--max-duration', '44', 'repo'), + output_file=None, extra_environment=None, working_directory=None, borg_local_path='borg', @@ -575,9 +667,37 @@ def test_check_archives_calls_borg_with_parameters(checks): ) +def test_check_archives_with_data_check_implies_archives_check_calls_borg_with_parameters(): + config = {} + flexmock(module).should_receive('make_check_name_flags').with_args( + {'data', 'archives'}, () + ).and_return(()) + flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) + insert_execute_command_mock(('borg', 'check', 'repo')) + + module.check_archives( + repository_path='repo', + config=config, + local_borg_version='1.2.3', + check_arguments=flexmock( + progress=None, + repair=None, + only_checks=None, + force=None, + match_archives=None, + max_duration=None, + ), + global_arguments=flexmock(log_json=False), + checks={'data'}, + archive_filter_flags=(), + ) + + def test_check_archives_with_log_info_passes_through_to_borg(): config = {} - flexmock(module).should_receive('make_check_name_flags').and_return(()) + flexmock(module).should_receive('make_check_name_flags').with_args( + {'repository'}, () + ).and_return(()) flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) insert_logging_mock(logging.INFO) insert_execute_command_mock(('borg', 'check', '--info', 'repo')) @@ -602,7 +722,9 @@ def test_check_archives_with_log_info_passes_through_to_borg(): def test_check_archives_with_log_debug_passes_through_to_borg(): config = {} - flexmock(module).should_receive('make_check_name_flags').and_return(()) + flexmock(module).should_receive('make_check_name_flags').with_args( + {'repository'}, () + ).and_return(()) flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) insert_logging_mock(logging.DEBUG) insert_execute_command_mock(('borg', 'check', '--debug', '--show-rc', 'repo')) @@ -806,7 +928,9 @@ def test_check_archives_with_retention_prefix(): def test_check_archives_with_extra_borg_options_passes_through_to_borg(): config = {'extra_borg_options': {'check': '--extra --options'}} - flexmock(module).should_receive('make_check_name_flags').and_return(()) + flexmock(module).should_receive('make_check_name_flags').with_args( + {'repository'}, () + ).and_return(()) flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) insert_execute_command_mock(('borg', 'check', '--extra', '--options', 'repo')) @@ -829,15 +953,16 @@ def test_check_archives_with_extra_borg_options_passes_through_to_borg(): def test_check_archives_with_match_archives_passes_through_to_borg(): - config = {} - flexmock(module).should_receive('make_check_name_flags').and_return( - ('--match-archives', 'foo-*') - ) + config = {'checks': [{'name': 'archives'}]} + flexmock(module).should_receive('make_check_name_flags').with_args( + {'archives'}, object + ).and_return(('--match-archives', 'foo-*')) 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').with_args( ('borg', 'check', '--match-archives', 'foo-*', 'repo'), + output_file=None, extra_environment=None, working_directory=None, borg_local_path='borg', @@ -864,7 +989,9 @@ def test_check_archives_with_match_archives_passes_through_to_borg(): def test_check_archives_calls_borg_with_working_directory(): config = {'working_directory': '/working/dir'} - flexmock(module).should_receive('make_check_name_flags').and_return(()) + flexmock(module).should_receive('make_check_name_flags').with_args( + {'repository'}, () + ).and_return(()) 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) From 5a2cd1b2619291137f5a6c141eb539c99c95f726 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 6 Feb 2025 14:21:36 -0800 Subject: [PATCH 8/8] Add support for Python 3.13. --- NEWS | 1 + scripts/run-full-tests | 3 +-- test_requirements.txt | 1 - tox.ini | 2 +- 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index 4d25f9dc..0b96a621 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,7 @@ refused to run checks in this situation. * #989: Fix the log message code to avoid using Python 3.10+ logging features. Now borgmatic will work with Python 3.9 again. + * Add support for Python 3.13. 1.9.9 * #635: Log the repository path or label on every relevant log message, not just some logs. diff --git a/scripts/run-full-tests b/scripts/run-full-tests index 8c5be55a..0bc18406 100755 --- a/scripts/run-full-tests +++ b/scripts/run-full-tests @@ -18,8 +18,7 @@ if [ -z "$TEST_CONTAINER" ]; then fi apk add --no-cache python3 py3-pip borgbackup postgresql17-client mariadb-client mongodb-tools \ - py3-mongo py3-regex py3-ruamel.yaml py3-ruamel.yaml.clib py3-tox py3-typed-ast py3-yaml bash \ - sqlite fish + py3-mongo py3-regex py3-ruamel.yaml py3-ruamel.yaml.clib py3-tox py3-yaml bash sqlite fish export COVERAGE_FILE=/tmp/.coverage tox --workdir /tmp/.tox --sitepackages diff --git a/test_requirements.txt b/test_requirements.txt index 1e3e8ef0..b80ffc4e 100644 --- a/test_requirements.txt +++ b/test_requirements.txt @@ -30,4 +30,3 @@ regex requests==2.32.2 ruamel.yaml>0.15.0 toml==0.10.2 -typed-ast diff --git a/tox.ini b/tox.ini index b2f0d484..6e3f4f05 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -env_list = py39,py310,py311,py312 +env_list = py39,py310,py311,py312,py313 skip_missing_interpreters = True package = editable min_version = 4.0