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

abarth at webkit.org abarth at webkit.org
Thu Apr 8 00:36:08 UTC 2010


The following commit has been merged in the webkit-1.2 branch:
commit 634023f07c40b1c138b74cbbaddd968a06615f57
Author: abarth at webkit.org <abarth at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Tue Dec 15 05:31:03 2009 +0000

    2009-12-14  Adam Barth  <abarth at webkit.org>
    
            Reviewed by Eric Seidel.
    
            [bzt] Convert rollout to StepSequence
            https://bugs.webkit.org/show_bug.cgi?id=32406
    
            * Scripts/modules/buildsteps.py:
            * Scripts/modules/commands/download.py:
            * Scripts/modules/commands/download_unittest.py:
            * Scripts/modules/mock_bugzillatool.py:
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@52130 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 17cda56..bef1979 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -2,6 +2,18 @@
 
         Reviewed by Eric Seidel.
 
+        [bzt] Convert rollout to StepSequence
+        https://bugs.webkit.org/show_bug.cgi?id=32406
+
+        * Scripts/modules/buildsteps.py:
+        * Scripts/modules/commands/download.py:
+        * Scripts/modules/commands/download_unittest.py:
+        * Scripts/modules/mock_bugzillatool.py:
+
+2009-12-14  Adam Barth  <abarth at webkit.org>
+
+        Reviewed by Eric Seidel.
+
         [bzt] Kill LandingSequence
         https://bugs.webkit.org/show_bug.cgi?id=32464
 
diff --git a/WebKitTools/Scripts/modules/buildsteps.py b/WebKitTools/Scripts/modules/buildsteps.py
index d08f635..4d5da58 100644
--- a/WebKitTools/Scripts/modules/buildsteps.py
+++ b/WebKitTools/Scripts/modules/buildsteps.py
@@ -33,6 +33,7 @@ from optparse import make_option
 from modules.comments import bug_comment_from_commit_text
 from modules.logging import log, error
 from modules.webkitport import WebKitPort
+from modules.changelogs import ChangeLog
 
 
 class CommandOptions(object):
@@ -48,6 +49,7 @@ class CommandOptions(object):
     close_bug = make_option("--no-close", action="store_false", dest="close_bug", default=True, help="Leave bug open after landing.")
     port = make_option("--port", action="store", dest="port", default=None, help="Specify a port (e.g., mac, qt, gtk, ...).")
     reviewer = make_option("-r", "--reviewer", action="store", type="string", dest="reviewer", help="Update ChangeLogs to say Reviewed by REVIEWER.")
+    complete_rollout = make_option("--complete-rollout", action="store_true", dest="complete_rollout", help="Commit the revert and re-open the original bug.")
 
 
 class AbstractStep(object):
@@ -75,11 +77,50 @@ class AbstractStep(object):
         raise NotImplementedError, "subclasses must implement"
 
 
+# FIXME: Unify with StepSequence?  I'm not sure yet which is the better design.
+class MetaStep(AbstractStep):
+    substeps = [] # Override in subclasses
+    def __init__(self, tool, options):
+        AbstractStep.__init__(self, tool, options)
+        self._step_instances = []
+        for step_class in self.substeps:
+            self._step_instances.append(step_class(tool, options))
+
+    @staticmethod
+    def _collect_options_from_steps(steps):
+        collected_options = []
+        for step in steps:
+            collected_options = collected_options + step.options()
+        return collected_options
+
+    @classmethod
+    def options(cls):
+        return cls._collect_options_from_steps(cls.substeps)
+
+    def run(self, state):
+        for step in self._step_instances:
+             step.run(state)
+
+
 class PrepareChangelogStep(AbstractStep):
     def run(self, state):
         self._run_script("prepare-ChangeLog")
 
 
+class PrepareChangelogForRevertStep(AbstractStep):
+    def run(self, state):
+        # First, discard the ChangeLog changes from the rollout.
+        os.chdir(self._tool.scm().checkout_root)
+        changelog_paths = self._tool.scm().modified_changelogs()
+        self._tool.scm().revert_files(changelog_paths)
+
+        # Second, make new ChangeLog entries for this rollout.
+        # This could move to prepare-ChangeLog by adding a --revert= option.
+        self._run_script("prepare-ChangeLog")
+        for changelog_path in changelog_paths:
+            ChangeLog(changelog_path).update_for_revert(state["revision"])
+
+
 class CleanWorkingDirectoryStep(AbstractStep):
     def __init__(self, tool, options, allow_local_commits=False):
         AbstractStep.__init__(self, tool, options)
