From f7fb33eed48cf233db6c6e68e4c33b5b2940cf11 Mon Sep 17 00:00:00 2001 From: "maruel@chromium.org" Date: Wed, 17 Jun 2009 12:59:15 +0000 Subject: [PATCH] Add better try server settings automatic detection support. This is a redo of http://codereview.chromium.org/123020 (revision 18198) that was reverted in revision 18232 because of a codereview.settings misconfiguration and not because the script was broken. The misconfiguration was fixed in http://codereview.chromium.org/126021 Review URL: http://codereview.chromium.org/125135 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@18612 0039d316-1c4b-4281-b951-d872f2087c98 --- tests/trychange_unittest.py | 14 +++--- trychange.py | 95 +++++++++++++++++++++---------------- 2 files changed, 61 insertions(+), 48 deletions(-) diff --git a/tests/trychange_unittest.py b/tests/trychange_unittest.py index 27f8db611..8690aae44 100644 --- a/tests/trychange_unittest.py +++ b/tests/trychange_unittest.py @@ -5,7 +5,6 @@ """Unit tests for trychange.py.""" -import os import unittest # Local imports @@ -23,18 +22,19 @@ class TryChangeUnittest(TryChangeTestsBase): """General trychange.py tests.""" def testMembersChanged(self): members = [ - 'EscapeDot', 'ExecuteTryServerScript', 'GIT', 'GetSourceRoot', 'GuessVCS', + 'EscapeDot', 'GIT', 'GetSourceRoot', + 'GetTryServerSettings', 'GuessVCS', 'HELP_STRING', 'InvalidScript', 'NoTryServerAccess', 'PathDifference', - 'RunCommand', 'SCM', 'SCRIPT_PATH', 'SVN', 'TryChange', 'USAGE', + 'RunCommand', 'SCM', 'SVN', 'TryChange', 'USAGE', 'datetime', 'gcl', 'gclient', 'getpass', 'logging', 'optparse', 'os', - 'shutil', 'sys', 'tempfile', 'traceback', 'urllib', + 'shutil', 'socket', 'sys', 'tempfile', 'traceback', 'upload', 'urllib', ] # If this test fails, you should add the relevant test. self.compareMembers(trychange, members) class SVNUnittest(TryChangeTestsBase): - """General trychange.py tests.""" + """trychange.SVN tests.""" def testMembersChanged(self): members = [ 'GenerateDiff', 'ProcessOptions', 'options' @@ -43,8 +43,8 @@ class SVNUnittest(TryChangeTestsBase): self.compareMembers(trychange.SVN(None), members) -class TryChangeUnittest(TryChangeTestsBase): - """General trychange.py tests.""" +class GITUnittest(TryChangeTestsBase): + """trychange.GIT tests.""" def testMembersChanged(self): members = [ 'GenerateDiff', 'GetEmail', 'GetPatchName', 'ProcessOptions', 'options' diff --git a/trychange.py b/trychange.py index 5c8c1c57b..7d79690ae 100755 --- a/trychange.py +++ b/trychange.py @@ -14,6 +14,7 @@ import logging import optparse import os import shutil +import socket import sys import tempfile import traceback @@ -28,7 +29,6 @@ __version__ = '1.1' # Constants HELP_STRING = "Sorry, Tryserver is not available." -SCRIPT_PATH = os.path.join('tools', 'tryserver', 'tryserver.py') USAGE = r"""%prog [options] Client-side script to send a try job to the try server. It communicates to @@ -75,27 +75,29 @@ def PathDifference(root, subpath): def GetSourceRoot(): """Returns the absolute directory one level up from the repository root.""" + # TODO(maruel): This is odd to assume that '..' is the source root. return os.path.abspath(os.path.join(gcl.GetRepositoryRoot(), '..')) -def ExecuteTryServerScript(): - """Locates the tryserver script, executes it and returns its dictionary. - - The try server script contains the repository-specific try server commands.""" - script_locals = {} - try: - # gcl.GetRepositoryRoot() may throw an exception. - script_path = os.path.join(gcl.GetRepositoryRoot(), SCRIPT_PATH) - except Exception: - return script_locals - if os.path.exists(script_path): +def GetTryServerSettings(): + """Grab try server settings local to the repository.""" + def _SafeResolve(host): try: - exec(gcl.ReadFile(script_path), script_locals) - except Exception, e: - # TODO(maruel): Need to specialize the exception trapper. - traceback.print_exc() - raise InvalidScript('%s is invalid.' % script_path) - return script_locals + return socket.getaddrinfo(host, None) + except socket.gaierror: + return None + + settings = {} + settings['http_port'] = gcl.GetCodeReviewSetting('TRYSERVER_HTTP_PORT') + settings['http_host'] = gcl.GetCodeReviewSetting('TRYSERVER_HTTP_HOST') + settings['svn_repo'] = gcl.GetCodeReviewSetting('TRYSERVER_SVN_URL') + # Use http is the http_host name resolve, fallback to svn otherwise. + if (settings['http_port'] and settings['http_host'] and + _SafeResolve(settings['http_host'])): + settings['default_transport'] = 'http' + elif settings.get('svn_repo'): + settings['default_transport'] = 'svn' + return settings def EscapeDot(name): @@ -256,18 +258,12 @@ def _ParseSendChangeOptions(options): def _SendChangeHTTP(options): """Send a change to the try server using the HTTP protocol.""" - script_locals = ExecuteTryServerScript() - if not options.host: - options.host = script_locals.get('try_server_http_host', None) - if not options.host: - raise NoTryServerAccess('Please use the --host option to specify the try ' - 'server host to connect to.') + raise NoTryServerAccess('Please use the --host option to specify the try ' + 'server host to connect to.') if not options.port: - options.port = script_locals.get('try_server_http_port', None) - if not options.port: - raise NoTryServerAccess('Please use the --port option to specify the try ' - 'server port to connect to.') + raise NoTryServerAccess('Please use the --port option to specify the try ' + 'server port to connect to.') values = _ParseSendChangeOptions(options) values['patch'] = options.diff @@ -298,12 +294,9 @@ def _SendChangeHTTP(options): def _SendChangeSVN(options): """Send a change to the try server by committing a diff file on a subversion server.""" - script_locals = ExecuteTryServerScript() if not options.svn_repo: - options.svn_repo = script_locals.get('try_server_svn', None) - if not options.svn_repo: - raise NoTryServerAccess('Please use the --svn_repo option to specify the' - ' try server svn repository to connect to.') + raise NoTryServerAccess('Please use the --svn_repo option to specify the' + ' try server svn repository to connect to.') values = _ParseSendChangeOptions(options) description = '' @@ -315,7 +308,9 @@ def _SendChangeSVN(options): temp_file = tempfile.NamedTemporaryFile() temp_file_name = temp_file.name try: - RunCommand(['svn', 'checkout', '--depth', 'empty', '--non-interactive', + # Don't use '--non-interactive', since we want it to prompt for + # crendentials if necessary. + RunCommand(['svn', 'checkout', '--depth', 'empty', options.svn_repo, temp_dir]) # TODO(maruel): Use a subdirectory per user? current_time = str(datetime.datetime.now()).replace(':', '.') @@ -391,6 +386,11 @@ def TryChange(argv, file_list, swallow_exception, prog=None): + default_settings = GetTryServerSettings() + transport_functions = { 'http': _SendChangeHTTP, 'svn': _SendChangeSVN } + default_transport = transport_functions.get( + default_settings.get('default_transport')) + # Parse argv parser = optparse.OptionParser(usage=USAGE, version=__version__, @@ -446,30 +446,43 @@ def TryChange(argv, parser.add_option_group(group) group = optparse.OptionGroup(parser, "Access the try server by HTTP") - group.add_option("--use_http", action="store_const", const=_SendChangeHTTP, - dest="send_patch", default=_SendChangeHTTP, + group.add_option("--use_http", + action="store_const", + const=_SendChangeHTTP, + dest="send_patch", + default=default_transport, help="Use HTTP to talk to the try server [default]") group.add_option("--host", + default=default_settings['http_host'], help="Host address") group.add_option("--port", + default=default_settings['http_port'], help="HTTP port") group.add_option("--proxy", help="HTTP proxy") parser.add_option_group(group) group = optparse.OptionGroup(parser, "Access the try server with SVN") - group.add_option("--use_svn", action="store_const", const=_SendChangeSVN, + group.add_option("--use_svn", + action="store_const", + const=_SendChangeSVN, dest="send_patch", help="Use SVN to talk to the try server") - group.add_option("--svn_repo", metavar="SVN_URL", + group.add_option("--svn_repo", + metavar="SVN_URL", + default=default_settings['svn_repo'], help="SVN url to use to write the changes in; --use_svn is " "implied when using --svn_repo") parser.add_option_group(group) options, args = parser.parse_args(argv) - # Switch the default accordingly. - if options.svn_repo: - options.send_patch = _SendChangeSVN + + # Switch the default accordingly if there was no default send_patch. + if not options.send_patch: + if options.http_port and options.http_host: + options.send_patch = _SendChangeHTTP + elif options.svn_repo: + options.send_patch = _SendChangeSVN if len(args) == 1 and args[0] == 'help': parser.print_help()