From b62017be4bbeae110f813d3bece3f51b49740522 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sat, 24 Jun 2023 15:35:10 -0700 Subject: [PATCH] Fix edge case in which "--config somepath.yaml" followed by an action alias (e.g. init for rcreate) wasn't parsed correctly (#716). --- borgmatic/commands/arguments.py | 15 ++++++++++----- tests/end-to-end/test_borgmatic.py | 2 +- tests/end-to-end/test_generate_config.py | 4 ++-- tests/end-to-end/test_override.py | 2 +- tests/end-to-end/test_validate_config.py | 6 +++--- tests/integration/commands/test_arguments.py | 11 +++++++++++ tests/unit/commands/test_arguments.py | 18 +++++++++--------- 7 files changed, 37 insertions(+), 21 deletions(-) diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index eac88c8..6b857f2 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -216,6 +216,16 @@ def parse_arguments_for_actions(unparsed_arguments, action_parsers, global_parse arguments['global'], remaining = global_parser.parse_known_args(unparsed_arguments) remaining_action_arguments.append(remaining) + # Prevent action names that follow "--config" paths from being considered as additional paths. + for argument_name in arguments.keys(): + if argument_name == 'global': + continue + + for action_name in [argument_name] + ACTION_ALIASES.get(argument_name, []): + if action_name in arguments['global'].config_paths: + arguments['global'].config_paths.remove(action_name) + break + return ( arguments, tuple(remaining_action_arguments) if arguments else unparsed_arguments, @@ -1263,11 +1273,6 @@ def parse_arguments(*unparsed_arguments): f"Unrecognized argument{'s' if len(unknown_arguments) > 1 else ''}: {' '.join(unknown_arguments)}" ) - # Prevent action names that follow "--config" paths from being considered as additional paths. - for argument_name in arguments.keys(): - if argument_name != 'global' and argument_name in arguments['global'].config_paths: - arguments['global'].config_paths.remove(argument_name) - if arguments['global'].excludes_filename: raise ValueError( 'The --excludes flag has been replaced with exclude_patterns in configuration.' diff --git a/tests/end-to-end/test_borgmatic.py b/tests/end-to-end/test_borgmatic.py index 5e915f0..f2b81a8 100644 --- a/tests/end-to-end/test_borgmatic.py +++ b/tests/end-to-end/test_borgmatic.py @@ -12,7 +12,7 @@ def generate_configuration(config_path, repository_path): to work for testing (including injecting the given repository path and tacking on an encryption passphrase). ''' - subprocess.check_call(f'generate-borgmatic-config --destination {config_path}'.split(' ')) + subprocess.check_call(f'borgmatic config generate --destination {config_path}'.split(' ')) config = ( open(config_path) .read() diff --git a/tests/end-to-end/test_generate_config.py b/tests/end-to-end/test_generate_config.py index b8cade9..c9293b7 100644 --- a/tests/end-to-end/test_generate_config.py +++ b/tests/end-to-end/test_generate_config.py @@ -8,9 +8,9 @@ def test_generate_borgmatic_config_with_merging_succeeds(): config_path = os.path.join(temporary_directory, 'test.yaml') new_config_path = os.path.join(temporary_directory, 'new.yaml') - subprocess.check_call(f'generate-borgmatic-config --destination {config_path}'.split(' ')) + subprocess.check_call(f'borgmatic config generate --destination {config_path}'.split(' ')) subprocess.check_call( - f'generate-borgmatic-config --source {config_path} --destination {new_config_path}'.split( + f'borgmatic config generate --source {config_path} --destination {new_config_path}'.split( ' ' ) ) diff --git a/tests/end-to-end/test_override.py b/tests/end-to-end/test_override.py index e86186d..e91e28b 100644 --- a/tests/end-to-end/test_override.py +++ b/tests/end-to-end/test_override.py @@ -10,7 +10,7 @@ def generate_configuration(config_path, repository_path): to work for testing (including injecting the given repository path and tacking on an encryption passphrase). ''' - subprocess.check_call(f'generate-borgmatic-config --destination {config_path}'.split(' ')) + subprocess.check_call(f'borgmatic config generate --destination {config_path}'.split(' ')) config = ( open(config_path) .read() diff --git a/tests/end-to-end/test_validate_config.py b/tests/end-to-end/test_validate_config.py index 5446503..4b86da4 100644 --- a/tests/end-to-end/test_validate_config.py +++ b/tests/end-to-end/test_validate_config.py @@ -8,7 +8,7 @@ def test_validate_config_command_with_valid_configuration_succeeds(): with tempfile.TemporaryDirectory() as temporary_directory: config_path = os.path.join(temporary_directory, 'test.yaml') - subprocess.check_call(f'generate-borgmatic-config --destination {config_path}'.split(' ')) + subprocess.check_call(f'borgmatic config generate --destination {config_path}'.split(' ')) exit_code = subprocess.call(f'validate-borgmatic-config --config {config_path}'.split(' ')) assert exit_code == 0 @@ -18,7 +18,7 @@ def test_validate_config_command_with_invalid_configuration_fails(): with tempfile.TemporaryDirectory() as temporary_directory: config_path = os.path.join(temporary_directory, 'test.yaml') - subprocess.check_call(f'generate-borgmatic-config --destination {config_path}'.split(' ')) + subprocess.check_call(f'borgmatic config generate --destination {config_path}'.split(' ')) config = open(config_path).read().replace('keep_daily: 7', 'keep_daily: "7"') config_file = open(config_path, 'w') config_file.write(config) @@ -33,7 +33,7 @@ def test_validate_config_command_with_show_flag_displays_configuration(): with tempfile.TemporaryDirectory() as temporary_directory: config_path = os.path.join(temporary_directory, 'test.yaml') - subprocess.check_call(f'generate-borgmatic-config --destination {config_path}'.split(' ')) + subprocess.check_call(f'borgmatic config generate --destination {config_path}'.split(' ')) output = subprocess.check_output( f'validate-borgmatic-config --config {config_path} --show'.split(' ') ).decode(sys.stdout.encoding) diff --git a/tests/integration/commands/test_arguments.py b/tests/integration/commands/test_arguments.py index 8914121..f992f9a 100644 --- a/tests/integration/commands/test_arguments.py +++ b/tests/integration/commands/test_arguments.py @@ -41,6 +41,17 @@ def test_parse_arguments_with_action_after_config_path_omits_action(): assert arguments['list'].json +def test_parse_arguments_with_action_after_config_path_omits_aliased_action(): + flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) + + arguments = module.parse_arguments('--config', 'myconfig', 'init', '--encryption', 'repokey') + + global_arguments = arguments['global'] + assert global_arguments.config_paths == ['myconfig'] + assert 'rcreate' in arguments + assert arguments['rcreate'].encryption_mode == 'repokey' + + def test_parse_arguments_with_verbosity_overrides_default(): config_paths = ['default'] flexmock(module.collect).should_receive('get_default_config_paths').and_return(config_paths) diff --git a/tests/unit/commands/test_arguments.py b/tests/unit/commands/test_arguments.py index 0969a6a..55c0eef 100644 --- a/tests/unit/commands/test_arguments.py +++ b/tests/unit/commands/test_arguments.py @@ -175,7 +175,7 @@ def test_parse_arguments_for_actions_consumes_action_arguments_after_action_name ) flexmock(module).should_receive('get_subactions_for_actions').and_return({}) action_parsers = {'action': flexmock(), 'other': flexmock()} - global_namespace = flexmock() + global_namespace = flexmock(config_paths=[]) global_parser = flexmock() global_parser.should_receive('parse_known_args').and_return((global_namespace, ())) @@ -204,7 +204,7 @@ def test_parse_arguments_for_actions_consumes_action_arguments_with_alias(): 'other': flexmock(), '-o': flexmock(), } - global_namespace = flexmock() + global_namespace = flexmock(config_paths=[]) global_parser = flexmock() global_parser.should_receive('parse_known_args').and_return((global_namespace, ())) flexmock(module).ACTION_ALIASES = {'action': ['-a'], 'other': ['-o']} @@ -232,7 +232,7 @@ def test_parse_arguments_for_actions_consumes_multiple_action_arguments(): 'action': flexmock(), 'other': flexmock(), } - global_namespace = flexmock() + global_namespace = flexmock(config_paths=[]) global_parser = flexmock() global_parser.should_receive('parse_known_args').and_return((global_namespace, ())) @@ -263,7 +263,7 @@ def test_parse_arguments_for_actions_respects_command_line_action_ordering(): 'action': flexmock(), 'other': flexmock(), } - global_namespace = flexmock() + global_namespace = flexmock(config_paths=[]) global_parser = flexmock() global_parser.should_receive('parse_known_args').and_return((global_namespace, ())) @@ -278,7 +278,7 @@ def test_parse_arguments_for_actions_respects_command_line_action_ordering(): def test_parse_arguments_for_actions_applies_default_action_parsers(): - global_namespace = flexmock() + global_namespace = flexmock(config_paths=[]) namespaces = { 'global': global_namespace, 'prune': flexmock(), @@ -327,7 +327,7 @@ def test_parse_arguments_for_actions_consumes_global_arguments(): 'action': flexmock(), 'other': flexmock(), } - global_namespace = flexmock() + global_namespace = flexmock(config_paths=[]) global_parser = flexmock() global_parser.should_receive('parse_known_args').and_return((global_namespace, ())) @@ -353,7 +353,7 @@ def test_parse_arguments_for_actions_passes_through_unknown_arguments_before_act 'action': flexmock(), 'other': flexmock(), } - global_namespace = flexmock() + global_namespace = flexmock(config_paths=[]) global_parser = flexmock() global_parser.should_receive('parse_known_args').and_return((global_namespace, ())) @@ -379,7 +379,7 @@ def test_parse_arguments_for_actions_passes_through_unknown_arguments_after_acti 'action': flexmock(), 'other': flexmock(), } - global_namespace = flexmock() + global_namespace = flexmock(config_paths=[]) global_parser = flexmock() global_parser.should_receive('parse_known_args').and_return((global_namespace, ())) @@ -405,7 +405,7 @@ def test_parse_arguments_for_actions_with_borg_action_skips_other_action_parsers 'borg': flexmock(), 'list': flexmock(), } - global_namespace = flexmock() + global_namespace = flexmock(config_paths=[]) global_parser = flexmock() global_parser.should_receive('parse_known_args').and_return((global_namespace, ()))