From e14391e94c850b8bd03680c23b38978db68687a8 Mon Sep 17 00:00:00 2001 From: John Reck Date: Thu, 4 Nov 2010 12:00:17 -0700 Subject: Merge Webkit at r70949: Initial merge by git. Change-Id: I77b8645c083b5d0da8dba73ed01d4014aab9848e --- .../Scripts/webkitpy/common/net/bugzilla.py | 25 +------ .../webkitpy/common/net/bugzilla_unittest.py | 7 -- .../Scripts/webkitpy/common/net/credentials.py | 57 +++++++++------- .../webkitpy/common/net/credentials_unittest.py | 66 ++++++++++++++---- .../Scripts/webkitpy/common/net/rietveld.py | 79 ---------------------- .../webkitpy/common/net/rietveld_unittest.py | 39 ----------- .../Scripts/webkitpy/common/net/statusserver.py | 2 +- 7 files changed, 89 insertions(+), 186 deletions(-) delete mode 100644 WebKitTools/Scripts/webkitpy/common/net/rietveld.py delete mode 100644 WebKitTools/Scripts/webkitpy/common/net/rietveld_unittest.py (limited to 'WebKitTools/Scripts/webkitpy/common/net') diff --git a/WebKitTools/Scripts/webkitpy/common/net/bugzilla.py b/WebKitTools/Scripts/webkitpy/common/net/bugzilla.py index 94519a7..1cc8e2e 100644 --- a/WebKitTools/Scripts/webkitpy/common/net/bugzilla.py +++ b/WebKitTools/Scripts/webkitpy/common/net/bugzilla.py @@ -113,9 +113,6 @@ class Attachment(object): def commit_queue(self): return self._attachment_dictionary.get("commit-queue") - def in_rietveld(self): - return self._attachment_dictionary.get("in-rietveld") - def url(self): # FIXME: This should just return # self._bugzilla().attachment_url_for_id(self.id()). scm_unittest.py @@ -221,9 +218,6 @@ class Bug(object): # a valid committer. return filter(lambda patch: patch.committer(), patches) - def in_rietveld_queue_patches(self): - return [patch for patch in self.patches() if patch.in_rietveld() == None] - # A container for all of the logic for making and parsing buzilla queries. class BugzillaQueries(object): @@ -287,16 +281,6 @@ class BugzillaQueries(object): return sum([self._fetch_bug(bug_id).commit_queued_patches() for bug_id in self.fetch_bug_ids_from_commit_queue()], []) - def fetch_first_patch_from_rietveld_queue(self): - # rietveld-queue processes all patches that don't have in-rietveld set. - query_url = "buglist.cgi?query_format=advanced&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&field0-0-0=flagtypes.name&type0-0-0=notsubstring&value0-0-0=in-rietveld&field0-1-0=attachments.ispatch&type0-1-0=equals&value0-1-0=1&order=Last+Changed&field0-2-0=attachments.isobsolete&type0-2-0=equals&value0-2-0=0" - bugs = self._fetch_bug_ids_advanced_query(query_url) - if not len(bugs): - return None - - patches = self._fetch_bug(bugs[0]).in_rietveld_queue_patches() - return patches[0] if len(patches) else None - def _fetch_bug_ids_from_review_queue(self): review_queue_url = "buglist.cgi?query_format=advanced&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&field0-0-0=flagtypes.name&type0-0-0=equals&value0-0-0=review?" return self._fetch_bug_ids_advanced_query(review_queue_url) @@ -502,8 +486,6 @@ class Bugzilla(object): self._parse_attachment_flag( element, 'review', attachment, 'reviewer_email') self._parse_attachment_flag( - element, 'in-rietveld', attachment, 'rietveld_uploader_email') - self._parse_attachment_flag( element, 'commit-queue', attachment, 'committer_email') return attachment @@ -591,11 +573,12 @@ class Bugzilla(object): self.authenticated = True return + credentials = Credentials(self.bug_server_host, git_prefix="bugzilla") + attempts = 0 while not self.authenticated: attempts += 1 - (username, password) = Credentials( - self.bug_server_host, git_prefix="bugzilla").read_credentials() + username, password = credentials.read_credentials() log("Logging in as %s..." % username) self.browser.open(self.bug_server_url + @@ -766,8 +749,6 @@ class Bugzilla(object): return self.browser.find_control(type='select', nr=0) elif flag_name == "commit-queue": return self.browser.find_control(type='select', nr=1) - elif flag_name == "in-rietveld": - return self.browser.find_control(type='select', nr=2) raise Exception("Don't know how to find flag named \"%s\"" % flag_name) def clear_attachment_flags(self, diff --git a/WebKitTools/Scripts/webkitpy/common/net/bugzilla_unittest.py b/WebKitTools/Scripts/webkitpy/common/net/bugzilla_unittest.py index 3a454d6..df1fcf6 100644 --- a/WebKitTools/Scripts/webkitpy/common/net/bugzilla_unittest.py +++ b/WebKitTools/Scripts/webkitpy/common/net/bugzilla_unittest.py @@ -83,11 +83,6 @@ class BugzillaTest(unittest.TestCase): status="+" setter="two@test.com" /> - ''' _expected_example_attachment_parsing = { @@ -103,8 +98,6 @@ class BugzillaTest(unittest.TestCase): 'reviewer_email' : 'one@test.com', 'commit-queue' : '+', 'committer_email' : 'two@test.com', - 'in-rietveld': '+', - 'rietveld_uploader_email': 'three@test.com', 'attacher_email' : 'christian.plesner.hansen@gmail.com', } diff --git a/WebKitTools/Scripts/webkitpy/common/net/credentials.py b/WebKitTools/Scripts/webkitpy/common/net/credentials.py index 1c3e6c0..30480b3 100644 --- a/WebKitTools/Scripts/webkitpy/common/net/credentials.py +++ b/WebKitTools/Scripts/webkitpy/common/net/credentials.py @@ -48,6 +48,7 @@ except ImportError: class Credentials(object): + _environ_prefix = "webkit_bugzilla_" def __init__(self, host, git_prefix=None, executive=None, cwd=os.getcwd(), keyring=keyring): @@ -58,8 +59,17 @@ class Credentials(object): self._keyring = keyring def _credentials_from_git(self): - return [Git.read_git_config(self.git_prefix + "username"), - Git.read_git_config(self.git_prefix + "password")] + try: + if not Git.in_working_directory(self.cwd): + return (None, None) + return (Git.read_git_config(self.git_prefix + "username"), + Git.read_git_config(self.git_prefix + "password")) + except OSError, e: + # Catch and ignore OSError exceptions such as "no such file + # or directory" (OSError errno 2), which imply that the Git + # command cannot be found/is not installed. + pass + return (None, None) def _keychain_value_with_label(self, label, source_text): match = re.search("%s\"(?P.+)\"" % label, @@ -110,21 +120,28 @@ class Credentials(object): else: return [None, None] - def read_credentials(self): - username = None - password = None + def _read_environ(self, key): + environ_key = self._environ_prefix + key + return os.environ.get(environ_key.upper()) - try: - if Git.in_working_directory(self.cwd): - (username, password) = self._credentials_from_git() - except OSError, e: - # Catch and ignore OSError exceptions such as "no such file - # or directory" (OSError errno 2), which imply that the Git - # command cannot be found/is not installed. - pass + def _credentials_from_environment(self): + return (self._read_environ("username"), self._read_environ("password")) + + def _offer_to_store_credentials_in_keyring(self, username, password): + if not self._keyring: + return + if not User().confirm("Store password in system keyring?", User.DEFAULT_NO): + return + self._keyring.set_password(self.host, username, password) + def read_credentials(self): + username, password = self._credentials_from_environment() + # FIXME: We don't currently support pulling the username from one + # source and the password from a separate source. + if not username or not password: + username, password = self._credentials_from_git() if not username or not password: - (username, password) = self._credentials_from_keychain(username) + username, password = self._credentials_from_keychain(username) if username and not password and self._keyring: password = self._keyring.get_password(self.host, username) @@ -132,13 +149,7 @@ class Credentials(object): if not username: username = User.prompt("%s login: " % self.host) if not password: - password = getpass.getpass("%s password for %s: " % (self.host, - username)) + password = getpass.getpass("%s password for %s: " % (self.host, username)) + self._offer_to_store_credentials_in_keyring(username, password) - if self._keyring: - store_password = User().confirm( - "Store password in system keyring?", User.DEFAULT_NO) - if store_password: - self._keyring.set_password(self.host, username, password) - - return [username, password] + return (username, password) diff --git a/WebKitTools/Scripts/webkitpy/common/net/credentials_unittest.py b/WebKitTools/Scripts/webkitpy/common/net/credentials_unittest.py index d30291b..6f2d909 100644 --- a/WebKitTools/Scripts/webkitpy/common/net/credentials_unittest.py +++ b/WebKitTools/Scripts/webkitpy/common/net/credentials_unittest.py @@ -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. +from __future__ import with_statement + import os import tempfile import unittest @@ -34,6 +36,21 @@ from webkitpy.common.system.executive import Executive from webkitpy.common.system.outputcapture import OutputCapture from webkitpy.thirdparty.mock import Mock + +# FIXME: Other unit tests probably want this class. +class _TemporaryDirectory(object): + def __init__(self, **kwargs): + self._kwargs = kwargs + self._directory_path = None + + def __enter__(self): + self._directory_path = tempfile.mkdtemp(**self._kwargs) + return self._directory_path + + def __exit__(self, type, value, traceback): + os.rmdir(self._directory_path) + + class CredentialsTest(unittest.TestCase): example_security_output = """keychain: "/Users/test/Library/Keychains/login.keychain" class: "inet" @@ -101,40 +118,59 @@ password: "SECRETSAUCE" self._assert_security_call() self._assert_security_call(username="foo") + def test_credentials_from_environment(self): + executive_mock = Mock() + credentials = Credentials("example.com", executive=executive_mock) + + saved_environ = os.environ.copy() + os.environ['WEBKIT_BUGZILLA_USERNAME'] = "foo" + os.environ['WEBKIT_BUGZILLA_PASSWORD'] = "bar" + username, password = credentials._credentials_from_environment() + self.assertEquals(username, "foo") + self.assertEquals(password, "bar") + os.environ = saved_environ + def test_read_credentials_without_git_repo(self): + # FIXME: This should share more code with test_keyring_without_git_repo class FakeCredentials(Credentials): def _is_mac_os_x(self): return True + def _credentials_from_keychain(self, username): - return ["test@webkit.org", "SECRETSAUCE"] + return ("test@webkit.org", "SECRETSAUCE") + + def _credentials_from_environment(self): + return (None, None) + + with _TemporaryDirectory(suffix="not_a_git_repo") as temp_dir_path: + credentials = FakeCredentials("bugs.webkit.org", cwd=temp_dir_path) + # FIXME: Using read_credentials here seems too broad as higher-priority + # credential source could be affected by the user's environment. + self.assertEqual(credentials.read_credentials(), ("test@webkit.org", "SECRETSAUCE")) - temp_dir_path = tempfile.mkdtemp(suffix="not_a_git_repo") - credentials = FakeCredentials("bugs.webkit.org", cwd=temp_dir_path) - self.assertEqual(credentials.read_credentials(), ["test@webkit.org", "SECRETSAUCE"]) - os.rmdir(temp_dir_path) def test_keyring_without_git_repo(self): + # FIXME: This should share more code with test_read_credentials_without_git_repo class MockKeyring(object): def get_password(self, host, username): return "NOMNOMNOM" class FakeCredentials(Credentials): - def __init__(self, cwd): - Credentials.__init__(self, "fake.hostname", cwd=cwd, - keyring=MockKeyring()) - def _is_mac_os_x(self): return True def _credentials_from_keychain(self, username): return ("test@webkit.org", None) - temp_dir_path = tempfile.mkdtemp(suffix="not_a_git_repo") - credentials = FakeCredentials(temp_dir_path) - try: - self.assertEqual(credentials.read_credentials(), ["test@webkit.org", "NOMNOMNOM"]) - finally: - os.rmdir(temp_dir_path) + def _credentials_from_environment(self): + return (None, None) + + with _TemporaryDirectory(suffix="not_a_git_repo") as temp_dir_path: + credentials = FakeCredentials("fake.hostname", cwd=temp_dir_path, keyring=MockKeyring()) + # FIXME: Using read_credentials here seems too broad as higher-priority + # credential source could be affected by the user's environment. + self.assertEqual(credentials.read_credentials(), ("test@webkit.org", "NOMNOMNOM")) + if __name__ == '__main__': unittest.main() diff --git a/WebKitTools/Scripts/webkitpy/common/net/rietveld.py b/WebKitTools/Scripts/webkitpy/common/net/rietveld.py deleted file mode 100644 index b9a0821..0000000 --- a/WebKitTools/Scripts/webkitpy/common/net/rietveld.py +++ /dev/null @@ -1,79 +0,0 @@ -# Copyright (c) 2010 Google Inc. All rights reserved. -# -# Redistribution and use in source and binary forms, with or without -# modification, are permitted provided that the following conditions are -# met: -# -# * Redistributions of source code must retain the above copyright -# notice, this list of conditions and the following disclaimer. -# * Redistributions in binary form must reproduce the above -# copyright notice, this list of conditions and the following disclaimer -# in the documentation and/or other materials provided with the -# distribution. -# * Neither the name of Google Inc. nor the names of its -# contributors may be used to endorse or promote products derived from -# this software without specific prior written permission. -# -# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -# (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 logging -import os -import re -import stat - -import webkitpy.common.config as config -from webkitpy.common.system.deprecated_logging import log -from webkitpy.common.system.executive import ScriptError -import webkitpy.thirdparty.autoinstalled.rietveld.upload as upload - - -class Rietveld(object): - def __init__(self, executive, dryrun=False): - self.dryrun = dryrun - self._executive = executive - - def url_for_issue(self, codereview_issue): - if not codereview_issue: - return None - return "%s%s" % (config.codereview_server_url, codereview_issue) - - def post(self, diff, patch_id, codereview_issue, message=None, cc=None): - if not message: - raise ScriptError("Rietveld requires a message.") - - # Rietveld has a 100 character limit on message length. - if len(message) > 100: - message = message[:100] - - args = [ - # First argument is empty string to mimic sys.argv. - "", - "--assume_yes", - "--server=%s" % config.codereview_server_host, - "--message=%s" % message, - "--webkit_patch_id=%s" % patch_id, - ] - if codereview_issue: - args.append("--issue=%s" % codereview_issue) - if cc: - args.append("--cc=%s" % cc) - - if self.dryrun: - log("Would have run %s" % args) - return - - # Use RealMain instead of calling upload from the commandline so that - # we can pass in the diff ourselves. Otherwise, upload will just use - # git diff for git checkouts, which doesn't respect --git-commit. - issue, patchset = upload.RealMain(args, data=diff) - return issue diff --git a/WebKitTools/Scripts/webkitpy/common/net/rietveld_unittest.py b/WebKitTools/Scripts/webkitpy/common/net/rietveld_unittest.py deleted file mode 100644 index 9c5a29e..0000000 --- a/WebKitTools/Scripts/webkitpy/common/net/rietveld_unittest.py +++ /dev/null @@ -1,39 +0,0 @@ -# Copyright (c) 2010 Google Inc. All rights reserved. -# -# Redistribution and use in source and binary forms, with or without -# modification, are permitted provided that the following conditions are -# met: -# -# * Redistributions of source code must retain the above copyright -# notice, this list of conditions and the following disclaimer. -# * Redistributions in binary form must reproduce the above -# copyright notice, this list of conditions and the following disclaimer -# in the documentation and/or other materials provided with the -# distribution. -# * Neither the name of Google Inc. nor the names of its -# contributors may be used to endorse or promote products derived from -# this software without specific prior written permission. -# -# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -# (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 unittest - -from webkitpy.common.net.rietveld import Rietveld -from webkitpy.thirdparty.mock import Mock - - -class RietveldTest(unittest.TestCase): - def test_url_for_issue(self): - rietveld = Rietveld(Mock()) - self.assertEqual(rietveld.url_for_issue(34223), - "https://wkrietveld.appspot.com/34223") diff --git a/WebKitTools/Scripts/webkitpy/common/net/statusserver.py b/WebKitTools/Scripts/webkitpy/common/net/statusserver.py index 3d03dcd..64dd77b 100644 --- a/WebKitTools/Scripts/webkitpy/common/net/statusserver.py +++ b/WebKitTools/Scripts/webkitpy/common/net/statusserver.py @@ -127,7 +127,7 @@ class StatusServer: self._browser.submit() def release_work_item(self, queue_name, patch): - _log.debug("Releasing work item %s from %s" % (patch.id(), queue_name)) + _log.info("Releasing work item %s from %s" % (patch.id(), queue_name)) return NetworkTransaction(convert_404_to_None=True).run(lambda: self._post_release_work_item(queue_name, patch)) def update_work_items(self, queue_name, work_items): -- cgit v1.1