Add merge-to-branch python port.

- To ease a line-by-line review, the script is intentionally close to the former bash version
- Disambiguate the existing "-r" option for reviewer in the other scripts
- The options design will be refactored in a follow up CL

TEST=python -m unittest test_scripts.ScriptTest.testMergeToBranch
R=ulan@chromium.org

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

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

tools/merge-to-branch.sh

@@ -235,7 +235,6 @@ if [ $START_STEP -le $CURRENT_STEP ] ; then
   echo ">>> Step $CURRENT_STEP: Apply patches for selected revisions."
   restore_if_unset "MERGE_TO_BRANCH"
   restore_patch_commit_hashes_if_unset "PATCH_COMMIT_HASHES"
-  rm -f "$TOUCHED_FILES_FILE"
   for HASH in ${PATCH_COMMIT_HASHES[@]} ; do
     echo "Applying patch for $HASH to $MERGE_TO_BRANCH..."
     git log -1 -p $HASH > "$TEMPORARY_PATCH_FILE"

tools/push-to-trunk/auto_roll.py

@@ -52,7 +52,6 @@ class AutoRollOptions(CommonOptions):
     super(AutoRollOptions, self).__init__(options)
     self.requires_editor = False
     self.status_password = options.status_password
-    self.r = options.r
     self.c = options.c
     self.push = getattr(options, 'push', False)
     self.author = getattr(options, 'a', None)
@@ -162,7 +161,7 @@ class PushToTrunk(Step):
               RunPushToTrunk,
               push_to_trunk.CONFIG,
               PushToTrunkOptions.MakeForcedOptions(self._options.author,
-                                                   self._options.r,
+                                                   self._options.reviewer,
                                                    self._options.c),
               self._side_effect_handler)
       finally:
@@ -197,7 +196,7 @@ def BuildOptions():
   result.add_option("-p", "--push",
                     help="Push to trunk if possible. Dry run if unspecified.",
                     default=False, action="store_true")
