From 4cc9df660bde6afe2f2250092e107292daf46a72 Mon Sep 17 00:00:00 2001 From: Daniel Norman Date: Thu, 18 Jul 2019 10:11:07 -0700 Subject: [PATCH] Clean up merge_target_files.py. - Removes functions that can be replaced with one-line external methods - Moves read_config_list to common alongside similar methods LoadDictionaryFrom* - Runs pyformat on merge_target_files.py Bug: 137853921 Test: python -m unittest test_merge_target_files Test: Using merge_target_files.py to create a merged build, & booting. Change-Id: I833c1086db41e1374057cc7447fc50d1915734a7 --- tools/releasetools/common.py | 7 +- tools/releasetools/merge_target_files.py | 91 ++++--------------- tools/releasetools/test_common.py | 17 ++++ tools/releasetools/test_merge_target_files.py | 20 +--- 4 files changed, 43 insertions(+), 92 deletions(-) diff --git a/tools/releasetools/common.py b/tools/releasetools/common.py index 54e7a0de9..913601fdb 100644 --- a/tools/releasetools/common.py +++ b/tools/releasetools/common.py @@ -463,10 +463,13 @@ def LoadBuildProp(read_helper, prop_file): return LoadDictionaryFromLines(data.split("\n")) -def LoadDictionaryFromFile(file_path): +def LoadListFromFile(file_path): with open(file_path) as f: - lines = list(f.read().splitlines()) + return f.read().splitlines() + +def LoadDictionaryFromFile(file_path): + lines = LoadListFromFile(file_path) return LoadDictionaryFromLines(lines) diff --git a/tools/releasetools/merge_target_files.py b/tools/releasetools/merge_target_files.py index f73bae1cd..7343f389b 100755 --- a/tools/releasetools/merge_target_files.py +++ b/tools/releasetools/merge_target_files.py @@ -13,9 +13,8 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations under # the License. - -""" -This script merges two partial target files packages. +# +"""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 @@ -239,8 +238,7 @@ def extract_items(target_files, target_files_temp_dir, extract_item_list): # Filter the extract_item_list to remove any items that do not exist in the # zip file. Otherwise, the extraction step will fail. - with zipfile.ZipFile( - target_files, allowZip64=True) as target_files_zipfile: + with zipfile.ZipFile(target_files, allowZip64=True) as target_files_zipfile: target_files_namelist = target_files_zipfile.namelist() filtered_extract_item_list = [] @@ -282,21 +280,6 @@ def copy_items(from_dir, to_dir, patterns): shutil.copyfile(original_file_path, copied_file_path) -def read_config_list(config_file_path): - """Reads a config file into a list of strings. - - Expects the file to be newline-separated. - - Args: - config_file_path: The path to the config file to open and read. - - Returns: - The list of strings in the config file. - """ - with open(config_file_path) as config_file: - return config_file.read().splitlines() - - def validate_config_lists(framework_item_list, framework_misc_info_keys, vendor_item_list): """Performs validations on the merge config lists. @@ -748,14 +731,14 @@ def files_from_path(target_path, extra_args=None): 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, + 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): + framework_misc_info_keys, rebuild_recovery): """Merges two target files packages into one target files structure. Args: @@ -875,8 +858,7 @@ def generate_super_empty_image(target_dir, output_super_empty): """ # Create super_empty.img using the merged misc_info.txt. - misc_info_txt = os.path.join(target_dir, 'META', - 'misc_info.txt') + misc_info_txt = os.path.join(target_dir, 'META', 'misc_info.txt') use_dynamic_partitions = common.LoadDictionaryFromFile(misc_info_txt).get( 'use_dynamic_partitions') @@ -885,8 +867,7 @@ def generate_super_empty_image(target_dir, output_super_empty): raise ValueError( 'Building super_empty.img requires use_dynamic_partitions=true.') elif use_dynamic_partitions == 'true': - super_empty_img = os.path.join(target_dir, 'IMAGES', - 'super_empty.img') + super_empty_img = os.path.join(target_dir, 'IMAGES', 'super_empty.img') build_super_image_args = [ misc_info_txt, super_empty_img, @@ -898,21 +879,6 @@ def generate_super_empty_image(target_dir, output_super_empty): shutil.copyfile(super_empty_img, output_super_empty) -def create_img_archive(source_path, target_path): - """Creates IMG archive in target path from source package. - - Args: - source_path: Path of the source package to be packed. - target_path: Create IMG package from the source package. - """ - - img_from_target_files_args = [ - source_path, - target_path, - ] - img_from_target_files.main(img_from_target_files_args) - - def create_target_files_archive(output_file, source_dir, temp_dir): """Creates archive from target package. @@ -923,13 +889,12 @@ def create_target_files_archive(output_file, source_dir, temp_dir): """ output_target_files_list = os.path.join(temp_dir, 'output.list') output_zip = os.path.abspath(output_file) - output_target_files_meta_dir = os.path.join(source_dir, - 'META') + output_target_files_meta_dir = os.path.join(source_dir, 'META') meta_content = files_from_path(output_target_files_meta_dir) - other_content = files_from_path(source_dir, - ['-path', output_target_files_meta_dir, - '-prune', '-o', '-print']) + other_content = files_from_path( + source_dir, + ['-path', output_target_files_meta_dir, '-prune', '-o', '-print']) with open(output_target_files_list, 'w') as f: f.write(meta_content) @@ -953,20 +918,6 @@ def create_target_files_archive(output_file, source_dir, temp_dir): return output_zip -def create_ota_package(zip_package, output_ota): - """Creates OTA package from archived package. - - Args: - zip_package: The name of the zip archived package. - output_ota: The name of the output zip archive ota package. - """ - ota_from_target_files_args = [ - zip_package, - output_ota, - ] - ota_from_target_files.main(ota_from_target_files_args) - - def merge_target_files(temp_dir, framework_target_files, framework_item_list, framework_misc_info_keys, vendor_target_files, vendor_item_list, output_target_files, output_dir, @@ -1024,7 +975,7 @@ def merge_target_files(temp_dir, framework_target_files, framework_item_list, if output_img: # Create the IMG package from the merged target files (before zipping, in # order to avoid an unnecessary unzip and copy). - create_img_archive(output_target_files_temp_dir, output_img) + img_from_target_files.main([output_target_files_temp_dir, output_img]) # Finally, create the output target files zip archive and/or copy the # output items to the output target files directory. @@ -1042,7 +993,7 @@ def merge_target_files(temp_dir, framework_target_files, framework_item_list, # Create the OTA package from the merged target files package. if output_ota: - create_ota_package(output_zip, output_ota) + ota_from_target_files.main([output_zip, output_ota]) def call_func_with_temp_dir(func, keep_tmp): @@ -1095,10 +1046,8 @@ def main(): elif o == '--framework-item-list': OPTIONS.framework_item_list = a elif o == '--system-misc-info-keys': - logger.warning( - '--system-misc-info-keys has been renamed to ' - '--framework-misc-info-keys' - ) + logger.warning('--system-misc-info-keys has been renamed to ' + '--framework-misc-info-keys') OPTIONS.framework_misc_info_keys = a elif o == '--framework-misc-info-keys': OPTIONS.framework_misc_info_keys = a @@ -1167,23 +1116,23 @@ def main(): sys.exit(1) if OPTIONS.framework_item_list: - framework_item_list = read_config_list(OPTIONS.framework_item_list) + framework_item_list = common.LoadListFromFile(OPTIONS.framework_item_list) else: framework_item_list = DEFAULT_FRAMEWORK_ITEM_LIST if OPTIONS.framework_misc_info_keys: - framework_misc_info_keys = read_config_list( + framework_misc_info_keys = common.LoadListFromFile( OPTIONS.framework_misc_info_keys) else: framework_misc_info_keys = DEFAULT_FRAMEWORK_MISC_INFO_KEYS if OPTIONS.vendor_item_list: - vendor_item_list = read_config_list(OPTIONS.vendor_item_list) + vendor_item_list = common.LoadListFromFile(OPTIONS.vendor_item_list) else: vendor_item_list = DEFAULT_VENDOR_ITEM_LIST if OPTIONS.output_item_list: - output_item_list = read_config_list(OPTIONS.output_item_list) + output_item_list = common.LoadListFromFile(OPTIONS.output_item_list) else: output_item_list = None diff --git a/tools/releasetools/test_common.py b/tools/releasetools/test_common.py index c0ebd8932..3a2198ceb 100644 --- a/tools/releasetools/test_common.py +++ b/tools/releasetools/test_common.py @@ -912,6 +912,23 @@ class CommonUtilsTest(test_utils.ReleaseToolsTestCase): 'recovery_as_boot': 'true', } + def test_LoadListFromFile(self): + file_path = os.path.join(self.testdata_dir, + 'merge_config_framework_item_list') + contents = common.LoadListFromFile(file_path) + expected_contents = [ + 'META/apkcerts.txt', + 'META/filesystem_config.txt', + 'META/root_filesystem_config.txt', + 'META/system_manifest.xml', + 'META/system_matrix.xml', + 'META/update_engine_config.txt', + 'PRODUCT/*', + 'ROOT/*', + 'SYSTEM/*', + ] + self.assertEqual(sorted(contents), sorted(expected_contents)) + @staticmethod def _test_LoadInfoDict_createTargetFiles(info_dict, fstab_path): target_files = common.MakeTempFile(prefix='target_files-', suffix='.zip') diff --git a/tools/releasetools/test_merge_target_files.py b/tools/releasetools/test_merge_target_files.py index bca29e5a0..1b1c7259a 100644 --- a/tools/releasetools/test_merge_target_files.py +++ b/tools/releasetools/test_merge_target_files.py @@ -18,7 +18,7 @@ import os.path import common import test_utils -from merge_target_files import (read_config_list, validate_config_lists, +from merge_target_files import (validate_config_lists, DEFAULT_FRAMEWORK_ITEM_LIST, DEFAULT_VENDOR_ITEM_LIST, DEFAULT_FRAMEWORK_MISC_INFO_KEYS, copy_items, @@ -83,24 +83,6 @@ class MergeTargetFilesTest(test_utils.ReleaseToolsTestCase): self.assertEqual( os.readlink(os.path.join(output_dir, 'a_link.cpp')), 'a.cpp') - def test_read_config_list(self): - framework_item_list_file = os.path.join(self.testdata_dir, - 'merge_config_framework_item_list') - framework_item_list = read_config_list(framework_item_list_file) - expected_framework_item_list = [ - 'META/apkcerts.txt', - 'META/filesystem_config.txt', - 'META/root_filesystem_config.txt', - 'META/system_manifest.xml', - 'META/system_matrix.xml', - 'META/update_engine_config.txt', - 'PRODUCT/*', - 'ROOT/*', - 'SYSTEM/*', - ] - self.assertEqual(sorted(framework_item_list), - sorted(expected_framework_item_list)) - def test_validate_config_lists_ReturnsFalseIfMissingDefaultItem(self): framework_item_list = list(DEFAULT_FRAMEWORK_ITEM_LIST) framework_item_list.remove('SYSTEM/*')