diff --git a/gclient_utils.py b/gclient_utils.py index 733d43fca..97c8227c0 100644 --- a/gclient_utils.py +++ b/gclient_utils.py @@ -17,10 +17,13 @@ import time import xml.dom.minidom import xml.parsers.expat -import subprocess2 -# Keep an alias for now. -Popen = subprocess2.Popen +def hack_subprocess(): + """subprocess functions may throw exceptions when used in multiple threads. + + See http://bugs.python.org/issue1731717 for more information. + """ + subprocess._cleanup = lambda: None class Error(Exception): @@ -52,6 +55,32 @@ class CheckCallError(OSError, Error): return out +def Popen(args, **kwargs): + """Calls subprocess.Popen() with hacks to work around certain behaviors. + + Ensure English outpout for svn and make it work reliably on Windows. + """ + logging.debug(u'%s, cwd=%s' % (u' '.join(args), kwargs.get('cwd', ''))) + if not 'env' in kwargs: + # It's easier to parse the stdout if it is always in English. + kwargs['env'] = os.environ.copy() + kwargs['env']['LANGUAGE'] = 'en' + if not 'shell' in kwargs: + # *Sigh*: Windows needs shell=True, or else it won't search %PATH% for the + # executable, but shell=True makes subprocess on Linux fail when it's called + # with a list because it only tries to execute the first item in the list. + kwargs['shell'] = (sys.platform=='win32') + try: + return subprocess.Popen(args, **kwargs) + except OSError, e: + if e.errno == errno.EAGAIN and sys.platform == 'cygwin': + raise Error( + 'Visit ' + 'http://code.google.com/p/chromium/wiki/CygwinDllRemappingFailure to ' + 'learn how to fix this error; you need to rebase your cygwin dlls') + raise + + def CheckCall(command, print_error=True, **kwargs): """Similar subprocess.check_call() but redirects stdout and returns (stdout, stderr). @@ -537,6 +566,7 @@ class ExecutionQueue(object): def __init__(self, jobs, progress): """jobs specifies the number of concurrent tasks to allow. progress is a Progress instance.""" + hack_subprocess() # Set when a thread is done or a new item is enqueued. self.ready_cond = threading.Condition() # Maximum number of concurrent tasks. diff --git a/subprocess2.py b/subprocess2.py index 9aefeea45..d59035b48 100644 --- a/subprocess2.py +++ b/subprocess2.py @@ -7,7 +7,6 @@ In theory you shouldn't need anything else in subprocess, or this module failed. """ -import errno import logging import os import subprocess @@ -80,7 +79,7 @@ def get_english_env(env): def Popen(args, **kwargs): - """Wraps subprocess.Popen() with various workarounds. + """Wraps subprocess.Popen(). Forces English output since it's easier to parse the stdout if it is always in English. @@ -88,8 +87,7 @@ def Popen(args, **kwargs): Sets shell=True on windows by default. You can override this by forcing shell parameter to a value. - Popen() can throw OSError when cwd or args[0] doesn't exist. Translate - exceptions generated by cygwin when it fails trying to emulate fork(). + Popen() can throw OSError when cwd or args[0] doesn't exist. """ # Make sure we hack subprocess if necessary. hack_subprocess() @@ -108,20 +106,7 @@ def Popen(args, **kwargs): if kwargs.get('cwd', None): tmp_str += '; cwd=%s' % kwargs['cwd'] logging.debug(tmp_str) - try: - return subprocess.Popen(args, **kwargs) - except OSError, e: - if e.errno == errno.EAGAIN and sys.platform == 'cygwin': - # Convert fork() emulation failure into a CalledProcessError(). - raise CalledProcessError( - e.errno, - args, - kwargs.get('cwd'), - 'Visit ' - 'http://code.google.com/p/chromium/wiki/CygwinDllRemappingFailure to ' - 'learn how to fix this error; you need to rebase your cygwin dlls', - None) - raise + return subprocess.Popen(args, **kwargs) def call(args, timeout=None, **kwargs): diff --git a/tests/gclient_utils_test.py b/tests/gclient_utils_test.py index a5fe72c08..8027675b0 100755 --- a/tests/gclient_utils_test.py +++ b/tests/gclient_utils_test.py @@ -33,8 +33,8 @@ class GclientUtilsUnittest(GclientUtilBase): 'ParseXML', 'Popen', 'PrintableObject', 'RemoveDirectory', 'SoftClone', 'SplitUrlRevision', 'SyntaxErrorToError', 'WorkItem', - 'errno', 'logging', 'os', 'Queue', 're', 'rmtree', - 'stat', 'subprocess', 'subprocess2', 'sys','threading', 'time', 'xml', + 'errno', 'hack_subprocess', 'logging', 'os', 'Queue', 're', 'rmtree', + 'stat', 'subprocess', 'sys','threading', 'time', 'xml', ] # If this test fails, you should add the relevant test. self.compareMembers(gclient_utils, members)