diff --git a/tests/trychange_unittest.py b/tests/trychange_unittest.py index 8690aae44..27f8db611 100644 --- a/tests/trychange_unittest.py +++ b/tests/trychange_unittest.py @@ -5,6 +5,7 @@ """Unit tests for trychange.py.""" +import os import unittest # Local imports @@ -22,19 +23,18 @@ class TryChangeUnittest(TryChangeTestsBase): """General trychange.py tests.""" def testMembersChanged(self): members = [ - 'EscapeDot', 'GIT', 'GetSourceRoot', - 'GetTryServerSettings', 'GuessVCS', + 'EscapeDot', 'ExecuteTryServerScript', 'GIT', 'GetSourceRoot', 'GuessVCS', 'HELP_STRING', 'InvalidScript', 'NoTryServerAccess', 'PathDifference', - 'RunCommand', 'SCM', 'SVN', 'TryChange', 'USAGE', + 'RunCommand', 'SCM', 'SCRIPT_PATH', 'SVN', 'TryChange', 'USAGE', 'datetime', 'gcl', 'gclient', 'getpass', 'logging', 'optparse', 'os', - 'shutil', 'socket', 'sys', 'tempfile', 'traceback', 'upload', 'urllib', + 'shutil', 'sys', 'tempfile', 'traceback', 'urllib', ] # If this test fails, you should add the relevant test. self.compareMembers(trychange, members) class SVNUnittest(TryChangeTestsBase): - """trychange.SVN tests.""" + """General trychange.py tests.""" def testMembersChanged(self): members = [ 'GenerateDiff', 'ProcessOptions', 'options' @@ -43,8 +43,8 @@ class SVNUnittest(TryChangeTestsBase): self.compareMembers(trychange.SVN(None), members) -class GITUnittest(TryChangeTestsBase): - """trychange.GIT tests.""" +class TryChangeUnittest(TryChangeTestsBase): + """General trychange.py tests.""" def testMembersChanged(self): members = [ 'GenerateDiff', 'GetEmail', 'GetPatchName', 'ProcessOptions', 'options' diff --git a/trychange.py b/trychange.py index 7d79690ae..5c8c1c57b 100755 --- a/trychange.py +++ b/trychange.py @@ -14,7 +14,6 @@ import logging import optparse import os import shutil -import socket import sys import tempfile import traceback @@ -29,6 +28,7 @@ __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,29 +75,27 @@ 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 GetTryServerSettings(): - """Grab try server settings local to the repository.""" - def _SafeResolve(host): +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): try: - 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 + 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 def EscapeDot(name): @@ -258,12 +256,18 @@ def _ParseSendChangeOptions(options): def _SendChangeHTTP(options): """Send a change to the try server using the HTTP protocol.""" + script_locals = ExecuteTryServerScript() + if not options.host: - raise NoTryServerAccess('Please use the --host option to specify the try ' - 'server host to connect to.') + 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.') if not options.port: - raise NoTryServerAccess('Please use the --port option to specify the try ' - 'server port to connect to.') + 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.') values = _ParseSendChangeOptions(options) values['patch'] = options.diff @@ -294,9 +298,12 @@ 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: - raise NoTryServerAccess('Please use the --svn_repo option to specify the' - ' try server svn repository to connect to.') + 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.') values = _ParseSendChangeOptions(options) description = '' @@ -308,9 +315,7 @@ def _SendChangeSVN(options): temp_file = tempfile.NamedTemporaryFile() temp_file_name = temp_file.name try: - # Don't use '--non-interactive', since we want it to prompt for - # crendentials if necessary. - RunCommand(['svn', 'checkout', '--depth', 'empty', + RunCommand(['svn', 'checkout', '--depth', 'empty', '--non-interactive', options.svn_repo, temp_dir]) # TODO(maruel): Use a subdirectory per user? current_time = str(datetime.datetime.now()).replace(':', '.') @@ -386,11 +391,6 @@ 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,43 +446,30 @@ 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=default_transport, + group.add_option("--use_http", action="store_const", const=_SendChangeHTTP, + dest="send_patch", default=_SendChangeHTTP, 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", - default=default_settings['svn_repo'], + group.add_option("--svn_repo", metavar="SVN_URL", 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 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 + # Switch the default accordingly. + if options.svn_repo: + options.send_patch = _SendChangeSVN if len(args) == 1 and args[0] == 'help': parser.print_help()