[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:15 UTC 2010


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

    2009-12-14  Adam Barth  <abarth at webkit.org>
    
            Reviewed by Eric Seidel.
    
            [bzt] Make download commands declarative
            https://bugs.webkit.org/show_bug.cgi?id=32469
    
            This patch "properly" factors most of the download commands.  These
            commands are now largely declarative, which is the final step of this
            grand refactoring.
    
            * Scripts/modules/buildsteps.py:
            * Scripts/modules/commands/download.py:
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@52133 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index d353a12..ed59ae1 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -2,6 +2,20 @@
 
         Reviewed by Eric Seidel.
 
+        [bzt] Make download commands declarative
+        https://bugs.webkit.org/show_bug.cgi?id=32469
+
+        This patch "properly" factors most of the download commands.  These
+        commands are now largely declarative, which is the final step of this
+        grand refactoring.
+
+        * Scripts/modules/buildsteps.py:
+        * Scripts/modules/commands/download.py:
+
+2009-12-14  Adam Barth  <abarth at webkit.org>
+
+        Reviewed by Eric Seidel.
+
         [bzt] Add AbstractPatchSequencingCommand to remove redundant code
         https://bugs.webkit.org/show_bug.cgi?id=32468
 
diff --git a/WebKitTools/Scripts/modules/buildsteps.py b/WebKitTools/Scripts/modules/buildsteps.py
index 9b53f12..abff522 100644
--- a/WebKitTools/Scripts/modules/buildsteps.py
+++ b/WebKitTools/Scripts/modules/buildsteps.py
@@ -211,6 +211,18 @@ class EnsureBuildersAreGreenStep(AbstractStep):
         error("Builders [%s] are red, please do not commit.\nSee http://%s.\nPass --ignore-builders to bypass this check." % (", ".join(red_builders_names), self._tool.buildbot.buildbot_host))
 
 
+class EnsureLocalCommitIfNeeded(AbstractStep):
+    @classmethod
+    def options(cls):
+        return [
+            CommandOptions.local_commit,
+        ]
+
+    def run(self, state):
+        if self._options.local_commit and not self._tool.scm().supports_local_commits():
+            error("--local-commit passed, but %s does not support local commits" % self._tool.scm.display_name())
+
+
 class UpdateChangelogsWithReviewerStep(AbstractStep):
     @classmethod
     def options(cls):
diff --git a/WebKitTools/Scripts/modules/commands/download.py b/WebKitTools/Scripts/modules/commands/download.py
index 8966baf..bcb244f 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, EnsureBuildersAreGreenStep, UpdateChangelogsWithReviewerStep, CleanWorkingDirectoryStep, CleanWorkingDirectoryWithLocalCommitsStep, UpdateStep, ApplyPatchStep, ApplyPatchWithLocalCommitStep, BuildStep, CheckStyleStep, RunTestsStep, CommitStep, ClosePatchStep, CloseBugStep, CloseBugForLandDiffStep, PrepareChangelogStep, PrepareChangelogForRevertStep, RevertRevisionStep, CompleteRollout
+from modules.buildsteps import CommandOptions, EnsureBuildersAreGreenStep, EnsureLocalCommitIfNeeded, UpdateChangelogsWithReviewerStep, CleanWorkingDirectoryStep, CleanWorkingDirectoryWithLocalCommitsStep, UpdateStep, ApplyPatchStep, ApplyPatchWithLocalCommitStep, 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
@@ -44,55 +44,69 @@ from modules.multicommandtool import Command
 from modules.stepsequence import StepSequence
 
 
-class Build(Command):
-    name = "build"
-    show_in_main_help = False
+# FIXME: Move this to a more general location.
+class AbstractDeclarativeCommmand(Command):
+    help_text = None
+    argument_names = None
+    def __init__(self, options):
+        Command.__init__(self, self.help_text, self.argument_names, options)
+
+
+class AbstractSequencedCommmand(AbstractDeclarativeCommmand):
+    steps = None
     def __init__(self):
-        self._sequence = StepSequence([
-            CleanWorkingDirectoryStep,
-            UpdateStep,
-            BuildStep
-        ])
-        Command.__init__(self, "Update working copy and build", "", self._sequence.options())
+        self._sequence = StepSequence(self.steps)
+        AbstractDeclarativeCommmand.__init__(self, self._sequence.options())
+
+    def _prepare_state(self, options, args, tool):
+        return None
 
     def execute(self, options, args, tool):
-        self._sequence.run_and_handle_errors(tool, options)
+        self._sequence.run_and_handle_errors(tool, options, self._prepare_state(options, args, tool))
 
 
-class LandDiff(Command):
+class Build(AbstractSequencedCommmand):
+    name = "build"
+    help_text = "Update working copy and build"
+    argument_names = ""
+    show_in_main_help = False
+    steps = [
+        CleanWorkingDirectoryStep,
+        UpdateStep,
+        BuildStep,
+    ]
+
+
+class LandDiff(AbstractSequencedCommmand):
     name = "land-diff"
+    help_text = "Land the current working directory diff and updates the associated bug if any"
+    argument_names = "[BUGID]"
     show_in_main_help = True
-    def __init__(self):
-        self._sequence = StepSequence([
-            EnsureBuildersAreGreenStep,
-            UpdateChangelogsWithReviewerStep,
-            EnsureBuildersAreGreenStep,
-            BuildStep,
-            RunTestsStep,
-            CommitStep,
-            CloseBugForLandDiffStep,
-        ])
-        Command.__init__(self, "Land the current working directory diff and updates the associated bug if any", "[BUGID]", options=self._sequence.options())
+    steps = [
+        EnsureBuildersAreGreenStep,
+        UpdateChangelogsWithReviewerStep,
+        EnsureBuildersAreGreenStep,
+        BuildStep,
+        RunTestsStep,
+        CommitStep,
+        CloseBugForLandDiffStep,
+    ]
 
-    def execute(self, options, args, tool):
-        bug_id = (args and args[0]) or parse_bug_id(tool.scm().create_patch())
-        fake_patch = {
-            "id": None,
-            "bug_id": bug_id,
+    def _prepare_state(self, options, args, tool):
+        return {
+            "patch": {
+                "id": None,
+                "bug_id": (args and args[0]) or parse_bug_id(tool.scm().create_patch()),
+            }
         }
-        state = {"patch": fake_patch}
-        self._sequence.run_and_handle_errors(tool, options, state)
 
 
-class AbstractPatchProcessingCommand(Command):
-    def __init__(self, help_text, args_description, options):
-        Command.__init__(self, help_text, args_description, options=options)
-
-    def _fetch_list_of_patches_to_process(self, options, args, tool):
-        raise NotImplementedError, "subclasses must implement"
-
-    def _prepare_to_process(self, options, args, tool):
-        raise NotImplementedError, "subclasses must implement"
+class AbstractPatchProcessingCommand(AbstractDeclarativeCommmand):
+    # Subclasses must implement the methods below.  We don't declare them here
+    # because we want to be able to implement them with mix-ins.
+    #
+    # def _fetch_list_of_patches_to_process(self, options, args, tool):
+    # def _prepare_to_process(self, options, args, tool):
 
     @staticmethod
     def _collect_patches_by_bug(patches):
@@ -125,12 +139,12 @@ class AbstractPatchSequencingCommand(AbstractPatchProcessingCommand):
         step_sequence = StepSequence(steps)
         return step_sequence, step_sequence.options()
 
-    def __init__(self, help_text, args_description):
+    def __init__(self):
         options = []
         self._prepare_sequence, prepare_options = self._create_step_sequence(self.prepare_steps)
         self._main_sequence, main_options = self._create_step_sequence(self.main_steps)
         options = sorted(set(prepare_options + main_options))
-        AbstractPatchProcessingCommand.__init__(self, help_text, args_description, options)
+        AbstractPatchProcessingCommand.__init__(self, options)
 
     def _prepare_to_process(self, options, args, tool):
         if self._prepare_sequence:
@@ -142,8 +156,25 @@ class AbstractPatchSequencingCommand(AbstractPatchProcessingCommand):
             self._main_sequence.run_and_handle_errors(tool, options, state)
 
 
-class CheckStyle(AbstractPatchSequencingCommand):
+class ProcessAttachmentsMixin(object):
+    def _fetch_list_of_patches_to_process(self, options, args, tool):
+        return map(lambda patch_id: tool.bugs.fetch_attachment(patch_id), args)
+
+
+class ProcessBugsMixin(object):
+    def _fetch_list_of_patches_to_process(self, options, args, tool):
+        all_patches = []
+        for bug_id in args:
+            patches = tool.bugs.fetch_reviewed_patches_from_bug(bug_id)
+            log("%s found on bug %s." % (pluralize("reviewed patch", len(patches)), bug_id))
+            all_patches += patches
+        return all_patches
+
+
+class CheckStyle(AbstractPatchSequencingCommand, ProcessAttachmentsMixin):
     name = "check-style"
+    help_text = "Run check-webkit-style on the specified attachments"
+    argument_names = "ATTACHMENT_ID [ATTACHMENT_IDS]"
     show_in_main_help = False
     main_steps = [
         CleanWorkingDirectoryStep,
@@ -151,15 +182,12 @@ class CheckStyle(AbstractPatchSequencingCommand):
         ApplyPatchStep,
         CheckStyleStep,
     ]
-    def __init__(self):
-        AbstractPatchSequencingCommand.__init__(self, "Run check-webkit-style on the specified attachments", "ATTACHMENT_ID [ATTACHMENT_IDS]")
-
-    def _fetch_list_of_patches_to_process(self, options, args, tool):
-        return map(lambda patch_id: tool.bugs.fetch_attachment(patch_id), args)
 
 
-class BuildAttachment(AbstractPatchSequencingCommand):
+class BuildAttachment(AbstractPatchSequencingCommand, ProcessAttachmentsMixin):
     name = "build-attachment"
+    help_text = "Apply and build patches from bugzilla"
+    argument_names = "ATTACHMENT_ID [ATTACHMENT_IDS]"
     show_in_main_help = False
     main_steps = [
         CleanWorkingDirectoryStep,
@@ -167,15 +195,11 @@ class BuildAttachment(AbstractPatchSequencingCommand):
         ApplyPatchStep,
         BuildStep,
     ]
-    def __init__(self):
-        AbstractPatchSequencingCommand.__init__(self, "Apply and build patches from bugzilla", "ATTACHMENT_ID [ATTACHMENT_IDS]")
-
-    def _fetch_list_of_patches_to_process(self, options, args, tool):
-        return map(lambda patch_id: tool.bugs.fetch_attachment(patch_id), args)
 
 
 class AbstractPatchApplyingCommand(AbstractPatchSequencingCommand):
     prepare_steps = [
+        EnsureLocalCommitIfNeeded,
         CleanWorkingDirectoryWithLocalCommitsStep,
         UpdateStep,
     ]
@@ -183,35 +207,19 @@ class AbstractPatchApplyingCommand(AbstractPatchSequencingCommand):
         ApplyPatchWithLocalCommitStep,
     ]
 
-    def _prepare_to_process(self, options, args, tool):
-        if options.local_commit and not tool.scm().supports_local_commits():
-            error("--local-commit passed, but %s does not support local commits" % scm.display_name())
-        AbstractPatchSequencingCommand._prepare_to_process(self, options, args, tool)
 
-
-class ApplyAttachment(AbstractPatchApplyingCommand):
+class ApplyAttachment(AbstractPatchApplyingCommand, ProcessAttachmentsMixin):
     name = "apply-attachment"
+    help_text = "Apply an attachment to the local working directory"
+    argument_names = "ATTACHMENT_ID [ATTACHMENT_IDS]"
     show_in_main_help = True
-    def __init__(self):
-        AbstractPatchApplyingCommand.__init__(self, "Apply an attachment to the local working directory", "ATTACHMENT_ID [ATTACHMENT_IDS]")
-
-    def _fetch_list_of_patches_to_process(self, options, args, tool):
-        return map(lambda patch_id: tool.bugs.fetch_attachment(patch_id), args)
 
 
-class ApplyPatches(AbstractPatchApplyingCommand):
+class ApplyPatches(AbstractPatchApplyingCommand, ProcessBugsMixin):
     name = "apply-patches"
+    help_text = "Apply reviewed patches from provided bugs to the local working directory"
+    argument_names = "BUGID [BUGIDS]"
     show_in_main_help = True
-    def __init__(self):
-        AbstractPatchApplyingCommand.__init__(self, "Apply reviewed patches from provided bugs to the local working directory", "BUGID [BUGIDS]")
-
-    def _fetch_list_of_patches_to_process(self, options, args, tool):
-        all_patches = []
-        for bug_id in args:
-            patches = tool.bugs.fetch_reviewed_patches_from_bug(bug_id)
-            log("%s found on bug %s." % (pluralize("reviewed patch", len(patches)), bug_id))
-            all_patches += patches
-        return all_patches
 
 
 class AbstractPatchLandingCommand(AbstractPatchSequencingCommand):
@@ -231,31 +239,21 @@ class AbstractPatchLandingCommand(AbstractPatchSequencingCommand):
     ]
 
 
