2011-05-04 Adam Barth <abarth@webkit.org>
authorabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 May 2011 10:44:18 +0000 (10:44 +0000)
committerabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 May 2011 10:44:18 +0000 (10:44 +0000)
        Reviewed by Eric Seidel.

        Fix circular dependency in webkitpy
        https://bugs.webkit.org/show_bug.cgi?id=60075

        These functions don't belong in bugzilla.py.  They only exist there
        because they are old.  Really, these functions shouldn't be free
        functions at all, but that's a patch for another day.

        * Scripts/webkitpy/common/checkout/changelog.py:
        * Scripts/webkitpy/common/checkout/changelog_unittest.py:
        * Scripts/webkitpy/common/checkout/checkout.py:
        * Scripts/webkitpy/common/config/urls.py:
        * Scripts/webkitpy/common/net/bugzilla/__init__.py:
        * Scripts/webkitpy/common/net/bugzilla/bugzilla.py:
        * Scripts/webkitpy/common/net/bugzilla/bugzilla_unittest.py:
        * Scripts/webkitpy/style/checkers/changelog.py:
        * Scripts/webkitpy/tool/bot/irc_command.py:
        * Scripts/webkitpy/tool/bot/sheriff.py:
        * Scripts/webkitpy/tool/commands/upload.py:

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

12 files changed:
Tools/ChangeLog
Tools/Scripts/webkitpy/common/checkout/changelog.py
Tools/Scripts/webkitpy/common/checkout/changelog_unittest.py
Tools/Scripts/webkitpy/common/checkout/checkout.py
Tools/Scripts/webkitpy/common/config/urls.py
Tools/Scripts/webkitpy/common/net/bugzilla/__init__.py
Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py
Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla_unittest.py
Tools/Scripts/webkitpy/style/checkers/changelog.py
Tools/Scripts/webkitpy/tool/bot/irc_command.py
Tools/Scripts/webkitpy/tool/bot/sheriff.py
Tools/Scripts/webkitpy/tool/commands/upload.py

index 3118470..36bae37 100644 (file)
@@ -1,3 +1,26 @@
+2011-05-04  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        Fix circular dependency in webkitpy
+        https://bugs.webkit.org/show_bug.cgi?id=60075
+
+        These functions don't belong in bugzilla.py.  They only exist there
+        because they are old.  Really, these functions shouldn't be free
+        functions at all, but that's a patch for another day.
+
+        * Scripts/webkitpy/common/checkout/changelog.py:
+        * Scripts/webkitpy/common/checkout/changelog_unittest.py:
+        * Scripts/webkitpy/common/checkout/checkout.py:
+        * Scripts/webkitpy/common/config/urls.py:
+        * Scripts/webkitpy/common/net/bugzilla/__init__.py:
+        * Scripts/webkitpy/common/net/bugzilla/bugzilla.py:
+        * Scripts/webkitpy/common/net/bugzilla/bugzilla_unittest.py:
+        * Scripts/webkitpy/style/checkers/changelog.py:
+        * Scripts/webkitpy/tool/bot/irc_command.py:
+        * Scripts/webkitpy/tool/bot/sheriff.py:
+        * Scripts/webkitpy/tool/commands/upload.py:
+
 2011-05-03  Pratik Solanki  <psolanki@apple.com>
 
         Reviewed by Dan Bernstein.
index 5f64c04..6ee8316 100644 (file)
@@ -34,9 +34,40 @@ import os.path
 import re
 import textwrap
 
-from webkitpy.common.system.deprecated_logging import log
 from webkitpy.common.config.committers import CommitterList
