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>

1 files changed, 7 insertions, 3 deletions

diff --git a/fetch.py b/fetch.py
index 91d40cda..d79f9479 100644
--- a/fetch.py
+++ b/fetch.py
@@ -18,7 +18,7 @@ import subprocess
 import sys
 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.
 
   Returns:
@@ -29,10 +29,14 @@ def fetch_file(url):
     cmd = ['gsutil', 'cat', url]
     try:
       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
     except subprocess.CalledProcessError as e:
-      print('fatal: error running "gsutil": %s' % e.output,
+      print('fatal: error running "gsutil": %s' % e.stderr,
             file=sys.stderr)
     sys.exit(1)
   if scheme == 'file':