-class LandAttachment(AbstractPatchLandingCommand):
+class LandAttachment(AbstractPatchLandingCommand, ProcessAttachmentsMixin):
     name = "land-attachment"
+    help_text = "Land patches from bugzilla, optionally building and testing them first"
+    argument_names = "ATTACHMENT_ID [ATTACHMENT_IDS]"
     show_in_main_help = True
-    def __init__(self):
-        AbstractPatchLandingCommand.__init__(self, "Land patches from bugzilla, optionally building and testing them first", "ATTACHMENT_ID [ATTACHMENT_IDS]")
-
-    def _fetch_list_of_patches_to_process(self, options, args, tool):
-        return map(lambda patch_id: tool.bugs.fetch_attachment(patch_id), args)
 
 
-class LandPatches(AbstractPatchLandingCommand):
+class LandPatches(AbstractPatchLandingCommand, ProcessBugsMixin):
     name = "land-patches"
+    help_text = "Land all patches on the given bugs, optionally building and testing them first"
+    argument_names = "BUGID [BUGIDS]"
     show_in_main_help = True
-    def __init__(self):
-        AbstractPatchLandingCommand.__init__(self, "Land all patches on the given bugs, optionally building and testing them first", "BUGID [BUGIDS]")
-
-    def _fetch_list_of_patches_to_process(self, options, args, tool):
-        all_patches = []
-        for bug_id in args:
-            patches = tool.bugs.fetch_reviewed_patches_from_bug(bug_id)
-            log("%s found on bug %s." % (pluralize("reviewed patch", len(patches)), bug_id))
-            all_patches += patches
-        return all_patches
 
 
+# FIXME: Make Rollout more declarative.
 class Rollout(Command):
     name = "rollout"
     show_in_main_help = True

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list