-from webkitpy.common.net.bugzilla import parse_bug_id_from_changelog
+import webkitpy.common.config.urls as config_urls
+from webkitpy.common.system.deprecated_logging import log
+
+
+# FIXME: parse_bug_id should not be a free function.
+# FIXME: Where should this function live in the dependency graph?
+def parse_bug_id(message):
+    if not message:
+        return None
+    match = re.search(config_urls.bug_url_short, message)
+    if match:
+        return int(match.group('bug_id'))
+    match = re.search(config_urls.bug_url_long, message)
+    if match:
+        return int(match.group('bug_id'))
+    return None
+
+
+# FIXME: parse_bug_id_from_changelog should not be a free function.
+# Parse the bug ID out of a Changelog message based on the format that is
+# used by prepare-ChangeLog
+def parse_bug_id_from_changelog(message):
+    if not message:
+        return None
+    match = re.search("^\s*" + config_urls.bug_url_short + "$", message, re.MULTILINE)
+    if match:
+        return int(match.group('bug_id'))
+    match = re.search("^\s*" + config_urls.bug_url_long + "$", message, re.MULTILINE)
+    if match:
+        return int(match.group('bug_id'))
+    # We weren't able to find a bug URL in the format used by prepare-ChangeLog. Fall back to the
+    # first bug URL found anywhere in the message.
+    return parse_bug_id(message)
 
 
 class ChangeLogEntry(object):
