Bug 1521996 - Part 1: Add `conditional_name` to mach @Command definition. r=ahal
authorNick Alexander <nalexander@mozilla.com>
Fri, 17 May 2019 21:39:31 +0000
changeset 533231 e3a0fedff65ea47dba3607665a1d34241d0c39c4
parent 533230 c062846ae62bc7817f35e4994cb7f2e524c017ff
child 533232 376efab28772a451060d9832e84c918b46aef8f4
push id11276
push userrgurzau@mozilla.com
push dateMon, 20 May 2019 13:11:24 +0000
treeherdermozilla-beta@847755a7c325 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersahal
bugs1521996, 901972, 1291335, 1305695
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1521996 - Part 1: Add `conditional_name` to mach @Command definition. r=ahal We want `mach run` for Android to be wildly different than `mach run` for Desktop. But right now, mach really doesn't support two different implementations of the same underlying named command. The avenues that might support different implementations, mostly run through `conditions`. `conditions` were added to mach commands in Bug 901972, and never really anticipated this use case: commands are keyed by name condition evaluation is delayed until dispatch-time. In order to have different commands with the same name, and have full support for `--help`, command matching, suggestions, etc, we really need conditions to evaluate at parse-time. Indeed, since Bug 901972 landed, we've moved context creation earlier in the dispatch flow and hacked in things that look like parse-time conditions (see Bug 1291335 and Bug 1305695). This approach is not the prettiest, but it handles this narrow use-case -- making `mach run` and `mach install` different on Android -- without much code churn. Differential Revision: https://phabricator.services.mozilla.com/D18290
python/mach/mach/decorators.py
python/mach/mach/main.py
python/mach/mach/registrar.py
--- a/python/mach/mach/decorators.py
+++ b/python/mach/mach/decorators.py
@@ -25,16 +25,19 @@ class _MachCommand(object):
         'subcommand',
         'category',
         'description',
         'conditions',
         '_parser',
         'arguments',
         'argument_group_names',
 
+        # When `conditions` are met, rename this command to the following name.
+        'conditional_name',
+
         # Describes how dispatch is performed.
 
         # The Python class providing the command. This is the class type not
         # an instance of the class. Mach will instantiate a new instance of
         # the class if the command is executed.
         'cls',
 
         # Whether the __init__ method of the class should receive a mach
@@ -48,25 +51,27 @@ class _MachCommand(object):
         'method',
 
         # Dict of string to _MachCommand defining sub-commands for this
         # command.
         'subcommand_handlers',
     )
 
     def __init__(self, name=None, subcommand=None, category=None,
-                 description=None, conditions=None, parser=None):
+                 description=None, conditions=None, parser=None,
+                 conditional_name=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 = []
+        self.conditional_name = conditional_name
 
         self.cls = None
         self.pass_context = None
         self.method = None
         self.subcommand_handlers = {}
 
     @property
     def parser(self):
--- a/python/mach/mach/main.py
+++ b/python/mach/mach/main.py
@@ -394,16 +394,18 @@ To see more help for a specific command,
         context = CommandContext(cwd=self.cwd,
                                  settings=self.settings, log_manager=self.log_manager,
                                  commands=Registrar)
 
         if self.populate_context_handler:
             self.populate_context_handler(context)
             context = ContextWrapper(context, self.populate_context_handler)
 
+        Registrar.register_conditional_names(context)
+
         parser = self.get_argument_parser(context)
 
         if not len(argv):
             # We don't register the usage until here because if it is globally
             # registered, argparse always prints it. This is not desired when
             # running with --help.
             parser.usage = Mach.USAGE
             parser.print_usage()
--- a/python/mach/mach/registrar.py
+++ b/python/mach/mach/registrar.py
@@ -42,52 +42,86 @@ class MachRegistrar(object):
 
     def register_settings_provider(self, cls):
         self.settings_providers.add(cls)
 
     def register_category(self, name, title, description, priority=50):
         self.categories[name] = (title, description, priority)
         self.commands_by_category[name] = set()
 
+    def register_conditional_names(self, context):
+        """For every handler with a conditional name, use that name if
+        the handler's conditions are met."""
+
+        # Avoid updating while iterating.
+        names = list(self.command_handlers.keys())
+
+        for name in names:
+            handler = self.command_handlers[name]
+            if handler.conditional_name:
+                instance = MachRegistrar._instance(handler, context)
+                fail_conditions = MachRegistrar._fail_conditions(handler, instance)
+
+                if fail_conditions:
+                    continue
+
+                # We passed our conditions.  Unregister the existing name.
+                del self.command_handlers[name]
+                self.commands_by_category[handler.category].remove(name)
+
+                # Register with the new name.
+                handler.name = handler.conditional_name
+                self.register_command_handler(handler)
+
     @classmethod
     def _condition_failed_message(cls, name, conditions):
         msg = ['\n']
         for c in conditions:
             part = ['  %s' % c.__name__]
             if c.__doc__ is not None:
                 part.append(c.__doc__)
             msg.append(' - '.join(part))
         return INVALID_COMMAND_CONTEXT % (name, '\n'.join(msg))
 
-    def _run_command_handler(self, handler, context=None, debug_command=False, **kwargs):
+    @classmethod
+    def _instance(_, handler, context, **kwargs):
         cls = handler.cls
 
         if handler.pass_context and not context:
             raise Exception('mach command class requires context.')
 
         if context:
             prerun = getattr(context, 'pre_dispatch_handler', None)
             if prerun:
                 prerun(context, handler, args=kwargs)
 
         if handler.pass_context:
             context.handler = handler
             instance = cls(context)
         else:
             instance = cls()
 
+        return instance
+
+    @classmethod
+    def _fail_conditions(_, handler, instance):
+        fail_conditions = []
         if handler.conditions:
-            fail_conditions = []
             for c in handler.conditions:
                 if not c(instance):
                     fail_conditions.append(c)
 
-            if fail_conditions:
-                print(self._condition_failed_message(handler.name, fail_conditions))
-                return 1
+        return fail_conditions
+
+    def _run_command_handler(self, handler, context=None, debug_command=False, **kwargs):
+        instance = MachRegistrar._instance(handler, context, **kwargs)
+        fail_conditions = MachRegistrar._fail_conditions(handler, instance)
+        if fail_conditions:
+            print(MachRegistrar._condition_failed_message(handler.name, fail_conditions))
+            return 1
 
         self.command_depth += 1
         fn = getattr(instance, handler.method)
 
         start_time = time.time()
 
         if debug_command:
             import pdb