Fix edge case in which "--config somepath.yaml" followed by an action alias (e.g. init for rcreate) wasn't parsed correctly (#716).

This commit is contained in:
Dan Helfman 2023-06-24 15:35:10 -07:00
parent 8debcbeaba
commit b62017be4b
7 changed files with 37 additions and 21 deletions

View file

@ -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) arguments['global'], remaining = global_parser.parse_known_args(unparsed_arguments)
remaining_action_arguments.append(remaining) 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 ( return (
arguments, arguments,
tuple(remaining_action_arguments) if arguments else unparsed_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)}" 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: if arguments['global'].excludes_filename:
raise ValueError( raise ValueError(
'The --excludes flag has been replaced with exclude_patterns in configuration.' 'The --excludes flag has been replaced with exclude_patterns in configuration.'

View file

@ -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 to work for testing (including injecting the given repository path and tacking on an encryption
passphrase). passphrase).
''' '''
subprocess.check_call(f'generate-borgmatic-config --destination {config_path}'.split(' ')) subprocess.check_call(f'borgmatic config generate --destination {config_path}'.split(' '))
config = ( config = (
open(config_path) open(config_path)
.read() .read()

View file

@ -8,9 +8,9 @@ def test_generate_borgmatic_config_with_merging_succeeds():
config_path = os.path.join(temporary_directory, 'test.yaml') config_path = os.path.join(temporary_directory, 'test.yaml')
new_config_path = os.path.join(temporary_directory, 'new.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( 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(
' ' ' '
) )
) )

View file

@ -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 to work for testing (including injecting the given repository path and tacking on an encryption
passphrase). passphrase).
''' '''
subprocess.check_call(f'generate-borgmatic-config --destination {config_path}'.split(' ')) subprocess.check_call(f'borgmatic config generate --destination {config_path}'.split(' '))
config = ( config = (
open(config_path) open(config_path)
.read() .read()

View file

@ -8,7 +8,7 @@ def test_validate_config_command_with_valid_configuration_succeeds():
with tempfile.TemporaryDirectory() as temporary_directory: with tempfile.TemporaryDirectory() as temporary_directory:
config_path = os.path.join(temporary_directory, 'test.yaml') 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(' ')) exit_code = subprocess.call(f'validate-borgmatic-config --config {config_path}'.split(' '))
assert exit_code == 0 assert exit_code == 0
@ -18,7 +18,7 @@ def test_validate_config_command_with_invalid_configuration_fails():
with tempfile.TemporaryDirectory() as temporary_directory: with tempfile.TemporaryDirectory() as temporary_directory:
config_path = os.path.join(temporary_directory, 'test.yaml') 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 = open(config_path).read().replace('keep_daily: 7', 'keep_daily: "7"')
config_file = open(config_path, 'w') config_file = open(config_path, 'w')
config_file.write(config) config_file.write(config)
@ -33,7 +33,7 @@ def test_validate_config_command_with_show_flag_displays_configuration():
with tempfile.TemporaryDirectory() as temporary_directory: with tempfile.TemporaryDirectory() as temporary_directory:
config_path = os.path.join(temporary_directory, 'test.yaml') 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( output = subprocess.check_output(
f'validate-borgmatic-config --config {config_path} --show'.split(' ') f'validate-borgmatic-config --config {config_path} --show'.split(' ')
).decode(sys.stdout.encoding) ).decode(sys.stdout.encoding)

View file

@ -41,6 +41,17 @@ def test_parse_arguments_with_action_after_config_path_omits_action():
assert arguments['list'].json 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(): def test_parse_arguments_with_verbosity_overrides_default():
config_paths = ['default'] config_paths = ['default']
flexmock(module.collect).should_receive('get_default_config_paths').and_return(config_paths) flexmock(module.collect).should_receive('get_default_config_paths').and_return(config_paths)

View file

@ -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({}) flexmock(module).should_receive('get_subactions_for_actions').and_return({})
action_parsers = {'action': flexmock(), 'other': flexmock()} action_parsers = {'action': flexmock(), 'other': flexmock()}
global_namespace = flexmock() global_namespace = flexmock(config_paths=[])
global_parser = flexmock() global_parser = flexmock()
global_parser.should_receive('parse_known_args').and_return((global_namespace, ())) 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(), 'other': flexmock(),
'-o': flexmock(), '-o': flexmock(),
} }
global_namespace = flexmock() global_namespace = flexmock(config_paths=[])
global_parser = flexmock() global_parser = flexmock()
global_parser.should_receive('parse_known_args').and_return((global_namespace, ())) global_parser.should_receive('parse_known_args').and_return((global_namespace, ()))
flexmock(module).ACTION_ALIASES = {'action': ['-a'], 'other': ['-o']} flexmock(module).ACTION_ALIASES = {'action': ['-a'], 'other': ['-o']}
@ -232,7 +232,7 @@ def test_parse_arguments_for_actions_consumes_multiple_action_arguments():
'action': flexmock(), 'action': flexmock(),
'other': flexmock(), 'other': flexmock(),
} }
global_namespace = flexmock() global_namespace = flexmock(config_paths=[])
global_parser = flexmock() global_parser = flexmock()
global_parser.should_receive('parse_known_args').and_return((global_namespace, ())) 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(), 'action': flexmock(),
'other': flexmock(), 'other': flexmock(),
} }
global_namespace = flexmock() global_namespace = flexmock(config_paths=[])
global_parser = flexmock() global_parser = flexmock()
global_parser.should_receive('parse_known_args').and_return((global_namespace, ())) 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(): def test_parse_arguments_for_actions_applies_default_action_parsers():
global_namespace = flexmock() global_namespace = flexmock(config_paths=[])
namespaces = { namespaces = {
'global': global_namespace, 'global': global_namespace,
'prune': flexmock(), 'prune': flexmock(),
@ -327,7 +327,7 @@ def test_parse_arguments_for_actions_consumes_global_arguments():
'action': flexmock(), 'action': flexmock(),
'other': flexmock(), 'other': flexmock(),
} }
global_namespace = flexmock() global_namespace = flexmock(config_paths=[])
global_parser = flexmock() global_parser = flexmock()
global_parser.should_receive('parse_known_args').and_return((global_namespace, ())) 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(), 'action': flexmock(),
'other': flexmock(), 'other': flexmock(),
} }
global_namespace = flexmock() global_namespace = flexmock(config_paths=[])
global_parser = flexmock() global_parser = flexmock()
global_parser.should_receive('parse_known_args').and_return((global_namespace, ())) 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(), 'action': flexmock(),
'other': flexmock(), 'other': flexmock(),
} }
global_namespace = flexmock() global_namespace = flexmock(config_paths=[])
global_parser = flexmock() global_parser = flexmock()
global_parser.should_receive('parse_known_args').and_return((global_namespace, ())) 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(), 'borg': flexmock(),
'list': flexmock(), 'list': flexmock(),
} }
global_namespace = flexmock() global_namespace = flexmock(config_paths=[])
global_parser = flexmock() global_parser = flexmock()
global_parser.should_receive('parse_known_args').and_return((global_namespace, ())) global_parser.should_receive('parse_known_args').and_return((global_namespace, ()))