Refactoring: Extract git checks in push and merge scripts.

This extracts the pattern "if call git fails: raise exception", which is spread all over the place. Now all calls to git are required to return gracefully and give a uniform exception message if they don't.

BUG=
R=ulan@chromium.org

Review URL: https://codereview.chromium.org/166903012

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19494 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/tools/push-to-trunk/common_includes.py b/tools/push-to-trunk/common_includes.py
index e28c3d7..c3d557a 100644 (file)
--- a/tools/push-to-trunk/common_includes.py
+++ b/tools/push-to-trunk/common_includes.py
@@ -220,6 +220,10 @@ class NoRetryException(Exception):
   pass
 
 
+class GitFailedException(Exception):
+  pass
+
+
 class CommonOptions(object):
   def __init__(self, options, manual=True):
     self.requires_editor = True
@@ -320,7 +324,10 @@ class Step(object):
 
   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:
+      raise GitFailedException("'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)
@@ -361,8 +368,7 @@ class Step(object):
       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
@@ -393,8 +399,7 @@ class Step(object):
         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.
@@ -457,7 +462,9 @@ class Step(object):
   # 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 GitFailedException:
       self.WaitForResolvingConflicts(patch_file)
 
   def FindLastTrunkPush(self):
@@ -484,8 +491,7 @@ class UploadStep(Step):
             % (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,

diff --git a/tools/push-to-trunk/merge_to_branch.py b/tools/push-to-trunk/merge_to_branch.py
index 8e126b1..757e617 100755 (executable)
--- a/tools/push-to-trunk/merge_to_branch.py
+++ b/tools/push-to-trunk/merge_to_branch.py
@@ -91,10 +91,8 @@ class CreateBranch(Step):
   MESSAGE = "Create a fresh branch for the patch."
 
   def RunStep(self):
-    args = "checkout -b %s svn/%s" % (self.Config(BRANCHNAME),
-                                      self["merge_to_branch"])
-    if self.Git(args) is None:
-      self.die("Creating branch %s failed." % self.Config(BRANCHNAME))
+    self.Git("checkout -b %s svn/%s" % (self.Config(BRANCHNAME),
+                                        self["merge_to_branch"]))
 
 
 class SearchArchitecturePorts(Step):
@@ -226,24 +224,17 @@ class CommitLocal(Step):
   MESSAGE = "Commit to local branch."
 
   def RunStep(self):
-    if self.Git("commit -a -F \"%s\"" % self.Config(COMMITMSG_FILE)) is None:
-      self.Die("'git commit -a' failed.")
+    self.Git("commit -a -F \"%s\"" % self.Config(COMMITMSG_FILE))
 
 
 class CommitRepository(Step):
   MESSAGE = "Commit to the repository."
 
   def RunStep(self):
-    if self.Git("checkout %s" % self.Config(BRANCHNAME)) is None:
-      self.Die("Cannot ensure that the current branch is %s"
-               % self.Config(BRANCHNAME))
+    self.Git("checkout %s" % self.Config(BRANCHNAME))
     self.WaitForLGTM()
-    if self.Git("cl presubmit", "PRESUBMIT_TREE_CHECK=\"skip\"") is None:
-      self.Die("Presubmit failed.")
-
-    if self.Git("cl dcommit -f --bypass-hooks",
-                retry_on=lambda x: x is None) is None:
-      self.Die("Failed to commit to %s" % self._status["merge_to_branch"])
+    self.Git("cl presubmit", "PRESUBMIT_TREE_CHECK=\"skip\"")
+    self.Git("cl dcommit -f --bypass-hooks", retry_on=lambda x: x is None)
 
 
 class PrepareSVN(Step):
@@ -252,8 +243,7 @@ class PrepareSVN(Step):
   def RunStep(self):
     if self._options.revert_bleeding_edge:
       return
-    if self.Git("svn fetch") is None:
-      self.Die("'git svn fetch' failed.")
+    self.Git("svn fetch")
     args = ("log -1 --format=%%H --grep=\"%s\" svn/%s"
             % (self["new_commit_msg"], self["merge_to_branch"]))
     commit_hash = self.Git(args).strip()

diff --git a/tools/push-to-trunk/push_to_trunk.py b/tools/push-to-trunk/push_to_trunk.py
index f5bda60..c1b52e6 100755 (executable)
--- a/tools/push-to-trunk/push_to_trunk.py
+++ b/tools/push-to-trunk/push_to_trunk.py
@@ -97,8 +97,7 @@ class FreshBranch(Step):
 
   def RunStep(self):
     args = "checkout -b %s svn/bleeding_edge" % self.Config(BRANCHNAME)
-    if self.Git(args) is None:
-      self.Die("Creating branch %s failed." % self.Config(BRANCHNAME))
+    self.Git(args)
 
 
 class DetectLastPush(Step):
@@ -268,9 +267,7 @@ class CommitLocal(Step):
       review = "\n\nTBR=%s" % self._options.reviewer
     else:
       review = ""
-    if self.Git("commit -a -m \"%s%s\""
-                % (self["prep_commit_msg"], review)) is None:
-      self.Die("'git commit -a' failed.")
+    self.Git("commit -a -m \"%s%s\"" % (self["prep_commit_msg"], review))
 
 
 class CommitRepository(Step):
@@ -283,12 +280,8 @@ class CommitRepository(Step):
     TextToFile(GetLastChangeLogEntries(self.Config(CHANGELOG_FILE)),
                self.Config(CHANGELOG_ENTRY_FILE))
 
-    if self.Git("cl presubmit", "PRESUBMIT_TREE_CHECK=\"skip\"") is None:
-      self.Die("'git cl presubmit' failed, please try again.")
-
-    if self.Git("cl dcommit -f --bypass-hooks",
-                retry_on=lambda x: x is None) is None:
-      self.Die("'git cl dcommit' failed, please try again.")
+    self.Git("cl presubmit", "PRESUBMIT_TREE_CHECK=\"skip\"")
+    self.Git("cl dcommit -f --bypass-hooks", retry_on=lambda x: x is None)
 
 
 class StragglerCommits(Step):
@@ -296,8 +289,7 @@ class StragglerCommits(Step):
              "started.")
 
   def RunStep(self):
-    if self.Git("svn fetch") is None:
-      self.Die("'git svn fetch' failed.")
+    self.Git("svn fetch")
     self.Git("checkout svn/bleeding_edge")
     args = "log -1 --format=%%H --grep=\"%s\"" % self["prep_commit_msg"]
     self["prepare_commit_hash"] = self.Git(args).strip()
@@ -342,9 +334,7 @@ class NewBranch(Step):
   MESSAGE = "Create a new branch from trunk."
 
   def RunStep(self):
-    if self.Git("checkout -b %s svn/trunk" % self.Config(TRUNKBRANCH)) is None:
-      self.Die("Checking out a new branch '%s' failed." %
-               self.Config(TRUNKBRANCH))
+    self.Git("checkout -b %s svn/trunk" % self.Config(TRUNKBRANCH))
 
 
 class ApplyChanges(Step):
@@ -380,8 +370,7 @@ class CommitTrunk(Step):
 
   def RunStep(self):
     self.Git("add \"%s\"" % self.Config(VERSION_FILE))
-    if self.Git("commit -F \"%s\"" % self.Config(COMMITMSG_FILE)) is None:
-      self.Die("'git commit' failed.")
+    self.Git("commit -F \"%s\"" % self.Config(COMMITMSG_FILE))
     Command("rm", "-f %s*" % self.Config(COMMITMSG_FILE))
 
 
@@ -423,10 +412,9 @@ class TagRevision(Step):
   MESSAGE = "Tag the new revision."
 
   def RunStep(self):
-    if self.Git(("svn tag %s -m \"Tagging version %s\""
-                 % (self["version"], self["version"])),
-                retry_on=lambda x: x is None) is None:
-      self.Die("'git svn tag' failed.")
+    self.Git(("svn tag %s -m \"Tagging version %s\""
+              % (self["version"], self["version"])),
+             retry_on=lambda x: x is None)
 
 
 class CheckChromium(Step):
@@ -466,14 +454,9 @@ class UpdateChromiumCheckout(Step):
 
   def RunStep(self):
     os.chdir(self["chrome_path"])
-    if self.Git("checkout master") is None:
-      self.Die("'git checkout master' failed.")
-    if self.Git("pull") is None:
-      self.Die("'git pull' failed, please try again.")
-
-    args = "checkout -b v8-roll-%s" % self["trunk_revision"]
-    if self.Git(args) is None:
-      self.Die("Failed to checkout a new branch.")
+    self.Git("checkout master")
+    self.Git("pull")
+    self.Git("checkout -b v8-roll-%s" % self["trunk_revision"])
 
 
 class UploadCL(Step):
@@ -497,17 +480,13 @@ class UploadCL(Step):
       print "Please enter the email address of a reviewer for the roll CL: ",
       self.DieNoManualMode("A reviewer must be specified in forced mode.")
       rev = self.ReadLine()
-    args = ("commit -am \"Update V8 to version %s "
-            "(based on bleeding_edge revision r%s).\n\nTBR=%s\""
-            % (self["version"], self["svn_revision"], rev))
-    if self.Git(args) is None:
-      self.Die("'git commit' failed.")
+    self.Git("commit -am \"Update V8 to version %s "
+             "(based on bleeding_edge revision r%s).\n\nTBR=%s\""
+             % (self["version"], self["svn_revision"], rev))
     author_option = self._options.author
     author = " --email \"%s\"" % author_option if author_option else ""
     force_flag = " -f" if self._options.force_upload else ""
-    if self.Git("cl upload%s --send-mail%s" % (author, force_flag),
-                pipe=False) is None:
-      self.Die("'git cl upload' failed, please try again.")
+    self.Git("cl upload%s --send-mail%s" % (author, force_flag), pipe=False)
     print "CL uploaded."