[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373

eric at webkit.org eric at webkit.org
Thu Apr 8 00:54:17 UTC 2010


The following commit has been merged in the webkit-1.2 branch:
commit 641a78cd7a887093180d2338de01697b7cbc3990
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Tue Jan 5 05:33:22 2010 +0000

    2010-01-04  Eric Seidel  <eric at webkit.org>
    
            Reviewed by Adam Barth.
    
            bugzilla-tool rollout should require a reason
            https://bugs.webkit.org/show_bug.cgi?id=30810
    
            * Scripts/webkitpy/changelogs.py: Add support for a reason, add auto-wrapping logic.
            * Scripts/webkitpy/changelogs_unittest.py: Test reason support.
            * Scripts/webkitpy/commands/download.py: rollout now requires a reason, remove unused BUGID argument
            * Scripts/webkitpy/commands/download_unittest.py: pass required reason
            * Scripts/webkitpy/steps/preparechangelogforrevert.py: pass reason to update_for_revert
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@52785 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 40eaa18..f67def5 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,16 @@
+2010-01-04  Eric Seidel  <eric at webkit.org>
+
+        Reviewed by Adam Barth.
+
+        bugzilla-tool rollout should require a reason
+        https://bugs.webkit.org/show_bug.cgi?id=30810
+
+        * Scripts/webkitpy/changelogs.py: Add support for a reason, add auto-wrapping logic.
+        * Scripts/webkitpy/changelogs_unittest.py: Test reason support.
+        * Scripts/webkitpy/commands/download.py: rollout now requires a reason, remove unused BUGID argument
+        * Scripts/webkitpy/commands/download_unittest.py: pass required reason
+        * Scripts/webkitpy/steps/preparechangelogforrevert.py: pass reason to update_for_revert
+
 2010-01-04  Jon Honeycutt  <jhoneycutt at apple.com>
 
         MSAA: <select> elements should broadcast value change events
diff --git a/WebKitTools/Scripts/webkitpy/changelogs.py b/WebKitTools/Scripts/webkitpy/changelogs.py
index 55287fe..17e4e61 100644
--- a/WebKitTools/Scripts/webkitpy/changelogs.py
+++ b/WebKitTools/Scripts/webkitpy/changelogs.py
@@ -30,6 +30,7 @@
 
 import fileinput # inplace file editing for set_reviewer_in_changelog
 import re
+import textwrap
 
 # FIMXE: This doesn't really belong in this file, but we don't have a better home for it yet.
 # Maybe eventually a webkit_config.py?
@@ -41,6 +42,8 @@ class ChangeLog:
     def __init__(self, path):
         self.path = path
 
+    _changelog_indent = " " * 8
+
     # e.g. 2009-06-03  Eric Seidel  <eric at webkit.org>
     date_line_regexp = re.compile('^(\d{4}-\d{2}-\d{2})' # Consume the date.
                                 + '\s+(.+)\s+' # Consume the name.
@@ -69,17 +72,38 @@ class ChangeLog:
         finally:
             changelog_file.close()
 
-    def update_for_revert(self, revision, bug_url=None):
-        reviewed_by_regexp = re.compile('Reviewed by NOBODY \(OOPS!\)\.')
+    # _wrap_line and _wrap_lines exist to work around http://bugs.python.org/issue1859
+    def _wrap_line(self, line):
+        return textwrap.fill(line,
+                             width=70,
+                             initial_indent=self._changelog_indent,
+                             break_long_words=False, # Don't break urls which may be longer than width.
+                             subsequent_indent=self._changelog_indent)
+
+    # Workaround as suggested by guido in http://bugs.python.org/issue1859#msg60040
+    def _wrap_lines(self, message):
+        wrapped_lines = [self._wrap_line(line) for line in message.splitlines()]
+        return "\n".join(wrapped_lines)
+
+    # This probably does not belong in changelogs.py
+    def _message_for_revert(self, revision, reason, bug_url):
+        message = "No review, rolling out r%s.\n" % revision
+        message += "%s\n" % view_source_url(revision)
+        if bug_url:
+            message += "%s\n" % bug_url
+        message += "\n" # Add an extra new line after the rollout links, before any reason.
+        if reason:
+            message += "%s\n\n" % reason
+        return self._wrap_lines(message)
+
+    def update_for_revert(self, revision, reason, bug_url=None):
+        reviewed_by_regexp = re.compile("%sReviewed by NOBODY \(OOPS!\)\." % self._changelog_indent)
         removing_boilerplate = False
         # inplace=1 creates a backup file and re-directs stdout to the file
         for line in fileinput.FileInput(self.path, inplace=1):
             if reviewed_by_regexp.search(line):
-                print reviewed_by_regexp.sub("No review, rolling out r%s." % revision, line),
-                print "        %s" % view_source_url(revision)
-                if bug_url:
-                    print "        %s" % bug_url
-                print # Add an extra new line after the rollout message.
+                message_lines = self._message_for_revert(revision, reason, bug_url)
+                print reviewed_by_regexp.sub(message_lines, line),
                 # Remove all the ChangeLog boilerplate between the Reviewed by line and the first changed file.
                 removing_boilerplate = True
             elif removing_boilerplate:
diff --git a/WebKitTools/Scripts/webkitpy/changelogs_unittest.py b/WebKitTools/Scripts/webkitpy/changelogs_unittest.py
index d8e8158..de3e60c 100644
--- a/WebKitTools/Scripts/webkitpy/changelogs_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/changelogs_unittest.py
@@ -124,12 +124,31 @@ class ChangeLogsTest(unittest.TestCase):
         os.remove(changelog_path)
         self.assertEquals(actual_contents, expected_contents)
 
+    _revert_message = """        No review, rolling out r12345.
+        http://trac.webkit.org/changeset/12345
+        http://example.com/123
+
+        This is a very long reason which should be long enough so that
+        _message_for_revert will need to wrap it.  We'll also include
+        a
+        https://veryveryveryveryverylongbugurl.com/reallylongbugthingy.cgi?bug_id=12354
+        link so that we can make sure we wrap that right too.
+"""
+
+    def test_message_for_revert(self):
+        changelog = ChangeLog("/fake/path")
+        long_reason = "This is a very long reason which should be long enough so that _message_for_revert will need to wrap it.  We'll also include a https://veryveryveryveryverylongbugurl.com/reallylongbugthingy.cgi?bug_id=12354 link so that we can make sure we wrap that right too."
+        message = changelog._message_for_revert(12345, long_reason, "http://example.com/123")
+        self.assertEquals(message, self._revert_message)
+
     _revert_entry_with_bug_url = '''2009-08-19  Eric Seidel  <eric at webkit.org>
 
         No review, rolling out r12345.
         http://trac.webkit.org/changeset/12345
         http://example.com/123
 
+        Reason
+
         * Scripts/bugzilla-tool:
 '''
 
@@ -138,6 +157,8 @@ class ChangeLogsTest(unittest.TestCase):
         No review, rolling out r12345.
         http://trac.webkit.org/changeset/12345
 
+        Reason
+
         * Scripts/bugzilla-tool:
 '''
 
@@ -151,8 +172,8 @@ class ChangeLogsTest(unittest.TestCase):
         self.assertEquals(actual_entry, expected_entry)
 
     def test_update_for_revert(self):
-        self._assert_update_for_revert_output([12345], self._revert_entry_without_bug_url)
-        self._assert_update_for_revert_output([12345, "http://example.com/123"], self._revert_entry_with_bug_url)
+        self._assert_update_for_revert_output([12345, "Reason"], self._revert_entry_without_bug_url)
+        self._assert_update_for_revert_output([12345, "Reason", "http://example.com/123"], self._revert_entry_with_bug_url)
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/WebKitTools/Scripts/webkitpy/commands/download.py b/WebKitTools/Scripts/webkitpy/commands/download.py
index d148b27..d732f44 100644
--- a/WebKitTools/Scripts/webkitpy/commands/download.py
+++ b/WebKitTools/Scripts/webkitpy/commands/download.py
@@ -238,12 +238,14 @@ class Rollout(AbstractSequencedCommmand):
     name = "rollout"
     show_in_main_help = True
     help_text = "Revert the given revision in the working copy and optionally commit the revert and re-open the original bug"
-    argument_names = "REVISION [BUGID]"
+    argument_names = "REVISION REASON"
     steps = [
         steps.CleanWorkingDirectory,
         steps.Update,
         steps.RevertRevision,
         steps.PrepareChangeLogForRevert,
+        steps.EditChangeLog,
+        steps.ConfirmDiff,
         steps.CompleteRollout,
     ]
 
@@ -262,6 +264,7 @@ class Rollout(AbstractSequencedCommmand):
 
     def execute(self, options, args, tool):
         revision = args[0]
+        reason = args[1]
         bug_id = self._parse_bug_id_from_revision_diff(tool, revision)
         if options.complete_rollout:
             if bug_id:
@@ -270,7 +273,8 @@ class Rollout(AbstractSequencedCommmand):
                 log("Failed to parse bug number from diff.  No bugs will be updated/reopened after the rollout.")
 
         state = {
-            "revision": revision,
-            "bug_id": bug_id,
+            "revision" : revision,
+            "bug_id" : bug_id,
+            "reason" : reason,
         }
         self._sequence.run_and_handle_errors(tool, options, state)
diff --git a/WebKitTools/Scripts/webkitpy/commands/download_unittest.py b/WebKitTools/Scripts/webkitpy/commands/download_unittest.py
index d914425..c9cd6d7 100644
--- a/WebKitTools/Scripts/webkitpy/commands/download_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/commands/download_unittest.py
@@ -91,10 +91,10 @@ class DownloadCommandsTest(CommandsTest):
 
     def test_rollout(self):
         expected_stderr = "Updating working directory\nRunning prepare-ChangeLog\n\nNOTE: Rollout support is experimental.\nPlease verify the rollout diff and use \"bugzilla-tool land-diff 12345\" to commit the rollout.\n"
-        self.assert_execute_outputs(Rollout(), [852], options=self._default_options(), expected_stderr=expected_stderr)
+        self.assert_execute_outputs(Rollout(), [852, "Reason"], options=self._default_options(), expected_stderr=expected_stderr)
 
     def test_complete_rollout(self):
         options = self._default_options()
         options.complete_rollout = True
         expected_stderr = "Will re-open bug 12345 after rollout.\nUpdating working directory\nRunning prepare-ChangeLog\nBuilding WebKit\n"
-        self.assert_execute_outputs(Rollout(), [852], options=options, expected_stderr=expected_stderr)
+        self.assert_execute_outputs(Rollout(), [852, "Reason"], options=options, expected_stderr=expected_stderr)
diff --git a/WebKitTools/Scripts/webkitpy/steps/preparechangelogforrevert.py b/WebKitTools/Scripts/webkitpy/steps/preparechangelogforrevert.py
index 2eff15d..88e5134 100644
--- a/WebKitTools/Scripts/webkitpy/steps/preparechangelogforrevert.py
+++ b/WebKitTools/Scripts/webkitpy/steps/preparechangelogforrevert.py
@@ -44,4 +44,6 @@ class PrepareChangeLogForRevert(AbstractStep):
         self._run_script("prepare-ChangeLog")
         bug_url = self._tool.bugs.bug_url_for_bug_id(state["bug_id"]) if state["bug_id"] else None
         for changelog_path in changelog_paths:
-            ChangeLog(changelog_path).update_for_revert(state["revision"], bug_url)
+            # FIXME: Seems we should prepare the message outside of changelogs.py and then just pass in
+            # text that we want to use to replace the reviewed by line.
+            ChangeLog(changelog_path).update_for_revert(state["revision"], state["reason"], bug_url)

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list