fetch: Fix stderr handling for gsutil

Previously gsutil stderr was getting piped into stdout, which
yields bad results if there are non-fatal warnings in stderr.

Additionally, we should fail outright if gsutil fails (by adding
`check = True`) rather than fail later on when we try to sync to
a manifest that is in fact just a stderr dump.

BUG=none
TEST=manual runs with bad gs urls

Change-Id: Id71791d0c3f180bd0601ef2c783a8e8e4afa8f59
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/321935
Tested-by: Jack Neus <jackneus@google.com>
Reviewed-by: Mike Frysinger <vapier@google.com>
This commit is contained in:
Jack Neus 2021-10-25 22:38:44 +00:00
parent 282d0cae89
commit 198838599c
2 changed files with 8 additions and 4 deletions

View File

@ -18,7 +18,7 @@ import subprocess
import sys import sys
from urllib.parse import urlparse from urllib.parse import urlparse
def fetch_file(url): def fetch_file(url, verbose=False):
"""Fetch a file from the specified source using the appropriate protocol. """Fetch a file from the specified source using the appropriate protocol.
Returns: Returns:
@ -29,10 +29,14 @@ def fetch_file(url):
cmd = ['gsutil', 'cat', url] cmd = ['gsutil', 'cat', url]
try: try:
result = subprocess.run( result = subprocess.run(
cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
check=True)
if result.stderr and verbose:
print('warning: non-fatal error running "gsutil": %s' % result.stderr,
file=sys.stderr)
return result.stdout return result.stdout
except subprocess.CalledProcessError as e: except subprocess.CalledProcessError as e:
print('fatal: error running "gsutil": %s' % e.output, print('fatal: error running "gsutil": %s' % e.stderr,
file=sys.stderr) file=sys.stderr)
sys.exit(1) sys.exit(1)
if scheme == 'file': if scheme == 'file':

View File

@ -298,7 +298,7 @@ to update the working directory files.
if standalone_manifest: if standalone_manifest:
if is_new: if is_new:
manifest_name = 'default.xml' manifest_name = 'default.xml'
manifest_data = fetch.fetch_file(opt.manifest_url) manifest_data = fetch.fetch_file(opt.manifest_url, verbose=opt.verbose)
dest = os.path.join(m.worktree, manifest_name) dest = os.path.join(m.worktree, manifest_name)
os.makedirs(os.path.dirname(dest), exist_ok=True) os.makedirs(os.path.dirname(dest), exist_ok=True)
with open(dest, 'wb') as f: with open(dest, 'wb') as f: