diff --git a/python/mach/mach/base.py b/python/mach/mach/base.py index 3e8e6477357..be031f9adfc 100644 --- a/python/mach/mach/base.py +++ b/python/mach/mach/base.py @@ -77,9 +77,6 @@ class MethodHandler(object): # Description of the purpose of this command. 'description', - # Whether to allow all arguments from the parser. - 'allow_all_arguments', - # Functions used to 'skip' commands if they don't meet the conditions # in a given context. 'conditions', @@ -94,15 +91,13 @@ class MethodHandler(object): ) def __init__(self, cls, method, name, category=None, description=None, - allow_all_arguments=False, conditions=None, parser=None, arguments=None, - pass_context=False): + conditions=None, parser=None, arguments=None, pass_context=False): self.cls = cls self.method = method self.name = name self.category = category self.description = description - self.allow_all_arguments = allow_all_arguments self.conditions = conditions or [] self.parser = parser self.arguments = arguments or [] diff --git a/python/mach/mach/decorators.py b/python/mach/mach/decorators.py index cb9a67428e7..3b60e890325 100644 --- a/python/mach/mach/decorators.py +++ b/python/mach/mach/decorators.py @@ -4,6 +4,7 @@ from __future__ import unicode_literals +import argparse import collections import inspect import types @@ -56,8 +57,8 @@ def CommandProvider(cls): if not isinstance(value, types.FunctionType): continue - command_name, category, description, allow_all, conditions, parser = getattr( - value, '_mach_command', (None, None, None, None, None, None)) + command_name, category, description, conditions, parser = getattr( + value, '_mach_command', (None, None, None, None, None)) if command_name is None: continue @@ -82,9 +83,8 @@ def CommandProvider(cls): arguments = getattr(value, '_mach_command_args', None) handler = MethodHandler(cls, attr, command_name, category=category, - description=description, allow_all_arguments=allow_all, - conditions=conditions, parser=parser, arguments=arguments, - pass_context=pass_context) + description=description, conditions=conditions, parser=parser, + arguments=arguments, pass_context=pass_context) Registrar.register_command_handler(handler) @@ -102,9 +102,6 @@ class Command(object): description -- A brief description of what the command does. - allow_all_args -- Bool indicating whether to allow unknown arguments - through to the command. - parser -- an optional argparse.ArgumentParser instance to use as the basis for the command arguments. @@ -114,18 +111,17 @@ class Command(object): def foo(self): pass """ - def __init__(self, name, category=None, description=None, - allow_all_args=False, conditions=None, parser=None): + def __init__(self, name, category=None, description=None, conditions=None, + parser=None): self._name = name self._category = category self._description = description - self._allow_all_args = allow_all_args self._conditions = conditions self._parser = parser def __call__(self, func): func._mach_command = (self._name, self._category, self._description, - self._allow_all_args, self._conditions, self._parser) + self._conditions, self._parser) return func @@ -145,6 +141,11 @@ class CommandArgument(object): pass """ def __init__(self, *args, **kwargs): + if kwargs.get('nargs') == argparse.REMAINDER: + # These are the assertions we make in dispatcher.py about + # those types of CommandArguments. + assert len(args) == 1 + assert all(k in ('default', 'nargs', 'help') for k in kwargs) self._command_args = (args, kwargs) def __call__(self, func): diff --git a/python/mach/mach/dispatcher.py b/python/mach/mach/dispatcher.py index 11e58cb9e2f..5aa5c7631f9 100644 --- a/python/mach/mach/dispatcher.py +++ b/python/mach/mach/dispatcher.py @@ -135,19 +135,27 @@ class CommandAction(argparse.Action): parser_args = { 'add_help': False, 'usage': '%(prog)s [global arguments] ' + command + - ' command arguments]', + ' [command arguments]', } - if handler.allow_all_arguments: - parser_args['prefix_chars'] = '+' - if handler.parser: subparser = handler.parser else: subparser = argparse.ArgumentParser(**parser_args) + remainder = None + for arg in handler.arguments: - subparser.add_argument(*arg[0], **arg[1]) + if arg[1].get('nargs') == argparse.REMAINDER: + # parse_known_args expects all argparse.REMAINDER ('...') + # arguments to be all stuck together. Instead, we want them to + # pick any extra argument, wherever they are. + # Assume a limited CommandArgument for those arguments. + assert len(arg[0]) == 1 + assert all(k in ('default', 'nargs', 'help') for k in arg[1]) + remainder = arg + else: + subparser.add_argument(*arg[0], **arg[1]) # We define the command information on the main parser result so as to # not interfere with arguments passed to the command. @@ -156,7 +164,32 @@ class CommandAction(argparse.Action): command_namespace, extra = subparser.parse_known_args(args) setattr(namespace, 'command_args', command_namespace) - if extra: + if remainder: + (name,), options = remainder + # parse_known_args usefully puts all arguments after '--' in + # extra, but also puts '--' there. We don't want to pass it down + # to the command handler. Note that if multiple '--' are on the + # command line, only the first one is removed, so that subsequent + # ones are passed down. + if '--' in extra: + extra.remove('--') + + # Commands with argparse.REMAINDER arguments used to force the + # other arguments to be '+' prefixed. If a user now passes such + # an argument, if will silently end up in extra. So, check if any + # of the allowed arguments appear in a '+' prefixed form, and error + # out if that's the case. + for args, _ in handler.arguments: + for arg in args: + arg = arg.replace('-', '+', 1) + if arg in extra: + raise UnrecognizedArgumentError(command, [arg]) + + if extra: + setattr(command_namespace, name, extra) + else: + setattr(command_namespace, name, options.get('default', [])) + elif extra: raise UnrecognizedArgumentError(command, extra) def _handle_main_help(self, parser, verbose): @@ -234,9 +267,6 @@ class CommandAction(argparse.Action): 'add_help': False, } - if handler.allow_all_arguments: - parser_args['prefix_chars'] = '+' - if handler.parser: c_parser = handler.parser c_parser.formatter_class = NoUsageFormatter diff --git a/python/mach_commands.py b/python/mach_commands.py index 2b97c3254a8..aa25787a427 100644 --- a/python/mach_commands.py +++ b/python/mach_commands.py @@ -25,7 +25,6 @@ from mach.decorators import ( @CommandProvider class MachCommands(MachCommandBase): @Command('python', category='devenv', - allow_all_args=True, description='Run Python.') @CommandArgument('args', nargs=argparse.REMAINDER) def python(self, args): diff --git a/python/mozbuild/mozbuild/mach_commands.py b/python/mozbuild/mozbuild/mach_commands.py index 45f14713f35..4a0e1096e4b 100644 --- a/python/mozbuild/mozbuild/mach_commands.py +++ b/python/mozbuild/mozbuild/mach_commands.py @@ -810,24 +810,21 @@ def get_run_args(mach_command, params, remote, background, noprofile): if params: args.extend(params) - if '--' in args: - args.remove('--') - return args @CommandProvider class RunProgram(MachCommandBase): """Launch the compiled binary""" - @Command('run', category='post-build', allow_all_args=True, + @Command('run', category='post-build', description='Run the compiled program.') - @CommandArgument('params', default=None, nargs='...', + @CommandArgument('params', nargs='...', help='Command-line arguments to be passed through to the program. Not specifying a -profile or -P option will result in a temporary profile being used.') - @CommandArgument('+remote', '+r', action='store_true', + @CommandArgument('-remote', '-r', action='store_true', help='Do not pass the -no-remote argument by default.') - @CommandArgument('+background', '+b', action='store_true', + @CommandArgument('-background', '-b', action='store_true', help='Do not pass the -foreground argument by default on Mac') - @CommandArgument('+noprofile', '+n', action='store_true', + @CommandArgument('-noprofile', '-n', action='store_true', help='Do not pass the -profile argument by default.') def run(self, params, remote, background, noprofile): args = get_run_args(self, params, remote, background, noprofile) @@ -841,26 +838,26 @@ class RunProgram(MachCommandBase): class DebugProgram(MachCommandBase): """Debug the compiled binary""" - @Command('debug', category='post-build', allow_all_args=True, + @Command('debug', category='post-build', description='Debug the compiled program.') - @CommandArgument('params', default=None, nargs='...', + @CommandArgument('params', nargs='...', help='Command-line arguments to be passed through to the program. Not specifying a -profile or -P option will result in a temporary profile being used.') - @CommandArgument('+remote', '+r', action='store_true', + @CommandArgument('-remote', '-r', action='store_true', help='Do not pass the -no-remote argument by default') - @CommandArgument('+background', '+b', action='store_true', + @CommandArgument('-background', '-b', action='store_true', help='Do not pass the -foreground argument by default on Mac') - @CommandArgument('+debugger', default=None, type=str, + @CommandArgument('-debugger', default=None, type=str, help='Name of debugger to launch') - @CommandArgument('+debugparams', default=None, metavar='params', type=str, + @CommandArgument('-debugparams', default=None, metavar='params', type=str, help='Command-line arguments to pass to the debugger itself; split as the Bourne shell would.') # Bug 933807 introduced JS_DISABLE_SLOW_SCRIPT_SIGNALS to avoid clever # segfaults induced by the slow-script-detecting logic for Ion/Odin JITted # code. If we don't pass this, the user will need to periodically type # "continue" to (safely) resume execution. There are ways to implement # automatic resuming; see the bug. - @CommandArgument('+slowscript', action='store_true', + @CommandArgument('-slowscript', action='store_true', help='Do not set the JS_DISABLE_SLOW_SCRIPT_SIGNALS env variable; when not set, recoverable but misleading SIGSEGV instances may occur in Ion/Odin JIT code') - @CommandArgument('+noprofile', '+n', action='store_true', + @CommandArgument('-noprofile', '-n', action='store_true', help='Do not pass the -profile argument by default.') def debug(self, params, remote, background, debugger, debugparams, slowscript, noprofile): # Parameters come from the CLI. We need to convert them before their use. @@ -868,7 +865,7 @@ class DebugProgram(MachCommandBase): import pymake.process argv, badchar = pymake.process.clinetoargv(debugparams, os.getcwd()) if badchar: - print("The +debugparams you passed require a real shell to parse them.") + print("The -debugparams you passed require a real shell to parse them.") print("(We can't handle the %r character.)" % (badchar,)) return 1 debugparams = argv; @@ -925,7 +922,7 @@ class RunDmd(MachCommandBase): @Command('dmd', category='post-build', description='Run the compiled program with DMD enabled.') - @CommandArgument('params', default=None, nargs='...', + @CommandArgument('params', nargs='...', help=('Command-line arguments to be passed through to the program. ' 'Not specifying a -profile or -P option will result in a ' 'temporary profile being used. If passing -params use a "--" to ' diff --git a/tools/mach_commands.py b/tools/mach_commands.py index 9e4481a7c31..7263dc70df1 100644 --- a/tools/mach_commands.py +++ b/tools/mach_commands.py @@ -294,7 +294,7 @@ class PastebinProvider(object): @CommandProvider class ReviewboardToolsProvider(MachCommandBase): - @Command('rbt', category='devenv', allow_all_args=True, + @Command('rbt', category='devenv', description='Run Reviewboard Tools') @CommandArgument('args', nargs='...', help='Arguments to rbt tool') def rbt(self, args):