Don't use sys.exit() in build_utils.CheckCallDie().

Calling sys.exit() in a thread spawned by
multiprocessing.pool.ThreadPool causes an infinite hang that even SIGINT
can't fix. This is bad. Instead of calling sys.exit(), raise an
exception, crafted so that the important information (the failed
command's stdout and stderr) comes after the stacktrace and the copyable
command.

This also renames CheckCallDie() to CheckOutput() and changes it to
print stderr but not stdout by default. Suppressing stderr has hid bugs
in the past (e.g. aapt crunch failures), and hiding stdout is what we
usually want anyway.

R=cjhopman@chromium.org

Review URL: https://codereview.chromium.org/106923002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@239276 0039d316-1c4b-4281-b951-d872f2087c98

14 files changed, 54 insertions, 52 deletions

diff --git a/build/android/gyp/ant.py b/build/android/gyp/ant.py
index 45ed3ed..06b0447 100755
--- a/build/android/gyp/ant.py
+++ b/build/android/gyp/ant.py
@@ -13,12 +13,20 @@ output up until the BUILD SUCCESSFUL line.
 """
 
 import sys
+import traceback
 
 from util import build_utils
 
 
 def main(argv):
-  stdout = build_utils.CheckCallDie(['ant'] + argv[1:], suppress_output=True)
+  try:
+    stdout = build_utils.CheckOutput(['ant'] + argv[1:])
+  except build_utils.CalledProcessError as e:
+    traceback.print_exc()
+    if '-quiet' in e.args:
+      sys.stderr.write('Tip: run the ant command above without the -quiet flag '
+          'to see more details on the error\n')
+    sys.exit(1)
   stdout = stdout.strip().split('\n')
   for line in stdout:
     if line.strip() == 'BUILD SUCCESSFUL':
diff --git a/build/android/gyp/create_standalone_apk.py b/build/android/gyp/create_standalone_apk.py
index de541a6..1952091 100755
--- a/build/android/gyp/create_standalone_apk.py
+++ b/build/android/gyp/create_standalone_apk.py
@@ -23,10 +23,9 @@ def CreateStandaloneApk(options):
       intermediate_path = intermediate_file.name
       shutil.copy(options.input_apk_path, intermediate_path)
       apk_path_abs = os.path.abspath(intermediate_path)
-      build_utils.CheckCallDie(
+      build_utils.CheckOutput(
           ['zip', '-r', '-1', apk_path_abs, 'lib'],
-          cwd=options.libraries_top_dir,
-          suppress_output=True)
+          cwd=options.libraries_top_dir)
       shutil.copy(intermediate_path, options.output_apk_path)
 
   input_paths = [options.input_apk_path, options.libraries_top_dir]
diff --git a/build/android/gyp/dex.py b/build/android/gyp/dex.py
index f90de95..1e6ca8a 100755
--- a/build/android/gyp/dex.py
+++ b/build/android/gyp/dex.py
@@ -22,7 +22,7 @@ def DoDex(options, paths):
 
   record_path = '%s.md5.stamp' % options.dex_path
   md5_check.CallAndRecordIfStale(
-      lambda: build_utils.CheckCallDie(dex_cmd, suppress_output=True),
+      lambda: build_utils.CheckOutput(dex_cmd, print_stderr=False),
       record_path=record_path,
       input_paths=paths,
       input_strings=dex_cmd)
diff --git a/build/android/gyp/emma_instr.py b/build/android/gyp/emma_instr.py
index 43951d2..86ee177 100755
--- a/build/android/gyp/emma_instr.py
+++ b/build/android/gyp/emma_instr.py
@@ -161,7 +161,7 @@ def _RunInstrumentCommand(command, options, args, option_parser):
            '-d', temp_dir,
            '-out', coverage_file,
            '-m', 'fullcopy']
-    build_utils.CheckCallDie(cmd, suppress_output=True)
+    build_utils.CheckOutput(cmd)
 
     if command == 'instrument_jar':
       for jar in os.listdir(os.path.join(temp_dir, 'lib')):
diff --git a/build/android/gyp/finalize_apk.py b/build/android/gyp/finalize_apk.py
index 0b1d2c2..6e4685c 100755
--- a/build/android/gyp/finalize_apk.py
+++ b/build/android/gyp/finalize_apk.py
@@ -26,7 +26,7 @@ def SignApk(keystore_path, unsigned_path, signed_path):
       signed_path,
       'chromiumdebugkey',
     ]
-  build_utils.CheckCallDie(sign_cmd)
+  build_utils.CheckOutput(sign_cmd)
 
 
 def AlignApk(android_sdk_root, unaligned_path, final_path):
@@ -36,7 +36,7 @@ def AlignApk(android_sdk_root, unaligned_path, final_path):
       unaligned_path,
       final_path,
       ]
-  build_utils.CheckCallDie(align_cmd)
+  build_utils.CheckOutput(align_cmd)
 
 
 def main(argv):
diff --git a/build/android/gyp/gcc_preprocess.py b/build/android/gyp/gcc_preprocess.py
index d4e30fc..2519254 100755
--- a/build/android/gyp/gcc_preprocess.py
+++ b/build/android/gyp/gcc_preprocess.py
@@ -27,7 +27,7 @@ def DoGcc(options):
       options.template
       ])
 
-  build_utils.CheckCallDie(gcc_cmd)
+  build_utils.CheckOutput(gcc_cmd)
 
 
 def main(argv):
diff --git a/build/android/gyp/jar.py b/build/android/gyp/jar.py
index 6d4fe12..cf731cd 100755
--- a/build/android/gyp/jar.py
+++ b/build/android/gyp/jar.py
@@ -30,7 +30,7 @@ def DoJar(options):
 
   record_path = '%s.md5.stamp' % options.jar_path
   md5_check.CallAndRecordIfStale(
-      lambda: build_utils.CheckCallDie(jar_cmd, cwd=jar_cwd),
+      lambda: build_utils.CheckOutput(jar_cmd, cwd=jar_cwd),
       record_path=record_path,
       input_paths=class_files,
       input_strings=jar_cmd)
diff --git a/build/android/gyp/jar_toc.py b/build/android/gyp/jar_toc.py
index 54e90bc..c05a42a 100755
--- a/build/android/gyp/jar_toc.py
+++ b/build/android/gyp/jar_toc.py
@@ -44,7 +44,7 @@ def CallJavap(classpath, classes):
       '-verbose',
       '-classpath', classpath
       ] + classes
-  return build_utils.CheckCallDie(javap_cmd, suppress_output=True)
+  return build_utils.CheckOutput(javap_cmd)
 
 
 def ExtractToc(disassembled_classes):
diff --git a/build/android/gyp/javac.py b/build/android/gyp/javac.py
index c117004..22a4f97 100755
--- a/build/android/gyp/javac.py
+++ b/build/android/gyp/javac.py
@@ -59,8 +59,7 @@ def DoJavac(options):
     # not contain the corresponding old .class file after running this action.
     build_utils.DeleteDirectory(output_dir)
     build_utils.MakeDirectory(output_dir)
-    suppress_output = not options.chromium_code
-    build_utils.CheckCallDie(javac_cmd, suppress_output=suppress_output)
+    build_utils.CheckOutput(javac_cmd, print_stdout=options.chromium_code)
 
   record_path = '%s/javac.md5.stamp' % options.output_dir
   md5_check.CallAndRecordIfStale(
diff --git a/build/android/gyp/process_resources.py b/build/android/gyp/process_resources.py
index cd7fbab..bf13175 100755
--- a/build/android/gyp/process_resources.py
+++ b/build/android/gyp/process_resources.py
@@ -104,7 +104,7 @@ def main():
     package_command.append('--non-constant-id')
   if options.custom_package:
     package_command += ['--custom-package', options.custom_package]
-  build_utils.CheckCallDie(package_command)
+  build_utils.CheckOutput(package_command)
 
   # Crunch image resources. This shrinks png files and is necessary for 9-patch
   # images to display correctly.
@@ -113,7 +113,7 @@ def main():
               'crunch',
               '-S', options.crunch_input_dir,
               '-C', options.crunch_output_dir]
-  build_utils.CheckCallDie(aapt_cmd, suppress_output=True, fail_if_stderr=True)
+  build_utils.CheckOutput(aapt_cmd, fail_if_stderr=True)
 
   MoveImagesToNonMdpiFolders(options.crunch_output_dir)
 
diff --git a/build/android/gyp/proguard.py b/build/android/gyp/proguard.py
index 6268caf..41d4a69 100755
--- a/build/android/gyp/proguard.py
+++ b/build/android/gyp/proguard.py
@@ -26,7 +26,8 @@ def DoProguard(options):
                   '-outjars', outjars,
                   '-libraryjars', libraryjars,
                   '@' + options.proguard_config]
-  build_utils.CheckCallDie(proguard_cmd)
+  build_utils.CheckOutput(proguard_cmd, print_stdout=True)
+
 
 def main(argv):
   parser = optparse.OptionParser()
diff --git a/build/android/gyp/strip_library_for_device.py b/build/android/gyp/strip_library_for_device.py
index 05eacfe..e4f5312 100755
--- a/build/android/gyp/strip_library_for_device.py
+++ b/build/android/gyp/strip_library_for_device.py
@@ -17,7 +17,7 @@ def StripLibrary(android_strip, android_strip_args, library_path, output_path):
     strip_cmd = ([android_strip] +
                  android_strip_args +
                  ['-o', output_path, library_path])
-    build_utils.CheckCallDie(strip_cmd)
+    build_utils.CheckOutput(strip_cmd)
 
 
 
diff --git a/build/android/gyp/util/build_utils.py b/build/android/gyp/util/build_utils.py
index a58fe4a..0bf9482 100644
--- a/build/android/gyp/util/build_utils.py
+++ b/build/android/gyp/util/build_utils.py
@@ -78,49 +78,44 @@ def ReadJson(path):
     return json.load(jsonfile)
 
 
-# This can be used in most cases like subprocess.check_call. The output,
+class CalledProcessError(Exception):
+  """This exception is raised when the process run by CheckOutput
+  exits with a non-zero exit code."""
+
+  def __init__(self, cwd, args, output):
+    self.cwd = cwd
+    self.args = args
+    self.output = output
+
+  def __str__(self):
+    # A user should be able to simply copy and paste the command that failed
+    # into their shell.
+    copyable_command = '( cd {}; {} )'.format(os.path.abspath(self.cwd),
+        ' '.join(map(pipes.quote, self.args)))
+    return 'Command failed: {}\n{}'.format(copyable_command, self.output)
+
+
+# This can be used in most cases like subprocess.check_output(). The output,
 # particularly when the command fails, better highlights the command's failure.
-# This call will directly exit on a failure in the subprocess so that no python
-# stacktrace is printed after the output of the failed command (and will
-# instead print a python stack trace before the output of the failed command)
-def CheckCallDie(args, suppress_output=False, cwd=None, fail_if_stderr=False):
+# If the command fails, raises a build_utils.CalledProcessError.
+def CheckOutput(args, cwd=None, print_stdout=False, print_stderr=True,
+                fail_if_stderr=False):
   if not cwd:
     cwd = os.getcwd()
 
   child = subprocess.Popen(args,
       stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd=cwd)
-
   stdout, stderr = child.communicate()
 
-  returncode = child.returncode
-  if fail_if_stderr and stderr and returncode == 0:
-    returncode = 1
-
-  if returncode:
-    stacktrace = traceback.extract_stack()
-    print >> sys.stderr, ''.join(traceback.format_list(stacktrace))
-    # A user should be able to simply copy and paste the command that failed
-    # into their shell.
-    copyable_command = ' '.join(map(pipes.quote, args))
-    copyable_command = ('( cd ' + os.path.abspath(cwd) + '; '
-        + copyable_command + ' )')
-    print >> sys.stderr, 'Command failed:', copyable_command, '\n'
-
-    if stdout:
-      print stdout,
-    if stderr:
-      print >> sys.stderr, stderr,
+  if child.returncode or (stderr and fail_if_stderr):
+    raise CalledProcessError(cwd, args, stdout + stderr)
 
-    # Directly exit to avoid printing stacktrace.
-    sys.exit(returncode)
+  if print_stdout:
+    sys.stdout.write(stdout)
+  if print_stderr:
+    sys.stderr.write(stderr)
 
-  else:
-    if not suppress_output:
-      if stdout:
-        print stdout,
-      if stderr:
-        print >> sys.stderr, stderr,
-    return stdout + stderr
+  return stdout
 
 
 def GetModifiedTime(path):
@@ -141,7 +136,7 @@ def IsTimeStale(output, inputs):
 
 
 def IsDeviceReady():
-  device_state = CheckCallDie(['adb', 'get-state'], suppress_output=True)
+  device_state = CheckOutput(['adb', 'get-state'])
   return device_state.strip() == 'device'
 
 
diff --git a/build/android/gyp/write_ordered_libraries.py b/build/android/gyp/write_ordered_libraries.py
index 11f12e0..23becb0 100755
--- a/build/android/gyp/write_ordered_libraries.py
+++ b/build/android/gyp/write_ordered_libraries.py
@@ -47,7 +47,7 @@ def CallReadElf(library_or_executable):
   readelf_cmd = [_options.readelf,
                  '-d',
                  library_or_executable]
-  return build_utils.CheckCallDie(readelf_cmd, suppress_output=True)
+  return build_utils.CheckOutput(readelf_cmd)
 
 
 def GetDependencies(library_or_executable):