2009-11-27 Adam Barth <abarth@webkit.org>
authorabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 Nov 2009 23:34:44 +0000 (23:34 +0000)
committerabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 Nov 2009 23:34:44 +0000 (23:34 +0000)
        Reviewed by Eric Seidel.

        style-queue should only process each patch once
        https://bugs.webkit.org/show_bug.cgi?id=31939

        Before processing a patch, the try-queues now ask the web service
        whether they have already processed the patch.  This is an initial cut
        of this functionality.  I expect we're make it richer over time.

        * Scripts/bugzilla-tool:
        * Scripts/modules/commands/queues.py:
        * Scripts/modules/patchcollection.py:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@51447 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebKitTools/ChangeLog
WebKitTools/Scripts/bugzilla-tool
WebKitTools/Scripts/modules/bugzilla.py
WebKitTools/Scripts/modules/commands/queues.py
WebKitTools/Scripts/modules/patchcollection.py

index 7edb6c6777ddaa636008101055a495bb1d54f81c..76f3560f6a60619f35f14ec54d78b42d7e2f7593 100644 (file)
@@ -1,3 +1,18 @@
+2009-11-27  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        style-queue should only process each patch once
+        https://bugs.webkit.org/show_bug.cgi?id=31939
+
+        Before processing a patch, the try-queues now ask the web service
+        whether they have already processed the patch.  This is an initial cut
+        of this functionality.  I expect we're make it richer over time.
+
+        * Scripts/bugzilla-tool:
+        * Scripts/modules/commands/queues.py:
+        * Scripts/modules/patchcollection.py:
+
 2009-11-27  Adam Barth  <abarth@webkit.org>
 
         Reviewed by Eric Seidel.
index 35a344a19a4ec77d1145052ed63a603771859de7..ba32b3abf3b35821c212368959bffd4fa24e6c75 100755 (executable)
@@ -46,7 +46,8 @@ class BugzillaTool(MultiCommandTool):
 
         self.bugs = Bugzilla()
         self.buildbot = BuildBot()
-        self.cached_scm = None
+        self._cached_scm = None
+        self._cached_status = None
         self.steps = BuildSteps()
 
     def dry_run_callback(self, option, opt, value, parser):
@@ -56,19 +57,24 @@ class BugzillaTool(MultiCommandTool):
     def scm(self):
         # Lazily initialize SCM to not error-out before command line parsing (or when running non-scm commands).
         original_cwd = os.path.abspath(".")
-        if not self.cached_scm:
-            self.cached_scm = detect_scm_system(original_cwd)
+        if not self._cached_scm:
+            self._cached_scm = detect_scm_system(original_cwd)
 
-        if not self.cached_scm:
+        if not self._cached_scm:
             script_directory = os.path.abspath(sys.path[0])
             webkit_directory = os.path.abspath(os.path.join(script_directory, "../.."))
-            self.cached_scm = detect_scm_system(webkit_directory)
-            if self.cached_scm:
+            self._cached_scm = detect_scm_system(webkit_directory)
+            if self._cached_scm:
                 log("The current directory (%s) is not a WebKit checkout, using %s" % (original_cwd, webkit_directory))
             else:
                 error("FATAL: Failed to determine the SCM system for either %s or %s" % (original_cwd, webkit_directory))
 
-        return self.cached_scm
+        return self._cached_scm
+
+    def status(self):
+        if not self._cached_status:
+            self._cached_status = StatusBot()
+        return self._cached_status
 
     def path(self):
         return __file__
index e66339833ff2b3a0728c2847a450820f1553f7ad..b923cf88a2f85e48acfb395c53d6305f4f7918d6 100644 (file)
@@ -324,10 +324,10 @@ class Bugzilla:
             patches_to_land += patches
         return patches_to_land
 
-    def fetch_patches_from_review_queue(self, limit):
+    def fetch_patches_from_review_queue(self, limit=None):
         patches_to_review = []
         for bug_id in self.fetch_bug_ids_from_review_queue():
-            if len(patches_to_review) >= limit:
+            if limit and len(patches_to_review) >= limit:
                 break
             patches = self.fetch_unreviewed_patches_from_bug(bug_id)
             patches_to_review += patches
