Bug 1176620 - Refactor how mach command metadata is stored; r=ahal
authorGregory Szorc <gps@mozilla.com>
Sun, 21 Jun 2015 12:59:29 -0700
changeset 268168 210526b4016b22ed788060d8d7be63c2256ec4c7
parent 268167 c0ecb2f5c0b12c0880e576ceaf9ae40014d73180
child 268169 968a784a76dacd9f8ff8917f34a9d479b1bc973e
push id4932
push userjlund@mozilla.com
push dateMon, 10 Aug 2015 18:23:06 +0000
treeherdermozilla-esr52@6dd5a4f5f745 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersahal
bugs1176620
milestone41.0a1
Bug 1176620 - Refactor how mach command metadata is stored; r=ahal Up to this point, mach command metadata has been stored in tuples. Initially, things weren't so bad. But they have evolved into tuples with many elements. Adding new attributes is cumbersome. Let's restructure the code to capture metadata in a dedicated class. Before, there existed a separate attribute on the @Command or @SubCommand decorated method for each mach decorator: @Command, @CommandArgument, @CommandArgumentGroup. With the magic of __ior__, we can now capture all metadata on a single type. This simplies processing, as we now only look at a single attribute on methods: _mach_command. Before, we used separate attributes to distinguish between mach commands and mach sub-commands. Now that we have a type that can hold all data, we combine things into the _mach_command attribute and look for the presence of the "subcommand" attribute on this type to identify sub-commands.
python/mach/mach/decorators.py
--- a/python/mach/mach/decorators.py
+++ b/python/mach/mach/decorators.py
@@ -13,16 +13,55 @@ from .base import (
     MachError,
     MethodHandler
 )
 
 from .config import ConfigProvider
 from .registrar import Registrar
 
 
+class _MachCommand(object):
+    """Container for mach command metadata.
+
+    Mach commands contain lots of attributes. This class exists to capture them
+    in a sane way so tuples, etc aren't used instead.
+    """
+    __slots__ = (
+        'name',
+        'subcommand',
+        'category',
+        'description',
+        'conditions',
+        'parser',
+        'arguments',
+        'argument_group_names',
+    )
+
+    def __init__(self, name=None, subcommand=None, category=None,
+                 description=None, conditions=None, parser=None):
+        self.name = name
+        self.subcommand = subcommand
+        self.category = category
+        self.description = description
+        self.conditions = conditions or []
+        self.parser = parser
+        self.arguments = []
+        self.argument_group_names = []
+
+    def __ior__(self, other):
+        if not isinstance(other, _MachCommand):
+            raise ValueError('can only operate on _MachCommand instances')
+
+        for a in self.__slots__:
+            if not getattr(self, a):
+                setattr(self, a, getattr(other, a))
+
+        return self
+
+
 def CommandProvider(cls):
     """Class decorator to denote that it provides subcommands for Mach.
 
     When this decorator is present, mach looks for commands being defined by
     methods inside the class.
     """
 
     # The implementation of this decorator relies on the parse-time behavior of
@@ -54,91 +93,94 @@ def CommandProvider(cls):
     # define commands multiple times. We also sort keys so commands defined in
     # the same class are grouped in a sane order.
     for attr in sorted(cls.__dict__.keys()):
         value = cls.__dict__[attr]
 
         if not isinstance(value, types.FunctionType):
             continue
 
-        command_name, category, description, conditions, parser = getattr(
-            value, '_mach_command', (None, None, None, None, None))
-
-        if command_name is None:
+        command = getattr(value, '_mach_command', None)
+        if not command:
             continue
 
-        seen_commands.add(command_name)
+        # Ignore subcommands for now: we handle them later.
+        if command.subcommand:
+            continue
 
-        if conditions is None and Registrar.require_conditions:
+        seen_commands.add(command.name)
+
+        if command.conditions is None and Registrar.require_conditions:
             continue
 
         msg = 'Mach command \'%s\' implemented incorrectly. ' + \
               'Conditions argument must take a list ' + \
               'of functions. Found %s instead.'
 
-        conditions = conditions or []
-        if not isinstance(conditions, collections.Iterable):
-            msg = msg % (command_name, type(conditions))
+        if not isinstance(command.conditions, collections.Iterable):
+            msg = msg % (command.name, type(command.conditions))
             raise MachError(msg)
 
-        for c in conditions:
+        for c in command.conditions:
             if not hasattr(c, '__call__'):
-                msg = msg % (command_name, type(c))
+                msg = msg % (command.name, type(c))
                 raise MachError(msg)
 
-        arguments = getattr(value, '_mach_command_args', None)
-
-        argument_group_names = getattr(value, '_mach_command_arg_group_names', None)
-
-        handler = MethodHandler(cls, attr, command_name, category=category,
-            description=description, docstring=value.__doc__,
-            conditions=conditions, parser=parser,
-            arguments=arguments, argument_group_names=argument_group_names,
-            pass_context=pass_context)
+        handler = MethodHandler(cls, attr,
+                                command.name,
+                                category=command.category,
+                                description=command.description,
+                                docstring=value.__doc__,
+                                conditions=command.conditions,
+                                parser=command.parser,
+                                arguments=command.arguments,
+                                argument_group_names=command.argument_group_names,
+                                pass_context=pass_context)
 
         Registrar.register_command_handler(handler)
 
     # Now do another pass to get sub-commands. We do this in two passes so
     # we can check the parent command existence without having to hold
     # state and reconcile after traversal.
     for attr in sorted(cls.__dict__.keys()):
         value = cls.__dict__[attr]
 
         if not isinstance(value, types.FunctionType):
             continue
 
