[Release] Update merge script to leverage auto-tag bot

tools/release/merge_to_branch.py

 #!/usr/bin/env python
 # Copyright 2014 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 81 matching lines @@
     if len(port_revision_list) > 0:
       if self.Confirm("Automatically add corresponding ports (%s)?"
                       % ", ".join(port_revision_list)):
         #: 'y': Add ports to revision list.
         self["full_revision_list"].extend(port_revision_list)
 
 
 class CreateCommitMessage(Step):
   MESSAGE = "Create commit message."
 
+  def _create_commit_description(self, commit_hash):
+    patch_merge_desc = self.GitLog(n=1, format="%s", git_hash=commit_hash)
+    description = "Merged: " + patch_merge_desc + "\n"
+    description = description + "Hash: " + commit_hash + "\n\n"

[Michael Achenbach, 2015/10/21 12:32:35]

Maybe s/Hash/Revision

[Michael Hablich, 2015/10/21 12:41:26]

You mean like Hash: 4df56dsssss r12345

?

[Michael Achenbach, 2015/10/21 12:51:17]

No I mean literally. Use the more commonly used word "Revision" instead of
"Hash", so that'd read, e.g.:
Revision: 4df56dsssss

[Michael Hablich, 2015/10/22 14:25:50]

Acknowledged.

+    return description
+
   def RunStep(self):
 
     # Stringify: ["abcde", "12345"] -> "abcde, 12345"
     self["revision_list"] = ", ".join(self["full_revision_list"])
 
     if not self["revision_list"]:  # pragma: no cover
       self.Die("Revision list is empty.")
 
-    action_text = "Merged %s"
+    msg_pieces = []
 
-    # The commit message title is added below after the version is specified.
-    msg_pieces = [
-      "\n".join(action_text % s for s in self["full_revision_list"]),
-    ]
-    msg_pieces.append("\n\n")
+    if len(self["full_revision_list"]) > 1:
+      self["commit_title"] = "Merged: Squashed multiple commits."
+    else:
+      commit_hash = self["full_revision_list"][0]

[Michael Achenbach, 2015/10/21 12:32:35]

Why not make a pure cherry-pick in case of one commit?

[Michael Hablich, 2015/10/21 12:41:26]

I didn't want to introduce more complexity into the script especially as a pure
cherry-pick is offering no additional value.

[Michael Achenbach, 2015/10/21 12:51:17]

It offers the full original commit message instead of only a title. + this seems
to be the practice for chromium branches. I thought that should become the
standard case and squashing would be an exception.

On the other hand (have to think more): It might get harder for the v8rel script
to still refer to the merged CLs.

[Michael Hablich, 2015/10/22 14:25:49]

So, I tried cherry-picks on our branches a few times. My impression was that the
full commit message is only confusing and there is no mentioning what revision
got cherry-picked. I think a squash is much easier to digest and does not
introduce extra complexity into the script.

+      patch_merge_desc = self.GitLog(n=1, format="%s", git_hash=commit_hash)
+      title = "Merged: " + patch_merge_desc
+      self["commit_title"] = title
 
     for commit_hash in self["full_revision_list"]:
-      patch_merge_desc = self.GitLog(n=1, format="%s", git_hash=commit_hash)
-      msg_pieces.append("%s\n\n" % patch_merge_desc)
+      msg_pieces.append(self._create_commit_description(commit_hash))
 
     bugs = []
     for commit_hash in self["full_revision_list"]:
       msg = self.GitLog(n=1, git_hash=commit_hash)
       for bug in re.findall(r"^[ \t]*BUG[ \t]*=[ \t]*(.*?)[ \t]*$", msg, re.M):
         bugs.extend(s.strip() for s in bug.split(","))
     bug_aggregate = ",".join(sorted(filter(lambda s: s and s != "none", bugs)))
     if bug_aggregate:
       msg_pieces.append("BUG=%s\nLOG=N\n" % bug_aggregate)
 
     self["new_commit_msg"] = "".join(msg_pieces)
 
 
 class ApplyPatches(Step):
   MESSAGE = "Apply patches for selected revisions."
 
   def RunStep(self):
     for commit_hash in self["full_revision_list"]:
       print("Applying patch for %s to %s..."
             % (commit_hash, self["merge_to_branch"]))
       patch = self.GitGetPatch(commit_hash)
       TextToFile(patch, self.Config("TEMPORARY_PATCH_FILE"))
       self.ApplyPatch(self.Config("TEMPORARY_PATCH_FILE"))
     if self._options.patch:
       self.ApplyPatch(self._options.patch)
 
