From 99cc468c0468ca79af6a17fd5627601b08119f5b Mon Sep 17 00:00:00 2001 From: Josip Sokcevic Date: Tue, 17 Aug 2021 17:02:44 +0000 Subject: [PATCH] Revert "Update gsutil to use gsutil version 4.66, python3" This reverts commit e8ef6259b967f7d76d96fe3039dc7c9614a52d6d. Reason for revert: https://crbug.com/1240673 High rate of bot_update failures, lots of gsutil defunct processes. Original change's description: > Update gsutil to use gsutil version 4.66, python3 > > This is a reland of 457736028de6b719c0f1907a54321385cf9a9eb9 > with following changes: > * bump version from 4.61 to 4.66, which contains several bugfixes > > Original change's description: > > Reland "Reland "Update gsutil to use gsutil version 4.61, python3."" > > > > This is a reland of e53a593956446795a68b5314459e86de38caa761 > > > > Additional bug fixes: > > * handle race condition in gsutil when creating its directory > > * limit to one gsutil process on darwin due to bug in python3 > > > > Original change's description: > > > Reland "Update gsutil to use gsutil version 4.61, python3." > > > > > > This reverts commit af121aeec9a2e4436c7609a39fcb61131a815b83. > > > > > > Reason for revert: re-landing with a switch back to vpython to get the compiled C extension version of crcmod for performance. > > > > > > Original change's description: > > > > Revert "Update gsutil to use gsutil version 4.61, python3." > > > > > > > > This reverts commit f059ec936899adfd83f63fff46fdb61ae3a39537. > > > > > > > > Reason for revert: Reverting because we probably need to be using vpython and a compiled crcmod instead. See, e.g.,. b/188591640. > > > > > > > > Original change's description: > > > > > Update gsutil to use gsutil version 4.61, python3. > > > > > > > > > > This CL updates the gsutil.py wrapper to download and use > > > > > v4.61 of GCP's gsutil, which is Python3-compatible. > > > > > > > > > > v4.61 appears to be fully self-contained and have all of the > > > > > packages it needs vendored into it. So, there's no reason to > > > > > use vpython anymore, and this CL removes that. > > > > > > > > > > Also, this CL removes the 'fallback' option to gsutil and > > > > > the ability to force a version switch, as this should no > > > > > longer be necessary (it was added for a migration back in 2014 > > > > > but apparently this code was never removed afterwards). > > > > > > > > > > This CL also updates download_from_google_storage.py and > > > > > upload_to_google_storage.py to similarly not have the version flags > > > > > and to just use regular python3, not vpython3. > > > > > > > > > > Bug: 1184108 > > > > > Change-Id: I0d1a8351dba2d3ad1f927afa333fb10959f19443 > > > > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2898439 > > > > > Reviewed-by: Mike Frysinger > > > > > Reviewed-by: Josip Sokcevic > > > > > Reviewed-by: Robbie Iannucci > > > > > Commit-Queue: Dirk Pranke > > > > > > > > Bug: 1184108 > > > > Change-Id: I8e21a9a40d81e4e185642f866855b6838f80f1c2 > > > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2905904 > > > > Bot-Commit: Rubber Stamper > > > > Commit-Queue: Dirk Pranke > > > > > > Bug: 1184108 > > > Change-Id: I5d6d6d06842e08517488471c144972818fcf02ff > > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2907155 > > > Reviewed-by: Mike Frysinger > > > Reviewed-by: Josip Sokcevic > > > Commit-Queue: Dirk Pranke > > > > Bug: 1184108 > > Change-Id: Ibb5d886fd22e3553521ff8ad6e2b4435844ef972 > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2988716 > > Reviewed-by: Dirk Pranke > > Reviewed-by: Mike Frysinger > > Reviewed-by: Robbie Iannucci > > Commit-Queue: Josip Sokcevic > > Bug: 1184108 > Change-Id: I33787dc75f6e45d6b462706e934d7a2a37703fa7 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3088085 > Reviewed-by: Dirk Pranke > Commit-Queue: Josip Sokcevic Bug: 1184108, 1240673 Change-Id: I74c3243ea29b99476e09b6ddb49cb052812a1e3e No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3100348 Auto-Submit: Josip Sokcevic Commit-Queue: Rubber Stamper Bot-Commit: Rubber Stamper --- download_from_google_storage.py | 7 +- gsutil.py | 58 ++++++---------- gsutil.py.vpython3 | 19 ------ gsutil.vpython | 115 ++++++++++++++++++++++++++++++++ tests/gsutil_test.py | 6 +- upload_to_google_storage.py | 2 +- 6 files changed, 143 insertions(+), 64 deletions(-) delete mode 100644 gsutil.py.vpython3 create mode 100644 gsutil.vpython diff --git a/download_from_google_storage.py b/download_from_google_storage.py index 27a67c8de..84bc8e30d 100755 --- a/download_from_google_storage.py +++ b/download_from_google_storage.py @@ -79,11 +79,12 @@ class Gsutil(object): VPYTHON3 = ('vpython3.bat' if GetNormalizedPlatform() == 'win32' else 'vpython3') - def __init__(self, path, boto_path=None): + def __init__(self, path, boto_path=None, version='4.28'): if not os.path.exists(path): raise FileNotFoundError('GSUtil not found in %s' % path) self.path = path self.boto_path = boto_path + self.version = version def get_sub_env(self): env = os.environ.copy() @@ -100,12 +101,12 @@ class Gsutil(object): return env def call(self, *args): - cmd = [self.VPYTHON3, self.path] + cmd = [self.VPYTHON3, self.path, '--force-version', self.version] cmd.extend(args) return subprocess2.call(cmd, env=self.get_sub_env()) def check_call(self, *args): - cmd = [self.VPYTHON3, self.path] + cmd = [self.VPYTHON3, self.path, '--force-version', self.version] cmd.extend(args) ((out, err), code) = subprocess2.communicate( cmd, diff --git a/gsutil.py b/gsutil.py index 8749c615b..d5423ad7e 100755 --- a/gsutil.py +++ b/gsutil.py @@ -31,11 +31,11 @@ API_URL = 'https://www.googleapis.com/storage/v1/b/pub/o/' THIS_DIR = os.path.dirname(os.path.abspath(__file__)) DEFAULT_BIN_DIR = os.path.join(THIS_DIR, 'external_bin', 'gsutil') +DEFAULT_FALLBACK_GSUTIL = os.path.join( + THIS_DIR, 'third_party', 'gsutil', 'gsutil') IS_WINDOWS = os.name == 'nt' -VERSION = '4.66' - class InvalidGsutilError(Exception): pass @@ -98,20 +98,7 @@ def ensure_gsutil(version, target, clean): return gsutil_bin if not os.path.exists(target): - try: - os.makedirs(target) - except FileExistsError: - # Another process is prepping workspace, so let's check if gsutil_bin is - # present. If after several checks it's still not, continue with - # downloading gsutil. - delay = 2 # base delay, in seconds - for _ in range(3): # make N attempts - # sleep first as it's not expected to have file ready just yet. - time.sleep(delay) - delay *= 1.5 # next delay increased by that factor - if os.path.isfile(gsutil_bin): - return gsutil_bin - + os.makedirs(target) with temporary_directory(target) as instance_dir: # Clean up if we're redownloading a corrupted gsutil. cleanup_path = os.path.join(instance_dir, 'clean') @@ -142,18 +129,13 @@ def ensure_gsutil(version, target, clean): return gsutil_bin -def run_gsutil(target, args, clean=False): - gsutil_bin = ensure_gsutil(VERSION, target, clean) - args_opt = ['-o', 'GSUtil:software_update_check_period=0'] +def run_gsutil(force_version, fallback, target, args, clean=False): + if force_version: + gsutil_bin = ensure_gsutil(force_version, target, clean) + else: + gsutil_bin = fallback + disable_update = ['-o', 'GSUtil:software_update_check_period=0'] - if sys.platform == 'darwin': - # We are experiencing problems with multiprocessing on MacOS where gsutil.py - # may hang. - # This behavior is documented in gsutil codebase, and recommendation is to - # set GSUtil:parallel_process_count=1. - # https://github.com/GoogleCloudPlatform/gsutil/blob/06efc9dc23719fab4fd5fadb506d252bbd3fe0dd/gslib/command.py#L1331 - # https://github.com/GoogleCloudPlatform/gsutil/issues/1100 - args_opt.extend(['-o', 'GSUtil:parallel_process_count=1']) if sys.platform == 'cygwin': # This script requires Windows Python, so invoke with depot_tools' # Python. @@ -165,10 +147,15 @@ def run_gsutil(target, args, clean=False): sys.exit(subprocess.call(cmd)) assert sys.platform != 'cygwin' + # Run "gsutil" through "vpython". We need to do this because on GCE instances, + # expectations are made about Python having access to "google-compute-engine" + # and "boto" packages that are not met with non-system Python (e.g., bundles). cmd = [ - sys.executable, + 'vpython', + '-vpython-spec', os.path.join(THIS_DIR, 'gsutil.vpython'), + '--', gsutil_bin - ] + args_opt + args + ] + disable_update + args return subprocess.call(cmd, shell=IS_WINDOWS) @@ -176,19 +163,13 @@ def parse_args(): bin_dir = os.environ.get('DEPOT_TOOLS_GSUTIL_BIN_DIR', DEFAULT_BIN_DIR) parser = argparse.ArgumentParser() - + parser.add_argument('--force-version', default='4.30') parser.add_argument('--clean', action='store_true', help='Clear any existing gsutil package, forcing a new download.') + parser.add_argument('--fallback', default=DEFAULT_FALLBACK_GSUTIL) parser.add_argument('--target', default=bin_dir, help='The target directory to download/store a gsutil version in. ' '(default is %(default)s).') - - # These two args exist for backwards-compatibility but are no-ops. - parser.add_argument('--force-version', default=VERSION, - help='(deprecated, this flag has no effect)') - parser.add_argument('--fallback', - help='(deprecated, this flag has no effect)') - parser.add_argument('args', nargs=argparse.REMAINDER) args, extras = parser.parse_known_args() @@ -201,7 +182,8 @@ def parse_args(): def main(): args = parse_args() - return run_gsutil(args.target, args.args, clean=args.clean) + return run_gsutil(args.force_version, args.fallback, args.target, args.args, + clean=args.clean) if __name__ == '__main__': diff --git a/gsutil.py.vpython3 b/gsutil.py.vpython3 deleted file mode 100644 index 40ddc80ee..000000000 --- a/gsutil.py.vpython3 +++ /dev/null @@ -1,19 +0,0 @@ -# vpython VirtualEnv protobuf for "gsutil". -# -# See: -# https://chromium.googlesource.com/infra/luci/luci-go/+/HEAD/vpython/api/vpython/spec.proto -# -# This is a "vpython" VirtualEnv specification applied to invocations of "gsutil" -# by the bootstrap wrapper, "gsutil.py". It ensures that any Python distribution -# has the expected Python packages installed. -# -# This is specifically relevant on Google Compute Engine invocations of -# "gsutil", where a stock system-deployed file in "/etc/" explicitly specifies -# that the "google_compute_engine" and "boto" Python packages are available. - -python_version: "3.8" - -wheel: < - name: "infra/python/wheels/crcmod/${vpython_platform}" - version: "version:1.7" -> diff --git a/gsutil.vpython b/gsutil.vpython new file mode 100644 index 000000000..aa47e5918 --- /dev/null +++ b/gsutil.vpython @@ -0,0 +1,115 @@ +# vpython VirtualEnv protobuf for "gsutil". +# +# See: +# https://chromium.googlesource.com/infra/luci/luci-go/+/HEAD/vpython/api/vpython/spec.proto +# +# This is a "vpython" VirtualEnv specification applied to invocations of "gsutil" +# by the bootstrap wrapper, "gsutil.py". It ensures that any Python distribution +# has the expected Python packages installed. +# +# This is specifically relevant on Google Compute Engine invocations of +# "gsutil", where a stock system-deployed file in "/etc/" explicitly specifies +# that the "google_compute_engine" and "boto" Python packages are available. + +python_version: "2.7" + +wheel < + name: "infra/python/wheels/google_compute_engine-py2_py3" + version: "version:2.6.2" +> +wheel < + name: "infra/python/wheels/boto-py2_py3" + version: "version:2.48.0" +> + +# "gsutil" on non-GCE can require PyOpenSSL, which, in turn, requires +# "cryptography". + +wheel: < + name: "infra/python/wheels/pyopenssl-py2_py3" + version: "version:17.2.0" +> + +## +# BEGIN "cryptography" dependencies. +## + +wheel: < + name: "infra/python/wheels/cryptography/${vpython_platform}" + version: "version:2.9.2" +> + +wheel: < + name: "infra/python/wheels/appdirs-py2_py3" + version: "version:1.4.3" +> + +wheel: < + name: "infra/python/wheels/enum34-py2" + version: "version:1.1.6" +> + +wheel: < + name: "infra/python/wheels/cffi/${vpython_platform}" + version: "version:1.14.5" +> + +wheel: < + name: "infra/python/wheels/idna-py2_py3" + version: "version:2.5" +> + +wheel: < + name: "infra/python/wheels/ipaddress-py2" + version: "version:1.0.18" +> + +wheel: < + name: "infra/python/wheels/packaging-py2_py3" + version: "version:16.8" +> + +wheel: < + name: "infra/python/wheels/pyasn1-py2_py3" + version: "version:0.2.3" +> + +wheel: < + name: "infra/python/wheels/pycparser-py2_py3" + version: "version:2.17" +> + +wheel: < + name: "infra/python/wheels/pyparsing-py2_py3" + version: "version:2.2.0" +> + +wheel: < + name: "infra/python/wheels/setuptools-py2_py3" + version: "version:34.3.2" +> + +wheel: < + name: "infra/python/wheels/six-py2_py3" + version: "version:1.10.0" +> + +## +# END "cryptography" dependencies. +## + +wheel: < + name: "infra/python/wheels/crcmod/${vpython_platform}" + version: "version:1.7" + match_tag: < + abi: "cp27mu" + platform: "manylinux1_i686" + > + match_tag: < + abi: "cp27mu" + platform: "manylinux1_x86_64" + > + match_tag: < + platform: "macosx_10_6_intel" + > +> diff --git a/tests/gsutil_test.py b/tests/gsutil_test.py index d9b7bbbd5..cd8129104 100755 --- a/tests/gsutil_test.py +++ b/tests/gsutil_test.py @@ -72,7 +72,7 @@ class GsutilUnitTests(unittest.TestCase): setattr(subprocess, 'call', self.old_call) def test_download_gsutil(self): - version = gsutil.VERSION + version = '4.2' filename = 'gsutil_%s.zip' % version full_filename = os.path.join(self.tempdir, filename) fake_file = b'This is gsutil.zip' @@ -113,7 +113,7 @@ class GsutilUnitTests(unittest.TestCase): self.assertEqual(self.fake.expectations, []) def test_ensure_gsutil_full(self): - version = gsutil.VERSION + version = '4.2' gsutil_dir = os.path.join(self.tempdir, 'gsutil_%s' % version, 'gsutil') gsutil_bin = os.path.join(gsutil_dir, 'gsutil') gsutil_flag = os.path.join(gsutil_dir, 'install.flag') @@ -137,7 +137,7 @@ class GsutilUnitTests(unittest.TestCase): self.assertEqual(self.fake.expectations, []) def test_ensure_gsutil_short(self): - version = gsutil.VERSION + version = '4.2' gsutil_dir = os.path.join(self.tempdir, 'gsutil_%s' % version, 'gsutil') gsutil_bin = os.path.join(gsutil_dir, 'gsutil') gsutil_flag = os.path.join(gsutil_dir, 'install.flag') diff --git a/upload_to_google_storage.py b/upload_to_google_storage.py index 2ecdf4e33..332bb9a47 100755 --- a/upload_to_google_storage.py +++ b/upload_to_google_storage.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python3 +#!/usr/bin/env python # Copyright (c) 2012 The Chromium Authors. All rights reserved. # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file.