@@ -127,6 +168,11 @@ class ApplyPatchStep(AbstractStep):
         self._tool.scm().apply_patch(state["patch"], force=self._options.non_interactive)
 
 
+class RevertRevisionStep(AbstractStep):
+    def run(self, state):
+        self._tool.scm().apply_reverse_diff(state["revision"])
+
+
 class EnsureBuildersAreGreenStep(AbstractStep):
     @classmethod
     def options(cls):
@@ -280,6 +326,36 @@ class CloseBugForLandDiffStep(AbstractStep):
             log("No bug id provided.")
 
 
+class CompleteRollout(MetaStep):
+    substeps = [
+        BuildStep,
+        CommitStep,
+    ]
+
+    @classmethod
+    def options(cls):
+        collected_options = cls._collect_options_from_steps(cls.substeps)
+        collected_options.append(CommandOptions.complete_rollout)
+        return collected_options
+
+    def run(self, state):
+        bug_id = state["bug_id"]
+        # FIXME: Fully automated rollout is not 100% idiot-proof yet, so for now just log with instructions on how to complete the rollout.
+        # Once we trust rollout we will remove this option.
+        if not self._options.complete_rollout:
+            log("\nNOTE: Rollout support is experimental.\nPlease verify the rollout diff and use \"bugzilla-tool land-diff %s\" to commit the rollout." % bug_id)
+            return
+
+        MetaStep.run(self, state)
+
+        if not bug_id:
+            log(state["commit_text"])
+            log("No bugs were updated or re-opened to reflect this rollout.")
+            return
+        # FIXME: I'm not sure state["commit_text"] is quite right here.
+        self._tool.bugs.reopen_bug(bug_id, state["commit_text"])
+
+
 # FIXME: This class is a dinosaur and should be extinct soon.
 class BuildSteps:
     # FIXME: The options should really live on each "Step" object.
diff --git a/WebKitTools/Scripts/modules/commands/download.py b/WebKitTools/Scripts/modules/commands/download.py
index 9a85aef..0b6e3aa 100644
--- a/WebKitTools/Scripts/modules/commands/download.py
+++ b/WebKitTools/Scripts/modules/commands/download.py
@@ -34,7 +34,7 @@ from optparse import make_option
 
 from modules.bugzilla import parse_bug_id
 # FIXME: This list is rediculous.  We need to learn the ways of __all__.
-from modules.buildsteps import CommandOptions, BuildSteps, EnsureBuildersAreGreenStep, UpdateChangelogsWithReviewerStep, CleanWorkingDirectoryStep, UpdateStep, ApplyPatchStep, BuildStep, CheckStyleStep, RunTestsStep, CommitStep, ClosePatchStep, CloseBugStep, CloseBugForLandDiffStep, PrepareChangelogStep
+from modules.buildsteps import CommandOptions, BuildSteps, EnsureBuildersAreGreenStep, UpdateChangelogsWithReviewerStep, CleanWorkingDirectoryStep, UpdateStep, ApplyPatchStep, BuildStep, CheckStyleStep, RunTestsStep, CommitStep, ClosePatchStep, CloseBugStep, CloseBugForLandDiffStep, PrepareChangelogStep, PrepareChangelogForRevertStep, RevertRevisionStep, CompleteRollout
 from modules.changelogs import ChangeLog
 from modules.comments import bug_comment_from_commit_text
 from modules.executive import ScriptError
@@ -271,28 +271,18 @@ class LandPatches(AbstractPatchLandingCommand):
         return all_patches
 
 
-# FIXME: Requires unit test.
 class Rollout(Command):
     name = "rollout"
     show_in_main_help = True
     def __init__(self):
