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

Index: third_party/WebKit/Tools/Scripts/webkitpy/tool/multi_command_tool.py
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/tool/multi_command_tool.py b/third_party/WebKit/Tools/Scripts/webkitpy/tool/multi_command_tool.py
index ad10d1a73f6d98b63afdbbd48bbd907037ad245d..656de706f8dfbebfde48bc4d6a4337c540c2a8fe 100644
--- a/third_party/WebKit/Tools/Scripts/webkitpy/tool/multi_command_tool.py
+++ b/third_party/WebKit/Tools/Scripts/webkitpy/tool/multi_command_tool.py
@@ -35,7 +35,6 @@ 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
 
@@ -48,29 +47,17 @@ class MultiCommandTool(object):
 
     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