Refactoring: Redesign option parsing in push and merge scripts.

tools/push-to-trunk/merge_to_branch.py

Index: tools/push-to-trunk/merge_to_branch.py
diff --git a/tools/push-to-trunk/merge_to_branch.py b/tools/push-to-trunk/merge_to_branch.py
index 5f0b6a6e5532c9276b7dff274a011a9bbe92d40b..61f4ef7ea7d18638ed76de974d1f14770dc9ca32 100755
--- a/tools/push-to-trunk/merge_to_branch.py
+++ b/tools/push-to-trunk/merge_to_branch.py
@@ -27,7 +27,7 @@
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 from collections import OrderedDict
-import optparse
+import argparse
 import sys
 
 from common_includes import *
@@ -50,36 +50,22 @@ CONFIG = {
 }
 
 
-class MergeToBranchOptions(CommonOptions):
-  def __init__(self, options, args):
-    super(MergeToBranchOptions, self).__init__(options, True)
-    self.requires_editor = True
-    self.wait_for_lgtm = True
-    self.delete_sentinel = options.f
-    self.message = getattr(options, "message", "")
-    self.revert = getattr(options, "r", False)
-    self.revert_bleeding_edge = getattr(options, "revert_bleeding_edge", False)
-    self.patch = getattr(options, "p", "")
-    self.args = args
-
-
 class Preparation(Step):
   MESSAGE = "Preparation."
 
   def RunStep(self):
     if os.path.exists(self.Config(ALREADY_MERGING_SENTINEL_FILE)):
-      if self._options.delete_sentinel:
+      if self._options.force:
         os.remove(self.Config(ALREADY_MERGING_SENTINEL_FILE))
-      elif self._options.s == 0:
+      elif self._options.step == 0:
         self.Die("A merge is already in progress")
     open(self.Config(ALREADY_MERGING_SENTINEL_FILE), "a").close()
 
     self.InitialEnvironmentChecks()
     if self._options.revert_bleeding_edge:
       self["merge_to_branch"] = "bleeding_edge"
-    elif self._options.args[0]:
-      self["merge_to_branch"] = self._options.args[0]
-      self._options.args = self._options.args[1:]
+    elif self._options.branch:
+      self["merge_to_branch"] = self._options.branch
     else:
       self.Die("Please specify a branch to merge to")
 
@@ -99,7 +85,8 @@ class SearchArchitecturePorts(Step):
   MESSAGE = "Search for corresponding architecture ports."
 
   def RunStep(self):
-    self["full_revision_list"] = list(OrderedDict.fromkeys(self._options.args))
+    self["full_revision_list"] = list(
+        OrderedDict.fromkeys(self._options.revisions))
     port_revision_list = []
     for revision in self["full_revision_list"]:
       # Search for commits which matches the "Port rXXX" pattern.
@@ -287,76 +274,58 @@ class CleanUp(Step):
         print "patches: %s" % self["revision_list"]
 
 
-def RunMergeToBranch(config,
-                     options,
-                     side_effect_handler=DEFAULT_SIDE_EFFECT_HANDLER):
-  step_classes = [
-    Preparation,
-    CreateBranch,
-    SearchArchitecturePorts,
-    FindGitRevisions,
-    ApplyPatches,
-    PrepareVersion,
-    IncrementVersion,
-    CommitLocal,
-    UploadStep,
-    CommitRepository,
-    PrepareSVN,
-    TagRevision,
-    CleanUp,
-  ]
-
-  RunScript(step_classes, config, options, side_effect_handler)
-
-
-def BuildOptions():
-  result = optparse.OptionParser()
-  result.set_usage("""%prog [OPTIONS]... [BRANCH] [REVISION]...
-
-Performs the necessary steps to merge revisions from bleeding_edge
-to other branches, including trunk.""")
-  result.add_option("-f",
-                    help="Delete sentinel file.",
-                    default=False, action="store_true")
-  result.add_option("-m", "--message",
-                    help="Specify a commit message for the patch.")
-  result.add_option("-r", "--revert",
-                    help="Revert specified patches.",
-                    default=False, action="store_true")
-  result.add_option("-R", "--revert-bleeding-edge",
-                    help="Revert specified patches from bleeding edge.",
-                    default=False, action="store_true")
-  result.add_option("-p", "--patch", dest="p",
-                    help="Specify a patch file to apply as part of the merge.")
-  result.add_option("-s", "--step", dest="s",
-                    help="Specify the step where to start work. Default: 0.",
-                    default=0, type="int")
-  return result
-
-
-def ProcessOptions(options, args):
-  revert_from_bleeding_edge = 1 if options.revert_bleeding_edge else 0
-  min_exp_args = 2 - revert_from_bleeding_edge
-  if len(args) < min_exp_args:
-    if not options.p:
-      print "Either a patch file or revision numbers must be specified"
-      return False
-    if not options.message:
-      print "You must specify a merge comment if no patches are specified"
-      return False
-  if options.s < 0:
-    print "Bad step number %d" % options.s
-    return False
-  return True
-
-
-def Main():
-  parser = BuildOptions()
-  (options, args) = parser.parse_args()
-  if not ProcessOptions(options, args):
-    parser.print_help()
-    return 1
-  RunMergeToBranch(CONFIG, MergeToBranchOptions(options, args))
+class MergeToBranch(ScriptsBase):
+  def _Description(self):
+    return ("Performs the necessary steps to merge revisions from "
+            "bleeding_edge to other branches, including trunk.")
+
+  def _PrepareOptions(self, parser):
+    group = parser.add_mutually_exclusive_group(required=True)
+    group.add_argument("--branch", help="The branch to merge to.")
+    group.add_argument("-R", "--revert-bleeding-edge",
+                       help="Revert specified patches from bleeding edge.",
+                       default=False, action="store_true")
+    parser.add_argument("revisions", nargs="*",
+                        help="The revisions to merge.")
+    parser.add_argument("-f", "--force",
+                        help="Delete sentinel file.",
+                        default=False, action="store_true")
+    parser.add_argument("-m", "--message",
+                        help="A commit message for the patch.")
+    parser.add_argument("--revert",
+                        help="Revert specified patches.",
+                        default=False, action="store_true")
+    parser.add_argument("-p", "--patch",
+                        help="A patch file to apply as part of the merge.")
+
+  def _ProcessOptions(self, options):
+    # TODO(machenbach): Add a test that covers revert from bleeding_edge
+    if len(options.revisions) < 1:
+      if not options.patch:
+        print "Either a patch file or revision numbers must be specified"
+        return False
+      if not options.message:
+        print "You must specify a merge comment if no patches are specified"
+        return False
+    return True
+
+  def _Steps(self):
+    return [
+      Preparation,
+      CreateBranch,
+      SearchArchitecturePorts,
+      FindGitRevisions,
+      ApplyPatches,
+      PrepareVersion,
+      IncrementVersion,
+      CommitLocal,
+      UploadStep,
+      CommitRepository,
+      PrepareSVN,
+      TagRevision,
+      CleanUp,
+    ]
+
 
 if __name__ == "__main__":
-  sys.exit(Main())
+  sys.exit(MergeToBranch(CONFIG).Run())