-        options = BuildSteps.cleaning_options()
-        options += BuildSteps.build_options()
-        options += BuildSteps.land_options()
-        options.append(make_option("--complete-rollout", action="store_true", dest="complete_rollout", help="Commit the revert and re-open the original bug."))
-        Command.__init__(self, "Revert the given revision in the working copy and optionally commit the revert and re-open the original bug", "REVISION [BUGID]", options=options)
-
-    @staticmethod
-    def _create_changelogs_for_revert(tool, revision):
-        # First, discard the ChangeLog changes from the rollout.
-        changelog_paths = tool.scm().modified_changelogs()
-        tool.scm().revert_files(changelog_paths)
-
-        # Second, make new ChangeLog entries for this rollout.
-        # This could move to prepare-ChangeLog by adding a --revert= option.
-        PrepareChangelogStep(tool, None).run({})
-        for changelog_path in changelog_paths:
-            ChangeLog(changelog_path).update_for_revert(revision)
+        self._sequence = StepSequence([
+            CleanWorkingDirectoryStep,
+            UpdateStep,
+            RevertRevisionStep,
+            PrepareChangelogForRevertStep,
+            CompleteRollout,
+        ])
+        Command.__init__(self, "Revert the given revision in the working copy and optionally commit the revert and re-open the original bug", "REVISION [BUGID]", options=self._sequence.options())
 
     @staticmethod
     def _parse_bug_id_from_revision_diff(tool, revision):
@@ -316,17 +306,8 @@ class Rollout(Command):
             else:
                 log("Failed to parse bug number from diff.  No bugs will be updated/reopened after the rollout.")
 
-        CleanWorkingDirectoryStep(tool, options).run({})
-        UpdateStep(tool, options).run({})
-        tool.scm().apply_reverse_diff(revision)
-        self._create_changelogs_for_revert(tool, revision)
-
-        # FIXME: Fully automated rollout is not 100% idiot-proof yet, so for now just log with instructions on how to complete the rollout.
-        # Once we trust rollout we will remove this option.
-        if not options.complete_rollout:
-            log("\nNOTE: Rollout support is experimental.\nPlease verify the rollout diff and use \"bugzilla-tool land-diff %s\" to commit the rollout." % bug_id)
-        else:
-            # FIXME: This function does not exist!!
-            # comment_text = WebKitLandingScripts.build_and_commit(tool.scm(), options)
-            raise ScriptError("OOPS! This option is not implemented (yet).")
-            self._reopen_bug_after_rollout(tool, bug_id, comment_text)
+        state = {
+            "revision": revision,
+            "bug_id": bug_id,
+        }
+        self._sequence.run_and_handle_errors(tool, options, state)
diff --git a/WebKitTools/Scripts/modules/commands/download_unittest.py b/WebKitTools/Scripts/modules/commands/download_unittest.py
index ff209d9..e7bda41 100644
--- a/WebKitTools/Scripts/modules/commands/download_unittest.py
+++ b/WebKitTools/Scripts/modules/commands/download_unittest.py
@@ -44,6 +44,7 @@ class DownloadCommandsTest(CommandsTest):
         options.build = True
         options.test = True
         options.close_bug = True
+        options.complete_rollout = False
         return options
 
     def test_build(self):
@@ -83,3 +84,13 @@ class DownloadCommandsTest(CommandsTest):
     def test_land_patches(self):
         expected_stderr = "2 reviewed patches found on bug 42.\nProcessing 2 patches from 1 bug.\nUpdating working directory\nProcessing patch 197 from bug 42.\nBuilding WebKit\nUpdating working directory\nProcessing patch 128 from bug 42.\nBuilding WebKit\n"
         self.assert_execute_outputs(LandPatches(), [42], options=self._default_options(), expected_stderr=expected_stderr)
+
+    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)
+
+    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)
diff --git a/WebKitTools/Scripts/modules/mock_bugzillatool.py b/WebKitTools/Scripts/modules/mock_bugzillatool.py
index e600947..6beae73 100644
--- a/WebKitTools/Scripts/modules/mock_bugzillatool.py
+++ b/WebKitTools/Scripts/modules/mock_bugzillatool.py
@@ -133,6 +133,9 @@ class MockSCM(Mock):
             return "Patch2"
         raise Exception("Bogus commit_id in commit_message_for_local_commit.")
 
+    def diff_for_revision(self, revision):
+        return "DiffForRevision%s\nhttp://bugs.webkit.org/show_bug.cgi?id=12345" % revision
+
     def modified_changelogs(self):
         # Ideally we'd return something more interesting here.
         # The problem is that LandDiff will try to actually read the path from disk!

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list