index cef33d8..8c54deb 100644 (file)
@@ -91,6 +91,51 @@ class ChangeLogTest(unittest.TestCase):
 == Rolled over to ChangeLog-2009-06-16 ==
 """
 
+    def test_parse_bug_id_from_changelog(self):
+        commit_text = '''
+2011-03-23  Ojan Vafai  <ojan@chromium.org>
+
+        Add failing result for WebKit2. All tests that require
+        focus fail on WebKit2. See https://bugs.webkit.org/show_bug.cgi?id=56988.
+
+        * platform/mac-wk2/fast/css/pseudo-any-expected.txt: Added.
+
+        '''
+
+        self.assertEquals(56988, parse_bug_id_from_changelog(commit_text))
+
+        commit_text = '''
+2011-03-23  Ojan Vafai  <ojan@chromium.org>
+
+        Add failing result for WebKit2. All tests that require
+        focus fail on WebKit2. See https://bugs.webkit.org/show_bug.cgi?id=56988.
+        https://bugs.webkit.org/show_bug.cgi?id=12345
+
+        * platform/mac-wk2/fast/css/pseudo-any-expected.txt: Added.
+
+        '''
+
+        self.assertEquals(12345, parse_bug_id_from_changelog(commit_text))
+
+        commit_text = '''
+2011-03-31  Adam Roben  <aroben@apple.com>
+
+        Quote the executable path we pass to ::CreateProcessW
+
+        This will ensure that spaces in the path will be interpreted correctly.
+
+        Fixes <http://webkit.org/b/57569> Web process sometimes fails to launch when there are
+        spaces in its path
+
+        Reviewed by Steve Falkenburg.
+
+        * UIProcess/Launcher/win/ProcessLauncherWin.cpp:
+        (WebKit::ProcessLauncher::launchProcess): Surround the executable path in quotes.
+
+        '''
+
+        self.assertEquals(57569, parse_bug_id_from_changelog(commit_text))
+
     def test_latest_entry_parse(self):
         changelog_contents = u"%s\n%s" % (self._example_entry, self._example_changelog)
         changelog_file = StringIO(changelog_contents)
index 5c21028..8f17b03 100644 (file)
@@ -30,12 +30,11 @@ import os
 import StringIO
 
 from webkitpy.common.config import urls
-from webkitpy.common.checkout.changelog import ChangeLog
+from webkitpy.common.checkout.changelog import ChangeLog, parse_bug_id_from_changelog
 from webkitpy.common.checkout.commitinfo import CommitInfo
 from webkitpy.common.checkout.scm import CommitMessage
 from webkitpy.common.checkout.deps import DEPS
 from webkitpy.common.memoized import memoized
-from webkitpy.common.net.bugzilla import parse_bug_id_from_changelog
 from webkitpy.common.system.executive import Executive, run_command, ScriptError
 from webkitpy.common.system.deprecated_logging import log
 
index ddaef97..b609a0c 100644 (file)
@@ -26,6 +26,8 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+import re
+
 
 def view_source_url(local_path):
     return "http://trac.webkit.org/browser/trunk/%s" % local_path
@@ -34,6 +36,13 @@ def view_source_url(local_path):
 def view_revision_url(revision_number):
     return "http://trac.webkit.org/changeset/%s" % revision_number
 
+
 chromium_lkgr_url = "http://chromium-status.appspot.com/lkgr"
 
 contribution_guidelines = "http://webkit.org/coding/contributing.html"
+
+bug_server_host = "bugs.webkit.org"
+_bug_server_regex = "https?://%s/" % re.sub('\.', '\\.', bug_server_host)
+bug_server_url = "https://%s/" % bug_server_host
+bug_url_long = _bug_server_regex + r"show_bug\.cgi\?id=(?P<bug_id>\d+)(&ctype=xml)?"
+bug_url_short = r"http\://webkit\.org/b/(?P<bug_id>\d+)"
index bde67c6..c427b18 100644 (file)
@@ -1,8 +1,7 @@
 # Required for Python to search this directory for module files
 
 # We only export public API here.
-# FIXME: parse_bug_id should not be a free function.
-from .bugzilla import Bugzilla, parse_bug_id, parse_bug_id_from_changelog
+from .bugzilla import Bugzilla
 # Unclear if Bug and Attachment need to be public classes.
 from .bug import Bug
 from .attachment import Attachment
index 3cc9f65..aff03f7 100644 (file)
@@ -43,40 +43,12 @@ from .bug import Bug
 
 from webkitpy.common.system.deprecated_logging import log
 from webkitpy.common.config import committers
+import webkitpy.common.config.urls as config_urls
 from webkitpy.common.net.credentials import Credentials
 from webkitpy.common.system.user import User
 from webkitpy.thirdparty.BeautifulSoup import BeautifulSoup, BeautifulStoneSoup, SoupStrainer
 
 
-# FIXME: parse_bug_id should not be a free function.
-def parse_bug_id(message):
-    if not message:
-        return None
-    match = re.search(Bugzilla.bug_url_short, message)
-    if match:
-        return int(match.group('bug_id'))
-    match = re.search(Bugzilla.bug_url_long, message)
-    if match:
-        return int(match.group('bug_id'))
-    return None
-
-
-# FIXME: parse_bug_id_from_changelog should not be a free function.
-# Parse the bug ID out of a Changelog message based on the format that is
-# used by prepare-ChangeLog
-def parse_bug_id_from_changelog(message):
-    if not message:
-        return None
-    match = re.search("^\s*" + Bugzilla.bug_url_short + "$", message, re.MULTILINE)
-    if match:
-        return int(match.group('bug_id'))
-    match = re.search("^\s*" + Bugzilla.bug_url_long + "$", message, re.MULTILINE)
-    if match:
-        return int(match.group('bug_id'))
-    # We weren't able to find a bug URL in the format used by prepare-ChangeLog. Fall back to the
-    # first bug URL found anywhere in the message.
-    return parse_bug_id(message)
-
 def timestamp():
     return datetime.now().strftime("%Y%m%d%H%M%S")
 
@@ -107,7 +79,7 @@ class BugzillaQueries(object):
 
     def _load_query(self, query):
         self._bugzilla.authenticate()
-        full_url = "%s%s" % (self._bugzilla.bug_server_url, query)
+        full_url = "%s%s" % (config_urls.bug_server_url, query)
         return self._bugzilla.browser.open(full_url)
 
     def _fetch_bugs_from_advanced_query(self, query):
@@ -220,14 +192,6 @@ class Bugzilla(object):
         # script.
         self.browser.set_handle_robots(False)
 
-    # FIXME: Much of this should go into some sort of config module,
-    # such as common.config.urls.
-    bug_server_host = "bugs.webkit.org"
-    bug_server_regex = "https?://%s/" % re.sub('\.', '\\.', bug_server_host)
-    bug_server_url = "https://%s/" % bug_server_host
-    bug_url_long = bug_server_regex + r"show_bug\.cgi\?id=(?P<bug_id>\d+)(&ctype=xml)?"
-    bug_url_short = r"http\://webkit\.org/b/(?P<bug_id>\d+)"
-
     def quips(self):
         # We only fetch and parse the list of quips once per instantiation
         # so that we do not burden bugs.webkit.org.
@@ -239,7 +203,7 @@ class Bugzilla(object):
         if not bug_id:
             return None
         content_type = "&ctype=xml" if xml else ""
-        return "%sshow_bug.cgi?id=%s%s" % (self.bug_server_url, bug_id, content_type)
+        return "%sshow_bug.cgi?id=%s%s" % (config_urls.bug_server_url, bug_id, content_type)
 
     def short_bug_url_for_bug_id(self, bug_id):
         if not bug_id:
@@ -247,7 +211,7 @@ class Bugzilla(object):
         return "http://webkit.org/b/%s" % bug_id
 
     def add_attachment_url(self, bug_id):
-        return "%sattachment.cgi?action=enter&bugid=%s" % (self.bug_server_url, bug_id)
+        return "%sattachment.cgi?action=enter&bugid=%s" % (config_urls.bug_server_url, bug_id)
 
     def attachment_url_for_id(self, attachment_id, action="view"):
         if not attachment_id:
@@ -255,7 +219,7 @@ class Bugzilla(object):
         action_param = ""
         if action and action != "view":
             action_param = "&action=%s" % action
-        return "%sattachment.cgi?id=%s%s" % (self.bug_server_url,
+        return "%sattachment.cgi?id=%s%s" % (config_urls.bug_server_url,
                                              attachment_id,
                                              action_param)
 
@@ -405,7 +369,7 @@ class Bugzilla(object):
             self.authenticated = True
             return
 
-        credentials = Credentials(self.bug_server_host, git_prefix="bugzilla")
+        credentials = Credentials(config_urls.bug_server_host, git_prefix="bugzilla")
 
         attempts = 0
         while not self.authenticated:
@@ -413,7 +377,7 @@ class Bugzilla(object):
             username, password = credentials.read_credentials()
 
             log("Logging in as %s..." % username)
-            self.browser.open(self.bug_server_url +
+            self.browser.open(config_urls.bug_server_url +
                               "index.cgi?GoAheadAndLogIn=1")
             self.browser.select_form(name="login")
             self.browser['Bugzilla_login'] = username
@@ -574,7 +538,7 @@ class Bugzilla(object):
             # FIXME: This will make some paths fail, as they assume this returns an id.
             return
 
-        self.browser.open(self.bug_server_url + "enter_bug.cgi?product=WebKit")
+        self.browser.open(config_urls.bug_server_url + "enter_bug.cgi?product=WebKit")
         self.browser.select_form(name="Create")
         component_items = self.browser.find_control('component').items
         component_names = map(lambda item: item.name, component_items)
@@ -610,7 +574,7 @@ class Bugzilla(object):
 
         bug_id = self._check_create_bug_response(response.read())
         log("Bug %s created." % bug_id)
-        log("%sshow_bug.cgi?id=%s" % (self.bug_server_url, bug_id))
+        log("%sshow_bug.cgi?id=%s" % (config_urls.bug_server_url, bug_id))
         return bug_id
 
     def _find_select_element_for_flag(self, flag_name):
index b996b7c..a3d1092 100644 (file)
@@ -30,8 +30,9 @@ import unittest
 import datetime
 import StringIO
 
-from .bugzilla import Bugzilla, BugzillaQueries, parse_bug_id, parse_bug_id_from_changelog
+from .bugzilla import Bugzilla, BugzillaQueries
 
+from webkitpy.common.checkout.changelog import parse_bug_id
 from webkitpy.common.system.outputcapture import OutputCapture
 from webkitpy.tool.mocktool import MockBrowser
 from webkitpy.thirdparty.mock import Mock
@@ -192,52 +193,6 @@ ZEZpbmlzaExvYWRXaXRoUmVhc29uOnJlYXNvbl07Cit9CisKIEBlbmQKIAogI2VuZGlmCg==
         }],
     }
 
-    def test_parse_bug_id_from_changelog(self):
-        commit_text = '''
-2011-03-23  Ojan Vafai  <ojan@chromium.org>
-
-        Add failing result for WebKit2. All tests that require
-        focus fail on WebKit2. See https://bugs.webkit.org/show_bug.cgi?id=56988.
-
-        * platform/mac-wk2/fast/css/pseudo-any-expected.txt: Added.
-
-        '''
-
-        self.assertEquals(56988, parse_bug_id_from_changelog(commit_text))
-
-        commit_text = '''
-2011-03-23  Ojan Vafai  <ojan@chromium.org>
-
-        Add failing result for WebKit2. All tests that require
-        focus fail on WebKit2. See https://bugs.webkit.org/show_bug.cgi?id=56988.
-        https://bugs.webkit.org/show_bug.cgi?id=12345
-
-        * platform/mac-wk2/fast/css/pseudo-any-expected.txt: Added.
-
-        '''
-
-        self.assertEquals(12345, parse_bug_id_from_changelog(commit_text))
-
-        commit_text = '''
-2011-03-31  Adam Roben  <aroben@apple.com>
-
-        Quote the executable path we pass to ::CreateProcessW
-
-        This will ensure that spaces in the path will be interpreted correctly.
-
-        Fixes <http://webkit.org/b/57569> Web process sometimes fails to launch when there are
-        spaces in its path
-
-        Reviewed by Steve Falkenburg.
-
-        * UIProcess/Launcher/win/ProcessLauncherWin.cpp:
-        (WebKit::ProcessLauncher::launchProcess): Surround the executable path in quotes.
-
-        '''
-
-        self.assertEquals(57569, parse_bug_id_from_changelog(commit_text))
-
-
     # FIXME: This should move to a central location and be shared by more unit tests.
     def _assert_dictionaries_equal(self, actual, expected):
         # Make sure we aren't parsing more or less than we expect
index 75754fa..bb857dc 100644 (file)
@@ -27,7 +27,7 @@
 
 import re
 from common import TabChecker
-from webkitpy.common.net.bugzilla import parse_bug_id_from_changelog
+from webkitpy.common.checkout.changelog import parse_bug_id_from_changelog
 
 
 class ChangeLogChecker(object):
index 4030960..b01d94f 100644 (file)
@@ -31,7 +31,7 @@ from webkitpy.common.config import irc as config_irc
 
 from webkitpy.common.config import urls
 from webkitpy.common.config.committers import CommitterList
-from webkitpy.common.net.bugzilla import parse_bug_id
+from webkitpy.common.checkout.changelog import parse_bug_id
 from webkitpy.common.system.executive import ScriptError
 from webkitpy.tool.bot.queueengine import TerminateQueue
 from webkitpy.tool.grammar import join_with_separators
index a5edceb..f4d1edb 100644 (file)
@@ -27,7 +27,7 @@
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 from webkitpy.common.config import urls
-from webkitpy.common.net.bugzilla import parse_bug_id
+from webkitpy.common.checkout.changelog import parse_bug_id
 from webkitpy.common.system.deprecated_logging import log
 from webkitpy.common.system.executive import ScriptError
 from webkitpy.tool.grammar import join_with_separators
index 80715a7..ac402dc 100644 (file)
@@ -36,8 +36,8 @@ from optparse import make_option
 
 from webkitpy.tool import steps
 
+from webkitpy.common.checkout.changelog import parse_bug_id_from_changelog
 from webkitpy.common.config.committers import CommitterList
-from webkitpy.common.net.bugzilla import parse_bug_id_from_changelog
 from webkitpy.common.system.deprecated_logging import error, log
 from webkitpy.common.system.user import User
 from webkitpy.thirdparty.mock import Mock