diff --git a/tools/releasetools/build_image.py b/tools/releasetools/build_image.py index d5ab05525e..42f05a7aca 100755 --- a/tools/releasetools/build_image.py +++ b/tools/releasetools/build_image.py @@ -31,7 +31,6 @@ import os.path import re import shlex import shutil -import subprocess import sys import common @@ -52,54 +51,21 @@ class BuildImageError(Exception): Exception.__init__(self, message) -def RunCommand(cmd, verbose=None, env=None): - """Echo and run the given command. - - Args: - cmd: the command represented as a list of strings. - verbose: show commands being executed. - env: a dictionary of additional environment variables. - Returns: - A tuple of the output and the exit code. - """ - env_copy = None - if env is not None: - env_copy = os.environ.copy() - env_copy.update(env) - if verbose is None: - verbose = OPTIONS.verbose - if verbose: - print("Running: " + " ".join(cmd)) - p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, - env=env_copy) - output, _ = p.communicate() - - if verbose: - print(output.rstrip()) - return (output, p.returncode) - - def GetVerityFECSize(partition_size): cmd = ["fec", "-s", str(partition_size)] - output, exit_code = RunCommand(cmd, False) - if exit_code != 0: - raise BuildImageError("Failed to GetVerityFECSize:\n{}".format(output)) + output = common.RunAndCheckOutput(cmd, verbose=False) return int(output) def GetVerityTreeSize(partition_size): cmd = ["build_verity_tree", "-s", str(partition_size)] - output, exit_code = RunCommand(cmd, False) - if exit_code != 0: - raise BuildImageError("Failed to GetVerityTreeSize:\n{}".format(output)) + output = common.RunAndCheckOutput(cmd, verbose=False) return int(output) def GetVerityMetadataSize(partition_size): cmd = ["build_verity_metadata.py", "size", str(partition_size)] - output, exit_code = RunCommand(cmd, False) - if exit_code != 0: - raise BuildImageError("Failed to GetVerityMetadataSize:\n{}".format(output)) + output = common.RunAndCheckOutput(cmd, verbose=False) return int(output) @@ -125,10 +91,12 @@ def GetDiskUsage(path): Raises: BuildImageError: On error. """ - env = {"POSIXLY_CORRECT": "1"} + env_copy = os.environ.copy() + env_copy["POSIXLY_CORRECT"] = "1" cmd = ["du", "-s", path] - output, exit_code = RunCommand(cmd, verbose=False, env=env) - if exit_code != 0: + try: + output = common.RunAndCheckOutput(cmd, verbose=False, env=env_copy) + except common.ExternalError: raise BuildImageError("Failed to get disk usage:\n{}".format(output)) # POSIX du returns number of blocks with block size 512 return int(output.split()[0]) * 512 @@ -160,16 +128,13 @@ def AVBCalcMaxImageSize(avbtool, footer_type, partition_size, additional_args): The maximum image size. Raises: - BuildImageError: On error or getting invalid image size. + BuildImageError: On invalid image size. """ cmd = [avbtool, "add_%s_footer" % footer_type, "--partition_size", str(partition_size), "--calc_max_image_size"] cmd.extend(shlex.split(additional_args)) - output, exit_code = RunCommand(cmd) - if exit_code != 0: - raise BuildImageError( - "Failed to calculate max image size:\n{}".format(output)) + output = common.RunAndCheckOutput(cmd) image_size = int(output) if image_size <= 0: raise BuildImageError( @@ -250,9 +215,6 @@ def AVBAddFooter(image_path, avbtool, footer_type, partition_size, salt: The salt to use (a hexadecimal string) or None. additional_args: Additional arguments to pass to "avbtool add_hash_footer" or "avbtool add_hashtree_footer". - - Raises: - BuildImageError: On error. """ cmd = [avbtool, "add_%s_footer" % footer_type, "--partition_size", partition_size, @@ -266,10 +228,7 @@ def AVBAddFooter(image_path, avbtool, footer_type, partition_size, cmd.extend(shlex.split(additional_args)) - output, exit_code = RunCommand(cmd) - if exit_code != 0: - raise BuildImageError( - "Failed to add AVB footer:\n{}".format(output)) + common.RunAndCheckOutput(cmd) def AdjustPartitionSizeForVerity(partition_size, fec_supported): @@ -324,19 +283,13 @@ def BuildVerityFEC(sparse_image_path, verity_path, verity_fec_path, padding_size): cmd = ["fec", "-e", "-p", str(padding_size), sparse_image_path, verity_path, verity_fec_path] - output, exit_code = RunCommand(cmd) - if exit_code != 0: - raise BuildImageError( - "Failed to build FEC data:\n{}".format(output)) + common.RunAndCheckOutput(cmd) def BuildVerityTree(sparse_image_path, verity_image_path): cmd = ["build_verity_tree", "-A", FIXED_SALT, sparse_image_path, verity_image_path] - output, exit_code = RunCommand(cmd) - if exit_code != 0: - raise BuildImageError( - "Failed to build verity tree:\n{}".format(output)) + output = common.RunAndCheckOutput(cmd) root, salt = output.split() return root, salt @@ -350,10 +303,7 @@ def BuildVerityMetadata(image_size, verity_metadata_path, root_hash, salt, cmd.append("--signer_args=\"%s\"" % (' '.join(signer_args),)) if verity_disable: cmd.append("--verity_disable") - output, exit_code = RunCommand(cmd) - if exit_code != 0: - raise BuildImageError( - "Failed to build verity metadata:\n{}".format(output)) + common.RunAndCheckOutput(cmd) def Append2Simg(sparse_image_path, unsparse_image_path, error_message): @@ -367,9 +317,10 @@ def Append2Simg(sparse_image_path, unsparse_image_path, error_message): BuildImageError: On error. """ cmd = ["append2simg", sparse_image_path, unsparse_image_path] - output, exit_code = RunCommand(cmd) - if exit_code != 0: - raise BuildImageError("{}:\n{}".format(error_message, output)) + try: + common.RunAndCheckOutput(cmd) + except: + raise BuildImageError(error_message) def Append(target, file_to_append, error_message): @@ -413,12 +364,11 @@ def UnsparseImage(sparse_image_path, replace=True): else: return unsparse_image_path inflate_command = ["simg2img", sparse_image_path, unsparse_image_path] - inflate_output, exit_code = RunCommand(inflate_command) - if exit_code != 0: + try: + common.RunAndCheckOutput(inflate_command) + except: os.remove(unsparse_image_path) - raise BuildImageError( - "Error: '{}' failed with exit code {}:\n{}".format( - inflate_command, exit_code, inflate_output)) + raise return unsparse_image_path @@ -475,10 +425,7 @@ def MakeVerityEnabledImage(out_file, fec_supported, prop_dict): def ConvertBlockMapToBaseFs(block_map_file): base_fs_file = common.MakeTempFile(prefix="script_gen_", suffix=".base_fs") convert_command = ["blk_alloc_to_base_fs", block_map_file, base_fs_file] - output, exit_code = RunCommand(convert_command) - if exit_code != 0: - raise BuildImageError( - "Failed to call blk_alloc_to_base_fs:\n{}".format(output)) + common.RunAndCheckOutput(convert_command) return base_fs_file @@ -729,12 +676,15 @@ def BuildImage(in_dir, prop_dict, out_file, target_out=None): raise BuildImageError( "Error: unknown filesystem type: {}".format(fs_type)) - mkfs_output, exit_code = RunCommand(build_command) - if exit_code != 0: + try: + mkfs_output = common.RunAndCheckOutput(build_command) + except: try: du = GetDiskUsage(in_dir) du_str = "{} bytes ({} MB)".format(du, du // BYTES_IN_MB) - except BuildImageError as e: + # Suppress any errors from GetDiskUsage() to avoid hiding the real errors + # from common.RunAndCheckOutput(). + except Exception as e: # pylint: disable=broad-except print(e, file=sys.stderr) du_str = "unknown" print( @@ -750,10 +700,7 @@ def BuildImage(in_dir, prop_dict, out_file, target_out=None): int(prop_dict["image_size"]) // BYTES_IN_MB, int(prop_dict["partition_size"]), int(prop_dict["partition_size"]) // BYTES_IN_MB)) - - raise BuildImageError( - "Error: '{}' failed with exit code {}:\n{}".format( - build_command, exit_code, mkfs_output)) + raise # Check if there's enough headroom space available for ext4 image. if "partition_headroom" in prop_dict and fs_type.startswith("ext4"): @@ -792,15 +739,12 @@ def BuildImage(in_dir, prop_dict, out_file, target_out=None): # Run e2fsck on the inflated image file e2fsck_command = ["e2fsck", "-f", "-n", unsparse_image] # TODO(b/112062612): work around e2fsck failure with SANITIZE_HOST=address - env4e2fsck = {"ASAN_OPTIONS": "detect_odr_violation=0"} - e2fsck_output, exit_code = RunCommand(e2fsck_command, env=env4e2fsck) - - os.remove(unsparse_image) - - if exit_code != 0: - raise BuildImageError( - "Error: '{}' failed with exit code {}:\n{}".format( - e2fsck_command, exit_code, e2fsck_output)) + env4e2fsck = os.environ.copy() + env4e2fsck["ASAN_OPTIONS"] = "detect_odr_violation=0" + try: + common.RunAndCheckOutput(e2fsck_command, env=env4e2fsck) + finally: + os.remove(unsparse_image) def ImagePropFromGlobalDict(glob_dict, mount_point): diff --git a/tools/releasetools/common.py b/tools/releasetools/common.py index e38167671b..d1bfc8f2f3 100644 --- a/tools/releasetools/common.py +++ b/tools/releasetools/common.py @@ -37,6 +37,7 @@ from hashlib import sha1, sha256 import blockimgdiff import sparse_img + class Options(object): def __init__(self): platform_search_path = { @@ -72,16 +73,13 @@ class Options(object): OPTIONS = Options() - # Values for "certificate" in apkcerts that mean special things. SPECIAL_CERT_STRINGS = ("PRESIGNED", "EXTERNAL") - # The partitions allowed to be signed by AVB (Android verified boot 2.0). AVB_PARTITIONS = ('boot', 'recovery', 'system', 'vendor', 'product', 'product_services', 'dtbo', 'odm') - # Partitions that should have their care_map added to META/care_map.pb PARTITIONS_WITH_CARE_MAP = ('system', 'vendor', 'product', 'product_services', 'odm') @@ -144,6 +142,36 @@ def Run(args, verbose=None, **kwargs): return subprocess.Popen(args, **kwargs) +def RunAndCheckOutput(args, verbose=None, **kwargs): + """Runs the given command and returns the output. + + 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: + The output string. + + Raises: + ExternalError: On non-zero exit from the command. + """ + if verbose is None: + verbose = OPTIONS.verbose + proc = Run(args, verbose=verbose, **kwargs) + output, _ = proc.communicate() + if verbose: + print("{}".format(output.rstrip())) + if proc.returncode != 0: + raise ExternalError( + "Failed to run command '{}' (exit code {}):\n{}".format( + args, proc.returncode, output)) + return output + + def RoundUpTo4K(value): rounded_up = value + 4095 return rounded_up - (rounded_up % 4096) @@ -445,20 +473,13 @@ def GetAvbChainedPartitionArg(partition, info_dict, key=None): Returns: A string of form "partition:rollback_index_location:key" that can be used to build or verify vbmeta image. - - Raises: - AssertionError: When it fails to extract the public key with avbtool. """ if key is None: key = info_dict["avb_" + partition + "_key_path"] avbtool = os.getenv('AVBTOOL') or info_dict["avb_avbtool"] pubkey_path = MakeTempFile(prefix="avb-", suffix=".pubkey") - proc = Run( + RunAndCheckOutput( [avbtool, "extract_public_key", "--key", key, "--output", pubkey_path]) - stdoutdata, _ = proc.communicate() - assert proc.returncode == 0, \ - "Failed to extract pubkey for {}:\n{}".format( - partition, stdoutdata) rollback_index_location = info_dict[ "avb_" + partition + "_rollback_index_location"] @@ -561,10 +582,7 @@ def _BuildBootableImage(sourcedir, fs_config_file, info_dict=None, fn = os.path.join(sourcedir, "recovery_dtbo") cmd.extend(["--recovery_dtbo", fn]) - proc = Run(cmd) - output, _ = proc.communicate() - assert proc.returncode == 0, \ - "Failed to run mkbootimg of {}:\n{}".format(partition_name, output) + RunAndCheckOutput(cmd) if (info_dict.get("boot_signer") == "true" and info_dict.get("verity_key")): @@ -579,10 +597,7 @@ def _BuildBootableImage(sourcedir, fs_config_file, info_dict=None, cmd.extend([path, img.name, info_dict["verity_key"] + ".pk8", info_dict["verity_key"] + ".x509.pem", img.name]) - proc = Run(cmd) - output, _ = proc.communicate() - assert proc.returncode == 0, \ - "Failed to run boot_signer of {} image:\n{}".format(path, output) + RunAndCheckOutput(cmd) # Sign the image if vboot is non-empty. elif info_dict.get("vboot"): @@ -600,10 +615,7 @@ def _BuildBootableImage(sourcedir, fs_config_file, info_dict=None, info_dict["vboot_subkey"] + ".vbprivk", img_keyblock.name, img.name] - proc = Run(cmd) - proc.communicate() - assert proc.returncode == 0, \ - "Failed to run vboot_signer of {} image:\n{}".format(path, output) + RunAndCheckOutput(cmd) # Clean up the temp files. img_unsigned.close() @@ -620,11 +632,7 @@ def _BuildBootableImage(sourcedir, fs_config_file, info_dict=None, args = info_dict.get("avb_" + partition_name + "_add_hash_footer_args") if args and args.strip(): cmd.extend(shlex.split(args)) - proc = Run(cmd) - output, _ = proc.communicate() - assert proc.returncode == 0, \ - "Failed to run 'avbtool add_hash_footer' of {}:\n{}".format( - partition_name, output) + RunAndCheckOutput(cmd) img.seek(os.SEEK_SET, 0) data = img.read() @@ -696,12 +704,7 @@ def UnzipTemp(filename, pattern=None): cmd = ["unzip", "-o", "-q", filename, "-d", dirname] if pattern is not None: cmd.extend(pattern) - proc = Run(cmd) - stdoutdata, _ = proc.communicate() - if proc.returncode != 0: - raise ExternalError( - "Failed to unzip input target-files \"{}\":\n{}".format( - filename, stdoutdata)) + RunAndCheckOutput(cmd) tmp = MakeTempDir(prefix="targetfiles-") m = re.match(r"^(.*[.]zip)\+(.*[.]zip)$", filename, re.IGNORECASE) @@ -1280,7 +1283,7 @@ class PasswordManager(object): first_line = i + 4 f.close() - Run([self.editor, "+%d" % (first_line,), self.pwfile]).communicate() + RunAndCheckOutput([self.editor, "+%d" % (first_line,), self.pwfile]) return self.ReadFile() @@ -1408,10 +1411,7 @@ def ZipDelete(zip_filename, entries): if isinstance(entries, basestring): entries = [entries] cmd = ["zip", "-d", zip_filename] + entries - proc = Run(cmd) - stdoutdata, _ = proc.communicate() - assert proc.returncode == 0, \ - "Failed to delete {}:\n{}".format(entries, stdoutdata) + RunAndCheckOutput(cmd) def ZipClose(zip_file): @@ -1872,11 +1872,7 @@ class BlockDifference(object): '--output={}.new.dat.br'.format(self.path), '{}.new.dat'.format(self.path)] print("Compressing {}.new.dat with brotli".format(self.partition)) - proc = Run(brotli_cmd) - stdoutdata, _ = proc.communicate() - assert proc.returncode == 0, \ - 'Failed to compress {}.new.dat with brotli:\n{}'.format( - self.partition, stdoutdata) + RunAndCheckOutput(brotli_cmd) new_data_name = '{}.new.dat.br'.format(self.partition) ZipWrite(output_zip, diff --git a/tools/releasetools/test_build_image.py b/tools/releasetools/test_build_image.py index 94c31ee6db..a2df27886a 100644 --- a/tools/releasetools/test_build_image.py +++ b/tools/releasetools/test_build_image.py @@ -23,7 +23,7 @@ import unittest import common from build_image import ( AVBCalcMinPartitionSize, BLOCK_SIZE, BuildImageError, CheckHeadroom, - RunCommand, SetUpInDirAndFsConfig) + SetUpInDirAndFsConfig) class BuildImageTest(unittest.TestCase): @@ -91,8 +91,9 @@ class BuildImageTest(unittest.TestCase): output_image = common.MakeTempFile(suffix='.img') command = ['mkuserimg_mke2fs', input_dir, output_image, 'ext4', '/system', '409600', '-j', '0'] - ext4fs_output, exit_code = RunCommand(command) - self.assertEqual(0, exit_code) + proc = common.Run(command) + ext4fs_output, _ = proc.communicate() + self.assertEqual(0, proc.returncode) prop_dict = { 'fs_type' : 'ext4', diff --git a/tools/releasetools/test_common.py b/tools/releasetools/test_common.py index 5179900954..ec86eb2675 100644 --- a/tools/releasetools/test_common.py +++ b/tools/releasetools/test_common.py @@ -334,8 +334,8 @@ class CommonZipTest(unittest.TestCase): self.assertFalse('Test2' in entries) self.assertTrue('Test3' in entries) - self.assertRaises(AssertionError, common.ZipDelete, zip_file.name, - 'Test2') + self.assertRaises( + common.ExternalError, common.ZipDelete, zip_file.name, 'Test2') with zipfile.ZipFile(zip_file.name, 'r') as check_zip: entries = check_zip.namelist() self.assertTrue('Test1' in entries) @@ -782,7 +782,8 @@ class CommonUtilsTest(unittest.TestCase): 'avb_system_rollback_index_location': 2, } self.assertRaises( - AssertionError, common.GetAvbChainedPartitionArg, 'system', info_dict) + common.ExternalError, common.GetAvbChainedPartitionArg, 'system', + info_dict) INFO_DICT_DEFAULT = { 'recovery_api_version': 3,