Revision: 18057
Author: [email protected]
Date: Mon Nov 25 14:20:39 2013 UTC
Log: Improve and refactor push-to-trunk script.
Let change log formatter squash title and bug reference. Repair wrongly
indented first line in change log. Add automatic rietveld reload of commit
messages to enable late corrections.
BUG=
[email protected]
Review URL: https://codereview.chromium.org/83603002
http://code.google.com/p/v8/source/detail?r=18057
Modified:
/branches/bleeding_edge/tools/push-to-trunk/common_includes.py
/branches/bleeding_edge/tools/push-to-trunk/push_to_trunk.py
/branches/bleeding_edge/tools/push-to-trunk/test_scripts.py
=======================================
--- /branches/bleeding_edge/tools/push-to-trunk/common_includes.py Fri Nov
22 09:48:43 2013 UTC
+++ /branches/bleeding_edge/tools/push-to-trunk/common_includes.py Mon Nov
25 14:20:39 2013 UTC
@@ -99,8 +99,8 @@
result = ""
added_titles = set()
for (title, body, author) in commit_messages:
- # TODO(machenbach): Reload the commit description from rietveld in
order to
- # catch late changes.
+ # TODO(machenbach): Better check for reverts. A revert should remove
the
+ # original CL from the actual log entry.
title = title.strip()
if auto_format:
# Only add commits that set the LOG flag correctly.
@@ -114,16 +114,12 @@
if title in added_titles:
continue
- # TODO(machenbach): Let python do all formatting. Get raw git title,
attach
- # issue and add/move dot to the end - all in one line. Make formatting
and
- # indentation afterwards.
-
- # Add the commit's title line.
- result += "%s\n" % Fill80(title)
+ # Add and format the commit's title and bug reference. Move dot to the
end.
added_titles.add(title)
-
- # Add bug references.
- result += MakeChangeLogBugReference(body)
+ raw_title = re.sub(r"(\.|\?|!)$", "", title)
+ bug_reference = MakeChangeLogBugReference(body)
+ space = " " if bug_reference else ""
+ result += "%s\n" % Fill80("%s%s%s." % (raw_title, space,
bug_reference))
# Append the commit's author for reference if not in auto-format mode.
if not auto_format:
@@ -169,8 +165,7 @@
FormatIssues("Chromium ", crbugs)
if len(bug_groups) > 0:
- # Format with 8 characters indentation and max 80 character lines.
- return "%s\n" % Fill80("(%s)" % ", ".join(bug_groups))
+ return "(%s)" % ", ".join(bug_groups)
else:
return ""
=======================================
--- /branches/bleeding_edge/tools/push-to-trunk/push_to_trunk.py Fri Nov 22
09:48:43 2013 UTC
+++ /branches/bleeding_edge/tools/push-to-trunk/push_to_trunk.py Mon Nov 25
14:20:39 2013 UTC
@@ -30,6 +30,7 @@
import optparse
import sys
import tempfile
+import urllib2
from common_includes import *
@@ -91,6 +92,21 @@
class PrepareChangeLog(Step):
MESSAGE = "Prepare raw ChangeLog entry."
+ def Reload(self, body):
+ """Attempts to reload the commit message from rietveld in order to
allow
+ late changes to the LOG flag. Note: This is brittle to future changes
of
+ the web page name or structure.
+ """
+ match = re.search(r"^Review URL:
https://codereview\.chromium\.org/(\d+)$",
+ body, flags=re.M)
+ if match:
+ cl_url = "https://codereview.chromium.org/%s/description"; %
match.group(1)
+ try:
+ body = self.ReadURL(cl_url)
+ except urllib2.URLError:
+ pass
+ return body
+
def RunStep(self):
self.RestoreIfUnset("last_push")
@@ -112,7 +128,7 @@
commit_messages = [
[
self.Git("log -1 %s --format=\"%%s\"" % commit),
- self.Git("log -1 %s --format=\"%%B\"" % commit),
+ self.Reload(self.Git("log -1 %s --format=\"%%B\"" % commit)),
self.Git("log -1 %s --format=\"%%an\"" % commit),
] for commit in commits.splitlines()
]
@@ -151,6 +167,7 @@
changelog_entry =
FileToText(self.Config(CHANGELOG_ENTRY_FILE)).rstrip()
changelog_entry = StripComments(changelog_entry)
changelog_entry = "\n".join(map(Fill80, changelog_entry.splitlines()))
+ changelog_entry = changelog_entry.lstrip()
if changelog_entry == "":
self.Die("Empty ChangeLog entry.")
@@ -541,6 +558,12 @@
if options.s < 0:
print "Bad step number %d" % options.s
return False
+ if options.f and not options.r:
+ print "A reviewer (-r) is required in forced mode."
+ return False
+ if options.f and not options.c:
+ print "A chromium checkout (-c) is required in forced mode."
+ return False
return True
=======================================
--- /branches/bleeding_edge/tools/push-to-trunk/test_scripts.py Fri Nov 22
09:48:43 2013 UTC
+++ /branches/bleeding_edge/tools/push-to-trunk/test_scripts.py Mon Nov 25
14:20:39 2013 UTC
@@ -71,14 +71,13 @@
["Title text 1",
"Title text 1\n\nBUG=\n",
"[email protected]"],
- ["Title text 2",
+ ["Title text 2.",
"Title text 2\n\nBUG=1234\n",
"[email protected]"],
]
- self.assertEquals(" Title text 1\n"
+ self.assertEquals(" Title text 1.\n"
" ([email protected])\n\n"
- " Title text 2\n"
- " (Chromium issue 1234)\n"
+ " Title text 2 (Chromium issue 1234).\n"
" ([email protected])\n\n",
MakeChangeLogBody(commits))
@@ -87,7 +86,7 @@
def testMakeChangeLogBodyAutoFormat(self):
commits = [
- ["Title text 1",
+ ["Title text 1!",
"Title text 1\nLOG=y\nBUG=\n",
"[email protected]"],
["Title text 2",
@@ -100,9 +99,8 @@
"Title text 4\n\nBUG=1234\nLOG=\n",
"[email protected]"],
]
- self.assertEquals(" Title text 1\n\n"
- " Title text 3\n"
- " (Chromium issue 1234)\n\n",
+ self.assertEquals(" Title text 1.\n\n"
+ " Title text 3 (Chromium issue 1234).\n\n",
MakeChangeLogBody(commits, True))
def testMakeChangeLogBugReferenceEmpty(self):
@@ -112,13 +110,13 @@
self.assertEquals("", MakeChangeLogBugReference("BUG=none\t"))
def testMakeChangeLogBugReferenceSimple(self):
- self.assertEquals(" (issue 987654)\n",
+ self.assertEquals("(issue 987654)",
MakeChangeLogBugReference("BUG = v8:987654"))
- self.assertEquals(" (Chromium issue 987654)\n",
+ self.assertEquals("(Chromium issue 987654)",
MakeChangeLogBugReference("BUG=987654 "))
def testMakeChangeLogBugReferenceFromBody(self):
- self.assertEquals(" (Chromium issue 1234567)\n",
+ self.assertEquals("(Chromium issue 1234567)",
MakeChangeLogBugReference("Title\n\nTBR=\nBUG=\n"
"
BUG=\tchromium:1234567\t\n"
"R=somebody\n"))
@@ -126,54 +124,56 @@
def testMakeChangeLogBugReferenceMultiple(self):
# All issues should be sorted and grouped. Multiple references to the
same
# issue should be filtered.
- self.assertEquals(" (issues 123, 234, Chromium issue 345)\n",
+ self.assertEquals("(issues 123, 234, Chromium issue 345)",
MakeChangeLogBugReference("Title\n\n"
"BUG=v8:234\n"
" BUG\t= 345, \tv8:234,\n"
"BUG=v8:123\n"
"R=somebody\n"))
- self.assertEquals(" (Chromium issues 123, 234)\n",
+ self.assertEquals("(Chromium issues 123, 234)",
MakeChangeLogBugReference("Title\n\n"
"BUG=234,,chromium:123 \n"
"R=somebody\n"))
- self.assertEquals(" (Chromium issues 123, 234)\n",
+ self.assertEquals("(Chromium issues 123, 234)",
MakeChangeLogBugReference("Title\n\n"
"BUG=chromium:234, , 123\n"
"R=somebody\n"))
- self.assertEquals(" (issues 345, 456)\n",
+ self.assertEquals("(issues 345, 456)",
MakeChangeLogBugReference("Title\n\n"
"\t\tBUG=v8:345,v8:456\n"
"R=somebody\n"))
- self.assertEquals(" (issue 123, Chromium issues 345, 456)\n",
+ self.assertEquals("(issue 123, Chromium issues 345, 456)",
MakeChangeLogBugReference("Title\n\n"
"BUG=chromium:456\n"
"BUG = none\n"
"R=somebody\n"
"BUG=456,v8:123, 345"))
+ # TODO(machenbach): These test don't make much sense when the formatting
is
+ # done later.
def testMakeChangeLogBugReferenceLong(self):
# -----------------00--------10--------20--------30--------
- self.assertEquals(" (issues 234, 1234567890, 1234567"
- "8901234567890, Chromium issues 12345678,\n"
- " 123456789)\n",
+ self.assertEquals("(issues 234, 1234567890, 1234567"
+ "8901234567890, Chromium issues 12345678,"
+ " 123456789)",
MakeChangeLogBugReference("BUG=v8:234\n"
"BUG=v8:1234567890\n"
"BUG=v8:12345678901234567890\n"
"BUG=123456789\n"
"BUG=12345678\n"))
# -----------------00--------10--------20--------30--------
- self.assertEquals(" (issues 234, 1234567890, 1234567"
- "8901234567890, Chromium issues\n"
- " 123456789, 1234567890)\n",
+ self.assertEquals("(issues 234, 1234567890, 1234567"
+ "8901234567890, Chromium issues"
+ " 123456789, 1234567890)",
MakeChangeLogBugReference("BUG=v8:234\n"
"BUG=v8:12345678901234567890\n"
"BUG=v8:1234567890\n"
"BUG=123456789\n"
"BUG=1234567890\n"))
# -----------------00--------10--------20--------30--------
- self.assertEquals(" (Chromium issues 234, 1234567890"
- ", 12345678901234567,\n"
- " 1234567890123456789)\n",
+ self.assertEquals("(Chromium issues 234, 1234567890"
+ ", 12345678901234567, "
+ "1234567890123456789)",
MakeChangeLogBugReference("BUG=234\n"
"BUG=12345678901234567\n"
"BUG=1234567890123456789\n"
@@ -194,7 +194,7 @@
try:
expected_call = self._recipe[self._index]
except IndexError:
- raise Exception("Calling %s %s" % (name, " ".join(args)))
+ raise Exception("Calling %s %s" % (self._name, " ".join(args)))
# Pack expectations without arguments into a list.
if not isinstance(expected_call, list):
@@ -397,18 +397,29 @@
TEST_CONFIG[CHANGELOG_ENTRY_FILE] = self.MakeEmptyTempFile()
self.ExpectGit([
- ["log 1234..HEAD --format=%H", "rev1\nrev2\nrev3"],
+ ["log 1234..HEAD --format=%H", "rev1\nrev2\nrev3\nrev4"],
["log -1 rev1 --format=\"%s\"", "Title text 1"],
["log -1 rev1 --format=\"%B\"", "Title\n\nBUG=\nLOG=y\n"],
["log -1 rev1 --format=\"%an\"", "[email protected]"],
- ["log -1 rev2 --format=\"%s\"", "Title text 2"],
+ ["log -1 rev2 --format=\"%s\"", "Title text 2."],
["log -1 rev2 --format=\"%B\"", "Title\n\nBUG=123\nLOG= \n"],
["log -1 rev2 --format=\"%an\"", "[email protected]"],
["log -1 rev3 --format=\"%s\"", "Title text 3"],
["log -1 rev3 --format=\"%B\"", "Title\n\nBUG=321\nLOG=true\n"],
["log -1 rev3 --format=\"%an\"", "[email protected]"],
+ ["log -1 rev4 --format=\"%s\"", "Title text 4"],
+ ["log -1 rev4 --format=\"%B\"",
+ ("Title\n\nBUG=456\nLOG=Y\n\n"
+ "Review URL: https://codereview.chromium.org/9876543210\n";)],
+ ["log -1 rev4 --format=\"%an\"", "[email protected]"],
])
+ # The cl for rev4 on rietveld has an updated LOG flag.
+ self.ExpectReadURL([
+ ["https://codereview.chromium.org/9876543210/description";,
+ "Title\n\nBUG=456\nLOG=N\n\n"],
+ ])
+
self.MakeStep().Persist("last_push", "1234")
self.MakeStep(PrepareChangeLog).Run()
@@ -418,10 +429,9 @@
# comparison here instead of a regexp match.
expected_cl = """\\d+\\-\\d+\\-\\d+: Version 3\\.22\\.5
- Title text 1
+ Title text 1.
- Title text 3
- \\(Chromium issue 321\\)
+ Title text 3 \\(Chromium issue 321\\).
Performance and stability improvements on all platforms\\.
#
@@ -429,17 +439,18 @@
# commit messages from the list below are included\\.
# All lines starting with # will be stripped\\.
#
-# Title text 1
+# Title text 1.
# \\(author1@chromium\\.org\\)
#
-# Title text 2
-# \\(Chromium issue 123\\)
+# Title text 2 \\(Chromium issue 123\\).
# \\(author2@chromium\\.org\\)
#
-# Title text 3
-# \\(Chromium issue 321\\)
+# Title text 3 \\(Chromium issue 321\\).
# \\(author3@chromium\\.org\\)
#
+# Title text 4 \\(Chromium issue 456\\).
+# \\(author4@chromium\\.org\\)
+#
#"""
self.assertTrue(re.match(expected_cl, actual_cl))
@@ -461,7 +472,7 @@
self.MakeStep(EditChangeLog).Run()
- self.assertEquals(" New\n Lines\n\n\n Original
CL",
+ self.assertEquals("New\n Lines\n\n\n Original CL",
FileToText(TEST_CONFIG[CHANGELOG_FILE]))
def testIncrementVersion(self):
@@ -539,8 +550,7 @@
def CheckPreparePush():
cl = FileToText(TEST_CONFIG[CHANGELOG_FILE])
self.assertTrue(re.search(r"Version 3.22.5", cl))
- self.assertTrue(re.search(r" Log text 1", cl))
- self.assertTrue(re.search(r" \(issue 321\)", cl))
+ self.assertTrue(re.search(r" Log text 1 \(issue 321\).", cl))
self.assertFalse(re.search(r" \(author1@chromium\.org\)", cl))
# Make sure all comments got stripped.
@@ -555,7 +565,7 @@
def CheckSVNCommit():
commit = FileToText(TEST_CONFIG[COMMITMSG_FILE])
self.assertTrue(re.search(r"Version 3.22.5", commit))
- self.assertTrue(re.search(r"Log text 1. \(issue 321\)", commit))
+ self.assertTrue(re.search(r"Log text 1 \(issue 321\).", 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))
@@ -643,9 +653,8 @@
self.assertTrue(re.search("\"v8_revision\": \"123456\"", deps))
cl = FileToText(TEST_CONFIG[CHANGELOG_FILE])
- self.assertTrue(re.search(r"\d\d\d\d\-\d+\-\d+: Version 3\.22\.5", cl))
- self.assertTrue(re.search(r" Log text 1", cl))
- self.assertTrue(re.search(r" \(issue 321\)", cl))
+ self.assertTrue(re.search(r"^\d\d\d\d\-\d+\-\d+: Version 3\.22\.5",
cl))
+ self.assertTrue(re.search(r" Log text 1 \(issue 321\).", cl))
self.assertTrue(re.search(r"1999\-04\-05: Version 3\.22\.4", cl))
# Note: The version file is on build number 5 again in the end of this
test
@@ -686,3 +695,27 @@
self.assertEquals("100", self.MakeStep().Restore("lkgr"))
self.assertEquals("101", self.MakeStep().Restore("latest"))
+
+
+class SystemTest(unittest.TestCase):
+ def testReload(self):
+ step = MakeStep(step_class=PrepareChangeLog, number=0, state={},
config={},
+ options=None,
+ side_effect_handler=DEFAULT_SIDE_EFFECT_HANDLER)
+ body = step.Reload(
+"""------------------------------------------------------------------------
+r17997 | [email protected] | 2013-11-22 11:04:04 +0100 (...) | 6
lines
+
+Prepare push to trunk. Now working on version 3.23.11.
+
[email protected]
+
+Review URL: https://codereview.chromium.org/83173002
+
+------------------------------------------------------------------------""")
+ self.assertEquals(
+"""Prepare push to trunk. Now working on version 3.23.11.
+
[email protected]
+
+Committed: https://code.google.com/p/v8/source/detail?r=17997""";, body)
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
