From 242549409fc3efa1ec2b5090a4fae06f1d0057bb Mon Sep 17 00:00:00 2001 From: "maruel@chromium.org" Date: Wed, 30 Mar 2011 01:05:04 +0000 Subject: [PATCH] Removed gclient_utils.Popen() and use subprocess2's version instead. R=dpranke@chromium.org BUG= TEST= Review URL: http://codereview.chromium.org/6770028 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@79779 0039d316-1c4b-4281-b951-d872f2087c98 --- gclient_utils.py | 36 +++--------------------------------- subprocess2.py | 21 ++++++++++++++++++--- tests/gclient_utils_test.py | 4 ++-- 3 files changed, 23 insertions(+), 38 deletions(-) diff --git a/gclient_utils.py b/gclient_utils.py index 97c8227c0..733d43fca 100644 --- a/gclient_utils.py +++ b/gclient_utils.py @@ -17,13 +17,10 @@ import time import xml.dom.minidom import xml.parsers.expat +import subprocess2 -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 +# Keep an alias for now. +Popen = subprocess2.Popen class Error(Exception): @@ -55,32 +52,6 @@ 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). @@ -566,7 +537,6 @@ 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 d59035b48..9aefeea45 100644 --- a/subprocess2.py +++ b/subprocess2.py @@ -7,6 +7,7 @@ In theory you shouldn't need anything else in subprocess, or this module failed. """ +import errno import logging import os import subprocess @@ -79,7 +80,7 @@ def get_english_env(env): def Popen(args, **kwargs): - """Wraps subprocess.Popen(). + """Wraps subprocess.Popen() with various workarounds. Forces English output since it's easier to parse the stdout if it is always in English. @@ -87,7 +88,8 @@ 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. + Popen() can throw OSError when cwd or args[0] doesn't exist. Translate + exceptions generated by cygwin when it fails trying to emulate fork(). """ # Make sure we hack subprocess if necessary. hack_subprocess() @@ -106,7 +108,20 @@ def Popen(args, **kwargs): if kwargs.get('cwd', None): tmp_str += '; cwd=%s' % kwargs['cwd'] logging.debug(tmp_str) - return subprocess.Popen(args, **kwargs) + 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 def call(args, timeout=None, **kwargs): diff --git a/tests/gclient_utils_test.py b/tests/gclient_utils_test.py index 8027675b0..a5fe72c08 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', 'hack_subprocess', 'logging', 'os', 'Queue', 're', 'rmtree', - 'stat', 'subprocess', 'sys','threading', 'time', 'xml', + 'errno', 'logging', 'os', 'Queue', 're', 'rmtree', + 'stat', 'subprocess', 'subprocess2', 'sys','threading', 'time', 'xml', ] # If this test fails, you should add the relevant test. self.compareMembers(gclient_utils, members)