Changeset 47111 in webkit

Timestamp:

Aug 12, 2009 9:26:19 AM (15 years ago)

Author:

eric@webkit.org

Message:

2009-08-11 Eric Seidel <​eric@webkit.org>

Reviewed by Adam Barth.

bugzilla-tool : various improvements for running the commit-queue
​https://bugs.webkit.org/show_bug.cgi?id=28199

Make run_and_throw_if_fail silence STDERR as well as STDIN.
I also changed run_and_throw_if_fail to use the /dev/null trick instead of .communicate() to avoid ever buffering the out
Change a few "print" statements to "log" so they appear in the output.
Changed all string + uses to use string formatting instead (this is less error prone as it will automatically convert non
Added a little more logging so that --quiet mode is easier to understand.
Changed clear_attachment_review_flag to clear_attachment_flags and made it clear the commit-queue flag as well.
Added the ability for bugzilla-tool to reject patches from the commit-queue when they fail to compile/apply/etc.
Added _find_select_element_for_flag to make the code for finding flag <select> elements clearer.
Made curl call (downloading patch files) quieter.

• Scripts/bugzilla-tool:
• Scripts/modules/bugzilla.py:
• Scripts/modules/scm.py:

Location:

trunk/WebKitTools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/bugzilla-tool (modified) (5 diffs)
• Scripts/modules/bugzilla.py (modified) (7 diffs)
• Scripts/modules/scm.py (modified) (1 diff)

trunk/WebKitTools/ChangeLog

@@ +1 @@
+2009-08-11  Eric Seidel  <eric@webkit.org>
+
+        Reviewed by Adam Barth.
+
+        bugzilla-tool : various improvements for running the commit-queue
+        https://bugs.webkit.org/show_bug.cgi?id=28199
+
+        Make run_and_throw_if_fail silence STDERR as well as STDIN.
+        I also changed run_and_throw_if_fail to use the /dev/null trick instead of .communicate() to avoid ever buffering the out
+        Change a few "print" statements to "log" so they appear in the output.
+        Changed all string + uses to use string formatting instead (this is less error prone as it will automatically convert non
+        Added a little more logging so that --quiet mode is easier to understand.
+        Changed clear_attachment_review_flag to clear_attachment_flags and made it clear the commit-queue flag as well.
+        Added the ability for bugzilla-tool to reject patches from the commit-queue when they fail to compile/apply/etc.
+        Added _find_select_element_for_flag to make the code for finding flag <select> elements clearer.
+        Made curl call (downloading patch files) quieter.
+
+        * Scripts/bugzilla-tool:
+        * Scripts/modules/bugzilla.py:
+        * Scripts/modules/scm.py:
+
 2009-08-11  Eric Seidel  <eric@webkit.org>
 

trunk/WebKitTools/Scripts/bugzilla-tool

@@ -243 +243 @@
      @staticmethod
      def run_and_throw_if_fail(args, quiet=False):
-         child_stdout = subprocess.PIPE if quiet else None
-         child_process = subprocess.Popen(args, stdout=child_stdout)
-         if child_process.stdout:
-             child_process.communicate()
+         # Passing None will use the default input/outputs
+         child_output = open(os.devnull, "w") if quiet else None
+
+         child_process = subprocess.Popen(args, stdout=child_output, stderr=child_output)
          return_code = child_process.wait()
+
+         if child_output:
+             child_output.close()
+
          if return_code:
              raise ScriptError("%s failed with exit code %d" % (" ".join(args), return_code))
@@ -258 +262 @@
      @classmethod
      def run_webkit_script(cls, script_name, quiet=False):
-         print "Running WebKit Script " + script_name
+         log("Running %s" % script_name)
          cls.run_and_throw_if_fail(cls.webkit_script_path(script_name), quiet)
 
@@ -357 +361 @@
             for patch in patches:
                 tool.scm().update_webkit() # Update before every patch in case the tree has changed
+                log("Applying %s from bug %s." % (patch['id'], bug_id))
                 tool.scm().apply_patch(patch, force=options.commit_queue)
                 comment_text = WebKitLandingScripts.build_and_commit(tool.scm(), options)
-                tool.bugs.clear_attachment_review_flag(patch['id'], comment_text)
+                tool.bugs.clear_attachment_flags(patch['id'], comment_text)
 
             if options.close_bug:
                 tool.bugs.close_bug_as_fixed(bug_id, "All reviewed patches have been landed.  Closing bug.")
         except ScriptError, e:
+            tool.bugs.reject_patch_from_commit_queue(patch['id'], e)
             # We should add a comment to the bug, and r- the patch on failure
             error(e)
@@ -379 +385 @@
             else:
                 patches = tool.bugs.fetch_reviewed_patches_from_bug(bug_id)
-            if not len(patches):
-                log("No reviewed patches found on %s." % bug_id)
-                continue
-            patch_count += len(patches)
-            bugs_to_patches[bug_id] = patches
+
+            patches_found = len(patches)
+            log("%s found on %s." % (pluralize("reviewed patch", patches_found), bug_id))
+
+            patch_count += patches_found
+            if patches_found:
+                bugs_to_patches[bug_id] = patches
 
         log("Landing %s from %s." % (pluralize("patch", patch_count), pluralize("bug", len(args))))
@@ -545 +553 @@
     def prompt_for_bug_title_and_comment(self):
         bug_title = raw_input("Bug title: ")
-        print("Bug comment (hit ^D on blank line to end):")
+        print "Bug comment (hit ^D on blank line to end):"
         lines = sys.stdin.readlines()
         sys.stdin.seek(0, os.SEEK_END)

trunk/WebKitTools/Scripts/modules/bugzilla.py

@@ -96 +96 @@
 def read_config(key):
     # Need a way to read from svn too
-    config_process = subprocess.Popen("git config --get bugzilla." + key, stdout=subprocess.PIPE, shell=True)
+    config_process = subprocess.Popen("git config --get bugzilla.%s" % key, stdout=subprocess.PIPE, shell=True)
     value = config_process.communicate()[0]
     return_code = config_process.wait()
@@ -142 +142 @@
         action_param = ""
         if action and action != "view":
-            action_param = "&action=" + action
+            action_param = "&action=%s" % action
         return "%sattachment.cgi?id=%s%s" % (self.bug_server_url, attachment_id, action_param)
 
@@ -171 +171 @@
     def fetch_attachments_from_bug(self, bug_id):
         bug_url = self.bug_url_for_bug_id(bug_id, xml=True)
-        log("Fetching: " + bug_url)
+        log("Fetching: %s" % bug_url)
 
         page = urllib2.urlopen(bug_url)
@@ -258 +258 @@
             return
         
-        self.browser.open(self.bug_server_url + "attachment.cgi?action=enter&bugid=" + bug_id)
+        self.browser.open("%sattachment.cgi?action=enter&bugid=%s" % (self.bug_server_url, bug_id))
         self.browser.select_form(name="entryform")
         self.browser['description'] = description
@@ -318 +318 @@
         bug_id = self._check_create_bug_response(response.read())
         log("Bug %s created." % bug_id)
-        log(self.bug_server_url + "show_bug.cgi?id=" + bug_id)
+        log("%sshow_bug.cgi?id=%s" % (self.bug_server_url, bug_id))
         return bug_id
 
-    def clear_attachment_review_flag(self, attachment_id, additional_comment_text=None):
-        self.authenticate()
-
-        comment_text = "Clearing review flag on attachment: %s" % attachment_id
+    def _find_select_element_for_flag(self, flag_name):
+        # FIXME: This will break if we ever re-order attachment flags
+        if flag_name == "review":
+            return self.browser.find_control(type='select', nr=0)
+        if flag_name == "commit-queue":
+            return self.browser.find_control(type='select', nr=1)
+        raise Exception("Don't know how to find flag named \"%s\"" % flag_name)
+
+    def clear_attachment_flags(self, attachment_id, additional_comment_text=None):
+        self.authenticate()
+
+        comment_text = "Clearing flags on attachment: %s" % attachment_id
         if additional_comment_text:
-            comment_text += "\n\n" + additional_comment_text
+            comment_text += "\n\n%s" % additional_comment_text
         log(comment_text)
 
@@ -335 +343 @@
         self.browser.select_form(nr=1)
         self.browser.set_value(comment_text, name='comment', nr=0)
-        self.browser.find_control(type='select', nr=0).value = ("X",)
+        self._find_select_element_for_flag('review').value = ("X",)
+        self._find_select_element_for_flag('commit-queue').value = ("X",)
+        self.browser.submit()
+
+    # FIXME: We need a way to test this on a live bugzilla instance.
+    def reject_patch_from_commit_queue(self, attachment_id, additional_comment_text=None):
+        self.authenticate()
+
+        comment_text = "Rejecting patch %s from commit-queue.  This patch will require manual commit." % attachment_id
+        if additional_comment_text:
+            comment_text += "\n\n%s" % additional_comment_text
+        log(comment_text)
+
+        if self.dryrun:
+            return
+
+        self.browser.open(self.attachment_url_for_id(attachment_id, 'edit'))
+        self.browser.select_form(nr=1)
+        self.browser.set_value(comment_text, name='comment', nr=0)
+        self._find_select_element_for_flag('commit-queue').value = ("-",)
         self.browser.submit()
 
@@ -350 +377 @@
         self.browser.find_control('isobsolete').items[0].selected = True
         # Also clear any review flag (to remove it from review/commit queues)
-        self.browser.find_control(type='select', nr=0).value = ("X",)
+        self._find_select_element_for_flag('review').value = ("X",)
+        self._find_select_element_for_flag('commit-queue').value = ("X",)
         if comment_text:
             log(comment_text)

trunk/WebKitTools/Scripts/modules/scm.py

@@ -124 +124 @@
         # It's possible that the patch was not made from the root directory.
         # We should detect and handle that case.
-        curl_process = subprocess.Popen(['curl', patch['url']], stdout=subprocess.PIPE)
+        curl_process = subprocess.Popen(['curl', '--silent', '--show-error', patch['url']], stdout=subprocess.PIPE)
         args = [self.script_path('svn-apply'), '--reviewer', patch['reviewer']]
         if force: