2009-11-16 Eric Seidel <eric@webkit.org>
authoreric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Nov 2009 08:39:15 +0000 (08:39 +0000)
committereric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Nov 2009 08:39:15 +0000 (08:39 +0000)
        Reviewed by Adam Barth.

        bugzilla-tool needs apply-attachment
        https://bugs.webkit.org/show_bug.cgi?id=31528

        * Scripts/bugzilla-tool:
         - Add ApplyAttachment command.
         - Abstract applying code into WebKitApplyingScripts.
         - Rename setup_for_landing to prepare_clean_working_directory and make local_commit checking optional.
        * Scripts/modules/bugzilla.py:
         - Add fetch_attachment and bug_id_for_attachment_id.
        * Scripts/modules/bugzilla_unittest.py:
         - Add test for new parsing.
         - Fix previous parsing test which broke with Adam's check-style patch (bug 31515).

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

WebKitTools/ChangeLog
WebKitTools/Scripts/bugzilla-tool
WebKitTools/Scripts/modules/bugzilla.py
WebKitTools/Scripts/modules/bugzilla_unittest.py

index a8cec7abb4b02c4a16580cd00645db0dcfba7317..24caf022283e27e022fdeaf85d1812f272d417a7 100644 (file)
@@ -1,3 +1,20 @@
+2009-11-16  Eric Seidel  <eric@webkit.org>
+
+        Reviewed by Adam Barth.
+
+        bugzilla-tool needs apply-attachment
+        https://bugs.webkit.org/show_bug.cgi?id=31528
+
+        * Scripts/bugzilla-tool:
+         - Add ApplyAttachment command.
+         - Abstract applying code into WebKitApplyingScripts.
+         - Rename setup_for_landing to prepare_clean_working_directory and make local_commit checking optional.
+        * Scripts/modules/bugzilla.py:
+         - Add fetch_attachment and bug_id_for_attachment_id.
+        * Scripts/modules/bugzilla_unittest.py:
+         - Add test for new parsing.
+         - Fix previous parsing test which broke with Adam's check-style patch (bug 31515).
+
 2009-11-16  Eric Seidel  <eric@webkit.org>
 
         Reviewed by Adam Barth.
index 26ab7625ac4a131dd0fa2bc302c3a3467314b642..967e0c9d8b74beb8853b8f9fbbb891d070020149 100755 (executable)
@@ -160,43 +160,62 @@ class CheckStyleOnBug(Command):
         bug_id = args[0]
         patches = tool.bugs.fetch_unreviewed_patches_from_bug(bug_id)
 
-        WebKitLandingScripts.setup_for_landing(tool.scm(), options)
+        WebKitLandingScripts.prepare_clean_working_directory(tool.scm(), options)
 
         for patch in patches:
             self.check_style(bug_id, patch, options, tool)
 
 
+class ApplyAttachment(Command):
+    def __init__(self):
+        options = WebKitApplyingScripts.apply_options() + WebKitLandingScripts.cleaning_options()
+        Command.__init__(self, 'Applies an attachment to the local working directory.', 'ATTACHMENT_ID', options=options)
+
+    def execute(self, options, args, tool):
+        WebKitApplyingScripts.setup_for_patch_apply(tool.scm(), options)
+        attachment_id = args[0]
+        attachment = tool.bugs.fetch_attachment(attachment_id)
+        WebKitApplyingScripts.apply_patches_with_options(tool.scm(), [attachment], options)
+
+
 class ApplyPatchesFromBug(Command):
     def __init__(self):
