Refactoring: Extract git checks in push and merge scripts.

tools/push-to-trunk/common_includes.py

 #!/usr/bin/env python
 # Copyright 2013 the V8 project authors. 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
 #       copyright notice, this list of conditions and the following
@@ skipping 302 matching lines @@
   def ReadLine(self, default=None):
     # Don't prompt in forced mode.
     if self._options.force_readline_defaults and default is not None:
       print "%s (forced)" % default
       return default
     else:
       return self._side_effect_handler.ReadLine()
 
   def Git(self, args="", prefix="", pipe=True, retry_on=None):
     cmd = lambda: self._side_effect_handler.Command("git", args, prefix, pipe)
-    return self.Retry(cmd, retry_on, [5, 30])
+    result = self.Retry(cmd, retry_on, [5, 30])
+    if result is None:
+      self.Die("'git %s' failed." % args)
+    return result
 
   def SVN(self, args="", prefix="", pipe=True, retry_on=None):
     cmd = lambda: self._side_effect_handler.Command("svn", args, prefix, pipe)
     return self.Retry(cmd, retry_on, [5, 30])
 
   def Editor(self, args):
     if self._options.requires_editor:
       return self._side_effect_handler.Command(os.environ["EDITOR"], args,
                                                pipe=False)
 
@@ skipping 20 matching lines @@
     print "%s [Y/n] " % msg,
     answer = self.ReadLine(default="Y")
     return answer == "" or answer == "Y" or answer == "y"
 
   def DeleteBranch(self, name):
     git_result = self.Git("branch").strip()
     for line in git_result.splitlines():
       if re.match(r".*\s+%s$" % name, line):
         msg = "Branch %s exists, do you want to delete it?" % name
         if self.Confirm(msg):
-          if self.Git("branch -D %s" % name) is None:
-            self.Die("Deleting branch '%s' failed." % name)
+          self.Git("branch -D %s" % name)
           print "Branch %s deleted." % name
         else:
           msg = "Can't continue. Please delete branch %s and try again." % name
           self.Die(msg)
 
   def InitialEnvironmentChecks(self):
     # Cancel if this is not a git checkout.
     if not os.path.exists(self._config[DOT_GIT_LOCATION]):
       self.Die("This is not a git checkout, this script won't work for you.")
 
@@ skipping 10 matching lines @@
     # Persist current branch.
     self["current_branch"] = ""
     git_result = self.Git("status -s -b -uno").strip()
     for line in git_result.splitlines():
       match = re.match(r"^## (.+)", line)
       if match:
         self["current_branch"] = match.group(1)
         break
 
     # Fetch unfetched revisions.
-    if self.Git("svn fetch") is None:
-      self.Die("'git svn fetch' failed.")
+    self.Git("svn fetch")
 
   def PrepareBranch(self):
     # Get ahold of a safe temporary branch and check it out.
     if self["current_branch"] != self._config[TEMP_BRANCH]:
       self.DeleteBranch(self._config[TEMP_BRANCH])
       self.Git("checkout -b %s" % self._config[TEMP_BRANCH])
 
     # Delete the branch that will be created later if it exists already.
     self.DeleteBranch(self._config[BRANCHNAME])
 
@@ skipping 42 matching lines @@
       if answer == "ABORT":
         self.Die("Applying the patch failed.")
       if answer != "":
         print "That was not 'RESOLVED' or 'ABORT'."
       print "> ",
       answer = self.ReadLine()
 
   # Takes a file containing the patch to apply as first argument.
   def ApplyPatch(self, patch_file, reverse_patch=""):
     args = "apply --index --reject %s \"%s\"" % (reverse_patch, patch_file)
-    if self.Git(args) is None:
+    try:
+      self.Git(args)
+    except:

[ulan, 2014/02/19 14:43:37]

This will catch all exceptions. Is that intended? Maybe there is a git specific
exception you can use here.

[Michael Achenbach, 2014/02/19 14:56:06]

Done.

       self.WaitForResolvingConflicts(patch_file)
 
   def FindLastTrunkPush(self):
     push_pattern = "^Version [[:digit:]]*\.[[:digit:]]*\.[[:digit:]]* (based"
     args = "log -1 --format=%%H --grep=\"%s\" svn/trunk" % push_pattern
     return self.Git(args).strip()
 
 
 class UploadStep(Step):
   MESSAGE = "Upload for code review."
 
   def RunStep(self):
     if self._options.reviewer:
       print "Using account %s for review." % self._options.reviewer
       reviewer = self._options.reviewer
     else:
       print "Please enter the email address of a V8 reviewer for your patch: ",
       self.DieNoManualMode("A reviewer must be specified in forced mode.")
       reviewer = self.ReadLine()
     author_option = self._options.author
     author = " --email \"%s\"" % author_option if author_option else ""
     force_flag = " -f" if self._options.force_upload else ""
     args = ("cl upload%s -r \"%s\" --send-mail%s"
             % (author, reviewer, force_flag))
     # TODO(machenbach): Check output in forced mode. Verify that all required
     # base files were uploaded, if not retry.
-    if self.Git(args, pipe=False) is None:
-      self.Die("'git cl upload' failed, please try again.")
+    self.Git(args, pipe=False)
 
 
 def MakeStep(step_class=Step, number=0, state=None, config=None,
              options=None, side_effect_handler=DEFAULT_SIDE_EFFECT_HANDLER):
     # Allow to pass in empty dictionaries.
     state = state if state is not None else {}
     config = config if config is not None else {}
 
     try:
       message = step_class.MESSAGE
@@ skipping 17 matching lines @@
   if options.s == 0 and os.path.exists(state_file):
     os.remove(state_file)
   state = {}
   steps = []
   for (number, step_class) in enumerate(step_classes):
     steps.append(MakeStep(step_class, number, state, config,
                           options, side_effect_handler))
 
   for step in steps[options.s:]:
     step.Run()