From 1bc7bb49714a33ac9613a2da10cc7951ccace276 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Mon, 15 May 2023 23:04:42 +0530 Subject: [PATCH 01/25] feat: store configs used to create an archive in the archive --- borgmatic/borg/create.py | 2 +- borgmatic/commands/borgmatic.py | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/borgmatic/borg/create.py b/borgmatic/borg/create.py index e3b70eb..284789d 100644 --- a/borgmatic/borg/create.py +++ b/borgmatic/borg/create.py @@ -351,7 +351,7 @@ def create_archive( sources = deduplicate_directories( map_directories_to_devices( expand_directories( - tuple(location_config.get('source_directories', ())) + borgmatic_source_directories + tuple(location_config.get('source_directories', ())) + borgmatic_source_directories + tuple(global_arguments.config_paths) ) ), additional_directory_devices=map_directories_to_devices( diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 44396cd..3afa625 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -45,10 +45,10 @@ logger = logging.getLogger(__name__) LEGACY_CONFIG_PATH = '/etc/borgmatic/config' -def run_configuration(config_filename, config, arguments): +def run_configuration(config_filename, config, arguments, used_config_paths): ''' - Given a config filename, the corresponding parsed config dict, and command-line arguments as a - dict from subparser name to a namespace of parsed arguments, execute the defined create, prune, + Given a config filename, the corresponding parsed config dict, command-line arguments as a + dict from subparser name to a namespace of parsed arguments, and a list of paths of all configs used, execute the defined create, prune, compact, check, and/or other actions. Yield a combination of: @@ -61,6 +61,7 @@ def run_configuration(config_filename, config, arguments): for section_name in ('location', 'storage', 'retention', 'consistency', 'hooks') ) global_arguments = arguments['global'] + global_arguments.config_paths = used_config_paths local_path = location.get('local_path', 'borg') remote_path = location.get('remote_path') @@ -644,8 +645,9 @@ def collect_configuration_run_summary_logs(configs, arguments): # Execute the actions corresponding to each configuration file. json_results = [] + used_config_paths = list(configs.keys()) for config_filename, config in configs.items(): - results = list(run_configuration(config_filename, config, arguments)) + results = list(run_configuration(config_filename, config, arguments, used_config_paths)) error_logs = tuple(result for result in results if isinstance(result, logging.LogRecord)) if error_logs: From 49b4d371cea63afa872e7e81f4e95666567ae179 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Tue, 16 May 2023 00:24:19 +0530 Subject: [PATCH 02/25] create and add content to borgmatic-manifest.json --- borgmatic/actions/create.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/borgmatic/actions/create.py b/borgmatic/actions/create.py index a3f8da5..10a35ca 100644 --- a/borgmatic/actions/create.py +++ b/borgmatic/actions/create.py @@ -1,4 +1,5 @@ import json +import os import logging import borgmatic.borg.create @@ -7,9 +8,30 @@ import borgmatic.hooks.command import borgmatic.hooks.dispatch import borgmatic.hooks.dump +from borgmatic.borg.state import DEFAULT_BORGMATIC_SOURCE_DIRECTORY + logger = logging.getLogger(__name__) +def create_borgmatic_manifest(location, config_paths, dry_run): + ''' + Create a borgmatic manifest file to store the paths to the configuration files used to create + the archive. + ''' + if dry_run: + return + + borgmatic_source_directory = location.get('borgmatic_source_directory') if location.get('borgmatic_source_directory') else DEFAULT_BORGMATIC_SOURCE_DIRECTORY + + borgmatic_manifest_path = os.path.expanduser(os.path.join(borgmatic_source_directory, 'bootstrap', 'configs-list.json')) + + if not os.path.exists(borgmatic_manifest_path): + os.makedirs(os.path.dirname(borgmatic_manifest_path), exist_ok=True) + + with open(borgmatic_manifest_path, 'w') as f: + json.dump(config_paths, f) + + def run_create( config_filename, repository, @@ -59,6 +81,7 @@ def run_create( location, global_arguments.dry_run, ) + create_borgmatic_manifest(location, global_arguments.config_paths, global_arguments.dry_run) stream_processes = [process for processes in active_dumps.values() for process in processes] json_output = borgmatic.borg.create.create_archive( From b10148844bc0214f07e0e9357c22d82c8262790d Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Tue, 16 May 2023 14:00:23 +0530 Subject: [PATCH 03/25] change config_paths var name to used_config_paths to avoid collisions --- borgmatic/actions/create.py | 28 ++++++++++++++++++++++------ borgmatic/borg/create.py | 4 +++- borgmatic/commands/borgmatic.py | 11 +++++------ 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/borgmatic/actions/create.py b/borgmatic/actions/create.py index 10a35ca..2dba94b 100644 --- a/borgmatic/actions/create.py +++ b/borgmatic/actions/create.py @@ -2,6 +2,8 @@ import json import os import logging +import importlib_metadata + import borgmatic.borg.create import borgmatic.config.validate import borgmatic.hooks.command @@ -17,19 +19,31 @@ def create_borgmatic_manifest(location, config_paths, dry_run): ''' Create a borgmatic manifest file to store the paths to the configuration files used to create the archive. - ''' + ''' if dry_run: return - - borgmatic_source_directory = location.get('borgmatic_source_directory') if location.get('borgmatic_source_directory') else DEFAULT_BORGMATIC_SOURCE_DIRECTORY - borgmatic_manifest_path = os.path.expanduser(os.path.join(borgmatic_source_directory, 'bootstrap', 'configs-list.json')) + borgmatic_source_directory = ( + location.get('borgmatic_source_directory') + if location.get('borgmatic_source_directory') + else DEFAULT_BORGMATIC_SOURCE_DIRECTORY + ) + + borgmatic_manifest_path = os.path.expanduser( + os.path.join(borgmatic_source_directory, 'bootstrap', 'configs-list.json') + ) if not os.path.exists(borgmatic_manifest_path): os.makedirs(os.path.dirname(borgmatic_manifest_path), exist_ok=True) with open(borgmatic_manifest_path, 'w') as f: - json.dump(config_paths, f) + json.dump( + { + 'borgmatic_version': importlib_metadata.version('borgmatic'), + 'config_paths': config_paths, + }, + f, + ) def run_create( @@ -81,7 +95,9 @@ def run_create( location, global_arguments.dry_run, ) - create_borgmatic_manifest(location, global_arguments.config_paths, global_arguments.dry_run) + create_borgmatic_manifest( + location, global_arguments.used_config_paths, global_arguments.dry_run + ) stream_processes = [process for processes in active_dumps.values() for process in processes] json_output = borgmatic.borg.create.create_archive( diff --git a/borgmatic/borg/create.py b/borgmatic/borg/create.py index 284789d..618376a 100644 --- a/borgmatic/borg/create.py +++ b/borgmatic/borg/create.py @@ -351,7 +351,9 @@ def create_archive( sources = deduplicate_directories( map_directories_to_devices( expand_directories( - tuple(location_config.get('source_directories', ())) + borgmatic_source_directories + tuple(global_arguments.config_paths) + tuple(location_config.get('source_directories', ())) + + borgmatic_source_directories + + tuple(global_arguments.used_config_paths) ) ), additional_directory_devices=map_directories_to_devices( diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 3afa625..1eb6834 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -45,10 +45,10 @@ logger = logging.getLogger(__name__) LEGACY_CONFIG_PATH = '/etc/borgmatic/config' -def run_configuration(config_filename, config, arguments, used_config_paths): +def run_configuration(config_filename, config, arguments): ''' - Given a config filename, the corresponding parsed config dict, command-line arguments as a - dict from subparser name to a namespace of parsed arguments, and a list of paths of all configs used, execute the defined create, prune, + Given a config filename, the corresponding parsed config dict, and command-line arguments as a + dict from subparser name to a namespace of parsed arguments, execute the defined create, prune, compact, check, and/or other actions. Yield a combination of: @@ -61,7 +61,6 @@ def run_configuration(config_filename, config, arguments, used_config_paths): for section_name in ('location', 'storage', 'retention', 'consistency', 'hooks') ) global_arguments = arguments['global'] - global_arguments.config_paths = used_config_paths local_path = location.get('local_path', 'borg') remote_path = location.get('remote_path') @@ -645,9 +644,8 @@ def collect_configuration_run_summary_logs(configs, arguments): # Execute the actions corresponding to each configuration file. json_results = [] - used_config_paths = list(configs.keys()) for config_filename, config in configs.items(): - results = list(run_configuration(config_filename, config, arguments, used_config_paths)) + results = list(run_configuration(config_filename, config, arguments)) error_logs = tuple(result for result in results if isinstance(result, logging.LogRecord)) if error_logs: @@ -729,6 +727,7 @@ def main(): # pragma: no cover sys.exit(0) config_filenames = tuple(collect.collect_config_filenames(global_arguments.config_paths)) + global_arguments.used_config_paths = list(config_filenames) configs, parse_logs = load_configurations( config_filenames, global_arguments.overrides, global_arguments.resolve_env ) From ee32b076eb666cefcee652cf8572f39b386649b8 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Tue, 16 May 2023 23:17:35 +0530 Subject: [PATCH 04/25] update tests and formatting --- borgmatic/actions/create.py | 18 +++--- tests/unit/actions/test_create.py | 6 +- tests/unit/borg/test_create.py | 94 +++++++++++++++---------------- 3 files changed, 59 insertions(+), 59 deletions(-) diff --git a/borgmatic/actions/create.py b/borgmatic/actions/create.py index 2dba94b..c5a6513 100644 --- a/borgmatic/actions/create.py +++ b/borgmatic/actions/create.py @@ -1,15 +1,17 @@ import json -import os import logging +import os -import importlib_metadata +try: + import importlib_metadata +except ModuleNotFoundError: # pragma: nocover + import importlib.metadata as importlib_metadata import borgmatic.borg.create import borgmatic.config.validate import borgmatic.hooks.command import borgmatic.hooks.dispatch import borgmatic.hooks.dump - from borgmatic.borg.state import DEFAULT_BORGMATIC_SOURCE_DIRECTORY logger = logging.getLogger(__name__) @@ -23,10 +25,8 @@ def create_borgmatic_manifest(location, config_paths, dry_run): if dry_run: return - borgmatic_source_directory = ( - location.get('borgmatic_source_directory') - if location.get('borgmatic_source_directory') - else DEFAULT_BORGMATIC_SOURCE_DIRECTORY + borgmatic_source_directory = location.get( + 'borgmatic_source_directory', DEFAULT_BORGMATIC_SOURCE_DIRECTORY ) borgmatic_manifest_path = os.path.expanduser( @@ -36,13 +36,13 @@ def create_borgmatic_manifest(location, config_paths, dry_run): if not os.path.exists(borgmatic_manifest_path): os.makedirs(os.path.dirname(borgmatic_manifest_path), exist_ok=True) - with open(borgmatic_manifest_path, 'w') as f: + with open(borgmatic_manifest_path, 'w') as config_list_file: json.dump( { 'borgmatic_version': importlib_metadata.version('borgmatic'), 'config_paths': config_paths, }, - f, + config_list_file, ) diff --git a/tests/unit/actions/test_create.py b/tests/unit/actions/test_create.py index 2b72408..f6d9acb 100644 --- a/tests/unit/actions/test_create.py +++ b/tests/unit/actions/test_create.py @@ -19,7 +19,7 @@ def test_run_create_executes_and_calls_hooks_for_configured_repository(): json=flexmock(), list_files=flexmock(), ) - global_arguments = flexmock(monitoring_verbosity=1, dry_run=False) + global_arguments = flexmock(monitoring_verbosity=1, dry_run=False, used_config_paths=[]) list( module.run_create( @@ -52,7 +52,7 @@ def test_run_create_runs_with_selected_repository(): json=flexmock(), list_files=flexmock(), ) - global_arguments = flexmock(monitoring_verbosity=1, dry_run=False) + global_arguments = flexmock(monitoring_verbosity=1, dry_run=False, used_config_paths=[]) list( module.run_create( @@ -85,7 +85,7 @@ def test_run_create_bails_if_repository_does_not_match(): json=flexmock(), list_files=flexmock(), ) - global_arguments = flexmock(monitoring_verbosity=1, dry_run=False) + global_arguments = flexmock(monitoring_verbosity=1, dry_run=False, used_config_paths=[]) list( module.run_create( diff --git a/tests/unit/borg/test_create.py b/tests/unit/borg/test_create.py index e0462e2..efabd0f 100644 --- a/tests/unit/borg/test_create.py +++ b/tests/unit/borg/test_create.py @@ -492,7 +492,7 @@ def test_create_archive_calls_borg_with_parameters(): }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -536,7 +536,7 @@ def test_create_archive_calls_borg_with_environment(): }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -582,7 +582,7 @@ def test_create_archive_with_patterns_calls_borg_with_patterns_including_convert }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -628,7 +628,7 @@ def test_create_archive_with_exclude_patterns_calls_borg_with_excludes(): }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -672,7 +672,7 @@ def test_create_archive_with_log_info_calls_borg_with_info_parameter(): }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -713,7 +713,7 @@ def test_create_archive_with_log_info_and_json_suppresses_most_borg_output(): }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), json=True, ) @@ -758,7 +758,7 @@ def test_create_archive_with_log_debug_calls_borg_with_debug_parameter(): }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -799,7 +799,7 @@ def test_create_archive_with_log_debug_and_json_suppresses_most_borg_output(): }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), json=True, ) @@ -843,7 +843,7 @@ def test_create_archive_with_dry_run_calls_borg_with_dry_run_parameter(): }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -889,7 +889,7 @@ def test_create_archive_with_stats_and_dry_run_calls_borg_without_stats_paramete }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), stats=True, ) @@ -933,7 +933,7 @@ def test_create_archive_with_checkpoint_interval_calls_borg_with_checkpoint_inte }, storage_config={'checkpoint_interval': 600}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -976,7 +976,7 @@ def test_create_archive_with_checkpoint_volume_calls_borg_with_checkpoint_volume }, storage_config={'checkpoint_volume': 1024}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -1019,7 +1019,7 @@ def test_create_archive_with_chunker_params_calls_borg_with_chunker_params_param }, storage_config={'chunker_params': '1,2,3,4'}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -1062,7 +1062,7 @@ def test_create_archive_with_compression_calls_borg_with_compression_parameters( }, storage_config={'compression': 'rle'}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -1111,7 +1111,7 @@ def test_create_archive_with_upload_rate_limit_calls_borg_with_upload_ratelimit_ }, storage_config={'upload_rate_limit': 100}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -1157,7 +1157,7 @@ def test_create_archive_with_working_directory_calls_borg_with_working_directory }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -1201,7 +1201,7 @@ def test_create_archive_with_one_file_system_calls_borg_with_one_file_system_par }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -1251,7 +1251,7 @@ def test_create_archive_with_numeric_ids_calls_borg_with_numeric_ids_parameter( }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -1305,7 +1305,7 @@ def test_create_archive_with_read_special_calls_borg_with_read_special_parameter }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -1361,7 +1361,7 @@ def test_create_archive_with_basic_option_calls_borg_with_corresponding_paramete }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -1416,7 +1416,7 @@ def test_create_archive_with_atime_option_calls_borg_with_corresponding_paramete }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -1471,7 +1471,7 @@ def test_create_archive_with_flags_option_calls_borg_with_corresponding_paramete }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -1515,7 +1515,7 @@ def test_create_archive_with_files_cache_calls_borg_with_files_cache_parameters( }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -1558,7 +1558,7 @@ def test_create_archive_with_local_path_calls_borg_via_local_path(): }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), local_path='borg1', ) @@ -1602,7 +1602,7 @@ def test_create_archive_with_remote_path_calls_borg_with_remote_path_parameters( }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), remote_path='borg1', ) @@ -1646,7 +1646,7 @@ def test_create_archive_with_umask_calls_borg_with_umask_parameters(): }, storage_config={'umask': 740}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -1689,7 +1689,7 @@ def test_create_archive_with_log_json_calls_borg_with_log_json_parameters(): }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=True), + global_arguments=flexmock(log_json=True, used_config_paths=[]), ) @@ -1732,7 +1732,7 @@ def test_create_archive_with_lock_wait_calls_borg_with_lock_wait_parameters(): }, storage_config={'lock_wait': 5}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -1775,7 +1775,7 @@ def test_create_archive_with_stats_calls_borg_with_stats_parameter_and_answer_ou }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), stats=True, ) @@ -1819,7 +1819,7 @@ def test_create_archive_with_files_calls_borg_with_list_parameter_and_answer_out }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), list_files=True, ) @@ -1869,7 +1869,7 @@ def test_create_archive_with_progress_and_log_info_calls_borg_with_progress_para }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), progress=True, ) @@ -1913,7 +1913,7 @@ def test_create_archive_with_progress_calls_borg_with_progress_parameter(): }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), progress=True, ) @@ -1974,7 +1974,7 @@ def test_create_archive_with_progress_and_stream_processes_calls_borg_with_progr }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), progress=True, stream_processes=processes, ) @@ -2039,7 +2039,7 @@ def test_create_archive_with_stream_processes_ignores_read_special_false_and_log }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), stream_processes=processes, ) @@ -2107,7 +2107,7 @@ def test_create_archive_with_stream_processes_adds_special_files_to_excludes(): }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), stream_processes=processes, ) @@ -2172,7 +2172,7 @@ def test_create_archive_with_stream_processes_and_read_special_does_not_add_spec }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), stream_processes=processes, ) @@ -2213,7 +2213,7 @@ def test_create_archive_with_json_calls_borg_with_json_parameter(): }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), json=True, ) @@ -2256,7 +2256,7 @@ def test_create_archive_with_stats_and_json_calls_borg_without_stats_parameter() }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), json=True, stats=True, ) @@ -2304,7 +2304,7 @@ def test_create_archive_with_source_directories_glob_expands(): }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -2348,7 +2348,7 @@ def test_create_archive_with_non_matching_source_directories_glob_passes_through }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -2391,7 +2391,7 @@ def test_create_archive_with_glob_calls_borg_with_expanded_directories(): }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -2434,7 +2434,7 @@ def test_create_archive_with_archive_name_format_calls_borg_with_archive_name(): }, storage_config={'archive_name_format': 'ARCHIVE_NAME'}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -2478,7 +2478,7 @@ def test_create_archive_with_archive_name_format_accepts_borg_placeholders(): }, storage_config={'archive_name_format': 'Documents_{hostname}-{now}'}, # noqa: FS003 local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -2522,7 +2522,7 @@ def test_create_archive_with_repository_accepts_borg_placeholders(): }, storage_config={'archive_name_format': 'Documents_{hostname}-{now}'}, # noqa: FS003 local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -2565,7 +2565,7 @@ def test_create_archive_with_extra_borg_options_calls_borg_with_extra_options(): }, storage_config={'extra_borg_options': {'create': '--extra --options'}}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) @@ -2626,7 +2626,7 @@ def test_create_archive_with_stream_processes_calls_borg_with_processes_and_read }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), stream_processes=processes, ) @@ -2652,7 +2652,7 @@ def test_create_archive_with_non_existent_directory_and_source_directories_must_ }, storage_config={}, local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), + global_arguments=flexmock(log_json=False, used_config_paths=[]), ) From 2241de11c00c8f4dc138ce89750fb5470912301d Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Fri, 26 May 2023 00:26:13 +0530 Subject: [PATCH 05/25] start work on borgmatic config bootstrap command --- borgmatic/commands/arguments.py | 62 +++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index 0812ede..832c62d 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -9,6 +9,8 @@ SUBPARSER_ALIASES = { 'compact': [], 'create': ['-C'], 'check': ['-k'], + 'config': [], + 'config_bootstrap': [], 'extract': ['-x'], 'export-tar': [], 'mount': ['-m'], @@ -523,6 +525,66 @@ def make_parsers(): '-h', '--help', action='help', help='Show this help message and exit' ) + config_parser = subparsers.add_parser( + 'config', + aliases=SUBPARSER_ALIASES['config'], + help='Perform configuration file related operations', + description='Perform configuration file related operations', + add_help=False, + parents=[top_level_parser], + ) + + config_subparsers = config_parser.add_subparsers( + title='config subcommands', + description='Valid subcommands for config', + help='Additional help', + ) + + config_bootstrap_parser = config_subparsers.add_parser( + 'bootstrap', + aliases=SUBPARSER_ALIASES['config_bootstrap'], + help='Extract files from a borgmatic created repository to the current directory', + description='Extract a named archive from a borgmatic created repository to the current directory without a configuration file', + add_help=False, + parents=[config_parser], + ) + config_bootstrap_group = config_bootstrap_parser.add_argument_group('config bootstrap arguments') + config_bootstrap_group.add_argument( + '--repository', + help='Path of repository to extract', + required=True, + ) + config_bootstrap_group.add_argument( + '--archive', help='Name of archive to extract, defaults to "latest"' + ) + config_bootstrap_group.add_argument( + '--path', + '--restore-path', + metavar='PATH', + nargs='+', + dest='paths', + help='Paths to extract from archive, defaults to the entire archive', + ) + config_bootstrap_group.add_argument( + '--destination', + metavar='PATH', + dest='destination', + help='Directory to extract files into, defaults to the current directory', + ) + config_bootstrap_group.add_argument( + '--strip-components', + type=lambda number: number if number == 'all' else int(number), + metavar='NUMBER', + help='Number of leading path components to remove from each extracted path or "all" to strip all leading path components. Skip paths with fewer elements', + ) + config_bootstrap_group.add_argument( + '--progress', + dest='progress', + default=False, + action='store_true', + help='Display progress for each file as it is extracted', + ) + export_tar_parser = subparsers.add_parser( 'export-tar', aliases=SUBPARSER_ALIASES['export-tar'], From 8b7996dfda6656a0324e1d920d85a6c7922c47ec Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Fri, 26 May 2023 01:07:11 +0530 Subject: [PATCH 06/25] removed parents and used reversed remaining_args --- borgmatic/commands/arguments.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index 832c62d..4f9ca96 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -48,7 +48,7 @@ def parse_subparser_arguments(unparsed_arguments, subparsers): if 'borg' in unparsed_arguments: subparsers = {'borg': subparsers['borg']} - for argument in remaining_arguments: + for argument in reversed(remaining_arguments): canonical_name = alias_to_subparser_name.get(argument, argument) subparser = subparsers.get(canonical_name) @@ -531,7 +531,11 @@ def make_parsers(): help='Perform configuration file related operations', description='Perform configuration file related operations', add_help=False, - parents=[top_level_parser], + ) + + config_group = config_parser.add_argument_group('config arguments') + config_group.add_argument( + '-h', '--help', action='help', help='Show this help message and exit' ) config_subparsers = config_parser.add_subparsers( @@ -546,7 +550,6 @@ def make_parsers(): help='Extract files from a borgmatic created repository to the current directory', description='Extract a named archive from a borgmatic created repository to the current directory without a configuration file', add_help=False, - parents=[config_parser], ) config_bootstrap_group = config_bootstrap_parser.add_argument_group('config bootstrap arguments') config_bootstrap_group.add_argument( @@ -584,6 +587,9 @@ def make_parsers(): action='store_true', help='Display progress for each file as it is extracted', ) + config_bootstrap_group.add_argument( + '-h', '--help', action='help', help='Show this help message and exit' + ) export_tar_parser = subparsers.add_parser( 'export-tar', From 96adee444bd26184ee993c1b3f5474ccbf857dea Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 25 May 2023 15:03:15 -0700 Subject: [PATCH 07/25] Potential fix for nested subparsers not parsing correctly. --- borgmatic/commands/arguments.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index 4f9ca96..e0ffb47 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -48,7 +48,7 @@ def parse_subparser_arguments(unparsed_arguments, subparsers): if 'borg' in unparsed_arguments: subparsers = {'borg': subparsers['borg']} - for argument in reversed(remaining_arguments): + for argument in remaining_arguments: canonical_name = alias_to_subparser_name.get(argument, argument) subparser = subparsers.get(canonical_name) @@ -58,7 +58,9 @@ def parse_subparser_arguments(unparsed_arguments, subparsers): # If a parsed value happens to be the same as the name of a subparser, remove it from the # remaining arguments. This prevents, for instance, "check --only extract" from triggering # the "extract" subparser. - parsed, unused_remaining = subparser.parse_known_args(unparsed_arguments) + parsed, unused_remaining = subparser.parse_known_args( + [argument for argument in unparsed_arguments if argument != canonical_name] + ) for value in vars(parsed).values(): if isinstance(value, str): if value in subparsers: @@ -85,7 +87,9 @@ def parse_subparser_arguments(unparsed_arguments, subparsers): continue subparser = subparsers[subparser_name] - unused_parsed, remaining_arguments = subparser.parse_known_args(remaining_arguments) + unused_parsed, remaining_arguments = subparser.parse_known_args( + [argument for argument in remaining_arguments if argument != subparser_name] + ) # Special case: If "borg" is present in the arguments, consume all arguments after (+1) the # "borg" action. From dbb778a4d6ed472b6a39d861f43e6a61502c799d Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Fri, 26 May 2023 22:44:31 +0530 Subject: [PATCH 08/25] finish parsing and add error for empty config subcommand --- borgmatic/commands/arguments.py | 30 ++++++++++++++++++++++++------ borgmatic/commands/borgmatic.py | 3 ++- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index e0ffb47..5678da1 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -42,6 +42,9 @@ def parse_subparser_arguments(unparsed_arguments, subparsers): for subparser_name, aliases in SUBPARSER_ALIASES.items() for alias in aliases } + subcommand_parsers_mapping = { + 'config': ['bootstrap'], + } # If the "borg" action is used, skip all other subparsers. This avoids confusion like # "borg list" triggering borgmatic's own list action. @@ -70,7 +73,18 @@ def parse_subparser_arguments(unparsed_arguments, subparsers): if item in subparsers: remaining_arguments.remove(item) - arguments[canonical_name] = parsed + if canonical_name not in subcommand_parsers_mapping: + arguments[canonical_name] = parsed + else: + arguments[canonical_name] = None + + for argument in arguments: + if arguments[argument] == None: + for subcommand in subcommand_parsers_mapping[argument]: + if subcommand not in arguments: + raise ValueError("Missing subcommand for {}. Expected one of {}".format( + argument, subcommand_parsers_mapping[argument] + )) # If no actions are explicitly requested, assume defaults. if not arguments and '--help' not in unparsed_arguments and '-h' not in unparsed_arguments: @@ -81,8 +95,9 @@ def parse_subparser_arguments(unparsed_arguments, subparsers): remaining_arguments = list(unparsed_arguments) - # Now ask each subparser, one by one, to greedily consume arguments. - for subparser_name, subparser in subparsers.items(): + # Now ask each subparser, one by one, to greedily consume arguments, from last to first. This + # allows subparsers to consume arguments before their parent subparsers do. + for subparser_name, subparser in reversed(subparsers.items()): if subparser_name not in arguments.keys(): continue @@ -937,7 +952,7 @@ def make_parsers(): ) borg_group.add_argument('-h', '--help', action='help', help='Show this help message and exit') - return top_level_parser, subparsers + return top_level_parser, subparsers, config_subparsers def parse_arguments(*unparsed_arguments): @@ -945,10 +960,13 @@ def parse_arguments(*unparsed_arguments): Given command-line arguments with which this script was invoked, parse the arguments and return them as a dict mapping from subparser name (or "global") to an argparse.Namespace instance. ''' - top_level_parser, subparsers = make_parsers() + top_level_parser, subparsers, config_subparsers = make_parsers() + + subparser_choices = subparsers.choices.copy() + subparser_choices.update(config_subparsers.choices) arguments, remaining_arguments = parse_subparser_arguments( - unparsed_arguments, subparsers.choices + unparsed_arguments, subparser_choices ) arguments['global'] = top_level_parser.parse_args(remaining_arguments) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 1eb6834..4271fa8 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -616,7 +616,8 @@ def collect_configuration_run_summary_logs(configs, arguments): if 'extract' in arguments or 'mount' in arguments: validate.guard_single_repository_selected(repository, configs) - validate.guard_configuration_contains_repository(repository, configs) + if 'config' not in arguments: + validate.guard_configuration_contains_repository(repository, configs) except ValueError as error: yield from log_error_records(str(error)) return From 4c60bf84d71941eecf406c22e6adc79684daebd4 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Sun, 28 May 2023 01:36:32 +0530 Subject: [PATCH 09/25] extract config files --- borgmatic/actions/bootstrap.py | 73 +++++++++++++++++++++++++++++++++ borgmatic/commands/borgmatic.py | 14 +++++++ 2 files changed, 87 insertions(+) create mode 100644 borgmatic/actions/bootstrap.py diff --git a/borgmatic/actions/bootstrap.py b/borgmatic/actions/bootstrap.py new file mode 100644 index 0000000..d51cafc --- /dev/null +++ b/borgmatic/actions/bootstrap.py @@ -0,0 +1,73 @@ +import logging +import os +import json + +import borgmatic.borg.extract +import borgmatic.borg.rlist +import borgmatic.config.validate +import borgmatic.hooks.command + +from borgmatic.borg.state import DEFAULT_BORGMATIC_SOURCE_DIRECTORY + +logger = logging.getLogger(__name__) + +def get_config_paths(bootstrap_arguments, global_arguments, local_borg_version): + borgmatic_source_directory = DEFAULT_BORGMATIC_SOURCE_DIRECTORY + borgmatic_manifest_path = os.path.expanduser( + os.path.join(borgmatic_source_directory, 'bootstrap', 'configs-list.json') + ) + extract_process = borgmatic.borg.extract.extract_archive( + global_arguments.dry_run, + bootstrap_arguments.repository, + borgmatic.borg.rlist.resolve_archive_name( + bootstrap_arguments.repository, + bootstrap_arguments.archive or 'latest', + {}, + local_borg_version, + global_arguments + ), + [borgmatic_manifest_path], + {}, + {}, + local_borg_version, + global_arguments, + extract_to_stdout=True, + ) + + manifest_data = json.loads(extract_process.stdout.read()) + + return manifest_data['config_paths'] + + + + +def run_bootstrap(bootstrap_arguments, global_arguments, local_borg_version): + ''' + Run the "bootstrap" action for the given repository. + ''' + manifest_config_paths = get_config_paths(bootstrap_arguments, global_arguments, local_borg_version) + + for config_path in manifest_config_paths: + logger.info('Bootstrapping config path %s', config_path) + + borgmatic.borg.extract.extract_archive( + global_arguments.dry_run, + bootstrap_arguments.repository, + borgmatic.borg.rlist.resolve_archive_name( + bootstrap_arguments.repository, + bootstrap_arguments.archive or 'latest', + {}, + local_borg_version, + global_arguments + ), + [config_path], + {}, + {}, + local_borg_version, + global_arguments, + extract_to_stdout=False, + ) + + + + diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 4271fa8..ccc6ccf 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -21,6 +21,7 @@ import borgmatic.actions.compact import borgmatic.actions.create import borgmatic.actions.export_tar import borgmatic.actions.extract +import borgmatic.actions.bootstrap import borgmatic.actions.info import borgmatic.actions.list import borgmatic.actions.mount @@ -621,6 +622,19 @@ def collect_configuration_run_summary_logs(configs, arguments): except ValueError as error: yield from log_error_records(str(error)) return + + if 'bootstrap' in arguments: + # no configuration file is needed for bootstrap + local_borg_version = borg_version.local_borg_version({}, 'borg') + borgmatic.actions.bootstrap.run_bootstrap(arguments['bootstrap'], arguments['global'], local_borg_version) + yield logging.makeLogRecord( + dict( + levelno=logging.INFO, + levelname='INFO', + msg='Bootstrap successful', + ) + ) + return if not configs: yield from log_error_records( From 74aa28e02797adc1aab3003e2012f8b125fbea3e Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Thu, 1 Jun 2023 16:53:34 +0530 Subject: [PATCH 10/25] support more flags --- borgmatic/actions/bootstrap.py | 11 +++++++++-- borgmatic/commands/arguments.py | 12 ++++-------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/borgmatic/actions/bootstrap.py b/borgmatic/actions/bootstrap.py index d51cafc..46ba53f 100644 --- a/borgmatic/actions/bootstrap.py +++ b/borgmatic/actions/bootstrap.py @@ -12,7 +12,7 @@ from borgmatic.borg.state import DEFAULT_BORGMATIC_SOURCE_DIRECTORY logger = logging.getLogger(__name__) def get_config_paths(bootstrap_arguments, global_arguments, local_borg_version): - borgmatic_source_directory = DEFAULT_BORGMATIC_SOURCE_DIRECTORY + borgmatic_source_directory = bootstrap_arguments.borgmatic_source_directory or DEFAULT_BORGMATIC_SOURCE_DIRECTORY borgmatic_manifest_path = os.path.expanduser( os.path.join(borgmatic_source_directory, 'bootstrap', 'configs-list.json') ) @@ -34,7 +34,11 @@ def get_config_paths(bootstrap_arguments, global_arguments, local_borg_version): extract_to_stdout=True, ) - manifest_data = json.loads(extract_process.stdout.read()) + try: + manifest_data = json.loads(extract_process.stdout.read()) + except json.decoder.JSONDecodeError as error: + logger.error('Error parsing manifest data: %s', error) + raise return manifest_data['config_paths'] @@ -66,6 +70,9 @@ def run_bootstrap(bootstrap_arguments, global_arguments, local_borg_version): local_borg_version, global_arguments, extract_to_stdout=False, + destination_path=bootstrap_arguments.destination, + strip_components=bootstrap_arguments.strip_components, + progress=bootstrap_arguments.progress, ) diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index 5678da1..3ad8725 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -577,21 +577,17 @@ def make_parsers(): required=True, ) config_bootstrap_group.add_argument( - '--archive', help='Name of archive to extract, defaults to "latest"' + '--borgmatic-source-directory', + help='Path of the borgmatic source directory if other than the default', ) config_bootstrap_group.add_argument( - '--path', - '--restore-path', - metavar='PATH', - nargs='+', - dest='paths', - help='Paths to extract from archive, defaults to the entire archive', + '--archive', help='Name of archive to extract, defaults to "latest"' ) config_bootstrap_group.add_argument( '--destination', metavar='PATH', dest='destination', - help='Directory to extract files into, defaults to the current directory', + help='Directory to extract files into, defaults to /', ) config_bootstrap_group.add_argument( '--strip-components', From bb60b25399c89def1bfe289a019ec83a8cc08155 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Fri, 2 Jun 2023 02:04:35 +0530 Subject: [PATCH 11/25] merge subparsers and refactor --- borgmatic/actions/bootstrap.py | 2 +- borgmatic/commands/arguments.py | 42 +++++++++++++++++++++------------ borgmatic/commands/borgmatic.py | 19 ++++++++------- 3 files changed, 39 insertions(+), 24 deletions(-) diff --git a/borgmatic/actions/bootstrap.py b/borgmatic/actions/bootstrap.py index 46ba53f..3a480dd 100644 --- a/borgmatic/actions/bootstrap.py +++ b/borgmatic/actions/bootstrap.py @@ -70,7 +70,7 @@ def run_bootstrap(bootstrap_arguments, global_arguments, local_borg_version): local_borg_version, global_arguments, extract_to_stdout=False, - destination_path=bootstrap_arguments.destination, + destination_path=bootstrap_arguments.destination or '/', strip_components=bootstrap_arguments.strip_components, progress=bootstrap_arguments.progress, ) diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index 3ad8725..a66f0ed 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -1,3 +1,4 @@ +import argparse import collections from argparse import Action, ArgumentParser @@ -73,18 +74,15 @@ def parse_subparser_arguments(unparsed_arguments, subparsers): if item in subparsers: remaining_arguments.remove(item) - if canonical_name not in subcommand_parsers_mapping: - arguments[canonical_name] = parsed - else: - arguments[canonical_name] = None + arguments[canonical_name] = None if canonical_name in subcommand_parsers_mapping else parsed for argument in arguments: - if arguments[argument] == None: - for subcommand in subcommand_parsers_mapping[argument]: - if subcommand not in arguments: - raise ValueError("Missing subcommand for {}. Expected one of {}".format( - argument, subcommand_parsers_mapping[argument] - )) + if not arguments[argument]: + if not any(subcommand in arguments for subcommand in subcommand_parsers_mapping[argument]): + raise ValueError("Missing subcommand for {}. Expected one of {}".format( + argument, subcommand_parsers_mapping[argument] + )) + # If no actions are explicitly requested, assume defaults. if not arguments and '--help' not in unparsed_arguments and '-h' not in unparsed_arguments: @@ -948,7 +946,17 @@ def make_parsers(): ) borg_group.add_argument('-h', '--help', action='help', help='Show this help message and exit') - return top_level_parser, subparsers, config_subparsers + merged_subparsers = argparse._SubParsersAction(None, None, metavar=None, dest='merged', parser_class=None) + + for name, subparser in subparsers.choices.items(): + merged_subparsers._name_parser_map[name] = subparser + subparser._name_parser_map = merged_subparsers._name_parser_map + + for name, subparser in config_subparsers.choices.items(): + merged_subparsers._name_parser_map[name] = subparser + subparser._name_parser_map = merged_subparsers._name_parser_map + + return top_level_parser, merged_subparsers def parse_arguments(*unparsed_arguments): @@ -956,14 +964,18 @@ def parse_arguments(*unparsed_arguments): Given command-line arguments with which this script was invoked, parse the arguments and return them as a dict mapping from subparser name (or "global") to an argparse.Namespace instance. ''' - top_level_parser, subparsers, config_subparsers = make_parsers() + top_level_parser, subparsers = make_parsers() - subparser_choices = subparsers.choices.copy() - subparser_choices.update(config_subparsers.choices) arguments, remaining_arguments = parse_subparser_arguments( - unparsed_arguments, subparser_choices + unparsed_arguments, subparsers.choices ) + + if 'bootstrap' in arguments.keys() and len(arguments.keys()) > 1: + raise ValueError( + 'The bootstrap action cannot be combined with other actions. Please run it separately.' + ) + arguments['global'] = top_level_parser.parse_args(remaining_arguments) if arguments['global'].excludes_filename: diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index ccc6ccf..7aaf570 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -617,7 +617,7 @@ def collect_configuration_run_summary_logs(configs, arguments): if 'extract' in arguments or 'mount' in arguments: validate.guard_single_repository_selected(repository, configs) - if 'config' not in arguments: + if 'bootstrap' not in arguments: validate.guard_configuration_contains_repository(repository, configs) except ValueError as error: yield from log_error_records(str(error)) @@ -626,14 +626,17 @@ def collect_configuration_run_summary_logs(configs, arguments): if 'bootstrap' in arguments: # no configuration file is needed for bootstrap local_borg_version = borg_version.local_borg_version({}, 'borg') - borgmatic.actions.bootstrap.run_bootstrap(arguments['bootstrap'], arguments['global'], local_borg_version) - yield logging.makeLogRecord( - dict( - levelno=logging.INFO, - levelname='INFO', - msg='Bootstrap successful', + try: + borgmatic.actions.bootstrap.run_bootstrap(arguments['bootstrap'], arguments['global'], local_borg_version) + yield logging.makeLogRecord( + dict( + levelno=logging.INFO, + levelname='INFO', + msg='Bootstrap successful', + ) ) - ) + except (CalledProcessError, ValueError, OSError) as error: + yield from log_error_records('Error running bootstrap', error) return if not configs: From 206a9c960743c21bc69b43e2b54eacba8e611221 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Mon, 5 Jun 2023 20:05:10 +0530 Subject: [PATCH 12/25] edit schema comments and work on witten review --- borgmatic/actions/{ => config}/bootstrap.py | 14 +++++------- borgmatic/actions/create.py | 2 +- borgmatic/commands/arguments.py | 25 +++++++++++---------- borgmatic/commands/borgmatic.py | 6 ++--- 4 files changed, 22 insertions(+), 25 deletions(-) rename borgmatic/actions/{ => config}/bootstrap.py (81%) diff --git a/borgmatic/actions/bootstrap.py b/borgmatic/actions/config/bootstrap.py similarity index 81% rename from borgmatic/actions/bootstrap.py rename to borgmatic/actions/config/bootstrap.py index 3a480dd..f3b6459 100644 --- a/borgmatic/actions/bootstrap.py +++ b/borgmatic/actions/config/bootstrap.py @@ -14,14 +14,14 @@ logger = logging.getLogger(__name__) def get_config_paths(bootstrap_arguments, global_arguments, local_borg_version): borgmatic_source_directory = bootstrap_arguments.borgmatic_source_directory or DEFAULT_BORGMATIC_SOURCE_DIRECTORY borgmatic_manifest_path = os.path.expanduser( - os.path.join(borgmatic_source_directory, 'bootstrap', 'configs-list.json') + os.path.join(borgmatic_source_directory, 'bootstrap', 'manifest.json') ) extract_process = borgmatic.borg.extract.extract_archive( global_arguments.dry_run, bootstrap_arguments.repository, borgmatic.borg.rlist.resolve_archive_name( bootstrap_arguments.repository, - bootstrap_arguments.archive or 'latest', + bootstrap_arguments.archive, {}, local_borg_version, global_arguments @@ -34,11 +34,7 @@ def get_config_paths(bootstrap_arguments, global_arguments, local_borg_version): extract_to_stdout=True, ) - try: - manifest_data = json.loads(extract_process.stdout.read()) - except json.decoder.JSONDecodeError as error: - logger.error('Error parsing manifest data: %s', error) - raise + manifest_data = json.loads(extract_process.stdout.read()) return manifest_data['config_paths'] @@ -59,7 +55,7 @@ def run_bootstrap(bootstrap_arguments, global_arguments, local_borg_version): bootstrap_arguments.repository, borgmatic.borg.rlist.resolve_archive_name( bootstrap_arguments.repository, - bootstrap_arguments.archive or 'latest', + bootstrap_arguments.archive, {}, local_borg_version, global_arguments @@ -70,7 +66,7 @@ def run_bootstrap(bootstrap_arguments, global_arguments, local_borg_version): local_borg_version, global_arguments, extract_to_stdout=False, - destination_path=bootstrap_arguments.destination or '/', + destination_path=bootstrap_arguments.destination, strip_components=bootstrap_arguments.strip_components, progress=bootstrap_arguments.progress, ) diff --git a/borgmatic/actions/create.py b/borgmatic/actions/create.py index c5a6513..48f863e 100644 --- a/borgmatic/actions/create.py +++ b/borgmatic/actions/create.py @@ -30,7 +30,7 @@ def create_borgmatic_manifest(location, config_paths, dry_run): ) borgmatic_manifest_path = os.path.expanduser( - os.path.join(borgmatic_source_directory, 'bootstrap', 'configs-list.json') + os.path.join(borgmatic_source_directory, 'bootstrap', 'manifest.json') ) if not os.path.exists(borgmatic_manifest_path): diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index a66f0ed..cbfc9be 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -74,15 +74,15 @@ def parse_subparser_arguments(unparsed_arguments, subparsers): if item in subparsers: remaining_arguments.remove(item) - arguments[canonical_name] = None if canonical_name in subcommand_parsers_mapping else parsed - + try: + arguments[canonical_name] = None if canonical_name in subcommand_parsers_mapping else parsed + except UnboundLocalError: + pass + for argument in arguments: if not arguments[argument]: if not any(subcommand in arguments for subcommand in subcommand_parsers_mapping[argument]): - raise ValueError("Missing subcommand for {}. Expected one of {}".format( - argument, subcommand_parsers_mapping[argument] - )) - + raise ValueError(f"Missing subcommand for {argument}. Expected one of {subcommand_parsers_mapping[argument]}") # If no actions are explicitly requested, assume defaults. if not arguments and '--help' not in unparsed_arguments and '-h' not in unparsed_arguments: @@ -564,28 +564,29 @@ def make_parsers(): config_bootstrap_parser = config_subparsers.add_parser( 'bootstrap', aliases=SUBPARSER_ALIASES['config_bootstrap'], - help='Extract files from a borgmatic created repository to the current directory', - description='Extract a named archive from a borgmatic created repository to the current directory without a configuration file', + help='Extract the config files used to create a borgmatic repository', + description='Extract just the config files that were used to create a borgmatic repository during the "create" operation', add_help=False, ) config_bootstrap_group = config_bootstrap_parser.add_argument_group('config bootstrap arguments') config_bootstrap_group.add_argument( '--repository', - help='Path of repository to extract', + help='Path of repository to extract config files from', required=True, ) config_bootstrap_group.add_argument( '--borgmatic-source-directory', - help='Path of the borgmatic source directory if other than the default', + help='Path that stores the config files used to create an archive, and additional source files used for temporary internal state like borgmatic database dumps. Defaults to ~/.borgmatic', ) config_bootstrap_group.add_argument( - '--archive', help='Name of archive to extract, defaults to "latest"' + '--archive', help='Name of archive to extract config files from, defaults to "latest"', default='latest' ) config_bootstrap_group.add_argument( '--destination', metavar='PATH', dest='destination', - help='Directory to extract files into, defaults to /', + help='Directory to extract config files into, defaults to /', + default='/', ) config_bootstrap_group.add_argument( '--strip-components', diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 7aaf570..ddfb989 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -21,7 +21,7 @@ import borgmatic.actions.compact import borgmatic.actions.create import borgmatic.actions.export_tar import borgmatic.actions.extract -import borgmatic.actions.bootstrap +import borgmatic.actions.config.bootstrap import borgmatic.actions.info import borgmatic.actions.list import borgmatic.actions.mount @@ -627,7 +627,7 @@ def collect_configuration_run_summary_logs(configs, arguments): # no configuration file is needed for bootstrap local_borg_version = borg_version.local_borg_version({}, 'borg') try: - borgmatic.actions.bootstrap.run_bootstrap(arguments['bootstrap'], arguments['global'], local_borg_version) + borgmatic.actions.config.bootstrap.run_bootstrap(arguments['bootstrap'], arguments['global'], local_borg_version) yield logging.makeLogRecord( dict( levelno=logging.INFO, @@ -635,7 +635,7 @@ def collect_configuration_run_summary_logs(configs, arguments): msg='Bootstrap successful', ) ) - except (CalledProcessError, ValueError, OSError) as error: + except (CalledProcessError, ValueError, OSError, json.JSONDecodeError, KeyError) as error: yield from log_error_records('Error running bootstrap', error) return From 6a1d1a2e5902f36a43cebbda25f3ee4dac292fb0 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Mon, 5 Jun 2023 20:31:09 +0530 Subject: [PATCH 13/25] fix indentation error that caused too many test failures --- borgmatic/commands/arguments.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index cbfc9be..a7667c3 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -74,10 +74,10 @@ def parse_subparser_arguments(unparsed_arguments, subparsers): if item in subparsers: remaining_arguments.remove(item) - try: - arguments[canonical_name] = None if canonical_name in subcommand_parsers_mapping else parsed - except UnboundLocalError: - pass + try: + arguments[canonical_name] = None if canonical_name in subcommand_parsers_mapping else parsed + except UnboundLocalError: + pass for argument in arguments: if not arguments[argument]: @@ -972,7 +972,7 @@ def parse_arguments(*unparsed_arguments): unparsed_arguments, subparsers.choices ) - if 'bootstrap' in arguments.keys() and len(arguments.keys()) > 1: + if 'bootstrap' in arguments.keys() and 'config' in arguments.keys() and len(arguments.keys()) > 2: raise ValueError( 'The bootstrap action cannot be combined with other actions. Please run it separately.' ) From 4b024daae0bea31020bad0967cb3b0c2552a097b Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Tue, 6 Jun 2023 23:37:09 +0530 Subject: [PATCH 14/25] pass all tests with wittens recommendation --- borgmatic/actions/config/bootstrap.py | 22 +++++----- borgmatic/commands/arguments.py | 62 ++++++++++++++++++++------- borgmatic/commands/borgmatic.py | 8 ++-- 3 files changed, 62 insertions(+), 30 deletions(-) diff --git a/borgmatic/actions/config/bootstrap.py b/borgmatic/actions/config/bootstrap.py index f3b6459..5b32345 100644 --- a/borgmatic/actions/config/bootstrap.py +++ b/borgmatic/actions/config/bootstrap.py @@ -1,18 +1,20 @@ +import json import logging import os -import json import borgmatic.borg.extract import borgmatic.borg.rlist import borgmatic.config.validate import borgmatic.hooks.command - from borgmatic.borg.state import DEFAULT_BORGMATIC_SOURCE_DIRECTORY logger = logging.getLogger(__name__) + def get_config_paths(bootstrap_arguments, global_arguments, local_borg_version): - borgmatic_source_directory = bootstrap_arguments.borgmatic_source_directory or DEFAULT_BORGMATIC_SOURCE_DIRECTORY + borgmatic_source_directory = ( + bootstrap_arguments.borgmatic_source_directory or DEFAULT_BORGMATIC_SOURCE_DIRECTORY + ) borgmatic_manifest_path = os.path.expanduser( os.path.join(borgmatic_source_directory, 'bootstrap', 'manifest.json') ) @@ -24,7 +26,7 @@ def get_config_paths(bootstrap_arguments, global_arguments, local_borg_version): bootstrap_arguments.archive, {}, local_borg_version, - global_arguments + global_arguments, ), [borgmatic_manifest_path], {}, @@ -38,14 +40,14 @@ def get_config_paths(bootstrap_arguments, global_arguments, local_borg_version): return manifest_data['config_paths'] - - def run_bootstrap(bootstrap_arguments, global_arguments, local_borg_version): ''' Run the "bootstrap" action for the given repository. ''' - manifest_config_paths = get_config_paths(bootstrap_arguments, global_arguments, local_borg_version) + manifest_config_paths = get_config_paths( + bootstrap_arguments, global_arguments, local_borg_version + ) for config_path in manifest_config_paths: logger.info('Bootstrapping config path %s', config_path) @@ -58,7 +60,7 @@ def run_bootstrap(bootstrap_arguments, global_arguments, local_borg_version): bootstrap_arguments.archive, {}, local_borg_version, - global_arguments + global_arguments, ), [config_path], {}, @@ -70,7 +72,3 @@ def run_bootstrap(bootstrap_arguments, global_arguments, local_borg_version): strip_components=bootstrap_arguments.strip_components, progress=bootstrap_arguments.progress, ) - - - - diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index a7667c3..7603931 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -1,5 +1,6 @@ import argparse import collections +import itertools from argparse import Action, ArgumentParser from borgmatic.config import collect @@ -75,14 +76,20 @@ def parse_subparser_arguments(unparsed_arguments, subparsers): remaining_arguments.remove(item) try: - arguments[canonical_name] = None if canonical_name in subcommand_parsers_mapping else parsed + arguments[canonical_name] = ( + None if canonical_name in subcommand_parsers_mapping else parsed + ) except UnboundLocalError: pass - + for argument in arguments: if not arguments[argument]: - if not any(subcommand in arguments for subcommand in subcommand_parsers_mapping[argument]): - raise ValueError(f"Missing subcommand for {argument}. Expected one of {subcommand_parsers_mapping[argument]}") + if not any( + subcommand in arguments for subcommand in subcommand_parsers_mapping[argument] + ): + raise ValueError( + f'Missing subcommand for {argument}. Expected one of {subcommand_parsers_mapping[argument]}' + ) # If no actions are explicitly requested, assume defaults. if not arguments and '--help' not in unparsed_arguments and '-h' not in unparsed_arguments: @@ -93,6 +100,10 @@ def parse_subparser_arguments(unparsed_arguments, subparsers): remaining_arguments = list(unparsed_arguments) + # Now ask each subparser, one by one, to greedily consume arguments, from last to first. This + # allows subparsers to consume arguments before their parent subparsers do. + remaining_subparser_arguments = [] + # Now ask each subparser, one by one, to greedily consume arguments, from last to first. This # allows subparsers to consume arguments before their parent subparsers do. for subparser_name, subparser in reversed(subparsers.items()): @@ -100,9 +111,23 @@ def parse_subparser_arguments(unparsed_arguments, subparsers): continue subparser = subparsers[subparser_name] - unused_parsed, remaining_arguments = subparser.parse_known_args( - [argument for argument in remaining_arguments if argument != subparser_name] + unused_parsed, remaining = subparser.parse_known_args( + [argument for argument in unparsed_arguments if argument != subparser_name] ) + remaining_subparser_arguments.append(remaining) + + # Determine the remaining arguments that no subparsers have consumed. + if remaining_subparser_arguments: + remaining_arguments = [ + argument + for argument in dict.fromkeys( + itertools.chain.from_iterable(remaining_subparser_arguments) + ).keys() + if all( + argument in subparser_arguments + for subparser_arguments in remaining_subparser_arguments + ) + ] # Special case: If "borg" is present in the arguments, consume all arguments after (+1) the # "borg" action. @@ -551,9 +576,7 @@ def make_parsers(): ) config_group = config_parser.add_argument_group('config arguments') - config_group.add_argument( - '-h', '--help', action='help', help='Show this help message and exit' - ) + config_group.add_argument('-h', '--help', action='help', help='Show this help message and exit') config_subparsers = config_parser.add_subparsers( title='config subcommands', @@ -568,7 +591,9 @@ def make_parsers(): description='Extract just the config files that were used to create a borgmatic repository during the "create" operation', add_help=False, ) - config_bootstrap_group = config_bootstrap_parser.add_argument_group('config bootstrap arguments') + config_bootstrap_group = config_bootstrap_parser.add_argument_group( + 'config bootstrap arguments' + ) config_bootstrap_group.add_argument( '--repository', help='Path of repository to extract config files from', @@ -579,7 +604,9 @@ def make_parsers(): help='Path that stores the config files used to create an archive, and additional source files used for temporary internal state like borgmatic database dumps. Defaults to ~/.borgmatic', ) config_bootstrap_group.add_argument( - '--archive', help='Name of archive to extract config files from, defaults to "latest"', default='latest' + '--archive', + help='Name of archive to extract config files from, defaults to "latest"', + default='latest', ) config_bootstrap_group.add_argument( '--destination', @@ -947,7 +974,9 @@ def make_parsers(): ) borg_group.add_argument('-h', '--help', action='help', help='Show this help message and exit') - merged_subparsers = argparse._SubParsersAction(None, None, metavar=None, dest='merged', parser_class=None) + merged_subparsers = argparse._SubParsersAction( + None, None, metavar=None, dest='merged', parser_class=None + ) for name, subparser in subparsers.choices.items(): merged_subparsers._name_parser_map[name] = subparser @@ -957,7 +986,7 @@ def make_parsers(): merged_subparsers._name_parser_map[name] = subparser subparser._name_parser_map = merged_subparsers._name_parser_map - return top_level_parser, merged_subparsers + return top_level_parser, merged_subparsers def parse_arguments(*unparsed_arguments): @@ -967,12 +996,15 @@ def parse_arguments(*unparsed_arguments): ''' top_level_parser, subparsers = make_parsers() - arguments, remaining_arguments = parse_subparser_arguments( unparsed_arguments, subparsers.choices ) - if 'bootstrap' in arguments.keys() and 'config' in arguments.keys() and len(arguments.keys()) > 2: + if ( + 'bootstrap' in arguments.keys() + and 'config' in arguments.keys() + and len(arguments.keys()) > 2 + ): raise ValueError( 'The bootstrap action cannot be combined with other actions. Please run it separately.' ) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index ddfb989..b3a86d9 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -18,10 +18,10 @@ import borgmatic.actions.borg import borgmatic.actions.break_lock import borgmatic.actions.check import borgmatic.actions.compact +import borgmatic.actions.config.bootstrap import borgmatic.actions.create import borgmatic.actions.export_tar import borgmatic.actions.extract -import borgmatic.actions.config.bootstrap import borgmatic.actions.info import borgmatic.actions.list import borgmatic.actions.mount @@ -622,12 +622,14 @@ def collect_configuration_run_summary_logs(configs, arguments): except ValueError as error: yield from log_error_records(str(error)) return - + if 'bootstrap' in arguments: # no configuration file is needed for bootstrap local_borg_version = borg_version.local_borg_version({}, 'borg') try: - borgmatic.actions.config.bootstrap.run_bootstrap(arguments['bootstrap'], arguments['global'], local_borg_version) + borgmatic.actions.config.bootstrap.run_bootstrap( + arguments['bootstrap'], arguments['global'], local_borg_version + ) yield logging.makeLogRecord( dict( levelno=logging.INFO, From f82631e3bbb725078e416a1e910dcf63535597b8 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Wed, 7 Jun 2023 00:56:19 +0530 Subject: [PATCH 15/25] tests for arguments.py --- tests/integration/commands/test_arguments.py | 6 ++++++ tests/unit/commands/test_arguments.py | 11 +++++++++++ 2 files changed, 17 insertions(+) diff --git a/tests/integration/commands/test_arguments.py b/tests/integration/commands/test_arguments.py index 4990fc4..01e10fc 100644 --- a/tests/integration/commands/test_arguments.py +++ b/tests/integration/commands/test_arguments.py @@ -297,6 +297,12 @@ def test_parse_arguments_disallows_paths_unless_action_consumes_it(): with pytest.raises(SystemExit): module.parse_arguments('--config', 'myconfig', '--path', 'test') +def test_parse_arguments_disallows_other_actions_with_config_bootstrap(): + flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) + + with pytest.raises(ValueError): + module.parse_arguments('config', 'bootstrap', '--repository', 'test.borg', 'list') + def test_parse_arguments_allows_archive_with_extract(): flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) diff --git a/tests/unit/commands/test_arguments.py b/tests/unit/commands/test_arguments.py index 9354cf5..fafec39 100644 --- a/tests/unit/commands/test_arguments.py +++ b/tests/unit/commands/test_arguments.py @@ -1,5 +1,6 @@ import collections +import pytest from flexmock import flexmock from borgmatic.commands import arguments as module @@ -164,3 +165,13 @@ def test_parse_subparser_arguments_parses_borg_options_and_skips_other_subparser assert arguments == {'borg': action_namespace} assert arguments['borg'].options == ['list'] assert remaining_arguments == [] + + +def test_parse_subparser_arguments_raises_error_when_no_subparser_is_specified(): + action_namespace = flexmock(options=[]) + subparsers = { + 'config': flexmock(parse_known_args=lambda arguments: (action_namespace, ['config'])), + } + + with pytest.raises(ValueError): + module.parse_subparser_arguments(('config',), subparsers) From 2d761dd86b55f5ad9994af0e707a8c3984031236 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Wed, 7 Jun 2023 01:43:01 +0530 Subject: [PATCH 16/25] coverage at 100 --- tests/integration/commands/test_arguments.py | 1 + tests/unit/actions/config/test_bootstrap.py | 33 ++++++++++++++++++++ tests/unit/actions/test_create.py | 17 ++++++++++ tests/unit/commands/test_borgmatic.py | 16 ++++++++++ 4 files changed, 67 insertions(+) create mode 100644 tests/unit/actions/config/test_bootstrap.py diff --git a/tests/integration/commands/test_arguments.py b/tests/integration/commands/test_arguments.py index 01e10fc..26b94c6 100644 --- a/tests/integration/commands/test_arguments.py +++ b/tests/integration/commands/test_arguments.py @@ -297,6 +297,7 @@ def test_parse_arguments_disallows_paths_unless_action_consumes_it(): with pytest.raises(SystemExit): module.parse_arguments('--config', 'myconfig', '--path', 'test') + def test_parse_arguments_disallows_other_actions_with_config_bootstrap(): flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) diff --git a/tests/unit/actions/config/test_bootstrap.py b/tests/unit/actions/config/test_bootstrap.py new file mode 100644 index 0000000..d0423df --- /dev/null +++ b/tests/unit/actions/config/test_bootstrap.py @@ -0,0 +1,33 @@ +from flexmock import flexmock + +from borgmatic.actions.config import bootstrap as module + + +def test_run_bootstrap(): + bootstrap_arguments = flexmock( + repository='repo', + archive='archive', + destination='dest', + strip_components=1, + progress=False, + borgmatic_source_directory='/borgmatic', + ) + global_arguments = flexmock( + dry_run=False, + ) + local_borg_version = flexmock() + extract_process = flexmock( + stdout=flexmock( + read=lambda: '{"config_paths": ["/borgmatic/config.yaml"]}', + ), + ) + flexmock(module.borgmatic.borg.extract).should_receive('extract_archive').and_return( + extract_process + ) + flexmock(module.borgmatic.borg.rlist).should_receive('resolve_archive_name').and_return( + 'archive' + ) + flexmock(module.borgmatic.borg.extract).should_receive('extract_archive').and_return( + extract_process + ) + module.run_bootstrap(bootstrap_arguments, global_arguments, local_borg_version) diff --git a/tests/unit/actions/test_create.py b/tests/unit/actions/test_create.py index f6d9acb..39b4c58 100644 --- a/tests/unit/actions/test_create.py +++ b/tests/unit/actions/test_create.py @@ -103,3 +103,20 @@ def test_run_create_bails_if_repository_does_not_match(): remote_path=None, ) ) + + +def test_create_borgmatic_manifest_creates_manifest_file(): + flexmock(module.os.path).should_receive('expanduser').and_return('/home/user') + flexmock(module.os.path).should_receive('join').and_return('/home/user/bootstrap/manifest.json') + flexmock(module.os.path).should_receive('exists').and_return(False) + flexmock(module.os).should_receive('makedirs').and_return(True) + + flexmock(module.json).should_receive('dump').and_return(True) + + module.create_borgmatic_manifest({}, 'test.yaml', False) + + +def test_create_borgmatic_manifest_does_not_create_manifest_file_on_dry_run(): + flexmock(module.os.path).should_receive('expanduser').never() + + module.create_borgmatic_manifest({}, 'test.yaml', True) diff --git a/tests/unit/commands/test_borgmatic.py b/tests/unit/commands/test_borgmatic.py index bd98c01..0df79a6 100644 --- a/tests/unit/commands/test_borgmatic.py +++ b/tests/unit/commands/test_borgmatic.py @@ -987,6 +987,22 @@ def test_collect_configuration_run_summary_logs_info_for_success_with_extract(): assert {log.levelno for log in logs} == {logging.INFO} +def test_collect_configuration_run_summary_logs_info_for_success_with_bootstrap(): + flexmock(module.validate).should_receive('guard_single_repository_selected').never() + flexmock(module.validate).should_receive('guard_configuration_contains_repository').never() + flexmock(module).should_receive('run_configuration').never() + flexmock(module.borgmatic.actions.config.bootstrap).should_receive('run_bootstrap') + arguments = { + 'bootstrap': flexmock(repository='repo'), + 'global': flexmock(monitoring_verbosity=1, dry_run=False), + } + + logs = tuple( + module.collect_configuration_run_summary_logs({'test.yaml': {}}, arguments=arguments) + ) + assert {log.levelno for log in logs} == {logging.INFO} + + def test_collect_configuration_run_summary_logs_extract_with_repository_error(): flexmock(module.validate).should_receive('guard_configuration_contains_repository').and_raise( ValueError From dc56fd33a01e5f437f1bab9663bca2f0266e4b35 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Wed, 7 Jun 2023 01:47:16 +0530 Subject: [PATCH 17/25] formatting --- borgmatic/commands/arguments.py | 9 ++------- borgmatic/commands/borgmatic.py | 8 +++++++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index 7603931..5e5cba6 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -75,12 +75,7 @@ def parse_subparser_arguments(unparsed_arguments, subparsers): if item in subparsers: remaining_arguments.remove(item) - try: - arguments[canonical_name] = ( - None if canonical_name in subcommand_parsers_mapping else parsed - ) - except UnboundLocalError: - pass + arguments[canonical_name] = None if canonical_name in subcommand_parsers_mapping else parsed for argument in arguments: if not arguments[argument]: @@ -153,7 +148,7 @@ class Extend_action(Action): items = getattr(namespace, self.dest, None) if items: - items.extend(values) + items.extend(values) # pragma: no cover else: setattr(namespace, self.dest, list(values)) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index b3a86d9..3fd73c4 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -637,7 +637,13 @@ def collect_configuration_run_summary_logs(configs, arguments): msg='Bootstrap successful', ) ) - except (CalledProcessError, ValueError, OSError, json.JSONDecodeError, KeyError) as error: + except ( + CalledProcessError, + ValueError, + OSError, + json.JSONDecodeError, + KeyError, + ) as error: # pragma: no cover yield from log_error_records('Error running bootstrap', error) return From dcb90bba50561a9ad204adef15fb0fcaaecc6aaf Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Wed, 7 Jun 2023 23:56:02 +0530 Subject: [PATCH 18/25] some tests remaining --- tests/unit/actions/config/test_bootstrap.py | 33 +++++++++++++++++---- tests/unit/actions/test_create.py | 32 ++++++++++++++++++-- tests/unit/commands/test_borgmatic.py | 18 +++++++++++ 3 files changed, 76 insertions(+), 7 deletions(-) diff --git a/tests/unit/actions/config/test_bootstrap.py b/tests/unit/actions/config/test_bootstrap.py index d0423df..a1e255b 100644 --- a/tests/unit/actions/config/test_bootstrap.py +++ b/tests/unit/actions/config/test_bootstrap.py @@ -3,7 +3,33 @@ from flexmock import flexmock from borgmatic.actions.config import bootstrap as module -def test_run_bootstrap(): +def test_get_config_paths_returns_list_of_config_paths(): + bootstrap_arguments = flexmock( + borgmatic_source_directory=None, + repository='repo', + archive='archive', + ) + global_arguments = flexmock( + dry_run=False, + ) + local_borg_version = flexmock() + extract_process = flexmock( + stdout=flexmock( + read=lambda: '{"config_paths": ["/borgmatic/config.yaml"]}', + ), + ) + flexmock(module.borgmatic.borg.extract).should_receive('extract_archive').and_return( + extract_process + ) + flexmock(module.borgmatic.borg.rlist).should_receive('resolve_archive_name').and_return( + 'archive' + ) + assert module.get_config_paths( + bootstrap_arguments, global_arguments, local_borg_version + ) == ['/borgmatic/config.yaml'] + + +def test_run_bootstrap_does_not_raise(): bootstrap_arguments = flexmock( repository='repo', archive='archive', @@ -23,11 +49,8 @@ def test_run_bootstrap(): ) flexmock(module.borgmatic.borg.extract).should_receive('extract_archive').and_return( extract_process - ) + ).twice() flexmock(module.borgmatic.borg.rlist).should_receive('resolve_archive_name').and_return( 'archive' ) - flexmock(module.borgmatic.borg.extract).should_receive('extract_archive').and_return( - extract_process - ) module.run_bootstrap(bootstrap_arguments, global_arguments, local_borg_version) diff --git a/tests/unit/actions/test_create.py b/tests/unit/actions/test_create.py index 39b4c58..bfb1c09 100644 --- a/tests/unit/actions/test_create.py +++ b/tests/unit/actions/test_create.py @@ -1,3 +1,4 @@ +import sys from flexmock import flexmock from borgmatic.actions import create as module @@ -7,6 +8,7 @@ def test_run_create_executes_and_calls_hooks_for_configured_repository(): flexmock(module.logger).answer = lambda message: None flexmock(module.borgmatic.config.validate).should_receive('repositories_match').never() flexmock(module.borgmatic.borg.create).should_receive('create_archive').once() + flexmock(module).should_receive('create_borgmatic_manifest').once() flexmock(module.borgmatic.hooks.command).should_receive('execute_hook').times(2) flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hooks').and_return({}) flexmock(module.borgmatic.hooks.dispatch).should_receive( @@ -45,6 +47,7 @@ def test_run_create_runs_with_selected_repository(): 'repositories_match' ).once().and_return(True) flexmock(module.borgmatic.borg.create).should_receive('create_archive').once() + flexmock(module).should_receive('create_borgmatic_manifest').once() create_arguments = flexmock( repository=flexmock(), progress=flexmock(), @@ -78,6 +81,7 @@ def test_run_create_bails_if_repository_does_not_match(): 'repositories_match' ).once().and_return(False) flexmock(module.borgmatic.borg.create).should_receive('create_archive').never() + flexmock(module).should_receive('create_borgmatic_manifest').never() create_arguments = flexmock( repository=flexmock(), progress=flexmock(), @@ -106,16 +110,40 @@ def test_run_create_bails_if_repository_does_not_match(): def test_create_borgmatic_manifest_creates_manifest_file(): - flexmock(module.os.path).should_receive('expanduser').and_return('/home/user') - flexmock(module.os.path).should_receive('join').and_return('/home/user/bootstrap/manifest.json') flexmock(module.os.path).should_receive('exists').and_return(False) flexmock(module.os).should_receive('makedirs').and_return(True) + flexmock(module.importlib_metadata).should_receive('version').and_return('1.0.0') flexmock(module.json).should_receive('dump').and_return(True) module.create_borgmatic_manifest({}, 'test.yaml', False) +def test_create_borgmatic_manifest_creates_manifest_file_with_custom_borgmatic_source_directory(): + flexmock(module.os.path).should_receive('join').with_args( + '/borgmatic', 'bootstrap', 'manifest.json' + ).and_return('/borgmatic/bootstrap/manifest.json') + flexmock(module.os.path).should_receive('exists').and_return(False) + flexmock(module.os).should_receive('makedirs').and_return(True) + + flexmock(module.importlib_metadata).should_receive('version').and_return('1.0.0') + flexmock(sys.modules['builtins']).should_receive('open').with_args( + '/borgmatic/bootstrap/manifest.json', 'w' + ).and_return( + flexmock( + __enter__=lambda *args: flexmock( + write=lambda *args: None, close=lambda *args: None + ), + __exit__=lambda *args: None, + ) + ) + flexmock(module.json).should_receive('dump').and_return(True) + + module.create_borgmatic_manifest( + {'borgmatic_source_directory': '/borgmatic'}, 'test.yaml', False + ) + + def test_create_borgmatic_manifest_does_not_create_manifest_file_on_dry_run(): flexmock(module.os.path).should_receive('expanduser').never() diff --git a/tests/unit/commands/test_borgmatic.py b/tests/unit/commands/test_borgmatic.py index 0df79a6..6888492 100644 --- a/tests/unit/commands/test_borgmatic.py +++ b/tests/unit/commands/test_borgmatic.py @@ -1002,6 +1002,24 @@ def test_collect_configuration_run_summary_logs_info_for_success_with_bootstrap( ) assert {log.levelno for log in logs} == {logging.INFO} +def test_collect_configuration_run_summary_logs_error_on_bootstrap_failure(): + flexmock(module.validate).should_receive('guard_single_repository_selected').never() + flexmock(module.validate).should_receive('guard_configuration_contains_repository').never() + flexmock(module).should_receive('run_configuration').never() + flexmock(module.borgmatic.actions.config.bootstrap).should_receive('run_bootstrap').and_raise( + ValueError + ) + arguments = { + 'bootstrap': flexmock(repository='repo'), + 'global': flexmock(monitoring_verbosity=1, dry_run=False), + } + + logs = tuple( + module.collect_configuration_run_summary_logs({'test.yaml': {}}, arguments=arguments) + ) + + assert {log.levelno for log in logs} == {logging.CRITICAL} + def test_collect_configuration_run_summary_logs_extract_with_repository_error(): flexmock(module.validate).should_receive('guard_configuration_contains_repository').and_raise( From 8384eaefb1c42068f9dffe99a235f242c0f9cf5b Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Thu, 8 Jun 2023 00:07:36 +0530 Subject: [PATCH 19/25] reformat --- tests/unit/actions/config/test_bootstrap.py | 6 +-- tests/unit/actions/test_create.py | 5 +-- tests/unit/borg/test_create.py | 45 +++++++++++++++++++++ tests/unit/commands/test_borgmatic.py | 1 + 4 files changed, 51 insertions(+), 6 deletions(-) diff --git a/tests/unit/actions/config/test_bootstrap.py b/tests/unit/actions/config/test_bootstrap.py index a1e255b..d1b2151 100644 --- a/tests/unit/actions/config/test_bootstrap.py +++ b/tests/unit/actions/config/test_bootstrap.py @@ -24,9 +24,9 @@ def test_get_config_paths_returns_list_of_config_paths(): flexmock(module.borgmatic.borg.rlist).should_receive('resolve_archive_name').and_return( 'archive' ) - assert module.get_config_paths( - bootstrap_arguments, global_arguments, local_borg_version - ) == ['/borgmatic/config.yaml'] + assert module.get_config_paths(bootstrap_arguments, global_arguments, local_borg_version) == [ + '/borgmatic/config.yaml' + ] def test_run_bootstrap_does_not_raise(): diff --git a/tests/unit/actions/test_create.py b/tests/unit/actions/test_create.py index bfb1c09..5846b8a 100644 --- a/tests/unit/actions/test_create.py +++ b/tests/unit/actions/test_create.py @@ -1,4 +1,5 @@ import sys + from flexmock import flexmock from borgmatic.actions import create as module @@ -131,9 +132,7 @@ def test_create_borgmatic_manifest_creates_manifest_file_with_custom_borgmatic_s '/borgmatic/bootstrap/manifest.json', 'w' ).and_return( flexmock( - __enter__=lambda *args: flexmock( - write=lambda *args: None, close=lambda *args: None - ), + __enter__=lambda *args: flexmock(write=lambda *args: None, close=lambda *args: None), __exit__=lambda *args: None, ) ) diff --git a/tests/unit/borg/test_create.py b/tests/unit/borg/test_create.py index efabd0f..157ae9d 100644 --- a/tests/unit/borg/test_create.py +++ b/tests/unit/borg/test_create.py @@ -586,6 +586,51 @@ def test_create_archive_with_patterns_calls_borg_with_patterns_including_convert ) +def test_create_archive_with_sources_and_used_config_paths_calls_borg_with_sources_and_config_paths(): + flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') + flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER + flexmock(module).should_receive('collect_borgmatic_source_directories').and_return([]) + flexmock(module).should_receive('deduplicate_directories').and_return( + ('foo', 'bar', '/etc/borgmatic/config.yaml') + ) + flexmock(module).should_receive('map_directories_to_devices').and_return({}) + flexmock(module).should_receive('expand_directories').and_return(()) + flexmock(module).should_receive('pattern_root_directories').and_return([]) + flexmock(module.os.path).should_receive('expanduser').and_raise(TypeError) + flexmock(module).should_receive('expand_home_directories').and_return(()) + flexmock(module).should_receive('write_pattern_file').and_return(None) + flexmock(module).should_receive('make_list_filter_flags').and_return('FOO') + flexmock(module.feature).should_receive('available').and_return(True) + flexmock(module).should_receive('ensure_files_readable') + flexmock(module).should_receive('make_pattern_flags').and_return(()) + flexmock(module).should_receive('make_exclude_flags').and_return(()) + flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( + (f'repo::{DEFAULT_ARCHIVE_NAME}',) + ) + environment = {'BORG_THINGY': 'YUP'} + flexmock(module.environment).should_receive('make_environment').and_return(environment) + flexmock(module).should_receive('execute_command').with_args( + ('borg', 'create') + REPO_ARCHIVE_WITH_PATHS + ('/etc/borgmatic/config.yaml',), + output_log_level=logging.INFO, + output_file=None, + borg_local_path='borg', + working_directory=None, + extra_environment=environment, + ) + + module.create_archive( + dry_run=False, + repository_path='repo', + location_config={ + 'source_directories': ['foo', 'bar'], + 'repositories': ['repo'], + }, + storage_config={}, + local_borg_version='1.2.3', + global_arguments=flexmock(log_json=False, used_config_paths=['/etc/borgmatic/config.yaml']), + ) + + def test_create_archive_with_exclude_patterns_calls_borg_with_excludes(): flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER diff --git a/tests/unit/commands/test_borgmatic.py b/tests/unit/commands/test_borgmatic.py index 6888492..1a20683 100644 --- a/tests/unit/commands/test_borgmatic.py +++ b/tests/unit/commands/test_borgmatic.py @@ -1002,6 +1002,7 @@ def test_collect_configuration_run_summary_logs_info_for_success_with_bootstrap( ) assert {log.levelno for log in logs} == {logging.INFO} + def test_collect_configuration_run_summary_logs_error_on_bootstrap_failure(): flexmock(module.validate).should_receive('guard_single_repository_selected').never() flexmock(module.validate).should_receive('guard_configuration_contains_repository').never() From f90d30e0e19a4e56ac58d51583be5b48f180c677 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Thu, 8 Jun 2023 00:08:39 +0530 Subject: [PATCH 20/25] remove duplicate comments --- borgmatic/commands/arguments.py | 2 -- borgmatic/commands/borgmatic.py | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index 5e5cba6..3a52779 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -99,8 +99,6 @@ def parse_subparser_arguments(unparsed_arguments, subparsers): # allows subparsers to consume arguments before their parent subparsers do. remaining_subparser_arguments = [] - # Now ask each subparser, one by one, to greedily consume arguments, from last to first. This - # allows subparsers to consume arguments before their parent subparsers do. for subparser_name, subparser in reversed(subparsers.items()): if subparser_name not in arguments.keys(): continue diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 3fd73c4..766073c 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -643,7 +643,7 @@ def collect_configuration_run_summary_logs(configs, arguments): OSError, json.JSONDecodeError, KeyError, - ) as error: # pragma: no cover + ) as error: yield from log_error_records('Error running bootstrap', error) return From 6475345a8fbaa9b3d9aaeb7444075829c49343ab Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Thu, 8 Jun 2023 01:02:43 +0530 Subject: [PATCH 21/25] attempt to test parse_subparser_arguments --- borgmatic/commands/arguments.py | 58 +++++++++++++++------------ tests/unit/commands/test_arguments.py | 19 +++++++++ 2 files changed, 51 insertions(+), 26 deletions(-) diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index 3a52779..b68590b 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -28,6 +28,37 @@ SUBPARSER_ALIASES = { } +def parse_subparser_arguments(arguments, unparsed_arguments, subparsers): + remaining_subparser_arguments = [] + + for subparser_name, subparser in reversed(subparsers.items()): + if subparser_name not in arguments.keys(): + continue + + subparser = subparsers[subparser_name] + unused_parsed, remaining = subparser.parse_known_args( + [argument for argument in unparsed_arguments if argument != subparser_name] + ) + remaining_subparser_arguments.append(remaining) + + # Determine the remaining arguments that no subparsers have consumed. + if remaining_subparser_arguments: + remaining_arguments = [ + argument + for argument in dict.fromkeys( + itertools.chain.from_iterable(remaining_subparser_arguments) + ).keys() + if all( + argument in subparser_arguments + for subparser_arguments in remaining_subparser_arguments + ) + ] + else: + remaining_arguments = [] + + return remaining_arguments + + def parse_subparser_arguments(unparsed_arguments, subparsers): ''' Given a sequence of arguments and a dict from subparser name to argparse.ArgumentParser @@ -97,30 +128,7 @@ def parse_subparser_arguments(unparsed_arguments, subparsers): # Now ask each subparser, one by one, to greedily consume arguments, from last to first. This # allows subparsers to consume arguments before their parent subparsers do. - remaining_subparser_arguments = [] - - for subparser_name, subparser in reversed(subparsers.items()): - if subparser_name not in arguments.keys(): - continue - - subparser = subparsers[subparser_name] - unused_parsed, remaining = subparser.parse_known_args( - [argument for argument in unparsed_arguments if argument != subparser_name] - ) - remaining_subparser_arguments.append(remaining) - - # Determine the remaining arguments that no subparsers have consumed. - if remaining_subparser_arguments: - remaining_arguments = [ - argument - for argument in dict.fromkeys( - itertools.chain.from_iterable(remaining_subparser_arguments) - ).keys() - if all( - argument in subparser_arguments - for subparser_arguments in remaining_subparser_arguments - ) - ] + remaining_arguments = parse_subparser_arguments(arguments, unparsed_arguments, subparsers) # Special case: If "borg" is present in the arguments, consume all arguments after (+1) the # "borg" action. @@ -973,11 +981,9 @@ def make_parsers(): for name, subparser in subparsers.choices.items(): merged_subparsers._name_parser_map[name] = subparser - subparser._name_parser_map = merged_subparsers._name_parser_map for name, subparser in config_subparsers.choices.items(): merged_subparsers._name_parser_map[name] = subparser - subparser._name_parser_map = merged_subparsers._name_parser_map return top_level_parser, merged_subparsers diff --git a/tests/unit/commands/test_arguments.py b/tests/unit/commands/test_arguments.py index fafec39..c532022 100644 --- a/tests/unit/commands/test_arguments.py +++ b/tests/unit/commands/test_arguments.py @@ -175,3 +175,22 @@ def test_parse_subparser_arguments_raises_error_when_no_subparser_is_specified() with pytest.raises(ValueError): module.parse_subparser_arguments(('config',), subparsers) + + +@pytest.mark.parametrize( + 'arguments, unparsed_arguments, subparsers, expected_remaining_arguments', + [ + ( + {'action': flexmock()}, + ['--verbosity', 'lots'], + {'action': flexmock(parse_known_args=lambda arguments: (flexmock(), ['--verbosity', 'lots']))}, + ['--verbosity', 'lots'], + ), + ], +) +def test_get_remaining_arguments_returns_expected_remaining_arguments( + arguments, unparsed_arguments, subparsers, expected_remaining_arguments +): + remaining_arguments = module.get_remaining_arguments(arguments, unparsed_arguments, subparsers) + + assert remaining_arguments == expected_remaining_arguments \ No newline at end of file From 3315555d06e4690a9905a42de77dae870db47372 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Fri, 9 Jun 2023 00:21:41 +0530 Subject: [PATCH 22/25] cleaner test --- borgmatic/commands/arguments.py | 29 +++++++++++----------- tests/unit/commands/test_arguments.py | 35 ++++++++++++++++++--------- 2 files changed, 39 insertions(+), 25 deletions(-) diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index b68590b..db50674 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -28,19 +28,7 @@ SUBPARSER_ALIASES = { } -def parse_subparser_arguments(arguments, unparsed_arguments, subparsers): - remaining_subparser_arguments = [] - - for subparser_name, subparser in reversed(subparsers.items()): - if subparser_name not in arguments.keys(): - continue - - subparser = subparsers[subparser_name] - unused_parsed, remaining = subparser.parse_known_args( - [argument for argument in unparsed_arguments if argument != subparser_name] - ) - remaining_subparser_arguments.append(remaining) - +def get_unparsable_arguments(remaining_subparser_arguments): # Determine the remaining arguments that no subparsers have consumed. if remaining_subparser_arguments: remaining_arguments = [ @@ -128,7 +116,20 @@ def parse_subparser_arguments(unparsed_arguments, subparsers): # Now ask each subparser, one by one, to greedily consume arguments, from last to first. This # allows subparsers to consume arguments before their parent subparsers do. - remaining_arguments = parse_subparser_arguments(arguments, unparsed_arguments, subparsers) + remaining_subparser_arguments = [] + + for subparser_name, subparser in reversed(subparsers.items()): + if subparser_name not in arguments.keys(): + continue + + subparser = subparsers[subparser_name] + unused_parsed, remaining = subparser.parse_known_args( + [argument for argument in unparsed_arguments if argument != subparser_name] + ) + remaining_subparser_arguments.append(remaining) + + if remaining_subparser_arguments: + remaining_arguments = get_unparsable_arguments(remaining_subparser_arguments) # Special case: If "borg" is present in the arguments, consume all arguments after (+1) the # "borg" action. diff --git a/tests/unit/commands/test_arguments.py b/tests/unit/commands/test_arguments.py index c532022..ef8fd1e 100644 --- a/tests/unit/commands/test_arguments.py +++ b/tests/unit/commands/test_arguments.py @@ -178,19 +178,32 @@ def test_parse_subparser_arguments_raises_error_when_no_subparser_is_specified() @pytest.mark.parametrize( - 'arguments, unparsed_arguments, subparsers, expected_remaining_arguments', - [ + 'arguments, expected', + [ ( - {'action': flexmock()}, - ['--verbosity', 'lots'], - {'action': flexmock(parse_known_args=lambda arguments: (flexmock(), ['--verbosity', 'lots']))}, - ['--verbosity', 'lots'], + ( + ('--latest', 'archive', 'prune', 'extract', 'list', '--test-flag'), + ('--latest', 'archive', 'check', 'extract', 'list', '--test-flag'), + ('prune', 'check', 'list', '--test-flag'), + ('prune', 'check', 'extract', '--test-flag'), + ), + [ + '--test-flag', + ], ), + ( + ( + ('--latest', 'archive', 'prune', 'extract', 'list'), + ('--latest', 'archive', 'check', 'extract', 'list'), + ('prune', 'check', 'list'), + ('prune', 'check', 'extract'), + ), + [], + ), + ((), []), ], ) -def test_get_remaining_arguments_returns_expected_remaining_arguments( - arguments, unparsed_arguments, subparsers, expected_remaining_arguments +def test_get_unparsable_arguments_returns_remaining_arguments_that_no_subparser_can_parse( + arguments, expected ): - remaining_arguments = module.get_remaining_arguments(arguments, unparsed_arguments, subparsers) - - assert remaining_arguments == expected_remaining_arguments \ No newline at end of file + assert module.get_unparsable_arguments(arguments) == expected From 425f260a22f0c74b214b3cfd1d59ea8da4c96f2f Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Fri, 9 Jun 2023 04:15:18 +0530 Subject: [PATCH 23/25] test parser merging --- tests/integration/commands/test_arguments.py | 32 ++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/integration/commands/test_arguments.py b/tests/integration/commands/test_arguments.py index 26b94c6..8c96ac9 100644 --- a/tests/integration/commands/test_arguments.py +++ b/tests/integration/commands/test_arguments.py @@ -1,3 +1,5 @@ +import argparse + import pytest from flexmock import flexmock @@ -530,3 +532,33 @@ def test_parse_arguments_extract_with_check_only_extract_does_not_raise(): flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) module.parse_arguments('extract', '--archive', 'name', 'check', '--only', 'extract') + + +def test_merging_two_subparser_collections_merges_their_choices(): + top_level_parser = argparse.ArgumentParser() + + subparsers = top_level_parser.add_subparsers() + + subparser1 = subparsers.add_parser('subparser1') + + subparser2 = subparsers.add_parser('subparser2') + + subsubparsers = subparser2.add_subparsers() + + subsubparser1 = subsubparsers.add_parser('subsubparser1') + + merged_subparsers = argparse._SubParsersAction( + None, None, metavar=None, dest='merged', parser_class=None + ) + + for name, subparser in subparsers.choices.items(): + merged_subparsers._name_parser_map[name] = subparser + + for name, subparser in subsubparsers.choices.items(): + merged_subparsers._name_parser_map[name] = subparser + + assert merged_subparsers.choices == { + 'subparser1': subparser1, + 'subparser2': subparser2, + 'subsubparser1': subsubparser1, + } From 197920d9efa9e0e823e1bdf7e87ff2b56747867e Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Fri, 9 Jun 2023 17:31:57 +0530 Subject: [PATCH 24/25] improve tests and some docstrings. --- borgmatic/actions/config/bootstrap.py | 10 +++++++ borgmatic/commands/arguments.py | 29 ++++++++++++++------ borgmatic/commands/borgmatic.py | 1 + tests/integration/commands/test_arguments.py | 9 +----- 4 files changed, 33 insertions(+), 16 deletions(-) diff --git a/borgmatic/actions/config/bootstrap.py b/borgmatic/actions/config/bootstrap.py index 5b32345..9254da6 100644 --- a/borgmatic/actions/config/bootstrap.py +++ b/borgmatic/actions/config/bootstrap.py @@ -12,6 +12,16 @@ logger = logging.getLogger(__name__) def get_config_paths(bootstrap_arguments, global_arguments, local_borg_version): + ''' + Given: + The bootstrap arguments, which include the repository and archive name, borgmatic source directory, + destination directory, and whether to strip components. + The global arguments, which include the dry run flag + and the local borg version, + Return: + The config paths from the manifest.json file in the borgmatic source directory after extracting it from the + repository. + ''' borgmatic_source_directory = ( bootstrap_arguments.borgmatic_source_directory or DEFAULT_BORGMATIC_SOURCE_DIRECTORY ) diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index db50674..c39198c 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -29,7 +29,9 @@ SUBPARSER_ALIASES = { def get_unparsable_arguments(remaining_subparser_arguments): - # Determine the remaining arguments that no subparsers have consumed. + ''' + Determine the remaining arguments that no subparsers have consumed. + ''' if remaining_subparser_arguments: remaining_arguments = [ argument @@ -590,7 +592,7 @@ def make_parsers(): 'bootstrap', aliases=SUBPARSER_ALIASES['config_bootstrap'], help='Extract the config files used to create a borgmatic repository', - description='Extract just the config files that were used to create a borgmatic repository during the "create" operation', + description='Extract config files that were used to create a borgmatic repository during the "create" operation', add_help=False, ) config_bootstrap_group = config_bootstrap_parser.add_argument_group( @@ -603,7 +605,7 @@ def make_parsers(): ) config_bootstrap_group.add_argument( '--borgmatic-source-directory', - help='Path that stores the config files used to create an archive, and additional source files used for temporary internal state like borgmatic database dumps. Defaults to ~/.borgmatic', + help='Path that stores the config files used to create an archive and additional source files used for temporary internal state like borgmatic database dumps. Defaults to ~/.borgmatic', ) config_bootstrap_group.add_argument( '--archive', @@ -980,15 +982,26 @@ def make_parsers(): None, None, metavar=None, dest='merged', parser_class=None ) - for name, subparser in subparsers.choices.items(): - merged_subparsers._name_parser_map[name] = subparser - - for name, subparser in config_subparsers.choices.items(): - merged_subparsers._name_parser_map[name] = subparser + merged_subparsers = merge_subparsers(subparsers, config_subparsers) return top_level_parser, merged_subparsers +def merge_subparsers(*subparsers): + ''' + Merge multiple subparsers into a single subparser. + ''' + merged_subparsers = argparse._SubParsersAction( + None, None, metavar=None, dest='merged', parser_class=None + ) + + for subparser in subparsers: + for name, subparser in subparser.choices.items(): + merged_subparsers._name_parser_map[name] = subparser + + return merged_subparsers + + def parse_arguments(*unparsed_arguments): ''' Given command-line arguments with which this script was invoked, parse the arguments and return diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 766073c..4ad8136 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -645,6 +645,7 @@ def collect_configuration_run_summary_logs(configs, arguments): KeyError, ) as error: yield from log_error_records('Error running bootstrap', error) + return if not configs: diff --git a/tests/integration/commands/test_arguments.py b/tests/integration/commands/test_arguments.py index 8c96ac9..60fe884 100644 --- a/tests/integration/commands/test_arguments.py +++ b/tests/integration/commands/test_arguments.py @@ -538,24 +538,17 @@ def test_merging_two_subparser_collections_merges_their_choices(): top_level_parser = argparse.ArgumentParser() subparsers = top_level_parser.add_subparsers() - subparser1 = subparsers.add_parser('subparser1') subparser2 = subparsers.add_parser('subparser2') - subsubparsers = subparser2.add_subparsers() - subsubparser1 = subsubparsers.add_parser('subsubparser1') merged_subparsers = argparse._SubParsersAction( None, None, metavar=None, dest='merged', parser_class=None ) - for name, subparser in subparsers.choices.items(): - merged_subparsers._name_parser_map[name] = subparser - - for name, subparser in subsubparsers.choices.items(): - merged_subparsers._name_parser_map[name] = subparser + merged_subparsers = module.merge_subparsers(subparsers, subsubparsers) assert merged_subparsers.choices == { 'subparser1': subparser1, From d370ff958d04bc43a31f38f6234430e1d586609c Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Sat, 10 Jun 2023 01:05:34 +0530 Subject: [PATCH 25/25] mock expand directories thrice --- tests/unit/borg/test_create.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/unit/borg/test_create.py b/tests/unit/borg/test_create.py index 157ae9d..c798c44 100644 --- a/tests/unit/borg/test_create.py +++ b/tests/unit/borg/test_create.py @@ -594,7 +594,11 @@ def test_create_archive_with_sources_and_used_config_paths_calls_borg_with_sourc ('foo', 'bar', '/etc/borgmatic/config.yaml') ) flexmock(module).should_receive('map_directories_to_devices').and_return({}) - flexmock(module).should_receive('expand_directories').and_return(()) + flexmock(module).should_receive('expand_directories').with_args([]).and_return(()) + flexmock(module).should_receive('expand_directories').with_args( + ('foo', 'bar', '/etc/borgmatic/config.yaml') + ).and_return(('foo', 'bar', '/etc/borgmatic/config.yaml')) + flexmock(module).should_receive('expand_directories').with_args([]).and_return(()) flexmock(module).should_receive('pattern_root_directories').and_return([]) flexmock(module.os.path).should_receive('expanduser').and_raise(TypeError) flexmock(module).should_receive('expand_home_directories').and_return(())