[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373
eric at webkit.org
eric at webkit.org
Thu Apr 8 00:51:41 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit 45d51f09b29c5eac5df0e2eaff6c079c362c8b70
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Tue Dec 29 19:33:01 2009 +0000
2009-12-29 Eric Seidel <eric at webkit.org>
Reviewed by Adam Barth.
Add the start of a Bug object for bugzilla.py
https://bugs.webkit.org/show_bug.cgi?id=32995
This allowed us to get rid of some duplicated "is_obsolete" checks.
* Scripts/modules/bugzilla.py:
- Add a new Bug class, and move patches/unreviewed_patches filtering logic there.
- Add _fetch_bug_page for possible future mocking.
(I did not try to test fetch_*_from_bug now due to difficulties with our current validate_reviewer logic.)
- Rename fetch_bug to fetch_bug_dictionary and add a new fetch_bug which returns a Bug object.
- Use fetch_bug and attachments(), patches(), etc. instead of custom fetch_*_from_bug methods.
- Reduce code in fetch_patches_from_pending_commit_list and fetch_patches_from_review_queue
using list comprehensions. Use a sum(list, []) trick to flatten a list of lists into a single list.
* Scripts/modules/bugzilla_unittest.py:
- Remove an unneeded unicode string marker.
* Scripts/modules/buildsteps.py:
- define __all__ to include just the BuildSteps
* Scripts/modules/commands/download.py:
- import * now that we have an __all__ defined.
* Scripts/modules/commands/upload.py:
- Use fetch_bug_dictionary instead of fetch_bug.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@52628 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 61062df..ab5c663 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,29 @@
+2009-12-29 Eric Seidel <eric at webkit.org>
+
+ Reviewed by Adam Barth.
+
+ Add the start of a Bug object for bugzilla.py
+ https://bugs.webkit.org/show_bug.cgi?id=32995
+
+ This allowed us to get rid of some duplicated "is_obsolete" checks.
+
+ * Scripts/modules/bugzilla.py:
+ - Add a new Bug class, and move patches/unreviewed_patches filtering logic there.
+ - Add _fetch_bug_page for possible future mocking.
+ (I did not try to test fetch_*_from_bug now due to difficulties with our current validate_reviewer logic.)
+ - Rename fetch_bug to fetch_bug_dictionary and add a new fetch_bug which returns a Bug object.
+ - Use fetch_bug and attachments(), patches(), etc. instead of custom fetch_*_from_bug methods.
+ - Reduce code in fetch_patches_from_pending_commit_list and fetch_patches_from_review_queue
+ using list comprehensions. Use a sum(list, []) trick to flatten a list of lists into a single list.
+ * Scripts/modules/bugzilla_unittest.py:
+ - Remove an unneeded unicode string marker.
+ * Scripts/modules/buildsteps.py:
+ - define __all__ to include just the BuildSteps
+ * Scripts/modules/commands/download.py:
+ - import * now that we have an __all__ defined.
+ * Scripts/modules/commands/upload.py:
+ - Use fetch_bug_dictionary instead of fetch_bug.
+
2009-12-29 Daniel Bates <dbates at webkit.org>
Reviewed by Ariya Hidayat.
diff --git a/WebKitTools/Scripts/modules/bugzilla.py b/WebKitTools/Scripts/modules/bugzilla.py
index 9ae031a..3d895f3 100644
--- a/WebKitTools/Scripts/modules/bugzilla.py
+++ b/WebKitTools/Scripts/modules/bugzilla.py
@@ -63,11 +63,31 @@ class BugzillaError(Exception):
pass
+# FIXME: This class is kinda a hack for now. It exists so we have one place
+# to hold bug logic, even if much of the code deals with dictionaries still.
+class Bug(object):
+ def __init__(self, bug_dictionary):
+ self.bug_dictionary = bug_dictionary
+
+ # Rarely do we actually want obsolete attachments
+ def attachments(self, include_obsolete=False):
+ if include_obsolete:
+ return self.bug_dictionary["attachments"][:] # Return a copy in either case.
+ return [attachment for attachment in self.bug_dictionary["attachments"] if not attachment["is_obsolete"]]
+
+ def patches(self, include_obsolete=False):
+ return [patch for patch in self.attachments(include_obsolete) if patch["is_patch"]]
+
+ def unreviewed_patches(self):
+ return [patch for patch in self.patches() if patch.get("review") == "?"]
+
+
class Bugzilla(object):
def __init__(self, dryrun=False, committers=CommitterList()):
self.dryrun = dryrun
self.authenticated = False
+ # FIXME: We should use some sort of Browser mock object when in dryrun mode (to prevent any mistakes).
self.browser = Browser()
# Ignore bugs.webkit.org/robots.txt until we fix it to allow this script
self.browser.set_handle_robots(False)
@@ -123,18 +143,18 @@ class Bugzilla(object):
bug["attachments"] = [self._parse_attachment_element(element, bug["id"]) for element in soup.findAll('attachment')]
return bug
- def fetch_bug(self, bug_id):
+ # Makes testing fetch_*_from_bug() possible until we have a better BugzillaNetwork abstration.
+ def _fetch_bug_page(self, bug_id):
bug_url = self.bug_url_for_bug_id(bug_id, xml=True)
log("Fetching: %s" % bug_url)
- page = self.browser.open(bug_url)
- return self._parse_bug_page(page)
+ return self.browser.open(bug_url)
- # This should be an attachments() method on a Bug object.
- def fetch_attachments_from_bug(self, bug_id):
- bug = self.fetch_bug(bug_id)
- if not bug:
- return None
- return bug["attachments"]
+ def fetch_bug_dictionary(self, bug_id):
+ return self._parse_bug_page(self._fetch_bug_page(bug_id))
+
+ # FIXME: A BugzillaCache object should provide all these fetch_ methods.
+ def fetch_bug(self, bug_id):
+ return Bug(self.fetch_bug_dictionary(bug_id))
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.
@@ -157,8 +177,7 @@ class Bugzilla(object):
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:
+ for attachment in self.fetch_bug(bug_id).attachments(include_obsolete=True):
# FIXME: Once we have a real Attachment class we shouldn't paper over this possible comparison failure
# and we should remove the int() == int() hacks and leave it just ==.
if int(attachment['id']) == int(attachment_id):
@@ -166,8 +185,9 @@ class Bugzilla(object):
return attachment
return None # This should never be hit.
+ # fetch_patches_from_bug exists until we expose a Bug class outside of bugzilla.py
def fetch_patches_from_bug(self, bug_id):
- return [patch for patch in self.fetch_attachments_from_bug(bug_id) if patch['is_patch'] and not patch['is_obsolete']]
+ return self.fetch_bug(bug_id).patches()
# _view_source_link belongs in some sort of webkit_config.py module.
def _view_source_link(self, local_path):
@@ -215,22 +235,19 @@ class Bugzilla(object):
self._validate_reviewer(patch, reject_invalid_patches=False)
self._validate_committer(patch, reject_invalid_patches=False)
- def fetch_unreviewed_patches_from_bug(self, bug_id):
- return [patch for patch in self.fetch_attachments_from_bug(bug_id) if patch.get('review') == '?' and not patch['is_obsolete']]
-
# FIXME: fetch_reviewed_patches_from_bug and fetch_commit_queue_patches_from_bug
# should share more code and use list comprehensions.
def fetch_reviewed_patches_from_bug(self, bug_id, reject_invalid_patches=False):
reviewed_patches = []
- for attachment in self.fetch_attachments_from_bug(bug_id):
- if self._validate_reviewer(attachment, reject_invalid_patches) and not attachment['is_obsolete']:
+ for attachment in self.fetch_bug(bug_id).attachments():
+ if self._validate_reviewer(attachment, reject_invalid_patches):
reviewed_patches.append(attachment)
return reviewed_patches
def fetch_commit_queue_patches_from_bug(self, bug_id, reject_invalid_patches=False):
commit_queue_patches = []
for attachment in self.fetch_reviewed_patches_from_bug(bug_id, reject_invalid_patches):
- if self._validate_committer(attachment, reject_invalid_patches) and not attachment['is_obsolete']:
+ if self._validate_committer(attachment, reject_invalid_patches):
commit_queue_patches.append(attachment)
return commit_queue_patches
@@ -274,20 +291,10 @@ class Bugzilla(object):
return patches_to_land
def fetch_patches_from_pending_commit_list(self):
- patches_needing_commit = []
- for bug_id in self.fetch_bug_ids_from_needs_commit_list():
- patches = self.fetch_reviewed_patches_from_bug(bug_id)
- patches_needing_commit += patches
- return patches_needing_commit
+ return sum([self.fetch_reviewed_patches_from_bug(bug_id) for bug_id in self.fetch_bug_ids_from_needs_commit_list()], [])
def fetch_patches_from_review_queue(self, limit=None):
- patches_to_review = []
- for bug_id in self.fetch_bug_ids_from_review_queue():
- if limit and len(patches_to_review) >= limit:
- break
- patches = self.fetch_unreviewed_patches_from_bug(bug_id)
- patches_to_review += patches
- return patches_to_review
+ return sum([self.fetch_bug(bug_id).unreviewed_patches() for bug_id in self.fetch_bug_ids_from_review_queue()[:limit]], []) # [:None] returns the whole array.
def authenticate(self):
if self.authenticated:
@@ -444,6 +451,7 @@ class Bugzilla(object):
comment_text = "Rejecting patch %s from review queue." % attachment_id
self._set_flag_on_attachment(attachment_id, 'review', '-', comment_text, additional_comment_text)
+ # FIXME: All of these bug editing methods have a ridiculous amount of copy/paste code.
def obsolete_attachment(self, attachment_id, comment_text = None):
self.authenticate()
diff --git a/WebKitTools/Scripts/modules/bugzilla_unittest.py b/WebKitTools/Scripts/modules/bugzilla_unittest.py
index 4f255fc..59480dc 100644
--- a/WebKitTools/Scripts/modules/bugzilla_unittest.py
+++ b/WebKitTools/Scripts/modules/bugzilla_unittest.py
@@ -179,7 +179,7 @@ ZEZpbmlzaExvYWRXaXRoUmVhc29uOnJlYXNvbl07Cit9CisKIEBlbmQKIAogI2VuZGlmCg==
'name': u'Patch',
'url': 'https://bugs.webkit.org/attachment.cgi?id=45548',
'is_obsolete': False,
- 'review': u'?',
+ 'review': '?',
'is_patch': True,
'attacher_email': 'mjs at apple.com',
'bug_id': 32585,
diff --git a/WebKitTools/Scripts/modules/buildsteps.py b/WebKitTools/Scripts/modules/buildsteps.py
index 338376d..dfa6323 100644
--- a/WebKitTools/Scripts/modules/buildsteps.py
+++ b/WebKitTools/Scripts/modules/buildsteps.py
@@ -37,6 +37,32 @@ from modules.logging import log, error
from modules.webkitport import WebKitPort
from modules.changelogs import ChangeLog
+# FIXME: Why do some of these have "Step" in their name but not all?
+__all__ = [
+ "ApplyPatchStep",
+ "ApplyPatchWithLocalCommitStep",
+ "BuildStep",
+ "CheckStyleStep",
+ "CleanWorkingDirectoryStep",
+ "CleanWorkingDirectoryWithLocalCommitsStep",
+ "CloseBugForLandDiffStep",
+ "CloseBugStep",
+ "ClosePatchStep",
+ "CommitStep",
+ "CompleteRollout",
+ "CreateBugStep",
+ "EnsureBuildersAreGreenStep",
+ "EnsureLocalCommitIfNeeded",
+ "ObsoletePatchesOnBugStep",
+ "PostDiffToBugStep",
+ "PrepareChangeLogForRevertStep",
+ "PrepareChangeLogStep",
+ "PromptForBugOrTitleStep",
+ "RevertRevisionStep",
+ "RunTestsStep",
+ "UpdateChangeLogsWithReviewerStep",
+ "UpdateStep",
+]
class CommandOptions(object):
force_clean = make_option("--force-clean", action="store_true", dest="force_clean", default=False, help="Clean working directory before applying patches (removes local changes and commits)")
diff --git a/WebKitTools/Scripts/modules/commands/download.py b/WebKitTools/Scripts/modules/commands/download.py
index 41b5eb1..4d0ccc5 100644
--- a/WebKitTools/Scripts/modules/commands/download.py
+++ b/WebKitTools/Scripts/modules/commands/download.py
@@ -33,8 +33,8 @@ import os
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, EnsureLocalCommitIfNeeded, UpdateChangeLogsWithReviewerStep, CleanWorkingDirectoryStep, CleanWorkingDirectoryWithLocalCommitsStep, UpdateStep, ApplyPatchStep, ApplyPatchWithLocalCommitStep, BuildStep, CheckStyleStep, RunTestsStep, CommitStep, ClosePatchStep, CloseBugStep, CloseBugForLandDiffStep, PrepareChangeLogForRevertStep, RevertRevisionStep, CompleteRollout
+# We could instead use from modules import buildsteps and then prefix every buildstep with "buildsteps."
+from modules.buildsteps import *
from modules.changelogs import ChangeLog
from modules.comments import bug_comment_from_commit_text
from modules.executive import ScriptError
diff --git a/WebKitTools/Scripts/modules/commands/upload.py b/WebKitTools/Scripts/modules/commands/upload.py
index 72cd3c9..3432f26 100644
--- a/WebKitTools/Scripts/modules/commands/upload.py
+++ b/WebKitTools/Scripts/modules/commands/upload.py
@@ -250,7 +250,7 @@ class MarkBugFixed(Command):
needs_prompt = True
(bug_id, svn_revision) = self._determine_bug_id_and_svn_revision(tool, bug_id, svn_revision)
- log("Bug: <%s> %s" % (tool.bugs.short_bug_url_for_bug_id(bug_id), tool.bugs.fetch_bug(bug_id)["title"]))
+ log("Bug: <%s> %s" % (tool.bugs.short_bug_url_for_bug_id(bug_id), tool.bugs.fetch_bug_dictionary(bug_id)["title"]))
log("Revision: %s" % svn_revision)
if options.open_bug:
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list