Merge "releasetools: Clean up the logging while calling external commands."

tools/releasetools/ota_from_target_files.py

@@ -169,7 +169,6 @@ import os.path
 import shlex
 import shutil
 import struct
-import subprocess
 import sys
 import tempfile
 import zipfile
@@ -393,11 +392,7 @@ class PayloadSigner(object):
       cmd.extend(["-passin", "pass:" + pw] if pw else ["-nocrypt"])
       signing_key = common.MakeTempFile(prefix="key-", suffix=".key")
       cmd.extend(["-out", signing_key])
-
+      common.RunAndCheckOutput(cmd, verbose=False)
-      get_signing_key = common.Run(cmd, verbose=False)
-      stdoutdata, _ = get_signing_key.communicate()
-      assert get_signing_key.returncode == 0, \
-          "Failed to get signing key: {}".format(stdoutdata)
 
       self.signer = "openssl"
       self.signer_args = ["pkeyutl", "-sign", "-inkey", signing_key,
@@ -410,10 +405,7 @@ class PayloadSigner(object):
     """Signs the given input file. Returns the output filename."""
     out_file = common.MakeTempFile(prefix="signed-", suffix=".bin")
     cmd = [self.signer] + self.signer_args + ['-in', in_file, '-out', out_file]
-    signing = common.Run(cmd)
+    common.RunAndCheckOutput(cmd)
-    stdoutdata, _ = signing.communicate()
-    assert signing.returncode == 0, \
-        "Failed to sign the input file: {}".format(stdoutdata)
     return out_file
 
 
@@ -431,8 +423,6 @@ class Payload(object):
     Args:
       secondary: Whether it's generating a secondary payload (default: False).
     """
-    # The place where the output from the subprocess should go.
-    self._log_file = sys.stdout if OPTIONS.verbose else subprocess.PIPE
     self.payload_file = None
     self.payload_properties = None
     self.secondary = secondary
@@ -457,10 +447,7 @@ class Payload(object):
     if source_file is not None:
       cmd.extend(["--source_image", source_file])
     cmd.extend(additional_args)
-    p = common.Run(cmd, stdout=self._log_file, stderr=subprocess.STDOUT)
+    common.RunAndCheckOutput(cmd)
-    stdoutdata, _ = p.communicate()
-    assert p.returncode == 0, \
-        "brillo_update_payload generate failed: {}".format(stdoutdata)
 
     self.payload_file = payload_file
     self.payload_properties = None
@@ -484,9 +471,7 @@ class Payload(object):
            "--signature_size", "256",
            "--metadata_hash_file", metadata_sig_file,
            "--payload_hash_file", payload_sig_file]
-    p1 = common.Run(cmd, stdout=self._log_file, stderr=subprocess.STDOUT)
+    common.RunAndCheckOutput(cmd)
-    p1.communicate()
-    assert p1.returncode == 0, "brillo_update_payload hash failed"
 
     # 2. Sign the hashes.
     signed_payload_sig_file = payload_signer.Sign(payload_sig_file)
@@ -501,9 +486,7 @@ class Payload(object):
            "--signature_size", "256",
            "--metadata_signature_file", signed_metadata_sig_file,
            "--payload_signature_file", signed_payload_sig_file]
-    p1 = common.Run(cmd, stdout=self._log_file, stderr=subprocess.STDOUT)
+    common.RunAndCheckOutput(cmd)
-    p1.communicate()
-    assert p1.returncode == 0, "brillo_update_payload sign failed"
 
     # 4. Dump the signed payload properties.
     properties_file = common.MakeTempFile(prefix="payload-properties-",
@@ -511,9 +494,7 @@ class Payload(object):
     cmd = ["brillo_update_payload", "properties",
            "--payload", signed_payload_file,
            "--properties_file", properties_file]
-    p1 = common.Run(cmd, stdout=self._log_file, stderr=subprocess.STDOUT)
+    common.RunAndCheckOutput(cmd)
-    p1.communicate()
-    assert p1.returncode == 0, "brillo_update_payload properties failed"
 
     if self.secondary:
       with open(properties_file, "a") as f:

tools/releasetools/test_ota_from_target_files.py

@@ -1268,7 +1268,7 @@ class PayloadTest(test_utils.ReleaseToolsTestCase):
     target_file = construct_target_files()
     common.ZipDelete(target_file, 'IMAGES/vendor.img')
     payload = Payload()
-    self.assertRaises(AssertionError, payload.Generate, target_file)
+    self.assertRaises(common.ExternalError, payload.Generate, target_file)
 
   def test_Sign_full(self):
     payload = self._create_payload_full()
@@ -1316,7 +1316,7 @@ class PayloadTest(test_utils.ReleaseToolsTestCase):
     payload = self._create_payload_full()
     payload_signer = PayloadSigner()
     payload_signer.signer_args.append('bad-option')
-    self.assertRaises(AssertionError, payload.Sign, payload_signer)
+    self.assertRaises(common.ExternalError, payload.Sign, payload_signer)
 
   def test_WriteToZip(self):
     payload = self._create_payload_full()