Reland of Use an explicit list of webkit-patch commands instead of using auto-discovery.

third_party/WebKit/Tools/Scripts/webkitpy/tool/multi_command_tool.py

 # Copyright (c) 2009 Google Inc. All rights reserved.
 # Copyright (c) 2009 Apple Inc. All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
 # met:
 #
 #     * Redistributions of source code must retain the above copyright
 # notice, this list of conditions and the following disclaimer.
 #     * Redistributions in binary form must reproduce the above
@@ skipping 17 matching lines @@
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #
 # MultiCommandTool provides a framework for writing svn-like/git-like tools
 # which are called with the following format:
 # tool-name [global options] command-name [command options]
 
 import optparse
 import logging
 import sys
 
-from webkitpy.tool.commands.command import Command
 from webkitpy.tool.commands.command import HelpPrintingOptionParser
 from webkitpy.tool.commands.help_command import HelpCommand
 
 _log = logging.getLogger(__name__)
 
 
 class MultiCommandTool(object):
 
     global_options = None
 
     def __init__(self, name=None, commands=None):
         self._name = name or optparse.OptionParser(prog=name).get_prog_name()  # OptionParser has nice logic for fetching the name.
-        # Allow the unit tests to disable command auto-discovery.
-        self.commands = commands or [cls() for cls in self._find_all_commands() if cls.name]
+        self.commands = commands or []

[Dirk Pranke, 2016/06/17 19:09:25]

can we make name and commands mandatory parameters? Or remove name, since it
seems no one is using it?

Also, weren't we just going to merge this class into webkit_patch.py ?

[qyearsley, 2016/06/17 20:10:06]

That makes sense; now done.
Before we merge webkit_patch and multi_command_tool, I want to decide that we're
probably *not* going to have any more "multi-command tools". At this point, it
would be relatively easy to move commands over to separate MultiCommandTool
subclasses, so that we could have multiple tools, e.g.:

  $ Tools/Scripts/rebaseline help
  Usage: rebaseline [options] COMMAND [ARGS]

  Common rebaseline commands:
    from-expectations
    from-json
    from-try-bots
    auto-rebaseline
    rebaseline-server

  $ Tools/Scripts/query help
  Usage: query [options] COMMAND [ARGS]

  Common query commands:
    crash-log
    print-baselines
    print-expectations
    print-flaky-tests

After merging, then it's clearer that there's only one "multi-tool". The
benefits of splitting up the commands that I can think of are:
  (1) making some commands shorter and more memorable
  (2) separating the user interface of commands that serve separate purposes

The disadvantages are:
  (1) this may be a troublesome renaming change in multiple places with no real
practical benefit
  (2) there are a few "miscellaneous commands" with no obvious place to go

Why is the command called "webkit-patch" anyway? I assume this name was made up
a long time ago, before rebaselining became one of the main purposes of the
tool? Even if we keep all these commands under one multi-tool, maybe it would be
nice to rename it (blink-tool? multi-tool?).

         self.help_command = self.command_by_name(HelpCommand.name)
-        # Require a help command, even if the manual test list doesn't include one.
+        # Require the help command, even if the manual test list doesn't include one.
         if not self.help_command:
             self.help_command = HelpCommand()
             self.commands.append(self.help_command)
+        # FIXME: Since tool is passed to Command.execute, it may not be necessary to set a tool attribute on the
+        # command objects here - maybe this should be done inside of Command.execute for commands that use self._tool.
         for command in self.commands:
             command.bind_to_tool(self)
 
-    @classmethod
-    def _add_all_subclasses(cls, class_to_crawl, seen_classes):
-        for subclass in class_to_crawl.__subclasses__():
-            if subclass not in seen_classes:
-                seen_classes.add(subclass)
-                cls._add_all_subclasses(subclass, seen_classes)
-
-    @classmethod
-    def _find_all_commands(cls):
-        commands = set()
-        cls._add_all_subclasses(Command, commands)
-        return sorted(commands)
-
     def name(self):
         return self._name
 
     def _create_option_parser(self):
         usage = "Usage: %prog [options] COMMAND [ARGS]"
         return HelpPrintingOptionParser(epilog_method=self.help_command._help_epilog, prog=self.name(), usage=usage)
 
     @staticmethod
     def _split_command_name_from_args(args):
         # Assume the first argument which doesn't start with "-" is the command name.
@@ skipping 51 matching lines @@
 
         (should_execute, failure_reason) = self.should_execute_command(command)
         if not should_execute:
             _log.error(failure_reason)
             return 0  # FIXME: Should this really be 0?
 
         result = command.check_arguments_and_execute(options, args, self)
 
         self.command_completed()
         return result