-        options = [
+        options = WebKitApplyingScripts.apply_options() + WebKitLandingScripts.cleaning_options()
+        Command.__init__(self, 'Applies all patches on a bug to the local working directory.', 'BUGID', options=options)
+
+    def execute(self, options, args, tool):
+        WebKitApplyingScripts.setup_for_patch_apply(tool.scm(), options)
+        bug_id = args[0]
+        patches = tool.bugs.fetch_reviewed_patches_from_bug(bug_id)
+        WebKitApplyingScripts.apply_patches_with_options(tool.scm(), patches, options)
+
+
+class WebKitApplyingScripts:
+    @staticmethod
+    def apply_options():
+        return [
             make_option("--no-update", action="store_false", dest="update", default=True, help="Don't update the working directory before applying patches"),
             make_option("--local-commit", action="store_true", dest="local_commit", default=False, help="Make a local commit for each applied patch"),
         ]
-        options += WebKitLandingScripts.cleaning_options()
-        Command.__init__(self, 'Applies all patches on a bug to the local working directory without committing.', 'BUGID', options=options)
 
     @staticmethod
-    def apply_patches(patches, scm, commit_each):
+    def setup_for_patch_apply(scm, options):
+        WebKitLandingScripts.prepare_clean_working_directory(scm, options, allow_local_commits=True)
+        if options.update:
+            scm.update_webkit()
+
+    @staticmethod
+    def apply_patches_with_options(scm, patches, options):
+        if options.local_commit and not scm.supports_local_commits():
+            error("--local-commit passed, but %s does not support local commits" % scm.display_name())
+
         for patch in patches:
+            log("Applying attachment %s from bug %s" % (patch['id'], patch['bug_id']))
             scm.apply_patch(patch)
-            if commit_each:
+            if options.local_commit:
                 commit_message = commit_message_for_this_commit(scm)
                 scm.commit_locally_with_message(commit_message.message() or patch['name'])
 
-    def execute(self, options, args, tool):
-        bug_id = args[0]
-        patches = tool.bugs.fetch_reviewed_patches_from_bug(bug_id)
-        os.chdir(tool.scm().checkout_root)
-        if options.clean:
-            tool.scm().ensure_clean_working_directory(options.force_clean)
-        if options.update:
-            tool.scm().update_webkit()
-        
-        if options.local_commit and not tool.scm().supports_local_commits():
-            error("--local-commit passed, but %s does not support local commits" % tool.scm().display_name())
-        
-        self.apply_patches(patches, tool.scm(), options.local_commit)
-
 
 class WebKitLandingScripts:
     @staticmethod
@@ -278,9 +297,10 @@ class WebKitLandingScripts:
         cls.run_and_throw_if_fail(args)
 
     @staticmethod
-    def setup_for_landing(scm, options):
+    def prepare_clean_working_directory(scm, options, allow_local_commits=False):
         os.chdir(scm.checkout_root)
-        scm.ensure_no_local_commits(options.force_clean)
+        if not allow_local_commits:
+            scm.ensure_no_local_commits(options.force_clean)
         if options.clean:
             scm.ensure_clean_working_directory(force_clean=options.force_clean)
 
@@ -416,7 +436,7 @@ class LandPatchesFromBugs(Command):
 
         bugs_to_patches = self._fetch_list_of_patches_to_land(options, args, tool)
 
-        WebKitLandingScripts.setup_for_landing(tool.scm(), options)
+        WebKitLandingScripts.prepare_clean_working_directory(tool.scm(), options)
 
         for bug_id in bugs_to_patches.keys():
             self.land_patches(bug_id, bugs_to_patches[bug_id], options, tool)
@@ -576,7 +596,7 @@ class RolloutCommit(Command):
             else:
                 log("Failed to parse bug number from diff.  No bugs will be updated/reopened after the rollout.")
 
-        WebKitLandingScripts.setup_for_landing(tool.scm(), options)
+        WebKitLandingScripts.prepare_clean_working_directory(tool.scm(), options)
         tool.scm().update_webkit()
         tool.scm().apply_reverse_diff(revision)
         self._create_changelogs_for_revert(tool.scm(), revision)
@@ -771,6 +791,7 @@ class BugzillaTool:
             { 'name' : 'patches-to-commit', 'object' : PatchesInCommitQueue() },
             { 'name' : 'reviewed-patches', 'object' : ReviewedPatchesOnBug() },
             { 'name' : 'create-bug', 'object' : CreateBug() },
+            { 'name' : 'apply-attachment', 'object' : ApplyAttachment() },
             { 'name' : 'apply-patches', 'object' : ApplyPatchesFromBug() },
             { 'name' : 'land-diff', 'object' : LandAndUpdateBug() },
             { 'name' : 'land-patches', 'object' : LandPatchesFromBugs() },
index fbbaebc627f17417e7c1f4e839afdd837738349d..e690b3217e304dc7ea79f50ddad64d9bc3ba83aa 100644 (file)
@@ -194,6 +194,33 @@ class Bugzilla:
             attachments.append(attachment)
         return attachments
 
+    def _parse_bug_id_from_attachment_page(self, page):
+        up_link = BeautifulSoup(page).find('link', rel='Up') # The "Up" relation happens to point to the bug.
+        if not up_link:
+            return None # This attachment does not exist (or you don't have permissions to view it).
+        match = re.search("show_bug.cgi\?id=(?P<bug_id>\d+)", up_link['href'])
+        return int(match.group('bug_id'))
+
+    def bug_id_for_attachment_id(self, attachment_id):
+        attachment_url = self.attachment_url_for_id(attachment_id, 'edit')
+        log("Fetching: %s" % attachment_url)
+        page = urllib2.urlopen(attachment_url)
+        return self._parse_bug_id_from_attachment_page(page)
+
+    # This should really return an Attachment object
+    # which can lazily fetch any missing data.
+    def fetch_attachment(self, attachment_id):
+        # We could grab all the attachment details off of the attachment edit page
+        # but we already have working code to do so off of the bugs page, so re-use that.
+        bug_id = self.bug_id_for_attachment_id(attachment_id)
+        if not bug_id:
+            return None
+        attachments = self.fetch_attachments_from_bug(bug_id)
+        for attachment in attachments:
+            if attachment['id'] == attachment_id:
+                return attachment
+        return None # This should never be hit.
+
     def fetch_title_from_bug(self, bug_id):
         bug_url = self.bug_url_for_bug_id(bug_id, xml=True)
         page = urllib2.urlopen(bug_url)
index f08031e5747bb328e57114a42906ee9abc89f3fa..95964f3d6a12ce8b5767a2f80084427414788b29 100644 (file)
@@ -68,8 +68,10 @@ class BugzillaTest(unittest.TestCase):
         'url' : "https://bugs.webkit.org/attachment.cgi?id=33721",
         'name' : "Fixed whitespace issue",
         'type' : "text/plain",
+        'review' : '+',
         'reviewer_email' : 'one@test.com',
-        'committer_email' : 'two@test.com'
+        'commit-queue' : '+',
+        'committer_email' : 'two@test.com',
     }
 
     def test_attachment_parsing(self):
@@ -86,5 +88,20 @@ class BugzillaTest(unittest.TestCase):
         for key, expected_value in self._expected_example_attachment_parsing.items():
             self.assertEquals(attachment[key], expected_value, ("Failure for key: %s: Actual='%s' Expected='%s'" % (key, attachment[key], expected_value)))
 
+    _sample_attachment_detail_page = """
+<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"
+                      "http://www.w3.org/TR/html4/loose.dtd">
+<html>
+  <head>
+    <title>
+  Attachment 41073 Details for Bug 27314</title>
+<link rel="Top" href="https://bugs.webkit.org/">
+    <link rel="Up" href="show_bug.cgi?id=27314">
+"""
+
+    def test_attachment_detail_bug_parsing(self):
+        bugzilla = Bugzilla()
+        self.assertEquals(27314, bugzilla._parse_bug_id_from_attachment_page(self._sample_attachment_detail_page))
+
 if __name__ == '__main__':
     unittest.main()