From dbbf5a39123848d5c562c7004a88c4304ecbcd56 Mon Sep 17 00:00:00 2001 From: Daniel Norman Date: Thu, 22 Oct 2020 16:03:32 -0700 Subject: [PATCH] Formatting and general cleanup of merge_target_files. Also adds a new validation that IMAGES/.img must come from the same build that provides /*. Bug: 171431774 Test: python3 -m unittest test_merge_target_files Test: Use merge_target_files to merge a build. Change-Id: Ia1f3f2e65a0ca90962216fb0c5cdd24c4c150cae --- tools/releasetools/merge_target_files.py | 146 +++++++----------- tools/releasetools/test_merge_target_files.py | 18 ++- 2 files changed, 67 insertions(+), 97 deletions(-) diff --git a/tools/releasetools/merge_target_files.py b/tools/releasetools/merge_target_files.py index 6f414a505..2da5cc08d 100755 --- a/tools/releasetools/merge_target_files.py +++ b/tools/releasetools/merge_target_files.py @@ -16,11 +16,15 @@ # """This script merges two partial target files packages. -One package contains framework files, and the other contains vendor files. -It produces a complete target files package that can be used to generate an -OTA package. +One input package contains framework files, and the other contains vendor files. -Usage: merge_target_files.py [args] +This script produces a complete, merged target files package: + - This package can be used to generate a flashable IMG package. + See --output-img. + - This package can be used to generate an OTA package. See --output-ota. + - The merged package is checked for compatibility between the two inputs. + +Usage: merge_target_files [args] --framework-target-files framework-target-files-zip-archive The input target files package containing framework bits. This is a zip @@ -155,16 +159,9 @@ DEFAULT_FRAMEWORK_ITEM_LIST = ( 'SYSTEM/*', ) -# FRAMEWORK_EXTRACT_SPECIAL_ITEM_LIST is a list of items to extract from the -# partial framework target files package that need some special processing, such -# as some sort of combination with items from the partial vendor target files -# package. - -FRAMEWORK_EXTRACT_SPECIAL_ITEM_LIST = ('META/*',) - # DEFAULT_FRAMEWORK_MISC_INFO_KEYS is a list of keys to obtain from the -# framework instance of META/misc_info.txt. The remaining keys from the -# vendor instance. +# framework instance of META/misc_info.txt. The remaining keys should come +# from the vendor instance. DEFAULT_FRAMEWORK_MISC_INFO_KEYS = ( 'avb_system_hashtree_enable', @@ -205,13 +202,6 @@ DEFAULT_VENDOR_ITEM_LIST = ( 'VENDOR/*', ) -# VENDOR_EXTRACT_SPECIAL_ITEM_LIST is a list of items to extract from the -# partial vendor target files package that need some special processing, such as -# some sort of combination with items from the partial framework target files -# package. - -VENDOR_EXTRACT_SPECIAL_ITEM_LIST = ('META/*',) - # The merge config lists should not attempt to extract items from both # builds for any of the following partitions. The partitions in # SINGLE_BUILD_PARTITIONS should come entirely from a single build (either @@ -320,8 +310,8 @@ def validate_config_lists(framework_item_list, framework_misc_info_keys, framework_item_list: The list of items to extract from the partial framework target files package as is. framework_misc_info_keys: A list of keys to obtain from the framework - instance of META/misc_info.txt. The remaining keys from the vendor - instance. + instance of META/misc_info.txt. The remaining keys should come from the + vendor instance. vendor_item_list: The list of items to extract from the partial vendor target files package as is. @@ -346,10 +336,15 @@ def validate_config_lists(framework_item_list, framework_misc_info_keys, 'this script.') has_error = True + # Check that partitions only come from one input. for partition in SINGLE_BUILD_PARTITIONS: - in_framework = any( - item.startswith(partition) for item in framework_item_list) - in_vendor = any(item.startswith(partition) for item in vendor_item_list) + image_path = 'IMAGES/{}.img'.format(partition.lower().replace('/', '')) + in_framework = ( + any(item.startswith(partition) for item in framework_item_list) or + image_path in framework_item_list) + in_vendor = ( + any(item.startswith(partition) for item in vendor_item_list) or + image_path in vendor_item_list) if in_framework and in_vendor: logger.error( 'Cannot extract items from %s for both the framework and vendor' @@ -375,8 +370,8 @@ def process_ab_partitions_txt(framework_target_files_temp_dir, framework directory and the vendor directory, placing the merged result in the output directory. The precondition in that the files are already extracted. The post condition is that the output META/ab_partitions.txt contains the - merged content. The format for each ab_partitions.txt a one partition name per - line. The output file contains the union of the parition names. + merged content. The format for each ab_partitions.txt is one partition name + per line. The output file contains the union of the partition names. Args: framework_target_files_temp_dir: The name of a directory containing the @@ -429,8 +424,8 @@ def process_misc_info_txt(framework_target_files_temp_dir, create the output target files package after all the special cases are processed. framework_misc_info_keys: A list of keys to obtain from the framework - instance of META/misc_info.txt. The remaining keys from the vendor - instance. + instance of META/misc_info.txt. The remaining keys should come from the + vendor instance. """ misc_info_path = ['META', 'misc_info.txt'] @@ -674,8 +669,8 @@ def process_special_cases(framework_target_files_temp_dir, create the output target files package after all the special cases are processed. framework_misc_info_keys: A list of keys to obtain from the framework - instance of META/misc_info.txt. The remaining keys from the vendor - instance. + instance of META/misc_info.txt. The remaining keys should come from the + vendor instance. framework_partition_set: Partitions that are considered framework partitions. Used to filter apexkeys.txt and apkcerts.txt. vendor_partition_set: Partitions that are considered vendor partitions. Used @@ -721,26 +716,6 @@ def process_special_cases(framework_target_files_temp_dir, file_name='apexkeys.txt') -def files_from_path(target_path, extra_args=None): - """Gets files under given path. - - Get (sub)files from given target path and return sorted list. - - Args: - target_path: Target path to get subfiles. - extra_args: List of extra argument for find command. Optional. - - Returns: - Sorted files and directories list. - """ - - find_command = ['find', target_path] + (extra_args or []) - find_process = common.Run(find_command, stdout=subprocess.PIPE, verbose=False) - return common.RunAndCheckOutput(['sort'], - stdin=find_process.stdout, - verbose=False) - - def create_merged_package(temp_dir, framework_target_files, framework_item_list, vendor_target_files, vendor_item_list, framework_misc_info_keys, rebuild_recovery): @@ -762,64 +737,42 @@ def create_merged_package(temp_dir, framework_target_files, framework_item_list, target files package as is, meaning these items will land in the output target files package exactly as they appear in the input partial vendor target files package. - framework_misc_info_keys: The list of keys to obtain from the framework - instance of META/misc_info.txt. The remaining keys from the vendor - instance. + framework_misc_info_keys: A list of keys to obtain from the framework + instance of META/misc_info.txt. The remaining keys should come from the + vendor instance. rebuild_recovery: If true, rebuild the recovery patch used by non-A/B devices and write it to the system image. Returns: Path to merged package under temp directory. """ + # Extract "as is" items from the input framework and vendor partial target + # files packages directly into the output temporary directory, since these items + # do not need special case processing. - # Create directory names that we'll use when we extract files from framework, - # and vendor, and for zipping the final output. - - framework_target_files_temp_dir = os.path.join(temp_dir, 'framework') - vendor_target_files_temp_dir = os.path.join(temp_dir, 'vendor') output_target_files_temp_dir = os.path.join(temp_dir, 'output') - - # Extract "as is" items from the input framework partial target files package. - # We extract them directly into the output temporary directory since the - # items do not need special case processing. - extract_items( target_files=framework_target_files, target_files_temp_dir=output_target_files_temp_dir, extract_item_list=framework_item_list) - - # Extract "as is" items from the input vendor partial target files package. We - # extract them directly into the output temporary directory since the items - # do not need special case processing. - extract_items( target_files=vendor_target_files, target_files_temp_dir=output_target_files_temp_dir, extract_item_list=vendor_item_list) - # Extract "special" items from the input framework partial target files - # package. We extract these items to different directory since they require - # special processing before they will end up in the output directory. - + # Perform special case processing on META/* items. + # After this function completes successfully, all the files we need to create + # the output target files package are in place. + framework_target_files_temp_dir = os.path.join(temp_dir, 'framework') + vendor_target_files_temp_dir = os.path.join(temp_dir, 'vendor') extract_items( target_files=framework_target_files, target_files_temp_dir=framework_target_files_temp_dir, - extract_item_list=FRAMEWORK_EXTRACT_SPECIAL_ITEM_LIST) - - # Extract "special" items from the input vendor partial target files package. - # We extract these items to different directory since they require special - # processing before they will end up in the output directory. - + extract_item_list=('META/*',)) extract_items( target_files=vendor_target_files, target_files_temp_dir=vendor_target_files_temp_dir, - extract_item_list=VENDOR_EXTRACT_SPECIAL_ITEM_LIST) - - # Now that the temporary directories contain all the extracted files, perform - # special case processing on any items that need it. After this function - # completes successfully, all the files we need to create the output target - # files package are in place. - + extract_item_list=('META/*',)) process_special_cases( framework_target_files_temp_dir=framework_target_files_temp_dir, vendor_target_files_temp_dir=vendor_target_files_temp_dir, @@ -845,8 +798,10 @@ def generate_images(target_files_dir, rebuild_recovery): # Regenerate IMAGES in the target directory. - add_img_args = ['--verbose'] - add_img_args.append('--add_missing') + add_img_args = [ + '--verbose', + '--add_missing', + ] # TODO(b/132730255): Remove this if statement. if rebuild_recovery: add_img_args.append('--rebuild_recovery') @@ -899,6 +854,15 @@ def create_target_files_archive(output_file, source_dir, temp_dir): output_zip = os.path.abspath(output_file) output_target_files_meta_dir = os.path.join(source_dir, 'META') + def files_from_path(target_path, extra_args=None): + """Gets files under the given path and return a sorted list.""" + find_command = ['find', target_path] + (extra_args or []) + find_process = common.Run( + find_command, stdout=subprocess.PIPE, verbose=False) + return common.RunAndCheckOutput(['sort'], + stdin=find_process.stdout, + verbose=False) + meta_content = files_from_path(output_target_files_meta_dir) other_content = files_from_path( source_dir, @@ -947,9 +911,9 @@ def merge_target_files(temp_dir, framework_target_files, framework_item_list, target files package as is, meaning these items will land in the output target files package exactly as they appear in the input partial framework target files package. - framework_misc_info_keys: The list of keys to obtain from the framework - instance of META/misc_info.txt. The remaining keys from the vendor - instance. + framework_misc_info_keys: A list of keys to obtain from the framework + instance of META/misc_info.txt. The remaining keys should come from the + vendor instance. vendor_target_files: The name of the zip archive containing the vendor partial target files package. vendor_item_list: The list of items to extract from the partial vendor diff --git a/tools/releasetools/test_merge_target_files.py b/tools/releasetools/test_merge_target_files.py index ff8593b69..7ea7f9678 100644 --- a/tools/releasetools/test_merge_target_files.py +++ b/tools/releasetools/test_merge_target_files.py @@ -117,6 +117,15 @@ class MergeTargetFilesTest(test_utils.ReleaseToolsTestCase): DEFAULT_FRAMEWORK_MISC_INFO_KEYS, vendor_item_list)) + def test_validate_config_lists_ReturnsFalseIfSharedExtractedPartitionImage( + self): + vendor_item_list = list(DEFAULT_VENDOR_ITEM_LIST) + vendor_item_list.append('IMAGES/system.img') + self.assertFalse( + validate_config_lists(DEFAULT_FRAMEWORK_ITEM_LIST, + DEFAULT_FRAMEWORK_MISC_INFO_KEYS, + vendor_item_list)) + def test_validate_config_lists_ReturnsFalseIfBadSystemMiscInfoKeys(self): for bad_key in ['dynamic_partition_list', 'super_partition_groups']: framework_misc_info_keys = list(DEFAULT_FRAMEWORK_MISC_INFO_KEYS) @@ -144,8 +153,7 @@ class MergeTargetFilesTest(test_utils.ReleaseToolsTestCase): process_apex_keys_apk_certs_common(framework_dir, vendor_dir, output_dir, set(['product', 'system', 'system_ext']), - set(['odm', 'vendor']), - 'apexkeys.txt') + set(['odm', 'vendor']), 'apexkeys.txt') merged_entries = [] merged_path = os.path.join(self.testdata_dir, 'apexkeys_merge.txt') @@ -180,8 +188,7 @@ class MergeTargetFilesTest(test_utils.ReleaseToolsTestCase): self.assertRaises(ValueError, process_apex_keys_apk_certs_common, framework_dir, conflict_dir, output_dir, set(['product', 'system', 'system_ext']), - set(['odm', 'vendor']), - 'apexkeys.txt') + set(['odm', 'vendor']), 'apexkeys.txt') def test_process_apex_keys_apk_certs_HandlesApkCertsSyntax(self): output_dir = common.MakeTempDir() @@ -201,8 +208,7 @@ class MergeTargetFilesTest(test_utils.ReleaseToolsTestCase): process_apex_keys_apk_certs_common(framework_dir, vendor_dir, output_dir, set(['product', 'system', 'system_ext']), - set(['odm', 'vendor']), - 'apkcerts.txt') + set(['odm', 'vendor']), 'apkcerts.txt') merged_entries = [] merged_path = os.path.join(self.testdata_dir, 'apkcerts_merge.txt')