releasetools: Set default stdout and stderr in common.Run().
stdout and stderr will default to subprocess.PIPE and subprocess.STDOUT
respectively (which is the expected behavior from most of the existing
callers), unless caller specifies any of them.
Test: `m dist`
Test: python -m unittest \
test_common \
test_add_img_to_target_files \
test_ota_from_target_files \
test_validate_target_files
Change-Id: I43b3f08edfa8a9bcfe54baf9848dc705c048e327
diff --git a/tools/releasetools/add_img_to_target_files.py b/tools/releasetools/add_img_to_target_files.py
index d7d1bc8..2fa5f52 100755
--- a/tools/releasetools/add_img_to_target_files.py
+++ b/tools/releasetools/add_img_to_target_files.py
@@ -49,7 +49,6 @@
import os
import shlex
import shutil
-import subprocess
import sys
import uuid
import zipfile
@@ -259,10 +258,11 @@
args = OPTIONS.info_dict.get("avb_dtbo_add_hash_footer_args")
if args and args.strip():
cmd.extend(shlex.split(args))
- p = common.Run(cmd, stdout=subprocess.PIPE)
- p.communicate()
- assert p.returncode == 0, \
- "avbtool add_hash_footer of %s failed" % (img.name,)
+ proc = common.Run(cmd)
+ output, _ = proc.communicate()
+ assert proc.returncode == 0, \
+ "Failed to call 'avbtool add_hash_footer' for {}:\n{}".format(
+ img.name, output)
img.Write()
return img.name
@@ -451,9 +451,9 @@
assert found, 'Failed to find {}'.format(image_path)
cmd.extend(split_args)
- p = common.Run(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
- stdoutdata, _ = p.communicate()
- assert p.returncode == 0, \
+ proc = common.Run(cmd)
+ stdoutdata, _ = proc.communicate()
+ assert proc.returncode == 0, \
"avbtool make_vbmeta_image failed:\n{}".format(stdoutdata)
img.Write()
@@ -481,9 +481,9 @@
if args:
cmd.extend(shlex.split(args))
- p = common.Run(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
- stdoutdata, _ = p.communicate()
- assert p.returncode == 0, \
+ proc = common.Run(cmd)
+ stdoutdata, _ = proc.communicate()
+ assert proc.returncode == 0, \
"bpttool make_table failed:\n{}".format(stdoutdata)
img.Write()
@@ -600,12 +600,10 @@
temp_care_map = common.MakeTempFile(prefix="caremap-", suffix=".pb")
care_map_gen_cmd = ["care_map_generator", temp_care_map_text, temp_care_map]
- p = common.Run(care_map_gen_cmd, stdout=subprocess.PIPE,
- stderr=subprocess.STDOUT)
- output, _ = p.communicate()
- if OPTIONS.verbose:
- print(output.rstrip())
- assert p.returncode == 0, "Failed to generate the care_map proto message."
+ proc = common.Run(care_map_gen_cmd)
+ output, _ = proc.communicate()
+ assert proc.returncode == 0, \
+ "Failed to generate the care_map proto message:\n{}".format(output)
care_map_path = "META/care_map.pb"
if output_zip and care_map_path not in output_zip.namelist():
@@ -656,9 +654,9 @@
cmd += shlex.split(OPTIONS.info_dict.get('lpmake_args').strip())
cmd += ['--output', img.name]
- p = common.Run(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
- stdoutdata, _ = p.communicate()
- assert p.returncode == 0, \
+ proc = common.Run(cmd)
+ stdoutdata, _ = proc.communicate()
+ assert proc.returncode == 0, \
"lpmake tool failed:\n{}".format(stdoutdata)
img.Write()
diff --git a/tools/releasetools/blockimgdiff.py b/tools/releasetools/blockimgdiff.py
index aeb4379..189dba2 100644
--- a/tools/releasetools/blockimgdiff.py
+++ b/tools/releasetools/blockimgdiff.py
@@ -23,7 +23,6 @@
import os
import os.path
import re
-import subprocess
import sys
import threading
from collections import deque, OrderedDict
@@ -43,11 +42,10 @@
# Don't dump the bsdiff/imgdiff commands, which are not useful for the case
# here, since they contain temp filenames only.
- p = common.Run(cmd, verbose=False, stdout=subprocess.PIPE,
- stderr=subprocess.STDOUT)
- output, _ = p.communicate()
+ proc = common.Run(cmd, verbose=False)
+ output, _ = proc.communicate()
- if p.returncode != 0:
+ if proc.returncode != 0:
raise ValueError(output)
with open(patchfile, 'rb') as f:
@@ -1494,9 +1492,9 @@
"--block-limit={}".format(max_blocks_per_transfer),
"--split-info=" + patch_info_file,
src_file, tgt_file, patch_file]
- p = common.Run(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
- imgdiff_output, _ = p.communicate()
- assert p.returncode == 0, \
+ proc = common.Run(cmd)
+ imgdiff_output, _ = proc.communicate()
+ assert proc.returncode == 0, \
"Failed to create imgdiff patch between {} and {}:\n{}".format(
src_name, tgt_name, imgdiff_output)
diff --git a/tools/releasetools/check_ota_package_signature.py b/tools/releasetools/check_ota_package_signature.py
index 3cac90a..a580709 100755
--- a/tools/releasetools/check_ota_package_signature.py
+++ b/tools/releasetools/check_ota_package_signature.py
@@ -24,7 +24,6 @@
import re
import subprocess
import sys
-import tempfile
import zipfile
from hashlib import sha1
@@ -165,11 +164,11 @@
cmd = ['delta_generator',
'--in_file=' + payload_file,
'--public_key=' + pubkey]
- proc = common.Run(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+ proc = common.Run(cmd)
stdoutdata, _ = proc.communicate()
assert proc.returncode == 0, \
- 'Failed to verify payload with delta_generator: %s\n%s' % (package,
- stdoutdata)
+ 'Failed to verify payload with delta_generator: {}\n{}'.format(
+ package, stdoutdata)
common.ZipClose(package_zip)
# Verified successfully upon reaching here.
diff --git a/tools/releasetools/common.py b/tools/releasetools/common.py
index 4e2346c..e381676 100644
--- a/tools/releasetools/common.py
+++ b/tools/releasetools/common.py
@@ -121,15 +121,26 @@
def Run(args, verbose=None, **kwargs):
- """Create and return a subprocess.Popen object.
+ """Creates and returns a subprocess.Popen object.
- Caller can specify if the command line should be printed. The global
- OPTIONS.verbose will be used if not specified.
+ Args:
+ args: The command represented as a list of strings.
+ verbose: Whether the commands should be shown (default to OPTIONS.verbose
+ if unspecified).
+ kwargs: Any additional args to be passed to subprocess.Popen(), such as env,
+ stdin, etc. stdout and stderr will default to subprocess.PIPE and
+ subprocess.STDOUT respectively unless caller specifies any of them.
+
+ Returns:
+ A subprocess.Popen object.
"""
if verbose is None:
verbose = OPTIONS.verbose
+ if 'stdout' not in kwargs and 'stderr' not in kwargs:
+ kwargs['stdout'] = subprocess.PIPE
+ kwargs['stderr'] = subprocess.STDOUT
if verbose:
- print(" running: ", " ".join(args))
+ print(" Running: \"{}\"".format(" ".join(args)))
return subprocess.Popen(args, **kwargs)
@@ -443,8 +454,7 @@
avbtool = os.getenv('AVBTOOL') or info_dict["avb_avbtool"]
pubkey_path = MakeTempFile(prefix="avb-", suffix=".pubkey")
proc = Run(
- [avbtool, "extract_public_key", "--key", key, "--output", pubkey_path],
- stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+ [avbtool, "extract_public_key", "--key", key, "--output", pubkey_path])
stdoutdata, _ = proc.communicate()
assert proc.returncode == 0, \
"Failed to extract pubkey for {}:\n{}".format(
@@ -551,9 +561,10 @@
fn = os.path.join(sourcedir, "recovery_dtbo")
cmd.extend(["--recovery_dtbo", fn])
- p = Run(cmd, stdout=subprocess.PIPE)
- p.communicate()
- assert p.returncode == 0, "mkbootimg of %s image failed" % (partition_name,)
+ proc = Run(cmd)
+ output, _ = proc.communicate()
+ assert proc.returncode == 0, \
+ "Failed to run mkbootimg of {}:\n{}".format(partition_name, output)
if (info_dict.get("boot_signer") == "true" and
info_dict.get("verity_key")):
@@ -568,9 +579,10 @@
cmd.extend([path, img.name,
info_dict["verity_key"] + ".pk8",
info_dict["verity_key"] + ".x509.pem", img.name])
- p = Run(cmd, stdout=subprocess.PIPE)
- p.communicate()
- assert p.returncode == 0, "boot_signer of %s image failed" % path
+ proc = Run(cmd)
+ output, _ = proc.communicate()
+ assert proc.returncode == 0, \
+ "Failed to run boot_signer of {} image:\n{}".format(path, output)
# Sign the image if vboot is non-empty.
elif info_dict.get("vboot"):
@@ -588,9 +600,10 @@
info_dict["vboot_subkey"] + ".vbprivk",
img_keyblock.name,
img.name]
- p = Run(cmd, stdout=subprocess.PIPE)
- p.communicate()
- assert p.returncode == 0, "vboot_signer of %s image failed" % path
+ proc = Run(cmd)
+ proc.communicate()
+ assert proc.returncode == 0, \
+ "Failed to run vboot_signer of {} image:\n{}".format(path, output)
# Clean up the temp files.
img_unsigned.close()
@@ -607,10 +620,11 @@
args = info_dict.get("avb_" + partition_name + "_add_hash_footer_args")
if args and args.strip():
cmd.extend(shlex.split(args))
- p = Run(cmd, stdout=subprocess.PIPE)
- p.communicate()
- assert p.returncode == 0, "avbtool add_hash_footer of %s failed" % (
- partition_name,)
+ proc = Run(cmd)
+ output, _ = proc.communicate()
+ assert proc.returncode == 0, \
+ "Failed to run 'avbtool add_hash_footer' of {}:\n{}".format(
+ partition_name, output)
img.seek(os.SEEK_SET, 0)
data = img.read()
@@ -682,9 +696,9 @@
cmd = ["unzip", "-o", "-q", filename, "-d", dirname]
if pattern is not None:
cmd.extend(pattern)
- p = Run(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
- stdoutdata, _ = p.communicate()
- if p.returncode != 0:
+ proc = Run(cmd)
+ stdoutdata, _ = proc.communicate()
+ if proc.returncode != 0:
raise ExternalError(
"Failed to unzip input target-files \"{}\":\n{}".format(
filename, stdoutdata))
@@ -926,15 +940,14 @@
key + OPTIONS.private_key_suffix,
input_name, output_name])
- p = Run(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE,
- stderr=subprocess.STDOUT)
+ proc = Run(cmd, stdin=subprocess.PIPE)
if password is not None:
password += "\n"
- stdoutdata, _ = p.communicate(password)
- if p.returncode != 0:
+ stdoutdata, _ = proc.communicate(password)
+ if proc.returncode != 0:
raise ExternalError(
"Failed to run signapk.jar: return code {}:\n{}".format(
- p.returncode, stdoutdata))
+ proc.returncode, stdoutdata))
def CheckSize(data, target, info_dict):
@@ -1267,8 +1280,7 @@
first_line = i + 4
f.close()
- p = Run([self.editor, "+%d" % (first_line,), self.pwfile])
- _, _ = p.communicate()
+ Run([self.editor, "+%d" % (first_line,), self.pwfile]).communicate()
return self.ReadFile()
@@ -1396,10 +1408,10 @@
if isinstance(entries, basestring):
entries = [entries]
cmd = ["zip", "-d", zip_filename] + entries
- proc = Run(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+ proc = Run(cmd)
stdoutdata, _ = proc.communicate()
- assert proc.returncode == 0, "Failed to delete %s:\n%s" % (entries,
- stdoutdata)
+ assert proc.returncode == 0, \
+ "Failed to delete {}:\n{}".format(entries, stdoutdata)
def ZipClose(zip_file):
@@ -1860,9 +1872,9 @@
'--output={}.new.dat.br'.format(self.path),
'{}.new.dat'.format(self.path)]
print("Compressing {}.new.dat with brotli".format(self.partition))
- p = Run(brotli_cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
- stdoutdata, _ = p.communicate()
- assert p.returncode == 0, \
+ proc = Run(brotli_cmd)
+ stdoutdata, _ = proc.communicate()
+ assert proc.returncode == 0, \
'Failed to compress {}.new.dat with brotli:\n{}'.format(
self.partition, stdoutdata)
diff --git a/tools/releasetools/ota_from_target_files.py b/tools/releasetools/ota_from_target_files.py
index 755eda9..7ea53f8 100755
--- a/tools/releasetools/ota_from_target_files.py
+++ b/tools/releasetools/ota_from_target_files.py
@@ -394,8 +394,7 @@
signing_key = common.MakeTempFile(prefix="key-", suffix=".key")
cmd.extend(["-out", signing_key])
- get_signing_key = common.Run(cmd, verbose=False, stdout=subprocess.PIPE,
- stderr=subprocess.STDOUT)
+ 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)
@@ -411,7 +410,7 @@
"""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, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+ signing = common.Run(cmd)
stdoutdata, _ = signing.communicate()
assert signing.returncode == 0, \
"Failed to sign the input file: {}".format(stdoutdata)
diff --git a/tools/releasetools/test_add_img_to_target_files.py b/tools/releasetools/test_add_img_to_target_files.py
index a73746e..cc7b887 100644
--- a/tools/releasetools/test_add_img_to_target_files.py
+++ b/tools/releasetools/test_add_img_to_target_files.py
@@ -16,7 +16,6 @@
import os
import os.path
-import subprocess
import unittest
import zipfile
@@ -45,9 +44,11 @@
# Calls an external binary to convert the proto message.
cmd = ["care_map_generator", "--parse_proto", file_name, text_file]
- p = common.Run(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
- p.communicate()
- self.assertEqual(0, p.returncode)
+ proc = common.Run(cmd)
+ output, _ = proc.communicate()
+ self.assertEqual(
+ 0, proc.returncode,
+ "Failed to run care_map_generator:\n{}".format(output))
with open(text_file, 'r') as verify_fp:
plain_text = verify_fp.read()
diff --git a/tools/releasetools/test_ota_from_target_files.py b/tools/releasetools/test_ota_from_target_files.py
index 1d8a786..29e0d83 100644
--- a/tools/releasetools/test_ota_from_target_files.py
+++ b/tools/releasetools/test_ota_from_target_files.py
@@ -17,7 +17,6 @@
import copy
import os
import os.path
-import subprocess
import unittest
import zipfile
@@ -1024,11 +1023,11 @@
'--signature_size', str(self.SIGNATURE_SIZE),
'--metadata_hash_file', metadata_sig_file,
'--payload_hash_file', payload_sig_file]
- proc = common.Run(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+ proc = common.Run(cmd)
stdoutdata, _ = proc.communicate()
self.assertEqual(
0, proc.returncode,
- 'Failed to run brillo_update_payload: {}'.format(stdoutdata))
+ 'Failed to run brillo_update_payload:\n{}'.format(stdoutdata))
signed_metadata_sig_file = payload_signer.Sign(metadata_sig_file)
diff --git a/tools/releasetools/test_validate_target_files.py b/tools/releasetools/test_validate_target_files.py
index 0aaf069..ecb7fde 100644
--- a/tools/releasetools/test_validate_target_files.py
+++ b/tools/releasetools/test_validate_target_files.py
@@ -21,7 +21,6 @@
import os
import os.path
import shutil
-import subprocess
import unittest
import build_image
@@ -44,7 +43,7 @@
kernel_fp.write(os.urandom(10))
cmd = ['mkbootimg', '--kernel', kernel, '-o', output_file]
- proc = common.Run(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+ proc = common.Run(cmd)
stdoutdata, _ = proc.communicate()
self.assertEqual(
0, proc.returncode,
@@ -53,7 +52,7 @@
cmd = ['boot_signer', '/boot', output_file,
os.path.join(self.testdata_dir, 'testkey.pk8'),
os.path.join(self.testdata_dir, 'testkey.x509.pem'), output_file]
- proc = common.Run(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+ proc = common.Run(cmd)
stdoutdata, _ = proc.communicate()
self.assertEqual(
0, proc.returncode,
@@ -123,7 +122,7 @@
system_root = common.MakeTempDir()
cmd = ['mkuserimg_mke2fs', '-s', system_root, output_file, 'ext4',
'/system', str(image_size), '-j', '0']
- proc = common.Run(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+ proc = common.Run(cmd)
stdoutdata, _ = proc.communicate()
self.assertEqual(
0, proc.returncode,
diff --git a/tools/releasetools/validate_target_files.py b/tools/releasetools/validate_target_files.py
index 09f800f..1cc4a60 100755
--- a/tools/releasetools/validate_target_files.py
+++ b/tools/releasetools/validate_target_files.py
@@ -35,7 +35,6 @@
import logging
import os.path
import re
-import subprocess
import zipfile
import common
@@ -256,7 +255,7 @@
continue
cmd = ['boot_signer', '-verify', image_path, '-certificate', verity_key]
- proc = common.Run(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+ proc = common.Run(cmd)
stdoutdata, _ = proc.communicate()
assert proc.returncode == 0, \
'Failed to verify {} with boot_signer:\n{}'.format(image, stdoutdata)
@@ -299,7 +298,7 @@
continue
cmd = ['verity_verifier', image_path, '-mincrypt', verity_key_mincrypt]
- proc = common.Run(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+ proc = common.Run(cmd)
stdoutdata, _ = proc.communicate()
assert proc.returncode == 0, \
'Failed to verify {} with verity_verifier (key: {}):\n{}'.format(
@@ -328,7 +327,7 @@
partition, info_dict, options[key_name])
cmd.extend(["--expected_chain_partition", chained_partition_arg])
- proc = common.Run(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+ proc = common.Run(cmd)
stdoutdata, _ = proc.communicate()
assert proc.returncode == 0, \
'Failed to verify {} with verity_verifier (key: {}):\n{}'.format(