-        command, subcommand, description = getattr(value, '_mach_subcommand',
-            (None, None, None))
-
+        command = getattr(value, '_mach_command', None)
         if not command:
             continue
 
-        if command not in seen_commands:
-            raise MachError('Command referenced by sub-command does not '
-                'exist: %s' % command)
-
-        if command not in Registrar.command_handlers:
+        # It is a regular command.
+        if not command.subcommand:
             continue
 
-        arguments = getattr(value, '_mach_command_args', None)
-        argument_group_names = getattr(value, '_mach_command_arg_group_names', None)
+        if command.name not in seen_commands:
+            raise MachError('Command referenced by sub-command does not '
+                'exist: %s' % command.name)
+
+        if command.name not in Registrar.command_handlers:
+            continue
 
-        handler = MethodHandler(cls, attr, subcommand, description=description,
-            docstring=value.__doc__,
-            arguments=arguments, argument_group_names=argument_group_names,
-            pass_context=pass_context)
-        parent = Registrar.command_handlers[command]
+        handler = MethodHandler(cls, attr, command.subcommand,
+                                description=command.description,
+                                docstring=value.__doc__,
+                                arguments=command.arguments,
+                                argument_group_names=command.argument_group_names,
+                                pass_context=pass_context)
+        parent = Registrar.command_handlers[command.name]
 
         if parent.parser:
             raise MachError('cannot declare sub commands against a command '
                 'that has a parser installed: %s' % command)
-        if subcommand in parent.subcommand_handlers:
-            raise MachError('sub-command already defined: %s' % subcommand)
+        if command.subcommand in parent.subcommand_handlers:
+            raise MachError('sub-command already defined: %s' % command.subcommand)
 
-        parent.subcommand_handlers[subcommand] = handler
+        parent.subcommand_handlers[command.subcommand] = handler
 
     return cls
 
 
 class Command(object):
     """Decorator for functions or methods that provide a mach command.
 
     The decorator accepts arguments that define basic attributes of the
@@ -154,27 +196,24 @@ class Command(object):
              basis for the command arguments.
 
     For example:
 
         @Command('foo', category='misc', description='Run the foo action')
         def foo(self):
             pass
     """
-    def __init__(self, name, category=None, description=None, conditions=None,
-                 parser=None):
-        self._name = name
-        self._category = category
-        self._description = description
-        self._conditions = conditions
-        self._parser = parser
+    def __init__(self, name, **kwargs):
+        self._mach_command = _MachCommand(name=name, **kwargs)
 
     def __call__(self, func):
-        func._mach_command = (self._name, self._category, self._description,
-                              self._conditions, self._parser)
+        if not hasattr(func, '_mach_command'):
+            func._mach_command = _MachCommand()
+
+        func._mach_command |= self._mach_command
 
         return func
 
 class SubCommand(object):
     """Decorator for functions or methods that provide a sub-command.
 
     Mach commands can have sub-commands. e.g. ``mach command foo`` or
     ``mach command bar``. Each sub-command has its own parser and is
@@ -186,23 +225,24 @@ class SubCommand(object):
         command -- The string of the command this sub command should be
         attached to.
 
         subcommand -- The string name of the sub command to register.
 
         description -- A textual description for this sub command.
     """
     def __init__(self, command, subcommand, description=None):
-        self._command = command
-        self._subcommand = subcommand
-        self._description = description
+        self._mach_command = _MachCommand(name=command, subcommand=subcommand,
+                                          description=description)
 
     def __call__(self, func):
-        func._mach_subcommand = (self._command, self._subcommand,
-            self._description)
+        if not hasattr(func, '_mach_command'):
+            func._mach_command = _MachCommand()
+
+        func._mach_command |= self._mach_command
 
         return func
 
 class CommandArgument(object):
     """Decorator for additional arguments to mach subcommands.
 
     This decorator should be used to add arguments to mach commands. Arguments
     to the decorator are proxied to ArgumentParser.add_argument().
@@ -219,21 +259,20 @@ class CommandArgument(object):
         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', 'group') for k in kwargs)
         self._command_args = (args, kwargs)
 
     def __call__(self, func):
-        command_args = getattr(func, '_mach_command_args', [])
+        if not hasattr(func, '_mach_command'):
+            func._mach_command = _MachCommand()
 
-        command_args.insert(0, self._command_args)
-
-        func._mach_command_args = command_args
+        func._mach_command.arguments.insert(0, self._command_args)
 
         return func
 
 
 class CommandArgumentGroup(object):
     """Decorator for additional argument groups to mach commands.
 
     This decorator should be used to add arguments groups to mach commands.
@@ -252,21 +291,20 @@ class CommandArgumentGroup(object):
     The name should be chosen so that it makes sense as part of the phrase
     'Command Arguments for <name>' because that's how it will be shown in the
     help message.
     """
     def __init__(self, group_name):
         self._group_name = group_name
 
     def __call__(self, func):
-        command_arg_group_names = getattr(func, '_mach_command_arg_group_names', [])
+        if not hasattr(func, '_mach_command'):
+            func._mach_command = _MachCommand()
 
-        command_arg_group_names.insert(0, self._group_name)
-
-        func._mach_command_arg_group_names = command_arg_group_names
+        func._mach_command.argument_group_names.insert(0, self._group_name)
 
         return func
 
 
 def SettingsProvider(cls):
     """Class decorator to denote that this class provides Mach settings.
 
     When this decorator is encountered, the underlying class will automatically