Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-11-28 23:31:56 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1076649 - Remove the '+' prefixing from mach commands with allow_all_arguments=True. r=gps

Browse Source 
The reason to use '+' prefixing was to distinguish between options to the
mach command itself, and options that are passed down to whatever the
command does (like mach run passing down args to the built application).
That makes things unnecessarily awkward, and quite non-standard.

Instead, use standard '-' prefixing, and pass all the unknown arguments
down. If there is overlap between the known arguments and arguments supported
by the underlying tool (like -remote when using mach run), it is possible to
use '--' to mark all following arguments as being targetted at the underlying
tool.

For instance:
    mach run -- -remote something
would run
    firefox -remote something
while
    mach run -remote something
would run
    firefox something

As allow_all_arguments is redundant with the presence of a argparse.REMAINDER
CommandArgument, allow_all_arguments is removed. The only mach command with a
argparse.REMAINDER CommandArgument without allow_all_arguments was "mach dmd",
and it did so because it didn't want to use '+' prefixes.
This commit is contained in:
Mike Hommey 2014-10-07 07:36:27 +09:00
parent 68bb3e1d0c
commit 417a2977b3
6 changed files with 69 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
python/mach/mach/base.py
View File

@ -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 []

25
python/mach/mach/decorators.py
View File

@ -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):

48
python/mach/mach/dispatcher.py
View File

@ -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

1
python/mach_commands.py
View File

@ -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):

33
python/mozbuild/mozbuild/mach_commands.py
View File

@ -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 '

2
tools/mach_commands.py
View File

@ -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):