Pythonification and refactoring of push-to-trunk.

R=jkummerow@chromium.org

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

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

tools/push-to-trunk/auto_roll.py

@@ -39,8 +39,7 @@ CONFIG = {
 
 
 class Preparation(Step):
-  def __init__(self):
-    Step.__init__(self, "Preparation.")
+  MESSAGE = "Preparation."
 
   def RunStep(self):
     self.InitialEnvironmentChecks()
@@ -48,8 +47,7 @@ class Preparation(Step):
 
 
 class FetchLatestRevision(Step):
-  def __init__(self):
-    Step.__init__(self, "Fetching latest V8 revision.")
+  MESSAGE = "Fetching latest V8 revision."
 
   def RunStep(self):
     log = self.Git("svn log -1 --oneline").strip()
@@ -60,8 +58,7 @@ class FetchLatestRevision(Step):
 
 
 class FetchLKGR(Step):
-  def __init__(self):
-    Step.__init__(self, "Fetching V8 LKGR.")
+  MESSAGE = "Fetching V8 LKGR."
 
   def RunStep(self):
     lkgr_url = "https://v8-status.appspot.com/lkgr"
@@ -69,8 +66,7 @@ class FetchLKGR(Step):
 
 
 class PushToTrunk(Step):
-  def __init__(self):
-    Step.__init__(self, "Pushing to trunk if possible.")
+  MESSAGE = "Pushing to trunk if possible."
 
   def RunStep(self):
     self.RestoreIfUnset("latest")

tools/push-to-trunk/common_includes.py

@@ -70,6 +70,10 @@ def MSub(rexp, replacement, text):
 
 
 def Fill80(line):
+  # Replace tabs and remove surrounding space.
+  line = re.sub(r"\t", r"        ", line.strip())
+
+  # Format with 8 characters indentation and line width 80.
   return textwrap.fill(line, width=80, initial_indent="        ",
                        subsequent_indent="        ")
 
@@ -97,7 +101,7 @@ def MakeChangeLogBody(commit_messages, auto_format=False):
   for (title, body, author) in commit_messages:
     # TODO(machenbach): Reload the commit description from rietveld in order to
     # catch late changes.
-    title = title.rstrip()
+    title = title.strip()
     if auto_format:
       # Only add commits that set the LOG flag correctly.
       log_exp = r"^[ \t]*LOG[ \t]*=[ \t]*(?:Y(?:ES)?)|TRUE"
@@ -115,7 +119,7 @@ def MakeChangeLogBody(commit_messages, auto_format=False):
     # indentation afterwards.
 
     # Add the commit's title line.
-    result += "%s\n" % title
+    result += "%s\n" % Fill80(title)
     added_titles.add(title)
 
     # Add bug references.
@@ -123,7 +127,7 @@ def MakeChangeLogBody(commit_messages, auto_format=False):
 
     # Append the commit's author for reference if not in auto-format mode.
     if not auto_format:
-      result += "%s\n" % author.rstrip()
+      result += "%s\n" % Fill80("(%s)" % author.strip())
 
     result += "\n"
   return result
@@ -205,36 +209,23 @@ DEFAULT_SIDE_EFFECT_HANDLER = SideEffectHandler()
 
 
 class Step(object):
-  def __init__(self, text="", requires=None):
+  def __init__(self, text, requires, number, config, state, options, handler):
     self._text = text
-    self._number = -1
-    self._options = None
     self._requires = requires
-    self._side_effect_handler = DEFAULT_SIDE_EFFECT_HANDLER
-
-  def SetNumber(self, number):
     self._number = number
-
-  def SetConfig(self, config):
     self._config = config
-
-  def SetState(self, state):
     self._state = state
-
-  def SetOptions(self, options):
     self._options = options
-
-  def SetSideEffectHandler(self, handler):
     self._side_effect_handler = handler
+    assert self._number >= 0
+    assert self._config is not None
+    assert self._state is not None
+    assert self._side_effect_handler is not None
 
   def Config(self, key):
     return self._config[key]
 
   def Run(self):
-    assert self._number >= 0
-    assert self._config is not None
-    assert self._state is not None
-    assert self._side_effect_handler is not None
     if self._requires:
       self.RestoreIfUnset(self._requires)
       if not self._state[self._requires]:
@@ -411,8 +402,7 @@ class Step(object):
 
 
 class UploadStep(Step):
-  def __init__(self):
-    Step.__init__(self, "Upload for code review.")
+  MESSAGE = "Upload for code review."
 
   def RunStep(self):
     if self._options.r:
@@ -430,24 +420,35 @@ class UploadStep(Step):
       self.Die("'git cl upload' failed, please try again.")
 
 
+def MakeStep(step_class=Step, number=0, state=None, config=None,
+             options=None, side_effect_handler=DEFAULT_SIDE_EFFECT_HANDLER):
+    # Allow to pass in empty dictionaries.
+    state = state if state is not None else {}
+    config = config if config is not None else {}
+
+    try:
+      message = step_class.MESSAGE
+    except AttributeError:
+      message = step_class.__name__
+    try:
+      requires = step_class.REQUIRES
+    except AttributeError:
+      requires = None
+
+    return step_class(message, requires, number=number, config=config,
+                      state=state, options=options,
+                      handler=side_effect_handler)
+
+
 def RunScript(step_classes,
               config,
               options,
               side_effect_handler=DEFAULT_SIDE_EFFECT_HANDLER):
   state = {}
   steps = []
-  number = 0
-
-  for step_class in step_classes:
-    # TODO(machenbach): Factory methods.
-    step = step_class()
-    step.SetNumber(number)
-    step.SetConfig(config)
-    step.SetOptions(options)
-    step.SetState(state)
-    step.SetSideEffectHandler(side_effect_handler)
-    steps.append(step)
-    number += 1
+  for (number, step_class) in enumerate(step_classes):
+    steps.append(MakeStep(step_class, number, state, config,
+                          options, side_effect_handler))
 
   for step in steps[options.s:]:
     step.Run()

tools/push-to-trunk/push_to_trunk.py

@@ -53,8 +53,7 @@ CONFIG = {
 
 
 class Preparation(Step):
-  def __init__(self):
-    Step.__init__(self, "Preparation.")
+  MESSAGE = "Preparation."
 
   def RunStep(self):
     self.InitialEnvironmentChecks()
@@ -64,8 +63,7 @@ class Preparation(Step):
 
 
 class FreshBranch(Step):
-  def __init__(self):
-    Step.__init__(self, "Create a fresh branch.")
+  MESSAGE = "Create a fresh branch."
 
   def RunStep(self):
     args = "checkout -b %s svn/bleeding_edge" % self.Config(BRANCHNAME)
@@ -74,8 +72,7 @@ class FreshBranch(Step):
 
 
 class DetectLastPush(Step):
-  def __init__(self):
-    Step.__init__(self, "Detect commit ID of last push to trunk.")
+  MESSAGE = "Detect commit ID of last push to trunk."
 
   def RunStep(self):
     last_push = (self._options.l or
@@ -92,8 +89,7 @@ class DetectLastPush(Step):
 
 
 class PrepareChangeLog(Step):
-  def __init__(self):
-    Step.__init__(self, "Prepare raw ChangeLog entry.")
+  MESSAGE = "Prepare raw ChangeLog entry."
 
   def RunStep(self):
     self.RestoreIfUnset("last_push")
@@ -115,9 +111,9 @@ class PrepareChangeLog(Step):
     # Cache raw commit messages.
     commit_messages = [
       [
-        self.Git("log -1 %s --format=\"%%w(80,8,8)%%s\"" % commit),
+        self.Git("log -1 %s --format=\"%%s\"" % commit),
         self.Git("log -1 %s --format=\"%%B\"" % commit),
-        self.Git("log -1 %s --format=\"%%w(80,8,8)(%%an)\"" % commit),
+        self.Git("log -1 %s --format=\"%%an\"" % commit),
       ] for commit in commits.splitlines()
     ]
 
@@ -137,8 +133,7 @@ class PrepareChangeLog(Step):
 
 
 class EditChangeLog(Step):
-  def __init__(self):
-    Step.__init__(self, "Edit ChangeLog entry.")
+  MESSAGE = "Edit ChangeLog entry."
 
   def RunStep(self):
     print ("Please press <Return> to have your EDITOR open the ChangeLog "
@@ -152,14 +147,10 @@ class EditChangeLog(Step):
     handle, new_changelog = tempfile.mkstemp()
     os.close(handle)
 
-    # (1) Strip comments, (2) eliminate tabs, (3) fix too little and (4) too
-    # much indentation, and (5) eliminate trailing whitespace.
+    # Strip comments and reformat with correct indentation.
     changelog_entry = FileToText(self.Config(CHANGELOG_ENTRY_FILE)).rstrip()
     changelog_entry = StripComments(changelog_entry)
-    changelog_entry = MSub(r"\t", r"        ", changelog_entry)
-    changelog_entry = MSub(r"^ {1,7}([^ ])", r"        \1", changelog_entry)
-    changelog_entry = MSub(r"^ {9,80}([^ ])", r"        \1", changelog_entry)
-    changelog_entry = MSub(r" +$", r"", changelog_entry)
+    changelog_entry = "\n".join(map(Fill80, changelog_entry.splitlines()))
 
     if changelog_entry == "":
       self.Die("Empty ChangeLog entry.")
@@ -174,8 +165,7 @@ class EditChangeLog(Step):
 
 
 class IncrementVersion(Step):
-  def __init__(self):
-    Step.__init__(self, "Increment version number.")
+  MESSAGE = "Increment version number."
 
   def RunStep(self):
     self.RestoreIfUnset("build")
@@ -197,8 +187,7 @@ class IncrementVersion(Step):
 
 
 class CommitLocal(Step):
-  def __init__(self):
-    Step.__init__(self, "Commit to local branch.")
+  MESSAGE = "Commit to local branch."
 
   def RunStep(self):
     self.RestoreVersionIfUnset("new_")
@@ -212,8 +201,7 @@ class CommitLocal(Step):
 
 
 class CommitRepository(Step):
-  def __init__(self):
-    Step.__init__(self, "Commit to the repository.")
+  MESSAGE = "Commit to the repository."
 
   def RunStep(self):
     self.WaitForLGTM()
@@ -227,9 +215,8 @@ class CommitRepository(Step):
 
 
 class StragglerCommits(Step):
-  def __init__(self):
-    Step.__init__(self, "Fetch straggler commits that sneaked in since this "
-                        "script was started.")
+  MESSAGE = ("Fetch straggler commits that sneaked in since this script was "
+             "started.")
 
   def RunStep(self):
     if self.Git("svn fetch") is None:
@@ -242,8 +229,7 @@ class StragglerCommits(Step):
 
 
 class SquashCommits(Step):
-  def __init__(self):
-    Step.__init__(self, "Squash commits into one.")
+  MESSAGE = "Squash commits into one."
 
   def RunStep(self):
     # Instead of relying on "git rebase -i", we'll just create a diff, because
@@ -285,8 +271,7 @@ class SquashCommits(Step):
 
 
 class NewBranch(Step):
-  def __init__(self):
-    Step.__init__(self, "Create a new branch from trunk.")
+  MESSAGE = "Create a new branch from trunk."
 
   def RunStep(self):
     if self.Git("checkout -b %s svn/trunk" % self.Config(TRUNKBRANCH)) is None:
@@ -295,8 +280,7 @@ class NewBranch(Step):
 
 
 class ApplyChanges(Step):
-  def __init__(self):
-    Step.__init__(self, "Apply squashed changes.")
+  MESSAGE = "Apply squashed changes."
 
   def RunStep(self):
     self.ApplyPatch(self.Config(PATCH_FILE))
@@ -304,8 +288,7 @@ class ApplyChanges(Step):
 
 
 class SetVersion(Step):
-  def __init__(self):
-    Step.__init__(self, "Set correct version for trunk.")
+  MESSAGE = "Set correct version for trunk."
 
   def RunStep(self):
     self.RestoreVersionIfUnset()
@@ -326,8 +309,7 @@ class SetVersion(Step):
 
 
 class CommitTrunk(Step):
-  def __init__(self):
-    Step.__init__(self, "Commit to local trunk branch.")
+  MESSAGE = "Commit to local trunk branch."
 
   def RunStep(self):
     self.Git("add \"%s\"" % self.Config(VERSION_FILE))
@@ -337,8 +319,7 @@ class CommitTrunk(Step):
 
 
 class SanityCheck(Step):
-  def __init__(self):
-    Step.__init__(self, "Sanity check.")
+  MESSAGE = "Sanity check."
 
   def RunStep(self):
     if not self.Confirm("Please check if your local checkout is sane: Inspect "
@@ -348,8 +329,7 @@ class SanityCheck(Step):
 
 
 class CommitSVN(Step):
-  def __init__(self):
-    Step.__init__(self, "Commit to SVN.")
+  MESSAGE = "Commit to SVN."
 
   def RunStep(self):
     result = self.Git("svn dcommit 2>&1")
@@ -374,8 +354,7 @@ class CommitSVN(Step):
 
 
 class TagRevision(Step):
-  def __init__(self):
-    Step.__init__(self, "Tag the new revision.")
+  MESSAGE = "Tag the new revision."
 
   def RunStep(self):
     self.RestoreVersionIfUnset()
@@ -387,8 +366,7 @@ class TagRevision(Step):
 
 
 class CheckChromium(Step):
-  def __init__(self):
-    Step.__init__(self, "Ask for chromium checkout.")
+  MESSAGE = "Ask for chromium checkout."
 
   def Run(self):
     chrome_path = self._options.c
@@ -404,8 +382,8 @@ class CheckChromium(Step):
 
 
 class SwitchChromium(Step):
-  def __init__(self):
-    Step.__init__(self, "Switch to Chromium checkout.", requires="chrome_path")
+  MESSAGE = "Switch to Chromium checkout."
+  REQUIRES = "chrome_path"
 
   def RunStep(self):
     v8_path = os.getcwd()
@@ -421,9 +399,8 @@ class SwitchChromium(Step):
 
 
 class UpdateChromiumCheckout(Step):
-  def __init__(self):
-    Step.__init__(self, "Update the checkout and create a new branch.",
-                  requires="chrome_path")
+  MESSAGE = "Update the checkout and create a new branch."
+  REQUIRES = "chrome_path"
 
   def RunStep(self):
     os.chdir(self._state["chrome_path"])
@@ -439,8 +416,8 @@ class UpdateChromiumCheckout(Step):
 
 
 class UploadCL(Step):
-  def __init__(self):
-    Step.__init__(self, "Create and upload CL.", requires="chrome_path")
+  MESSAGE = "Create and upload CL."
+  REQUIRES = "chrome_path"
 
   def RunStep(self):
     os.chdir(self._state["chrome_path"])
@@ -474,8 +451,8 @@ class UploadCL(Step):
 
 
 class SwitchV8(Step):
-  def __init__(self):
-    Step.__init__(self, "Returning to V8 checkout.", requires="chrome_path")
+  MESSAGE = "Returning to V8 checkout."
+  REQUIRES = "chrome_path"
 
   def RunStep(self):
     self.RestoreIfUnset("v8_path")
@@ -483,8 +460,7 @@ class SwitchV8(Step):
 
 
 class CleanUp(Step):
-  def __init__(self):
-    Step.__init__(self, "Done!")
+  MESSAGE = "Done!"
 
   def RunStep(self):
     self.RestoreVersionIfUnset()

tools/push-to-trunk/test_scripts.py

@@ -68,18 +68,18 @@ class ToplevelTest(unittest.TestCase):
 
   def testMakeChangeLogBodySimple(self):
     commits = [
-          ["        Title text 1",
+          ["Title text 1",
            "Title text 1\n\nBUG=\n",
-           "        author1@chromium.org"],
-          ["        Title text 2",
+           "author1@chromium.org"],
+          ["Title text 2",
            "Title text 2\n\nBUG=1234\n",
-           "        author2@chromium.org"],
+           "author2@chromium.org"],
         ]
     self.assertEquals("        Title text 1\n"
-                      "        author1@chromium.org\n\n"
+                      "        (author1@chromium.org)\n\n"
                       "        Title text 2\n"
                       "        (Chromium issue 1234)\n"
-                      "        author2@chromium.org\n\n",
+                      "        (author2@chromium.org)\n\n",
                       MakeChangeLogBody(commits))
 
   def testMakeChangeLogBodyEmpty(self):
@@ -87,18 +87,18 @@ class ToplevelTest(unittest.TestCase):
 
   def testMakeChangeLogBodyAutoFormat(self):
     commits = [
-          ["        Title text 1",
+          ["Title text 1",
            "Title text 1\nLOG=y\nBUG=\n",
-           "        author1@chromium.org"],
-          ["        Title text 2",
+           "author1@chromium.org"],
+          ["Title text 2",
            "Title text 2\n\nBUG=1234\n",
-           "        author2@chromium.org"],
-          ["        Title text 3",
+           "author2@chromium.org"],
+          ["Title text 3",
            "Title text 3\n\nBUG=1234\nLOG = Yes\n",
-           "        author3@chromium.org"],
-          ["        Title text 3",
+           "author3@chromium.org"],
+          ["Title text 3",
            "Title text 4\n\nBUG=1234\nLOG=\n",
-           "        author4@chromium.org"],
+           "author4@chromium.org"],
         ]
     self.assertEquals("        Title text 1\n\n"
                       "        Title text 3\n"
@@ -245,13 +245,9 @@ class ScriptTest(unittest.TestCase):
     return name
 
   def MakeStep(self, step_class=Step, state=None):
-    state = state or {}
-    step = step_class()
-    step.SetConfig(TEST_CONFIG)
-    step.SetState(state)
-    step.SetNumber(0)
-    step.SetSideEffectHandler(self)
-    return step
+    """Convenience wrapper."""
+    return MakeStep(step_class=step_class, number=0, state=state,
+                    config=TEST_CONFIG, options=None, side_effect_handler=self)
 
   def GitMock(self, cmd, args="", pipe=True):
     return self._git_mock.Call(args)
@@ -402,18 +398,15 @@ class ScriptTest(unittest.TestCase):
 
     self.ExpectGit([
       ["log 1234..HEAD --format=%H", "rev1\nrev2\nrev3"],
-      ["log -1 rev1 --format=\"%w(80,8,8)%s\"", "        Title text 1"],
+      ["log -1 rev1 --format=\"%s\"", "Title text 1"],
       ["log -1 rev1 --format=\"%B\"", "Title\n\nBUG=\nLOG=y\n"],
-      ["log -1 rev1 --format=\"%w(80,8,8)(%an)\"",
-       "        author1@chromium.org"],
-      ["log -1 rev2 --format=\"%w(80,8,8)%s\"", "        Title text 2"],
+      ["log -1 rev1 --format=\"%an\"", "author1@chromium.org"],
+      ["log -1 rev2 --format=\"%s\"", "Title text 2"],
       ["log -1 rev2 --format=\"%B\"", "Title\n\nBUG=123\nLOG= \n"],
-      ["log -1 rev2 --format=\"%w(80,8,8)(%an)\"",
-       "        author2@chromium.org"],
-      ["log -1 rev3 --format=\"%w(80,8,8)%s\"", "        Title text 3"],
+      ["log -1 rev2 --format=\"%an\"", "author2@chromium.org"],
+      ["log -1 rev3 --format=\"%s\"", "Title text 3"],
       ["log -1 rev3 --format=\"%B\"", "Title\n\nBUG=321\nLOG=true\n"],
-      ["log -1 rev3 --format=\"%w(80,8,8)(%an)\"",
-       "        author3@chromium.org"],
+      ["log -1 rev3 --format=\"%an\"", "author3@chromium.org"],
     ])
 
     self.MakeStep().Persist("last_push", "1234")
@@ -437,15 +430,15 @@ class ScriptTest(unittest.TestCase):
 # All lines starting with # will be stripped\\.
 #
 #       Title text 1
-#       author1@chromium\\.org
+#       \\(author1@chromium\\.org\\)
 #
 #       Title text 2
 #       \\(Chromium issue 123\\)
-#       author2@chromium\\.org
+#       \\(author2@chromium\\.org\\)
 #
 #       Title text 3
 #       \\(Chromium issue 321\\)
-#       author3@chromium\\.org
+#       \\(author3@chromium\\.org\\)
 #
 #"""
 
@@ -548,7 +541,7 @@ class ScriptTest(unittest.TestCase):
       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.assertFalse(re.search(r"        author1@chromium\.org", cl))
+      self.assertFalse(re.search(r"        \(author1@chromium\.org\)", cl))
 
       # Make sure all comments got stripped.
       self.assertFalse(re.search(r"^#", cl, flags=re.M))
@@ -583,10 +576,9 @@ class ScriptTest(unittest.TestCase):
       ["log -1 --format=%H ChangeLog", "1234\n"],
       ["log -1 1234", "Last push ouput\n"],
       ["log 1234..HEAD --format=%H", "rev1\n"],
-      ["log -1 rev1 --format=\"%w(80,8,8)%s\"", "        Log text 1.\n"],
+      ["log -1 rev1 --format=\"%s\"", "Log text 1.\n"],
       ["log -1 rev1 --format=\"%B\"", "Text\nLOG=YES\nBUG=v8:321\nText\n"],
-      ["log -1 rev1 --format=\"%w(80,8,8)(%an)\"",
-       "        author1@chromium.org\n"],
+      ["log -1 rev1 --format=\"%an\"", "author1@chromium.org\n"],
       [("commit -a -m \"Prepare push to trunk.  "
         "Now working on version 3.22.6.\""),
        " 2 files changed\n",