Changeset 51447 in webkit

Timestamp:

Nov 27, 2009 3:34:44 PM (14 years ago)

Author:

abarth@webkit.org

Message:

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:

Location:

trunk/WebKitTools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/bugzilla-tool (modified) (2 diffs)
• Scripts/modules/bugzilla.py (modified) (1 diff)
• Scripts/modules/commands/queues.py (modified) (6 diffs)
• Scripts/modules/patchcollection.py (modified) (1 diff)

trunk/WebKitTools/ChangeLog

@@ +1 @@
+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>
 

trunk/WebKitTools/Scripts/bugzilla-tool

@@ -47 +47 @@
         self.bugs = Bugzilla()
         self.buildbot = BuildBot()
-        self.cached_scm = None
+        self._cached_scm = None
+        self._cached_status = None
         self.steps = BuildSteps()
 
@@ -57 +58 @@
         # 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):

trunk/WebKitTools/Scripts/modules/bugzilla.py

@@ -325 +325 @@
         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)

trunk/WebKitTools/Scripts/modules/commands/queues.py

@@ -48 +48 @@
 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
@@ -140 +140 @@
 
 
-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.
@@ -149 +162 @@
     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()
 
@@ -164 +175 @@
     def handle_unexpected_error(self, patch, message):
         log(message)
+        self._patches.done(patch)
 
 
@@ -177 +189 @@
     def process_work_item(self, patch):
         self.run_bugzilla_tool(["check-style", "--force-clean", patch["id"]])
+        self._patches.done(patch)
 
 
@@ -199 +212 @@
     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)

trunk/WebKitTools/Scripts/modules/patchcollection.py

@@ -58 +58 @@
     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)