From 58ef297bc522c5fd0a1ae5310737c7ee14fb7561 Mon Sep 17 00:00:00 2001
From: "maruel@chromium.org"
 <maruel@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Fri, 1 Apr 2011 21:00:11 +0000
Subject: [PATCH] Revert r80216 "Reapply r79779: "Removed gclient_utils.Popen()
 and use subprocess2's ...""

Horked windows slaves this time.

R=dpranke@chromium.org
BUG=
TEST=

Review URL: http://codereview.chromium.org/6689023

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@80220 0039d316-1c4b-4281-b951-d872f2087c98
---
 gclient_utils.py            | 36 +++++++++++++++++++++++++++++++++---
 subprocess2.py              | 21 +++------------------
 tests/gclient_utils_test.py |  4 ++--
 3 files changed, 38 insertions(+), 23 deletions(-)

diff --git a/gclient_utils.py b/gclient_utils.py
index 733d43fca2..97c8227c09 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 d4fa5781d6..c9057ac6ae 100644
--- a/subprocess2.py
+++ b/subprocess2.py
@@ -8,7 +8,6 @@ In theory you shouldn't need anything else in subprocess, or this module failed.
 """
 
 from __future__ import with_statement
-import errno
 import logging
 import os
 import subprocess
@@ -125,7 +124,7 @@ def get_english_env(env):
 
 
 def Popen(args, **kwargs):
-  """Wraps subprocess.Popen() with various workarounds.
+  """Wraps subprocess.Popen().
 
   Returns a subprocess.Popen object.
 
@@ -135,8 +134,7 @@ def Popen(args, **kwargs):
     shell parameter to a value.
   - Adds support for VOID to not buffer when not needed.
 
-  Note: Popen() can throw OSError when cwd or args[0] doesn't exist. Translate
-  exceptions generated by cygwin when it fails trying to emulate fork().
+  Note: Popen() can throw OSError when cwd or args[0] doesn't exist.
   """
   # Make sure we hack subprocess if necessary.
   hack_subprocess()
@@ -162,20 +160,7 @@ def Popen(args, **kwargs):
     kwargs['stdout'] = open(os.devnull, 'w')
   if kwargs.get('stderr') in (VOID, os.devnull):
     kwargs['stderr'] = open(os.devnull, 'w')
-  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 63369b2dfd..b00db3c80e 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)