Refactor and improve bug line generation for push-to-trunk.

tools/push-to-trunk/common_includes.py

Index: tools/push-to-trunk/common_includes.py
diff --git a/tools/push-to-trunk/common_includes.py b/tools/push-to-trunk/common_includes.py
index 06b7ebe53a3c3655e41048c186bef308b59402c0..55182d8019cdce85a29c1aab2e9c070bc8c5cbeb 100644
--- a/tools/push-to-trunk/common_includes.py
+++ b/tools/push-to-trunk/common_includes.py
@@ -30,6 +30,7 @@ import os
 import re
 import subprocess
 import sys
+import textwrap
 
 PERSISTFILE_BASENAME = "PERSISTFILE_BASENAME"
 TEMP_BRANCH = "TEMP_BRANCH"
@@ -67,6 +68,11 @@ def MSub(rexp, replacement, text):
   return re.sub(rexp, replacement, text, flags=re.MULTILINE)
 
 
+def Fill80(line):
+  return textwrap.fill(line, width=80, initial_indent="        ",
+                       subsequent_indent="        ")
+
+
 def GetLastChangeLogEntries(change_log_file):
   result = []
   for line in LinesInFile(change_log_file):
@@ -81,28 +87,66 @@ def MakeChangeLogBody(commit_generator):
     # Add the commit's title line.
     result += "%s\n" % title.rstrip()
 
-    # Grep for "BUG=xxxx" lines in the commit message and convert them to
-    # "(issue xxxx)".
-    out = body.splitlines()
-    out = filter(lambda x: re.search(r"^BUG=", x), out)
-    out = filter(lambda x: not re.search(r"BUG=$", x), out)
-    out = filter(lambda x: not re.search(r"BUG=none$", x), out)
-
-    # TODO(machenbach): Handle multiple entries (e.g. BUG=123, 234).
-    def FormatIssue(text):
-      text = re.sub(r"BUG=v8:(.*)$", r"(issue \1)", text)
-      text = re.sub(r"BUG=chromium:(.*)$", r"(Chromium issue \1)", text)
-      text = re.sub(r"BUG=(.*)$", r"(Chromium issue \1)", text)
-      return "        %s\n" % text
-
-    for line in map(FormatIssue, out):
-      result += line
+    # Add bug references.
+    result += MakeChangeLogBugReference(body)
 
     # Append the commit's author for reference.
     result += "%s\n\n" % author.rstrip()
   return result
 
 
+def MakeChangeLogBugReference(body):
+  # Grep for "BUG=xxxx" lines in the commit message and convert them to
+  # "(issue xxxx)".
+  out = body.splitlines()
+  out = filter(lambda x: re.search(r"^[ \t]*BUG[ \t]*=", x), out)

[Jakob Kummerow, 2013/11/19 13:39:29]

I don't think we need any of these three "filter" lines any more. AddIssues()
below should handle all of that anyway.

[Michael Achenbach, 2013/11/19 15:02:57]

Done.

+  out = filter(lambda x: not re.search(r"BUG[ \t]*=[ \t]*$", x), out)
+  out = filter(lambda x: not re.search(r"BUG[ \t]*=[ \t]*none[ \t]*$", x), out)
+
+  crbugs = []
+  v8bugs = []
+
+  def AddSafe(bugs, bug):

[Jakob Kummerow, 2013/11/19 13:39:29]

I think you don't need this function at all, if below in the regexes you match
against (\d+) instead of (.*).

[Michael Achenbach, 2013/11/19 15:02:57]

Done.

+    try:
+      bugs.append(int(bug))
+    except ValueError:
+      pass
+
+  def AddIssues(text):
+    ref = re.match(r"^[ \t]*BUG[ \t]*=[ \t]*(.*?)[ \t]*$", text)

[Jakob Kummerow, 2013/11/19 13:39:29]

I think the regex can be simplified to:

r"^[ \t]*BUG=(.*?)$"

Reasons:
- whitespace around '=' doesn't happen in practice
- whitespace at the end would be strip()'ed away below anyway.
- the simpler a regex, the better :-)

[Michael Achenbach, 2013/11/19 15:02:57]

Regarding the inner whitespace I had
https://code.google.com/p/chromium/codesearch#chromium/tools/depot_tools/git_...
as reference. That's what is allowed when depot tools use it.

Agreed on the outer whitespace. Added a strip.

+    if not ref:
+      return
+    for bug in ref.group(1).split(","):
+      bug = bug.strip()
+      match = re.match(r"^v8[ \t]*:[ \t]*(.*)$", bug)

[Jakob Kummerow, 2013/11/19 13:39:29]

similarly, we don't need to support whitespace around ':'

[Michael Achenbach, 2013/11/19 15:02:57]

Done.

+      if match: AddSafe(v8bugs, match.group(1))
+      else:
+        match = re.match(r"^(?:chromium[ \t]*:)?[ \t]*(.*)$", bug)
+        if match: AddSafe(crbugs, match.group(1))
+
+  # Add issues to crbugs and v8bugs.
+  map(AddIssues, out)
+
+  # Filter duplicates, sort, stringify.
+  crbugs = map(str, sorted(set(crbugs)))
+  v8bugs = map(str, sorted(set(v8bugs)))
+
+  bug_groups = []
+  def FormatIssues(prefix, bugs):
+    if len(bugs) > 0:
+      plural = "s" if len(bugs) > 1 else ""
+      bug_groups.append("%sissue%s %s" % (prefix, plural, ", ".join(bugs)))
+
+  FormatIssues("Chromium ", crbugs)
+  FormatIssues("", v8bugs)

[Jakob Kummerow, 2013/11/19 13:39:29]

V8 bugs first, please :-)

[Michael Achenbach, 2013/11/19 15:02:57]

Done. That required some test changes...

+
+  if len(bug_groups) > 0:
+    # Format with 8 characters indentation and max 80 character lines.
+    return "%s\n" % Fill80("(%s)" % ", ".join(bug_groups))
+  else:
+    return ""
+
+
 # Some commands don't like the pipe, e.g. calling vi from within the script or
 # from subscripts like git cl upload.
 def Command(cmd, args="", prefix="", pipe=True):