-
-class PrepareVersion(Step):
-  MESSAGE = "Prepare version file."
-
-  def RunStep(self):
-    # This is used to calculate the patch level increment.
-    self.ReadAndPersistVersion()
-
-
-class IncrementVersion(Step):
-  MESSAGE = "Increment version number."
-
-  def RunStep(self):
-    new_patch = str(int(self["patch"]) + 1)
-    if self.Confirm("Automatically increment V8_PATCH_LEVEL? (Saying 'n' will "
-                    "fire up your EDITOR on %s so you can make arbitrary "
-                    "changes. When you're done, save the file and exit your "
-                    "EDITOR.)" % VERSION_FILE):
-      text = FileToText(os.path.join(self.default_cwd, VERSION_FILE))
-      text = MSub(r"(?<=#define V8_PATCH_LEVEL)(?P<space>\s+)\d*$",
-                  r"\g<space>%s" % new_patch,
-                  text)
-      TextToFile(text, os.path.join(self.default_cwd, VERSION_FILE))
-    else:
-      self.Editor(os.path.join(self.default_cwd, VERSION_FILE))
-    self.ReadAndPersistVersion("new_")
-    self["version"] = "%s.%s.%s.%s" % (self["new_major"],
-                                       self["new_minor"],
-                                       self["new_build"],
-                                       self["new_patch"])
-
-
 class CommitLocal(Step):
   MESSAGE = "Commit to local branch."
 
   def RunStep(self):
     # Add a commit message title.
-    self["commit_title"] = "Version %s (cherry-pick)" % self["version"]
     self["new_commit_msg"] = "%s\n\n%s" % (self["commit_title"],
                                            self["new_commit_msg"])
     TextToFile(self["new_commit_msg"], self.Config("COMMITMSG_FILE"))
     self.GitCommit(file_name=self.Config("COMMITMSG_FILE"))
 
 
 class CommitRepository(Step):
   MESSAGE = "Commit to the repository."
 
   def RunStep(self):
     self.GitCheckout(self.Config("BRANCHNAME"))
     self.WaitForLGTM()
     self.GitPresubmit()
     self.vc.CLLand()
 
-
-class TagRevision(Step):
-  MESSAGE = "Create the tag."
-
-  def RunStep(self):
-    print "Creating tag %s" % self["version"]
-    self.vc.Tag(self["version"],
-                self.vc.RemoteBranch(self["merge_to_branch"]),
-                self["commit_title"])
-
-
 class CleanUp(Step):
   MESSAGE = "Cleanup."
 
   def RunStep(self):
     self.CommonCleanup()
     print "*** SUMMARY ***"
     print "version: %s" % self["version"]

[Michael Achenbach, 2015/10/21 12:32:35]

Remove version output.

[Michael Hablich, 2015/10/21 12:41:26]

Ack.

     print "branch: %s" % self["merge_to_branch"]
     if self["revision_list"]:
       print "patches: %s" % self["revision_list"]
 
 
 class MergeToBranch(ScriptsBase):
   def _Description(self):
     return ("Performs the necessary steps to merge revisions from "
             "master to other branches, including candidates.")
 
@@ skipping 41 matching lines @@
       "COMMITMSG_FILE": "/tmp/v8-prepare-merge-tempfile-commitmsg",
     }
 
   def _Steps(self):
     return [
       Preparation,
       CreateBranch,
       SearchArchitecturePorts,
       CreateCommitMessage,
       ApplyPatches,
-      PrepareVersion,
-      IncrementVersion,
+      # PrepareVersion,

[Michael Achenbach, 2015/10/21 12:32:35]

nit: Remove the comments.

[Michael Achenbach, 2015/10/21 12:38:37]

Done.

[Michael Hablich, 2015/10/21 12:41:26]

Done.

+      # IncrementVersion,
       CommitLocal,
       UploadStep,

[Michael Achenbach, 2015/10/21 12:32:35]

Maybe make the upload step configurable and let it add some text to the email
explaining the new merge script. Otherwise people might get confused.

[Michael Hablich, 2015/10/21 12:41:26]

I don't think more configuration will help. 

Adding a intermediate change announcement is a good idea though.

[Michael Achenbach, 2015/10/21 12:51:17]

I don't care how it would be done technically. But it would be nice if it ended
up in the email so that the merger and the reviewers see what's going on.

       CommitRepository,
-      TagRevision,
+      # TagRevision,
       CleanUp,
     ]
 
 
 if __name__ == "__main__":  # pragma: no cover
   sys.exit(MergeToBranch().Run())