From e3ba82cff24a2bf44c39230cf1f6d934c3f4c644 Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Wed, 21 Aug 2019 13:29:30 -0700 Subject: [PATCH 1/4] Add a script to check VINTF compat of target files package. Instead of checking META/{system,vendor}_{manifest,matrix}.xml (which is error-prone because ODM SKU-specific manifests are not checked), this script read the target files package, remaps its directory structure so that checkvintf understands it, and check VINTF compatibility. Also, put it in otatools.zip. Test: run it on an extracted target files package Bug: 131425279 Change-Id: I06036f9a8d7242d4bc11524028be40e780c508e8 --- core/Makefile | 50 ++-- tools/releasetools/Android.bp | 21 ++ .../releasetools/check_target_files_vintf.py | 232 ++++++++++++++++++ 3 files changed, 287 insertions(+), 16 deletions(-) create mode 100755 tools/releasetools/check_target_files_vintf.py diff --git a/core/Makefile b/core/Makefile index 99b86ee99..6f6eee2eb 100644 --- a/core/Makefile +++ b/core/Makefile @@ -2698,12 +2698,19 @@ $(BUILT_ASSEMBLED_VENDOR_MANIFEST): PRIVATE_FLAGS := # -- Kernel version and configurations. ifeq ($(PRODUCT_OTA_ENFORCE_VINTF_KERNEL_REQUIREMENTS),true) +intermediates := $(call intermediates-dir-for,ETC,$(notdir $(BUILT_ASSEMBLED_VENDOR_MANIFEST))) +BUILT_KERNEL_CONFIGS_FILE := $(intermediates)/kernel_configs.txt +BUILT_KERNEL_VERSION_FILE := $(intermediates)/kernel_version.txt + # BOARD_KERNEL_CONFIG_FILE and BOARD_KERNEL_VERSION can be used to override the values extracted # from INSTALLED_KERNEL_TARGET. ifdef BOARD_KERNEL_CONFIG_FILE ifdef BOARD_KERNEL_VERSION -$(BUILT_ASSEMBLED_VENDOR_MANIFEST): $(BOARD_KERNEL_CONFIG_FILE) -$(BUILT_ASSEMBLED_VENDOR_MANIFEST): PRIVATE_FLAGS += --kernel $(BOARD_KERNEL_VERSION):$(BOARD_KERNEL_CONFIG_FILE) +$(BUILT_KERNEL_CONFIGS_FILE): $(BOARD_KERNEL_CONFIG_FILE) + cp $< $@ +$(BUILT_KERNEL_VERSION_FILE): + echo $(BOARD_KERNEL_VERSION) > $@ + my_board_extracted_kernel := true endif # BOARD_KERNEL_VERSION endif # BOARD_KERNEL_CONFIG_FILE @@ -2718,7 +2725,6 @@ $(warning No INSTALLED_KERNEL_TARGET is defined when PRODUCT_OTA_ENFORCE_VINTF_K BOARD_KERNEL_VERSION manually; or (3) unsetting PRODUCT_OTA_ENFORCE_VINTF_KERNEL_REQUIREMENTS \ manually.) else -intermediates := $(call intermediates-dir-for,ETC,$(notdir $(BUILT_ASSEMBLED_VENDOR_MANIFEST))) # Tools for decompression that is not in PATH. # Check $(EXTRACT_KERNEL) for decompression algorithms supported by the script. @@ -2726,29 +2732,25 @@ intermediates := $(call intermediates-dir-for,ETC,$(notdir $(BUILT_ASSEMBLED_VEN my_decompress_tools := \ lz4:$(HOST_OUT_EXECUTABLES)/lz4 \ -my_kernel_configs := $(intermediates)/kernel_configs.txt -my_kernel_version := $(intermediates)/kernel_version.txt -$(my_kernel_configs): .KATI_IMPLICIT_OUTPUTS := $(my_kernel_version) -$(my_kernel_configs): PRIVATE_KERNEL_VERSION_FILE := $(my_kernel_version) -$(my_kernel_configs): PRIVATE_DECOMPRESS_TOOLS := $(my_decompress_tools) -$(my_kernel_configs): $(foreach pair,$(my_decompress_tools),$(call word-colon,2,$(pair))) -$(my_kernel_configs): $(EXTRACT_KERNEL) $(INSTALLED_KERNEL_TARGET) +$(BUILT_KERNEL_CONFIGS_FILE): .KATI_IMPLICIT_OUTPUTS := $(BUILT_KERNEL_VERSION_FILE) +$(BUILT_KERNEL_CONFIGS_FILE): PRIVATE_DECOMPRESS_TOOLS := $(my_decompress_tools) +$(BUILT_KERNEL_CONFIGS_FILE): $(foreach pair,$(my_decompress_tools),$(call word-colon,2,$(pair))) +$(BUILT_KERNEL_CONFIGS_FILE): $(EXTRACT_KERNEL) $(INSTALLED_KERNEL_TARGET) $< --tools $(PRIVATE_DECOMPRESS_TOOLS) --input $(INSTALLED_KERNEL_TARGET) \ --output-configs $@ \ - --output-version $(PRIVATE_KERNEL_VERSION_FILE) - -$(BUILT_ASSEMBLED_VENDOR_MANIFEST): $(my_kernel_configs) $(my_kernel_version) -$(BUILT_ASSEMBLED_VENDOR_MANIFEST): PRIVATE_FLAGS += --kernel $$(cat $(my_kernel_version)):$(my_kernel_configs) + --output-version $(BUILT_KERNEL_VERSION_FILE) intermediates := -my_kernel_configs := -my_kernel_version := my_decompress_tools := endif # my_board_extracted_kernel my_board_extracted_kernel := endif # INSTALLED_KERNEL_TARGET + +$(BUILT_ASSEMBLED_VENDOR_MANIFEST): $(BUILT_KERNEL_CONFIGS_FILE) $(BUILT_KERNEL_VERSION_FILE) +$(BUILT_ASSEMBLED_VENDOR_MANIFEST): PRIVATE_FLAGS += --kernel $$(cat $(BUILT_KERNEL_VERSION_FILE)):$(BUILT_KERNEL_CONFIGS_FILE) + endif # PRODUCT_OTA_ENFORCE_VINTF_KERNEL_REQUIREMENTS $(BUILT_ASSEMBLED_VENDOR_MANIFEST): @@ -3614,6 +3616,7 @@ INTERNAL_OTATOOLS_MODULES := \ care_map_generator \ check_ota_package_signature \ check_target_files_signatures \ + check_target_files_vintf \ checkvintf \ delta_generator \ e2fsck \ @@ -3846,6 +3849,13 @@ endif # BOARD_AVB_DTBO_KEY_PATH endif # BOARD_AVB_ENABLE endif # BOARD_PREBUILT_DTBOIMAGE $(call dump-dynamic-partitions-info,$@) + @# VINTF checks +ifeq ($(PRODUCT_ENFORCE_VINTF_MANIFEST),true) + $(hide) echo "vintf_enforce=true" >> $@ +endif +ifdef ODM_MANIFEST_SKUS + $(hide) echo "vintf_odm_manifest_skus=$(ODM_MANIFEST_SKUS)" >> $@ +endif .PHONY: misc_info misc_info: $(INSTALLED_MISC_INFO_TARGET) @@ -4005,6 +4015,8 @@ $(BUILT_TARGET_FILES_PACKAGE): \ $(BUILT_ASSEMBLED_VENDOR_MANIFEST) \ $(BUILT_SYSTEM_MATRIX) \ $(BUILT_VENDOR_MATRIX) \ + $(BUILT_KERNEL_CONFIGS_FILE) \ + $(BUILT_KERNEL_VERSION_FILE) \ | $(ACP) @echo "Package target files: $@" $(call create-system-vendor-symlink) @@ -4253,6 +4265,12 @@ endif ifdef BUILT_VENDOR_MATRIX $(hide) cp $(BUILT_VENDOR_MATRIX) $(zip_root)/META/vendor_matrix.xml endif +ifdef BUILT_KERNEL_CONFIGS_FILE + $(hide) cp $(BUILT_KERNEL_CONFIGS_FILE) $(zip_root)/META/kernel_configs.txt +endif +ifdef BUILT_KERNEL_VERSION_FILE + $(hide) cp $(BUILT_KERNEL_VERSION_FILE) $(zip_root)/META/kernel_version.txt +endif ifneq ($(BOARD_SUPER_PARTITION_GROUPS),) $(hide) echo "super_partition_groups=$(BOARD_SUPER_PARTITION_GROUPS)" > $(zip_root)/META/dynamic_partitions_info.txt @# Remove 'vendor' from the group partition list if the image is not available. This should only diff --git a/tools/releasetools/Android.bp b/tools/releasetools/Android.bp index 03028567e..43f52be7a 100644 --- a/tools/releasetools/Android.bp +++ b/tools/releasetools/Android.bp @@ -75,6 +75,19 @@ python_defaults { ], } +python_defaults { + name: "releasetools_check_target_files_vintf_defaults", + srcs: [ + "check_target_files_vintf.py", + ], + libs: [ + "releasetools_common", + ], + required: [ + "checkvintf", + ], +} + python_defaults { name: "releasetools_ota_from_target_files_defaults", srcs: [ @@ -265,6 +278,14 @@ python_binary_host { ], } +python_binary_host { + name: "check_target_files_vintf", + defaults: [ + "releasetools_binary_defaults", + "releasetools_check_target_files_vintf_defaults" + ], +} + python_binary_host { name: "img_from_target_files", defaults: [ diff --git a/tools/releasetools/check_target_files_vintf.py b/tools/releasetools/check_target_files_vintf.py new file mode 100755 index 000000000..543147c98 --- /dev/null +++ b/tools/releasetools/check_target_files_vintf.py @@ -0,0 +1,232 @@ +#!/usr/bin/env python +# +# Copyright (C) 2019 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +Check VINTF compatibility from a target files package. + +Usage: check_target_files_vintf target_files + +target_files can be a ZIP file or an extracted target files directory. +""" + +import logging +import subprocess +import sys +import os +import zipfile + +import common + +logger = logging.getLogger(__name__) + +OPTIONS = common.OPTIONS + +# Keys are paths that VINTF searches. Must keep in sync with libvintf's search +# paths (VintfObject.cpp). +# These paths are stored in different directories in target files package, so +# we have to search for the correct path and tell checkvintf to remap them. +DIR_SEARCH_PATHS = { + '/system': ('SYSTEM',), + '/vendor': ('VENDOR', 'SYSTEM/vendor'), + '/product': ('PRODUCT', 'SYSTEM/product'), + '/odm': ('ODM', 'VENDOR/odm'), +} + +UNZIP_PATTERN = ['META/*', '*/build.prop'] + + +def GetDirmap(input_tmp): + dirmap = {} + for device_path, target_files_rel_paths in DIR_SEARCH_PATHS.items(): + for target_files_rel_path in target_files_rel_paths: + target_files_path = os.path.join(input_tmp, target_files_rel_path) + if os.path.isdir(target_files_path): + dirmap[device_path] = target_files_path + break + if device_path not in dirmap: + raise ValueError("Can't determine path for device path " + device_path + + ". Searched the following:" + + ("\n".join(target_files_rel_paths))) + return dirmap + + +def GetArgsForSkus(info_dict): + skus = info_dict.get('vintf_odm_manifest_skus', '').strip().split() + if not skus: + logger.info("ODM_MANIFEST_SKUS is not defined. Check once without SKUs.") + skus = [''] + return [['--property', 'ro.boot.product.hardware.sku=' + sku] + for sku in skus] + + +def GetArgsForShippingApiLevel(info_dict): + shipping_api_level = info_dict['vendor.build.prop'].get( + 'ro.product.first_api_level') + if not shipping_api_level: + logger.warning('Cannot determine ro.product.first_api_level') + return [] + return ['--property', 'ro.product.first_api_level=' + shipping_api_level] + + +def GetArgsForKernel(input_tmp): + version_path = os.path.join(input_tmp, 'META/kernel_version.txt') + config_path = os.path.join(input_tmp, 'META/kernel_configs.txt') + + if not os.path.isfile(version_path) or not os.path.isfile(config_path): + logger.info('Skipping kernel config checks because ' + + 'PRODUCT_OTA_ENFORCE_VINTF_KERNEL_REQUIREMENTS is not set') + return [] + + with open(version_path) as f: + version = f.read().strip() + + return ['--kernel', '{}:{}'.format(version, config_path)] + + +def CheckVintfFromExtractedTargetFiles(input_tmp, info_dict=None): + """ + Checks VINTF metadata of an extracted target files directory. + + Args: + inp: path to the directory that contains the extracted target files archive. + info_dict: The build-time info dict. If None, it will be loaded from inp. + + Returns: + True if VINTF check is skipped or compatible, False if incompatible. Raise + a RuntimeError if any error occurs. + """ + + if info_dict is None: + info_dict = common.LoadInfoDict(input_tmp) + + if info_dict.get('vintf_enforce') != 'true': + logger.warning('PRODUCT_ENFORCE_VINTF_MANIFEST is not set, skipping checks') + return True + + dirmap = GetDirmap(input_tmp) + args_for_skus = GetArgsForSkus(info_dict) + shipping_api_level_args = GetArgsForShippingApiLevel(info_dict) + kernel_args = GetArgsForKernel(input_tmp) + + common_command = [ + 'checkvintf', + '--check-compat', + ] + for device_path, real_path in dirmap.items(): + common_command += ['--dirmap', '{}:{}'.format(device_path, real_path)] + common_command += kernel_args + common_command += shipping_api_level_args + + success = True + for sku_args in args_for_skus: + command = common_command + sku_args + proc = common.Run(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + out, err = proc.communicate() + if proc.returncode == 0: + logger.info("Command `%s` returns 'compatible'", ' '.join(command)) + elif out.strip() == "INCOMPATIBLE": + logger.info("Command `%s` returns 'incompatible'", ' '.join(command)) + success = False + else: + raise common.ExternalError( + "Failed to run command '{}' (exit code {}):\nstdout:{}\nstderr:{}" + .format(' '.join(command), proc.returncode, out, err)) + logger.info("stdout: %s", out) + logger.info("stderr: %s", err) + + return success + + +def GetVintfFileList(): + """ + Returns a list of VINTF metadata files that should be read from a target files + package before executing checkvintf. + """ + def PathToPatterns(path): + if path[-1] == '/': + path += '*' + for device_path, target_files_rel_paths in DIR_SEARCH_PATHS.items(): + if path.startswith(device_path): + suffix = path[len(device_path):] + return [rel_path + suffix for rel_path in target_files_rel_paths] + raise RuntimeError('Unrecognized path from checkvintf --dump-file-list: ' + + path) + + out = common.RunAndCheckOutput(['checkvintf', '--dump-file-list']) + paths = out.strip().split('\n') + paths = sum((PathToPatterns(path) for path in paths if path), []) + return paths + + +def CheckVintfFromTargetFiles(inp, info_dict=None): + """ + Checks VINTF metadata of a target files zip. + + Args: + inp: path to the target files archive. + info_dict: The build-time info dict. If None, it will be loaded from inp. + + Returns: + True if VINTF check is skipped or compatible, False if incompatible. Raise + a RuntimeError if any error occurs. + """ + input_tmp = common.UnzipTemp(inp, GetVintfFileList() + UNZIP_PATTERN) + return CheckVintfFromExtractedTargetFiles(input_tmp, info_dict) + + +def CheckVintf(inp, info_dict=None): + """ + Checks VINTF metadata of a target files zip or extracted target files + directory. + + Args: + inp: path to the (possibly extracted) target files archive. + info_dict: The build-time info dict. If None, it will be loaded from inp. + + Returns: + True if VINTF check is skipped or compatible, False if incompatible. Raise + a RuntimeError if any error occurs. + """ + if os.path.isdir(inp): + logger.info('Checking VINTF compatibility extracted target files...') + return CheckVintfFromExtractedTargetFiles(inp, info_dict) + + if zipfile.is_zipfile(inp): + logger.info('Checking VINTF compatibility target files...') + return CheckVintfFromTargetFiles(inp, info_dict) + + raise ValueError('{} is not a valid directory or zip file'.format(inp)) + + +def main(argv): + args = common.ParseOptions(argv, __doc__) + if len(args) != 1: + common.Usage(__doc__) + sys.exit(1) + common.InitLogging() + if not CheckVintf(args[0]): + sys.exit(1) + + +if __name__ == '__main__': + try: + common.CloseInheritedPipes() + main(sys.argv[1:]) + except common.ExternalError: + logger.exception('\n ERROR:\n') + sys.exit(1) + finally: + common.Cleanup() From ccb86fe7d47b01ce06a968c0d2bc8d4024b8b8a6 Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Thu, 22 Aug 2019 15:52:26 -0700 Subject: [PATCH 2/4] releasetools: add tests for check_target_files_vintf Test: run it Bug: 139300422 Change-Id: I265d1e6313177dc661bb5cdf1415b11684dec0a0 --- tools/releasetools/Android.bp | 11 +- .../test_check_target_files_vintf.py | 143 ++++++++++++++++++ .../kernel/SYSTEM/compatibility_matrix.xml | 7 + .../SYSTEM/compatibility_matrix.xml | 6 + .../sku_compat/ODM/etc/vintf/manifest_sku.xml | 7 + .../SYSTEM/compatibility_matrix.xml | 14 ++ .../ODM/etc/vintf/manifest_sku.xml | 7 + .../SYSTEM/compatibility_matrix.xml | 14 ++ 8 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 tools/releasetools/test_check_target_files_vintf.py create mode 100644 tools/releasetools/testdata/vintf/kernel/SYSTEM/compatibility_matrix.xml create mode 100644 tools/releasetools/testdata/vintf/matrix_incompat/SYSTEM/compatibility_matrix.xml create mode 100644 tools/releasetools/testdata/vintf/sku_compat/ODM/etc/vintf/manifest_sku.xml create mode 100644 tools/releasetools/testdata/vintf/sku_compat/SYSTEM/compatibility_matrix.xml create mode 100644 tools/releasetools/testdata/vintf/sku_incompat/ODM/etc/vintf/manifest_sku.xml create mode 100644 tools/releasetools/testdata/vintf/sku_incompat/SYSTEM/compatibility_matrix.xml diff --git a/tools/releasetools/Android.bp b/tools/releasetools/Android.bp index 43f52be7a..ef76c52ef 100644 --- a/tools/releasetools/Android.bp +++ b/tools/releasetools/Android.bp @@ -155,6 +155,14 @@ python_library_host { ], } +python_library_host { + name: "releasetools_check_target_files_vintf", + defaults: [ + "releasetools_library_defaults", + "releasetools_check_target_files_vintf_defaults", + ], +} + python_library_host { name: "releasetools_common", defaults: ["releasetools_library_defaults"], @@ -417,13 +425,14 @@ python_defaults { "releasetools_apex_utils", "releasetools_build_image", "releasetools_build_super_image", + "releasetools_check_target_files_vintf", "releasetools_common", "releasetools_img_from_target_files", "releasetools_ota_from_target_files", "releasetools_verity_utils", ], data: [ - "testdata/*", + "testdata/**/*", ], required: [ "otatools", diff --git a/tools/releasetools/test_check_target_files_vintf.py b/tools/releasetools/test_check_target_files_vintf.py new file mode 100644 index 000000000..a1328c237 --- /dev/null +++ b/tools/releasetools/test_check_target_files_vintf.py @@ -0,0 +1,143 @@ +# +# Copyright (C) 2019 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the 'License'); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an 'AS IS' BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +import os.path + +import common +import test_utils +from check_target_files_vintf import CheckVintf + +# A skeleton target files directory structure. This is VINTF compatible. +SKELETON_TARGET_FILE_STRUCTURE = { + # Empty files + 'PRODUCT/build.prop': '', + 'PRODUCT/etc/build.prop': '', + 'VENDOR/etc/build.prop': '', + 'ODM/build.prop': '', + 'ODM/etc/build.prop': '', + 'RECOVERY/RAMDISK/etc/recovery.fstab': '', + 'SYSTEM/build.prop': '', + 'SYSTEM/etc/build.prop': '', + 'SYSTEM_EXT/build.prop': '', + 'SYSTEM_EXT/etc/build.prop': '', + + # Non-empty files + 'SYSTEM/compatibility_matrix.xml':""" + + + 0.0 + 0 + + """, + 'SYSTEM/manifest.xml': + '', + 'VENDOR/build.prop': 'ro.product.first_api_level=29\n', + 'VENDOR/compatibility_matrix.xml': + '', + 'VENDOR/manifest.xml': + '', + 'META/misc_info.txt': + 'recovery_api_version=3\nfstab_version=2\nvintf_enforce=true\n', +} + + +def write_string_to_file(content, path, mode='w'): + if not os.path.isdir(os.path.dirname(path)): + os.makedirs(os.path.dirname(path)) + with open(path, mode=mode) as f: + f.write(content) + + +class CheckTargetFilesVintfTest(test_utils.ReleaseToolsTestCase): + + def setUp(self): + self.testdata_dir = test_utils.get_testdata_dir() + + def prepare_test_dir(self, test_delta_rel_path): + test_delta_dir = os.path.join(self.testdata_dir, test_delta_rel_path) + test_dir = common.MakeTempDir(prefix='check_target_files_vintf') + + # Create a skeleton directory structure of target files + for rel_path, content in SKELETON_TARGET_FILE_STRUCTURE.items(): + write_string_to_file(content, os.path.join(test_dir, rel_path)) + + # Overwrite with files from test_delta_rel_path + for root, _, files in os.walk(test_delta_dir): + rel_root = os.path.relpath(root, test_delta_dir) + for f in files: + output_file = os.path.join(test_dir, rel_root, f) + with open(os.path.join(root, f)) as inp: + write_string_to_file(inp.read(), output_file) + + return test_dir + + @test_utils.SkipIfExternalToolsUnavailable() + def test_CheckVintf_sanity(self): + msg = 'Sanity check with skeleton target files failed.' + test_dir = self.prepare_test_dir('does-not-exist') + self.assertTrue(CheckVintf(test_dir), msg=msg) + + @test_utils.SkipIfExternalToolsUnavailable() + def test_CheckVintf_matrix_incompat(self): + msg = 'vintf/matrix_incompat should be incompatible because sepolicy ' \ + 'version fails to match' + test_dir = self.prepare_test_dir('vintf/matrix_incompat') + self.assertFalse(CheckVintf(test_dir), msg=msg) + + @test_utils.SkipIfExternalToolsUnavailable() + def test_CheckVintf_kernel_compat(self): + msg = 'vintf/kernel with 4.14.1 kernel version should be compatible' + test_dir = self.prepare_test_dir('vintf/kernel') + write_string_to_file('', os.path.join(test_dir, 'META/kernel_configs.txt')) + write_string_to_file('4.14.1', + os.path.join(test_dir, 'META/kernel_version.txt')) + self.assertTrue(CheckVintf(test_dir), msg=msg) + + @test_utils.SkipIfExternalToolsUnavailable() + def test_CheckVintf_kernel_incompat(self): + msg = 'vintf/kernel with 4.14.0 kernel version should be incompatible ' \ + 'because 4.14.1 kernel version is required' + test_dir = self.prepare_test_dir('vintf/kernel') + write_string_to_file('', os.path.join(test_dir, 'META/kernel_configs.txt')) + write_string_to_file('4.14.0', + os.path.join(test_dir, 'META/kernel_version.txt')) + self.assertFalse(CheckVintf(test_dir), msg=msg) + + @test_utils.SkipIfExternalToolsUnavailable() + def test_CheckVintf_sku_compat(self): + msg = 'vintf/sku_compat should be compatible because ' \ + 'ODM/etc/vintf/manifest_sku.xml has the required HALs' + test_dir = self.prepare_test_dir('vintf/sku_compat') + write_string_to_file('vintf_odm_manifest_skus=sku', + os.path.join(test_dir, 'META/misc_info.txt'), mode='a') + self.assertTrue(CheckVintf(test_dir), msg=msg) + + @test_utils.SkipIfExternalToolsUnavailable() + def test_CheckVintf_sku_incompat(self): + msg = 'vintf/sku_compat should be compatible because ' \ + 'ODM/etc/vintf/manifest_sku.xml does not have the required HALs' + test_dir = self.prepare_test_dir('vintf/sku_incompat') + write_string_to_file('vintf_odm_manifest_skus=sku', + os.path.join(test_dir, 'META/misc_info.txt'), mode='a') + self.assertFalse(CheckVintf(test_dir), msg=msg) + + @test_utils.SkipIfExternalToolsUnavailable() + def test_CheckVintf_bad_xml(self): + test_dir = self.prepare_test_dir('does-not-exist') + write_string_to_file('not an XML', + os.path.join(test_dir, 'VENDOR/manifest.xml')) + # Should raise an error because a file has invalid format. + self.assertRaises(common.ExternalError, CheckVintf, test_dir) diff --git a/tools/releasetools/testdata/vintf/kernel/SYSTEM/compatibility_matrix.xml b/tools/releasetools/testdata/vintf/kernel/SYSTEM/compatibility_matrix.xml new file mode 100644 index 000000000..ed46b6b76 --- /dev/null +++ b/tools/releasetools/testdata/vintf/kernel/SYSTEM/compatibility_matrix.xml @@ -0,0 +1,7 @@ + + + + 0.0 + 0 + + diff --git a/tools/releasetools/testdata/vintf/matrix_incompat/SYSTEM/compatibility_matrix.xml b/tools/releasetools/testdata/vintf/matrix_incompat/SYSTEM/compatibility_matrix.xml new file mode 100644 index 000000000..5d891fa4a --- /dev/null +++ b/tools/releasetools/testdata/vintf/matrix_incompat/SYSTEM/compatibility_matrix.xml @@ -0,0 +1,6 @@ + + + 1.0 + 0 + + diff --git a/tools/releasetools/testdata/vintf/sku_compat/ODM/etc/vintf/manifest_sku.xml b/tools/releasetools/testdata/vintf/sku_compat/ODM/etc/vintf/manifest_sku.xml new file mode 100644 index 000000000..bcd7ce424 --- /dev/null +++ b/tools/releasetools/testdata/vintf/sku_compat/ODM/etc/vintf/manifest_sku.xml @@ -0,0 +1,7 @@ + + + foo + hwbinder + @1.0::IFoo/default + + diff --git a/tools/releasetools/testdata/vintf/sku_compat/SYSTEM/compatibility_matrix.xml b/tools/releasetools/testdata/vintf/sku_compat/SYSTEM/compatibility_matrix.xml new file mode 100644 index 000000000..19a9b6a0d --- /dev/null +++ b/tools/releasetools/testdata/vintf/sku_compat/SYSTEM/compatibility_matrix.xml @@ -0,0 +1,14 @@ + + + foo + 1.0 + + IFoo + default + + + + 0.0 + 0 + + diff --git a/tools/releasetools/testdata/vintf/sku_incompat/ODM/etc/vintf/manifest_sku.xml b/tools/releasetools/testdata/vintf/sku_incompat/ODM/etc/vintf/manifest_sku.xml new file mode 100644 index 000000000..bcd7ce424 --- /dev/null +++ b/tools/releasetools/testdata/vintf/sku_incompat/ODM/etc/vintf/manifest_sku.xml @@ -0,0 +1,7 @@ + + + foo + hwbinder + @1.0::IFoo/default + + diff --git a/tools/releasetools/testdata/vintf/sku_incompat/SYSTEM/compatibility_matrix.xml b/tools/releasetools/testdata/vintf/sku_incompat/SYSTEM/compatibility_matrix.xml new file mode 100644 index 000000000..e0e0d6c20 --- /dev/null +++ b/tools/releasetools/testdata/vintf/sku_incompat/SYSTEM/compatibility_matrix.xml @@ -0,0 +1,14 @@ + + + foo + 1.1 + + IFoo + default + + + + 0.0 + 0 + + From 9276cf0226d341927d0a7e682a8db677e25ae5dd Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Wed, 21 Aug 2019 16:37:04 -0700 Subject: [PATCH 3/4] Check VINTF compatibility at OTA generation time. Instead of relying on META/{system,vendor}_{manifest,matrix}.xml and blindly copy compatibility.zip from target files package to OTA package, do a static check on the input target files package before generating the OTA package. META/{system,vendor}_{manifest,matrix} does not contain ODM manifest fragments, which is incorrect. Also, the on-device check of compatibility.zip uses a old libvintf binary on the device, which may not understand the incoming VINTF metadata. This change removes the on-device check. Hence, it removes the requirement of forwards compatibility of libvintf. This behavior can be skipped with --skip-compatibility-check. Test: build OTA package Bug: 139300422 Bug: 131425279 Change-Id: I7fb93be9eb73f578fc05a182c6c9d1f073db2800 --- core/Makefile | 14 --- tools/releasetools/Android.bp | 1 + tools/releasetools/merge_target_files.py | 4 - tools/releasetools/ota_from_target_files.py | 124 ++++---------------- 4 files changed, 24 insertions(+), 119 deletions(-) diff --git a/core/Makefile b/core/Makefile index 6f6eee2eb..f56df1f2c 100644 --- a/core/Makefile +++ b/core/Makefile @@ -4011,10 +4011,6 @@ $(BUILT_TARGET_FILES_PACKAGE): \ $(HOST_OUT_EXECUTABLES)/fs_config \ $(ADD_IMG_TO_TARGET_FILES) \ $(MAKE_RECOVERY_PATCH) \ - $(BUILT_ASSEMBLED_FRAMEWORK_MANIFEST) \ - $(BUILT_ASSEMBLED_VENDOR_MANIFEST) \ - $(BUILT_SYSTEM_MATRIX) \ - $(BUILT_VENDOR_MATRIX) \ $(BUILT_KERNEL_CONFIGS_FILE) \ $(BUILT_KERNEL_VERSION_FILE) \ | $(ACP) @@ -4255,16 +4251,6 @@ ifdef BUILDING_SYSTEM_OTHER_IMAGE $(hide) $(call fs_config,$(zip_root)/SYSTEM_OTHER,system/) > $(zip_root)/META/system_other_filesystem_config.txt endif @# Metadata for compatibility verification. - $(hide) cp $(BUILT_SYSTEM_MATRIX) $(zip_root)/META/system_matrix.xml -ifdef BUILT_ASSEMBLED_FRAMEWORK_MANIFEST - $(hide) cp $(BUILT_ASSEMBLED_FRAMEWORK_MANIFEST) $(zip_root)/META/system_manifest.xml -endif -ifdef BUILT_ASSEMBLED_VENDOR_MANIFEST - $(hide) cp $(BUILT_ASSEMBLED_VENDOR_MANIFEST) $(zip_root)/META/vendor_manifest.xml -endif -ifdef BUILT_VENDOR_MATRIX - $(hide) cp $(BUILT_VENDOR_MATRIX) $(zip_root)/META/vendor_matrix.xml -endif ifdef BUILT_KERNEL_CONFIGS_FILE $(hide) cp $(BUILT_KERNEL_CONFIGS_FILE) $(zip_root)/META/kernel_configs.txt endif diff --git a/tools/releasetools/Android.bp b/tools/releasetools/Android.bp index ef76c52ef..a920ffb13 100644 --- a/tools/releasetools/Android.bp +++ b/tools/releasetools/Android.bp @@ -96,6 +96,7 @@ python_defaults { "target_files_diff.py", ], libs: [ + "releasetools_check_target_files_vintf", "releasetools_common", "releasetools_verity_utils", ], diff --git a/tools/releasetools/merge_target_files.py b/tools/releasetools/merge_target_files.py index b7ed6458a..916c80374 100755 --- a/tools/releasetools/merge_target_files.py +++ b/tools/releasetools/merge_target_files.py @@ -117,8 +117,6 @@ DEFAULT_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/*', @@ -163,8 +161,6 @@ DEFAULT_VENDOR_ITEM_LIST = ( 'META/otakeys.txt', 'META/releasetools.py', 'META/vendor_filesystem_config.txt', - 'META/vendor_manifest.xml', - 'META/vendor_matrix.xml', 'BOOT/*', 'DATA/*', 'ODM/*', diff --git a/tools/releasetools/ota_from_target_files.py b/tools/releasetools/ota_from_target_files.py index c6c859c85..60b73e798 100755 --- a/tools/releasetools/ota_from_target_files.py +++ b/tools/releasetools/ota_from_target_files.py @@ -72,7 +72,7 @@ Common options that apply to both of non-A/B and A/B OTAs --skip_postinstall is implied. --skip_compatibility_check - Skip adding the compatibility package to the generated OTA package. + Skip checking compatibility of the input target files package. --output_metadata_path Write a copy of the metadata to a separate file. Therefore, users can @@ -189,9 +189,9 @@ import shlex import shutil import struct import sys -import tempfile import zipfile +import check_target_files_vintf import common import edify_generator import verity_utils @@ -723,20 +723,15 @@ def HasPartition(target_files_zip, partition): return False -def HasVendorPartition(target_files_zip): - return HasPartition(target_files_zip, "vendor") +def HasTrebleEnabled(target_files, target_info): + def HasVendorPartition(target_files): + if os.path.isdir(target_files): + return os.path.isdir(os.path.join(target_files, "VENDOR")) + if zipfile.is_zipfile(target_files): + return HasPartition(zipfile.ZipFile(target_files), "vendor") + raise ValueError("Unknown target_files argument") - -def HasProductPartition(target_files_zip): - return HasPartition(target_files_zip, "product") - - -def HasOdmPartition(target_files_zip): - return HasPartition(target_files_zip, "odm") - - -def HasTrebleEnabled(target_files_zip, target_info): - return (HasVendorPartition(target_files_zip) and + return (HasVendorPartition(target_files) and target_info.GetBuildProp("ro.treble.enabled") == "true") @@ -761,74 +756,23 @@ def WriteFingerprintAssertion(script, target_info, source_info): source_info.GetBuildProp("ro.build.thumbprint")) -def AddCompatibilityArchiveIfTrebleEnabled(target_zip, output_zip, target_info, - source_info=None): - """Adds compatibility info into the output zip if it's Treble-enabled target. +def CheckVintfIfTrebleEnabled(target_files, target_info): + """Checks compatibility info of the input target files. - Metadata used for on-device compatibility verification is retrieved from - target_zip then added to compatibility.zip which is added to the output_zip - archive. + Metadata used for compatibility verification is retrieved from target_zip. - Compatibility archive should only be included for devices that have enabled + Compatibility should only be checked for devices that have enabled Treble support. Args: - target_zip: Zip file containing the source files to be included for OTA. - output_zip: Zip file that will be sent for OTA. + target_files: Path to zip file containing the source files to be included + for OTA. Can also be the path to extracted directory. target_info: The BuildInfo instance that holds the target build info. - source_info: The BuildInfo instance that holds the source build info, if - generating an incremental OTA; None otherwise. """ - def AddCompatibilityArchive(framework_updated, device_updated): - """Adds compatibility info based on update status of both sides of Treble - boundary. - - Args: - framework_updated: If True, the system / product image will be updated - and therefore their metadata should be included. - device_updated: If True, the vendor / odm image will be updated and - therefore their metadata should be included. - """ - # Determine what metadata we need. Files are names relative to META/. - compatibility_files = [] - device_metadata = ("vendor_manifest.xml", "vendor_matrix.xml") - framework_metadata = ("system_manifest.xml", "system_matrix.xml") - if device_updated: - compatibility_files += device_metadata - if framework_updated: - compatibility_files += framework_metadata - - # Create new archive. - compatibility_archive = tempfile.NamedTemporaryFile() - compatibility_archive_zip = zipfile.ZipFile( - compatibility_archive, "w", compression=zipfile.ZIP_DEFLATED) - - # Add metadata. - for file_name in compatibility_files: - target_file_name = "META/" + file_name - - if target_file_name in target_zip.namelist(): - data = target_zip.read(target_file_name) - common.ZipWriteStr(compatibility_archive_zip, file_name, data) - - # Ensure files are written before we copy into output_zip. - compatibility_archive_zip.close() - - # Only add the archive if we have any compatibility info. - if compatibility_archive_zip.namelist(): - common.ZipWrite(output_zip, compatibility_archive.name, - arcname="compatibility.zip", - compress_type=zipfile.ZIP_STORED) - - def FingerprintChanged(source_fp, target_fp): - if source_fp is None or target_fp is None: - return True - return source_fp != target_fp - # Will only proceed if the target has enabled the Treble support (as well as # having a /vendor partition). - if not HasTrebleEnabled(target_zip, target_info): + if not HasTrebleEnabled(target_files, target_info): return # Skip adding the compatibility package as a workaround for b/114240221. The @@ -836,28 +780,8 @@ def AddCompatibilityArchiveIfTrebleEnabled(target_zip, output_zip, target_info, if OPTIONS.skip_compatibility_check: return - # Full OTA carries the info for system/vendor/product/odm - if source_info is None: - AddCompatibilityArchive(True, True) - return - - source_fp = source_info.fingerprint - target_fp = target_info.fingerprint - system_updated = source_fp != target_fp - - # other build fingerprints could be possibly blacklisted at build time. For - # such a case, we consider those images being changed. - vendor_updated = FingerprintChanged(source_info.vendor_fingerprint, - target_info.vendor_fingerprint) - product_updated = HasProductPartition(target_zip) and \ - FingerprintChanged(source_info.product_fingerprint, - target_info.product_fingerprint) - odm_updated = HasOdmPartition(target_zip) and \ - FingerprintChanged(source_info.odm_fingerprint, - target_info.odm_fingerprint) - - AddCompatibilityArchive(system_updated or product_updated, - vendor_updated or odm_updated) + if not check_target_files_vintf.CheckVintf(target_files, target_info): + raise RuntimeError("VINTF compatibility check failed") def GetBlockDifferences(target_zip, source_zip, target_info, source_info, @@ -1068,7 +992,7 @@ else if get_stage("%(bcb_dev)s") == "3/3" then progress=progress_dict.get(block_diff.partition), write_verify_script=OPTIONS.verify) - AddCompatibilityArchiveIfTrebleEnabled(input_zip, output_zip, target_info) + CheckVintfIfTrebleEnabled(OPTIONS.input_tmp, target_info) boot_img = common.GetBootableImage( "boot.img", "boot.img", OPTIONS.input_tmp, "BOOT") @@ -1643,8 +1567,7 @@ def WriteBlockIncrementalOTAPackage(target_zip, source_zip, output_file): source_info=source_info, device_specific=device_specific) - AddCompatibilityArchiveIfTrebleEnabled( - target_zip, output_zip, target_info, source_info) + CheckVintfIfTrebleEnabled(OPTIONS.target_tmp, target_info) # Assertions (e.g. device properties check). target_info.WriteDeviceAssertions(script, OPTIONS.oem_no_mount) @@ -2079,11 +2002,10 @@ def GenerateAbOtaPackage(target_file, output_file, source_file=None): else: logger.warning("Cannot find care map file in target_file package") - AddCompatibilityArchiveIfTrebleEnabled( - target_zip, output_zip, target_info, source_info) - common.ZipClose(target_zip) + CheckVintfIfTrebleEnabled(target_file, target_info) + # We haven't written the metadata entry yet, which will be handled in # FinalizeMetadata(). common.ZipClose(output_zip) From ade0d3f4a2b0f0cb361fb20c79de74b3e674f17f Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Wed, 21 Aug 2019 16:37:11 -0700 Subject: [PATCH 4/4] merge_target_files: Checks VINTF metadata of resulting target files Bug: 139300422 Bug: 131425279 Test: test_merge_target_files Change-Id: I972db2fc40555ab0fc38316f0a6e6f5e8f163371 --- tools/releasetools/Android.bp | 1 + tools/releasetools/merge_target_files.py | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/tools/releasetools/Android.bp b/tools/releasetools/Android.bp index a920ffb13..fa65767a2 100644 --- a/tools/releasetools/Android.bp +++ b/tools/releasetools/Android.bp @@ -335,6 +335,7 @@ python_binary_host { libs: [ "releasetools_add_img_to_target_files", "releasetools_build_super_image", + "releasetools_check_target_files_vintf", "releasetools_common", "releasetools_img_from_target_files", "releasetools_ota_from_target_files", diff --git a/tools/releasetools/merge_target_files.py b/tools/releasetools/merge_target_files.py index 916c80374..61c4f4ecc 100755 --- a/tools/releasetools/merge_target_files.py +++ b/tools/releasetools/merge_target_files.py @@ -87,6 +87,7 @@ import zipfile import add_img_to_target_files import build_super_image +import check_target_files_vintf import common import img_from_target_files import ota_from_target_files @@ -906,6 +907,9 @@ def merge_target_files(temp_dir, framework_target_files, framework_item_list, vendor_target_files, vendor_item_list, framework_misc_info_keys, rebuild_recovery) + if not check_target_files_vintf.CheckVintf(output_target_files_temp_dir): + raise RuntimeError("Incompatible VINTF metadata") + generate_images(output_target_files_temp_dir, rebuild_recovery) generate_super_empty_image(output_target_files_temp_dir, output_super_empty)