index 1f02e61d9e3107200a802a0a4b8a0b2847b74811..b7ff871e0e13734f07f3a4e4f398cfdbe9fbc580 100644 (file)
@@ -47,7 +47,7 @@ from modules.grammar import pluralize
 from modules.landingsequence import LandingSequence, ConditionalLandingSequence
 from modules.logging import error, log, tee
 from modules.multicommandtool import MultiCommandTool, Command
-from modules.patchcollection import PatchCollection
+from modules.patchcollection import PatchCollection, PersistentPatchCollection, PersistentPatchCollectionDelegate
 from modules.processutils import run_and_throw_if_fail
 from modules.scm import CommitMessage, detect_scm_system, ScriptError, CheckoutNeedsUpdate
 from modules.statusbot import StatusBot
@@ -139,20 +139,31 @@ class CommitQueue(AbstractQueue):
         self.tool.bugs.reject_patch_from_commit_queue(patch["id"], message)
 
 
-class AbstractTryQueue(AbstractQueue):
+class AbstractTryQueue(AbstractQueue, PersistentPatchCollectionDelegate):
     def __init__(self, options=[]):
         AbstractQueue.__init__(self, options)
 
+    # PersistentPatchCollectionDelegate methods
+
+    def collection_name(self):
+        return self.name
+
+    def fetch_potential_patches(self):
+        return self.tool.bugs.fetch_patches_from_review_queue(limit=3)
+
+    def status_server(self):
+        return self.tool.status()
+
+    # AbstractQueue methods
+
     def status_host(self):
         return None # FIXME: A hack until we come up with a more generic status page.
 
     def begin_work_queue(self):
         AbstractQueue.begin_work_queue(self)
-        self._patches = PatchCollection(self.tool.bugs)
-        self._patches.add_patches(self.tool.bugs.fetch_patches_from_review_queue(limit=10))
+        self._patches = PersistentPatchCollection(self)
 
     def next_work_item(self):
-        self.log_progress(self._patches.patch_ids())
         return self._patches.next()
 
     def should_proceed_with_work_item(self, patch):
@@ -163,6 +174,7 @@ class AbstractTryQueue(AbstractQueue):
 
     def handle_unexpected_error(self, patch, message):
         log(message)
+        self._patches.done(patch)
 
 
 class StyleQueue(AbstractTryQueue):
@@ -176,6 +188,7 @@ class StyleQueue(AbstractTryQueue):
 
     def process_work_item(self, patch):
         self.run_bugzilla_tool(["check-style", "--force-clean", patch["id"]])
+        self._patches.done(patch)
 
 
 class BuildQueue(AbstractTryQueue):
@@ -198,3 +211,4 @@ class BuildQueue(AbstractTryQueue):
 
     def process_work_item(self, patch):
         self.run_bugzilla_tool(["build-attachment", self.port.flag(), "--force-clean", "--quiet", "--no-update", patch["id"]])
+        self._patches.done(patch)
index 2ab9310ece37738e84e661f26ffa4316d7b33e17..03fbea549ab99170c08cdaa0c812fc2be0718678 100644 (file)
@@ -57,3 +57,34 @@ class PatchCollection:
 
     def __len__(self):
         return len(self._patches)
+
+
+class PersistentPatchCollectionDelegate:
+    def collection_name(self):
+        raise NotImplementedError, "subclasses must implement"
+
+    def fetch_potential_patches(self):
+        raise NotImplementedError, "subclasses must implement"
+
+    def status_server(self):
+        raise NotImplementedError, "subclasses must implement"
+
+
+class PersistentPatchCollection:
+    _initial_status = "Attempted"
+    _terminal_status = "Done"
+    def __init__(self, delegate):
+        self._delegate = delegate
+        self._name = self._delegate.collection_name()
+        self._status = self._delegate.status_server()
+
+    def next(self):
+        patches = self._delegate.fetch_potential_patches()
+        for patch in patches:
+            last_status = self._status.patch_status(self._name, patch["id"])
+            if not last_status: # FIXME: Add support for "Try again"
+                self._status.update_status(self._name, self._initial_status, patch)
+                return patch
+
+    def done(self, patch):
+        self._status.update_status(self._name, self._terminal_status, patch)