-  result.add_option("-r", "--reviewer", dest="r",
+  result.add_option("-r", "--reviewer",
                     help=("Specify the account name to be used for reviews."))
   result.add_option("-s", "--step", dest="s",
                     help="Specify the step where to start work. Default: 0.",
@@ -210,7 +209,7 @@ def BuildOptions():
 def Main():
   parser = BuildOptions()
   (options, args) = parser.parse_args()
-  if not options.a or not options.c or not options.r:
+  if not options.a or not options.c or not options.reviewer:
     print "You need to specify author, chromium src location and reviewer."
     parser.print_help()
     return 1

tools/push-to-trunk/common_includes.py

@@ -227,6 +227,7 @@ class CommonOptions(object):
     self.force_readline_defaults = not manual
     self.force_upload = not manual
     self.manual = manual
+    self.reviewer = getattr(options, 'reviewer', None)
     self.author = getattr(options, 'a', None)
 
 
@@ -302,6 +303,10 @@ class Step(object):
     cmd = lambda: self._side_effect_handler.Command("git", args, prefix, pipe)
     return self.Retry(cmd, retry_on, [5, 30])
 
+  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,
@@ -462,9 +467,9 @@ class UploadStep(Step):
   MESSAGE = "Upload for code review."
 
   def RunStep(self):
-    if self._options.r:
-      print "Using account %s for review." % self._options.r
-      reviewer = self._options.r
+    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.")

tools/push-to-trunk/push_to_trunk.py

@@ -63,7 +63,6 @@ class PushToTrunkOptions(CommonOptions):
     options.l = None
     options.f = True
     options.m = False
-    options.r = reviewer
     options.c = chrome_path
     options.a = author
     return PushToTrunkOptions(options)
@@ -74,7 +73,7 @@ class PushToTrunkOptions(CommonOptions):
     self.wait_for_lgtm = not options.f
     self.tbr_commit = not options.m
     self.l = options.l
-    self.r = options.r
+    self.reviewer = options.reviewer
     self.c = options.c
     self.author = getattr(options, 'a', None)
 
@@ -240,7 +239,10 @@ class CommitLocal(Step):
 
     # Include optional TBR only in the git command. The persisted commit
     # message is used for finding the commit again later.
-    review = "\n\nTBR=%s" % self._options.r if self._options.tbr_commit else ""
+    if self._options.tbr_commit:
+      review = "\n\nTBR=%s" % self._options.reviewer
+    else:
+      review = ""
     if self.Git("commit -a -m \"%s%s\"" % (prep_commit_msg, review)) is None:
       self.Die("'git commit -a' failed.")
 
@@ -481,9 +483,9 @@ class UploadCL(Step):
     ver = "%s.%s.%s" % (self._state["major"],
                         self._state["minor"],
                         self._state["build"])
-    if self._options.r:
-      print "Using account %s for review." % self._options.r
-      rev = self._options.r
+    if self._options.reviewer:
+      print "Using account %s for review." % self._options.reviewer
+      rev = self._options.reviewer
     else:
       print "Please enter the email address of a reviewer for the roll CL: ",
       self.DieNoManualMode("A reviewer must be specified in forced mode.")
@@ -587,7 +589,7 @@ def BuildOptions():
   result.add_option("-m", "--manual", dest="m",
                     help="Prompt the user at every important step.",
                     default=False, action="store_true")
-  result.add_option("-r", "--reviewer", dest="r",
+  result.add_option("-r", "--reviewer",
                     help=("Specify the account name to be used for reviews."))
   result.add_option("-s", "--step", dest="s",
                     help="Specify the step where to start work. Default: 0.",
@@ -599,7 +601,7 @@ def ProcessOptions(options):
   if options.s < 0:
     print "Bad step number %d" % options.s
     return False
-  if not options.m and not options.r:
+  if not options.m and not options.reviewer:
     print "A reviewer (-r) is required in (semi-)automatic mode."
     return False
   if options.f and options.m:

tools/push-to-trunk/test_scripts.py

@@ -31,15 +31,17 @@ import tempfile
 import traceback
 import unittest
 
-import common_includes
-from common_includes import *
-import push_to_trunk
-from push_to_trunk import *
 import auto_roll
 from auto_roll import AutoRollOptions
 from auto_roll import CheckLastPush
 from auto_roll import FetchLatestRevision
 from auto_roll import SETTINGS_LOCATION
+import common_includes
+from common_includes import *
+import merge_to_branch
+from merge_to_branch import *
+import push_to_trunk
+from push_to_trunk import *
 
 
 TEST_CONFIG = {
@@ -56,11 +58,15 @@ TEST_CONFIG = {
   CHROMIUM: "/tmp/test-v8-push-to-trunk-tempfile-chromium",
   DEPS_FILE: "/tmp/test-v8-push-to-trunk-tempfile-chromium/DEPS",
   SETTINGS_LOCATION: None,
+  ALREADY_MERGING_SENTINEL_FILE:
+      "/tmp/test-merge-to-branch-tempfile-already-merging",
+  COMMIT_HASHES_FILE: "/tmp/test-merge-to-branch-tempfile-PATCH_COMMIT_HASHES",
+  TEMPORARY_PATCH_FILE: "/tmp/test-merge-to-branch-tempfile-temporary-patch",
 }
 
 
 def MakeOptions(s=0, l=None, f=False, m=True, r=None, c=None, a=None,
-                status_password=None):
+                status_password=None, revert_bleeding_edge=None, p=None):
   """Convenience wrapper."""
   class Options(object):
       pass
@@ -69,10 +75,12 @@ def MakeOptions(s=0, l=None, f=False, m=True, r=None, c=None, a=None,
   options.l = l
   options.f = f
   options.m = m
-  options.r = r
+  options.reviewer = r
   options.c = c
   options.a = a
+  options.p = p
   options.status_password = status_password
+  options.revert_bleeding_edge = revert_bleeding_edge
   return options
 
 
@@ -228,7 +236,7 @@ class SimpleMock(object):
     try:
       expected_call = self._recipe[self._index]
     except IndexError:
-      raise Exception("Calling %s %s" % (self._name, " ".join(args)))
+      raise NoRetryException("Calling %s %s" % (self._name, " ".join(args)))
 
     # Pack expectations without arguments into a list.
     if not isinstance(expected_call, list):
@@ -303,6 +311,10 @@ class ScriptTest(unittest.TestCase):
 
   MOCKS = {
     "git": GitMock,
+    # TODO(machenbach): Little hack to reuse the git mock for the one svn call
+    # in merge-to-branch. The command should be made explicit in the test
+    # expectations.
+    "svn": GitMock,
     "vi": LogMock,
   }
 
@@ -837,6 +849,124 @@ Performance and stability improvements on all platforms.""", commit)
       auto_roll.RunAutoRoll(TEST_CONFIG, AutoRollOptions(MakeOptions()), self)
     self.assertRaises(Exception, RunAutoRoll)
 
+  def testMergeToBranch(self):
+    TEST_CONFIG[ALREADY_MERGING_SENTINEL_FILE] = self.MakeEmptyTempFile()
+    TEST_CONFIG[DOT_GIT_LOCATION] = self.MakeEmptyTempFile()
+    TEST_CONFIG[VERSION_FILE] = self.MakeTempVersionFile()
+    os.environ["EDITOR"] = "vi"
+    extra_patch = self.MakeEmptyTempFile()
+
+    def VerifyPatch(patch):
+      return lambda: self.assertEquals(patch,
+          FileToText(TEST_CONFIG[TEMPORARY_PATCH_FILE]))
+
+    msg = """Merged r12345, r23456, r34567, r45678, r56789 into trunk branch.
+
+Title4
+
+Title2
+
+Title3
+
+Title1
+
+Title5
+
+BUG=123,234,345,456,567,v8:123
+LOG=N
+"""
+
+    def VerifySVNCommit():
+      commit = FileToText(TEST_CONFIG[COMMITMSG_FILE])
+      self.assertEquals(msg, commit)
+      version = FileToText(TEST_CONFIG[VERSION_FILE])
+      self.assertTrue(re.search(r"#define MINOR_VERSION\s+22", version))
+      self.assertTrue(re.search(r"#define BUILD_NUMBER\s+5", version))
+      self.assertTrue(re.search(r"#define PATCH_LEVEL\s+1", version))
+      self.assertTrue(re.search(r"#define IS_CANDIDATE_VERSION\s+0", version))
+
+    self.ExpectGit([
+      ["status -s -uno", ""],
+      ["status -s -b -uno", "## some_branch\n"],
+      ["svn fetch", ""],
+      ["branch", "  branch1\n* branch2\n"],
+      ["checkout -b %s" % TEST_CONFIG[TEMP_BRANCH], ""],
+      ["branch", "  branch1\n* branch2\n"],
+      ["checkout -b %s svn/trunk" % TEST_CONFIG[BRANCHNAME], ""],
+      ["log svn/bleeding_edge --reverse --format=%H --grep=\"Port r12345\"",
+       "hash1\nhash2"],
+      ["svn find-rev hash1 svn/bleeding_edge", "45678"],
+      ["log -1 --format=%s hash1", "Title1"],
+      ["svn find-rev hash2 svn/bleeding_edge", "23456"],
+      ["log -1 --format=%s hash2", "Title2"],
+      ["log svn/bleeding_edge --reverse --format=%H --grep=\"Port r23456\"",
+       ""],
+      ["log svn/bleeding_edge --reverse --format=%H --grep=\"Port r34567\"",
+       "hash3"],
+      ["svn find-rev hash3 svn/bleeding_edge", "56789"],
+      ["log -1 --format=%s hash3", "Title3"],
+      ["svn find-rev \"r12345\" svn/bleeding_edge", "hash4"],
+      ["svn find-rev \"r23456\" svn/bleeding_edge", "hash2"],
+      ["svn find-rev \"r34567\" svn/bleeding_edge", "hash3"],
+      ["svn find-rev \"r45678\" svn/bleeding_edge", "hash1"],
+      ["svn find-rev \"r56789\" svn/bleeding_edge", "hash5"],
+      ["log -1 --format=%s hash4", "Title4"],
+      ["log -1 --format=%s hash2", "Title2"],
+      ["log -1 --format=%s hash3", "Title3"],
+      ["log -1 --format=%s hash1", "Title1"],
+      ["log -1 --format=%s hash5", "Title5"],
+      ["log -1 hash4", "Title4\nBUG=123\nBUG=234"],
+      ["log -1 hash2", "Title2\n BUG = v8:123,345"],
+      ["log -1 hash3", "Title3\nLOG=n\nBUG=567, 456"],
+      ["log -1 hash1", "Title1"],
+      ["log -1 hash5", "Title5"],
+      ["log -1 -p hash4", "patch4"],
+      ["apply --index --reject  \"%s\"" % TEST_CONFIG[TEMPORARY_PATCH_FILE],
+       "", VerifyPatch("patch4")],
+      ["log -1 -p hash2", "patch2"],
+      ["apply --index --reject  \"%s\"" % TEST_CONFIG[TEMPORARY_PATCH_FILE],
+       "", VerifyPatch("patch2")],
+      ["log -1 -p hash3", "patch3"],
+      ["apply --index --reject  \"%s\"" % TEST_CONFIG[TEMPORARY_PATCH_FILE],
+       "", VerifyPatch("patch3")],
+      ["log -1 -p hash1", "patch1"],
+      ["apply --index --reject  \"%s\"" % TEST_CONFIG[TEMPORARY_PATCH_FILE],
+       "", VerifyPatch("patch1")],
+      ["log -1 -p hash5", "patch5"],
+      ["apply --index --reject  \"%s\"" % TEST_CONFIG[TEMPORARY_PATCH_FILE],
+       "", VerifyPatch("patch5")],
+      ["apply --index --reject  \"%s\"" % extra_patch, ""],
+      ["commit -a -F \"%s\"" % TEST_CONFIG[COMMITMSG_FILE], ""],
+      ["cl upload -r \"reviewer@chromium.org\" --send-mail", ""],
+      ["checkout %s" % TEST_CONFIG[BRANCHNAME], ""],
+      ["cl presubmit", "Presubmit successfull\n"],
+      ["cl dcommit -f --bypass-hooks", "Closing issue\n", VerifySVNCommit],
+      ["svn fetch", ""],
+      ["log -1 --format=%%H --grep=\"%s\" svn/trunk" % msg, "hash6"],
+      ["svn find-rev hash6", "1324"],
+      [("copy -r 1324 https://v8.googlecode.com/svn/trunk "
+        "https://v8.googlecode.com/svn/tags/3.22.5.1 -m "
+        "\"Tagging version 3.22.5.1\""), ""],
+      ["checkout -f some_branch", ""],
+      ["branch -D %s" % TEST_CONFIG[TEMP_BRANCH], ""],
+      ["branch -D %s" % TEST_CONFIG[BRANCHNAME], ""],
+    ])
+
+    self.ExpectReadline([
+      "Y",  # Automatically add corresponding ports (34567, 56789)?
+      "Y",  # Automatically increment patch level?
+      "reviewer@chromium.org",  # V8 reviewer.
+      "LGTM",  # Enter LGTM for V8 CL.
+    ])
+
+    options = MakeOptions(p=extra_patch, f=True)
+    # r12345 and r34567 are patches. r23456 (included) and r45678 are the MIPS
+    # ports of r12345. r56789 is the MIPS port of r34567.
+    args = ["trunk", "12345", "23456", "34567"]
+    self.assertTrue(merge_to_branch.ProcessOptions(options, args))
+    RunMergeToBranch(TEST_CONFIG, MergeToBranchOptions(options, args), self)
+
+
 class SystemTest(unittest.TestCase):
   def testReload(self):
     step = MakeStep(step_class=PrepareChangeLog, number=0, state={}, config={},