From 410ad8bb9203d20f9df2b1fa14031875fc56d9f6 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Fri, 24 Aug 2018 12:08:38 -0700 Subject: [PATCH] releasetools: common.LoadInfoDict() takes a bool parameter for repacking. It used to accept a non-None input_dir to indicate the need to re-generate images (called from add_img_to_target_files.py). During that flow, both of the two parameters redundantly refer to the same input dir. This CL replaces the second parameter with a bool value instead. Test: `python -m unittest test_common` Test: `m dist` with aosp_taimen-userdebug Test: `zip -d aosp_taimen-target_files-eng.zip IMAGES/\* && add_img_to_target_files.py -a aosp_taimen-target_files-eng.zip` Change-Id: I0a5a164366acb116407f94bb350872a3a0b207d1 --- tools/releasetools/add_img_to_target_files.py | 2 +- tools/releasetools/common.py | 71 +++++++++++++------ tools/releasetools/test_common.py | 35 +++++++++ 3 files changed, 86 insertions(+), 22 deletions(-) diff --git a/tools/releasetools/add_img_to_target_files.py b/tools/releasetools/add_img_to_target_files.py index e47c03888..a3dc6c324 100755 --- a/tools/releasetools/add_img_to_target_files.py +++ b/tools/releasetools/add_img_to_target_files.py @@ -664,7 +664,7 @@ def AddImagesToTargetFiles(filename): print("target_files appears to already contain images.") sys.exit(1) - OPTIONS.info_dict = common.LoadInfoDict(OPTIONS.input_tmp, OPTIONS.input_tmp) + OPTIONS.info_dict = common.LoadInfoDict(OPTIONS.input_tmp, repacking=True) has_recovery = OPTIONS.info_dict.get("no_recovery") != "true" diff --git a/tools/releasetools/common.py b/tools/releasetools/common.py index 227d7011a..8d51df6b2 100644 --- a/tools/releasetools/common.py +++ b/tools/releasetools/common.py @@ -149,9 +149,41 @@ def CloseInheritedPipes(): pass -def LoadInfoDict(input_file, input_dir=None): - """Read and parse the META/misc_info.txt key/value pairs from the - input target files and return a dict.""" +def LoadInfoDict(input_file, repacking=False): + """Loads the key/value pairs from the given input target_files. + + It reads `META/misc_info.txt` file in the target_files input, does sanity + checks and returns the parsed key/value pairs for to the given build. It's + usually called early when working on input target_files files, e.g. when + generating OTAs, or signing builds. Note that the function may be called + against an old target_files file (i.e. from past dessert releases). So the + property parsing needs to be backward compatible. + + In a `META/misc_info.txt`, a few properties are stored as links to the files + in the PRODUCT_OUT directory. It works fine with the build system. However, + they are no longer available when (re)generating images from target_files zip. + When `repacking` is True, redirect these properties to the actual files in the + unzipped directory. + + Args: + input_file: The input target_files file, which could be an open + zipfile.ZipFile instance, or a str for the dir that contains the files + unzipped from a target_files file. + repacking: Whether it's trying repack an target_files file after loading the + info dict (default: False). If so, it will rewrite a few loaded + properties (e.g. selinux_fc, root_dir) to point to the actual files in + target_files file. When doing repacking, `input_file` must be a dir. + + Returns: + A dict that contains the parsed key/value pairs. + + Raises: + AssertionError: On invalid input arguments. + ValueError: On malformed input values. + """ + if repacking: + assert isinstance(input_file, str), \ + "input_file must be a path str when doing repacking" def read_helper(fn): if isinstance(input_file, zipfile.ZipFile): @@ -168,36 +200,33 @@ def LoadInfoDict(input_file, input_dir=None): try: d = LoadDictionaryFromLines(read_helper("META/misc_info.txt").split("\n")) except KeyError: - raise ValueError("can't find META/misc_info.txt in input target-files") + raise ValueError("Failed to find META/misc_info.txt in input target-files") - assert "recovery_api_version" in d - assert "fstab_version" in d + if "recovery_api_version" not in d: + raise ValueError("Failed to find 'recovery_api_version'") + if "fstab_version" not in d: + raise ValueError("Failed to find 'fstab_version'") - # A few properties are stored as links to the files in the out/ directory. - # It works fine with the build system. However, they are no longer available - # when (re)generating from target_files zip. If input_dir is not None, we - # are doing repacking. Redirect those properties to the actual files in the - # unzipped directory. - if input_dir is not None: - # We carry a copy of file_contexts.bin under META/. If not available, - # search BOOT/RAMDISK/. Note that sometimes we may need a different file - # to build images than the one running on device, in that case, we must - # have the one for image generation copied to META/. + if repacking: + # We carry a copy of file_contexts.bin under META/. If not available, search + # BOOT/RAMDISK/. Note that sometimes we may need a different file to build + # images than the one running on device, in that case, we must have the one + # for image generation copied to META/. fc_basename = os.path.basename(d.get("selinux_fc", "file_contexts")) - fc_config = os.path.join(input_dir, "META", fc_basename) + fc_config = os.path.join(input_file, "META", fc_basename) assert os.path.exists(fc_config) d["selinux_fc"] = fc_config # Similarly we need to redirect "root_dir", and "root_fs_config". - d["root_dir"] = os.path.join(input_dir, "ROOT") + d["root_dir"] = os.path.join(input_file, "ROOT") d["root_fs_config"] = os.path.join( - input_dir, "META", "root_filesystem_config.txt") + input_file, "META", "root_filesystem_config.txt") # Redirect {system,vendor}_base_fs_file. if "system_base_fs_file" in d: basename = os.path.basename(d["system_base_fs_file"]) - system_base_fs_file = os.path.join(input_dir, "META", basename) + system_base_fs_file = os.path.join(input_file, "META", basename) if os.path.exists(system_base_fs_file): d["system_base_fs_file"] = system_base_fs_file else: @@ -207,7 +236,7 @@ def LoadInfoDict(input_file, input_dir=None): if "vendor_base_fs_file" in d: basename = os.path.basename(d["vendor_base_fs_file"]) - vendor_base_fs_file = os.path.join(input_dir, "META", basename) + vendor_base_fs_file = os.path.join(input_file, "META", basename) if os.path.exists(vendor_base_fs_file): d["vendor_base_fs_file"] = vendor_base_fs_file else: diff --git a/tools/releasetools/test_common.py b/tools/releasetools/test_common.py index f56f368fd..517990095 100644 --- a/tools/releasetools/test_common.py +++ b/tools/releasetools/test_common.py @@ -806,6 +806,9 @@ class CommonUtilsTest(unittest.TestCase): else: fstab_values = FSTAB_TEMPLATE.format('/system') common.ZipWriteStr(target_files_zip, fstab_path, fstab_values) + + common.ZipWriteStr( + target_files_zip, 'META/file_contexts', 'file-contexts') return target_files def test_LoadInfoDict(self): @@ -898,6 +901,38 @@ class CommonUtilsTest(unittest.TestCase): self.assertEqual(2, loaded_dict['fstab_version']) self.assertIsNone(loaded_dict['fstab']) + def test_LoadInfoDict_missingMetaMiscInfoTxt(self): + target_files = self._test_LoadInfoDict_createTargetFiles( + self.INFO_DICT_DEFAULT, + 'BOOT/RAMDISK/system/etc/recovery.fstab') + common.ZipDelete(target_files, 'META/misc_info.txt') + with zipfile.ZipFile(target_files, 'r') as target_files_zip: + self.assertRaises(ValueError, common.LoadInfoDict, target_files_zip) + + def test_LoadInfoDict_repacking(self): + target_files = self._test_LoadInfoDict_createTargetFiles( + self.INFO_DICT_DEFAULT, + 'BOOT/RAMDISK/system/etc/recovery.fstab') + unzipped = common.UnzipTemp(target_files) + loaded_dict = common.LoadInfoDict(unzipped, True) + self.assertEqual(3, loaded_dict['recovery_api_version']) + self.assertEqual(2, loaded_dict['fstab_version']) + self.assertIn('/', loaded_dict['fstab']) + self.assertIn('/system', loaded_dict['fstab']) + self.assertEqual( + os.path.join(unzipped, 'ROOT'), loaded_dict['root_dir']) + self.assertEqual( + os.path.join(unzipped, 'META', 'root_filesystem_config.txt'), + loaded_dict['root_fs_config']) + + def test_LoadInfoDict_repackingWithZipFileInput(self): + target_files = self._test_LoadInfoDict_createTargetFiles( + self.INFO_DICT_DEFAULT, + 'BOOT/RAMDISK/system/etc/recovery.fstab') + with zipfile.ZipFile(target_files, 'r') as target_files_zip: + self.assertRaises( + AssertionError, common.LoadInfoDict, target_files_zip, True) + class InstallRecoveryScriptFormatTest(unittest.TestCase): """Checks the format of install-recovery.sh.