From 5e879a426d77db166fd317ddfc50e62adb94d848 Mon Sep 17 00:00:00 2001 From: "hinoka@chromium.org" Date: Sat, 24 Jan 2015 00:55:46 +0000 Subject: [PATCH] Hook sys.stdio directly to the gsutil subprocess for the gsutil call So that gsutil.py config works. I would've preferred the execv solution, but apparently that didn't work on Windows :( BUG=451551 Review URL: https://codereview.chromium.org/870093003 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@293790 0039d316-1c4b-4281-b951-d872f2087c98 --- gsutil.py | 36 ++++-------------------------------- tests/gsutil_test.py | 30 +++++++++++++++--------------- 2 files changed, 19 insertions(+), 47 deletions(-) diff --git a/gsutil.py b/gsutil.py index 6578d7844..f2f2b72ac 100755 --- a/gsutil.py +++ b/gsutil.py @@ -27,31 +27,10 @@ DEFAULT_FALLBACK_GSUTIL = os.path.join( THIS_DIR, 'third_party', 'gsutil', 'gsutil') -class SubprocessError(Exception): - def __init__(self, message=None, code=0): - super(SubprocessError, self).__init__(message) - self.code = code - - class InvalidGsutilError(Exception): pass -def call(args, verbose=True, **kwargs): - kwargs['stdout'] = subprocess.PIPE - kwargs['stderr'] = subprocess.STDOUT - proc = subprocess.Popen(args, **kwargs) - out = [] - for line in proc.stdout: - out.append(line) - if verbose: - sys.stdout.write(line) - code = proc.wait() - if code: - raise SubprocessError('%s failed with %s' % (args, code), code) - return ''.join(out) - - def download_gsutil(version, target_dir): """Downloads gsutil into the target_dir.""" filename = 'gsutil_%s.zip' % version @@ -90,12 +69,9 @@ def download_gsutil(version, target_dir): def check_gsutil(gsutil_bin): """Run gsutil version and make sure it runs.""" - try: - call([sys.executable, gsutil_bin, 'version'], verbose=False) - return True - except SubprocessError: - return False - + return subprocess.call( + [sys.executable, gsutil_bin, 'version'], + stdout=subprocess.PIPE, stderr=subprocess.STDOUT) == 0 def ensure_gsutil(version, target): bin_dir = os.path.join(target, 'gsutil_%s' % version) @@ -127,11 +103,7 @@ def run_gsutil(force_version, fallback, target, args): else: gsutil_bin = fallback cmd = [sys.executable, gsutil_bin] + args - try: - call(cmd) - except SubprocessError as e: - return e.code - return 0 + return subprocess.call(cmd) def parse_args(): diff --git a/tests/gsutil_test.py b/tests/gsutil_test.py index 30648279d..76570dd31 100755 --- a/tests/gsutil_test.py +++ b/tests/gsutil_test.py @@ -7,16 +7,17 @@ import __builtin__ -import unittest +import base64 import hashlib -import zipfile +import json +import os import shutil +import subprocess import sys -import base64 import tempfile -import json -import os +import unittest import urllib2 +import zipfile # Add depot_tools to path @@ -62,8 +63,6 @@ class FakeCall(object): message = 'Expected:\n args: %s\n kwargs: %s\n' % (exp_args, exp_kwargs) message += 'Got:\n args: %s\n kwargs: %s\n' % (args, kwargs) raise TestError(message) - if isinstance(exp_returns, Exception): - raise exp_returns return exp_returns @@ -72,15 +71,15 @@ class GsutilUnitTests(unittest.TestCase): self.fake = FakeCall() self.tempdir = tempfile.mkdtemp() self.old_urlopen = getattr(urllib2, 'urlopen') - self.old_call = getattr(gsutil, 'call') + self.old_call = getattr(subprocess, 'call') setattr(urllib2, 'urlopen', self.fake) - setattr(gsutil, 'call', self.fake) + setattr(subprocess, 'call', self.fake) def tearDown(self): self.assertEqual(self.fake.expectations, []) shutil.rmtree(self.tempdir) setattr(urllib2, 'urlopen', self.old_urlopen) - setattr(gsutil, 'call', self.old_call) + setattr(subprocess, 'call', self.old_call) def test_download_gsutil(self): version = '4.2' @@ -126,8 +125,8 @@ class GsutilUnitTests(unittest.TestCase): os.makedirs(gsutil_dir) self.fake.add_expectation( - [sys.executable, gsutil_bin, 'version'], verbose=False, - _returns=gsutil.SubprocessError()) + [sys.executable, gsutil_bin, 'version'], stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, _returns=1) with open(gsutil_bin, 'w') as f: f.write('Foobar') @@ -140,8 +139,8 @@ class GsutilUnitTests(unittest.TestCase): with open(tempzip, 'rb') as f: self.fake.add_expectation(url, _returns=Buffer(f.read())) self.fake.add_expectation( - [sys.executable, gsutil_bin, 'version'], verbose=False, - _returns=gsutil.SubprocessError()) + [sys.executable, gsutil_bin, 'version'], stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, _returns=1) # This should delete the old bin and rewrite it with 'Fake gsutil' self.assertRaises( @@ -160,7 +159,8 @@ class GsutilUnitTests(unittest.TestCase): # Mock out call(). self.fake.add_expectation( - [sys.executable, gsutil_bin, 'version'], verbose=False, _returns=True) + [sys.executable, gsutil_bin, 'version'], + stdout=subprocess.PIPE, stderr=subprocess.STDOUT, _returns=0) with open(gsutil_bin, 'w') as f: f.write('Foobar')