diff options
20 files changed, 183 insertions, 202 deletions
diff --git a/ci/build_test_suites_x86_64-trunk_staging b/ci/build_test_suites index cd7ba8b70d..03f6731dcd 100755 --- a/ci/build_test_suites_x86_64-trunk_staging +++ b/ci/build_test_suites @@ -16,15 +16,4 @@ import sys import build_test_suites -build_test_suites.main( - sys.argv, - extra_targets=[ - 'acts_tests', - 'cts', - 'host-unit-tests', - 'robolectric-tests', - 'test_mapping', - 'tradefed-all', - 'vts', - ], -) +build_test_suites.main(sys.argv) diff --git a/ci/build_test_suites.py b/ci/build_test_suites.py index dad34f1b16..23e896d70a 100644 --- a/ci/build_test_suites.py +++ b/ci/build_test_suites.py @@ -36,15 +36,15 @@ REQUIRED_MODULES = frozenset( ) -def build_test_suites(argv, extra_targets: set[str]): +def build_test_suites(argv): args = parse_args(argv) if is_optimization_enabled(): # Call the class to map changed files to modules to build. # TODO(lucafarsi): Move this into a replaceable class. - build_affected_modules(args, extra_targets) + build_affected_modules(args) else: - build_everything(args, extra_targets) + build_everything(args) def parse_args(argv): @@ -71,20 +71,20 @@ def is_optimization_enabled() -> bool: return False -def build_everything(args: argparse.Namespace, extra_targets: set[str]): - build_command = base_build_command(args, extra_targets) +def build_everything(args: argparse.Namespace): + build_command = base_build_command(args, args.extra_targets) build_command.append('general-tests') run_command(build_command, print_output=True) -def build_affected_modules(args: argparse.Namespace, extra_targets: set[str]): +def build_affected_modules(args: argparse.Namespace): modules_to_build = find_modules_to_build( pathlib.Path(args.change_info), args.extra_required_modules ) # Call the build command with everything. - build_command = base_build_command(args, extra_targets) + build_command = base_build_command(args, args.extra_targets) build_command.extend(modules_to_build) # When not building general-tests we also have to build the general tests # shared libs. @@ -410,5 +410,5 @@ def get_soong_var(var: str, target_release: str) -> str: return value -def main(argv, extra_targets: set[str]): - build_test_suites(argv, extra_targets) +def main(argv): + build_test_suites(argv) diff --git a/core/android_soong_config_vars.mk b/core/android_soong_config_vars.mk index ac586c5cfe..ca87417eaa 100644 --- a/core/android_soong_config_vars.mk +++ b/core/android_soong_config_vars.mk @@ -128,13 +128,25 @@ endif # are controlled by the MODULE_BUILD_FROM_SOURCE environment variable by # default. INDIVIDUALLY_TOGGLEABLE_PREBUILT_MODULES := \ + adservices \ + appsearch \ btservices \ devicelock \ + configinfrastructure \ + conscrypt \ + healthfitness \ + ipsec \ + media \ + mediaprovider \ + ondevicepersonalization \ permission \ rkpd \ + scheduling \ + sdkext \ + statsd \ + tethering \ uwb \ wifi \ - mediaprovider \ $(foreach m, $(INDIVIDUALLY_TOGGLEABLE_PREBUILT_MODULES),\ $(if $(call soong_config_get,$(m)_module,source_build),,\ diff --git a/core/config.mk b/core/config.mk index c9f752d6d4..c5df45c70f 100644 --- a/core/config.mk +++ b/core/config.mk @@ -407,33 +407,26 @@ ifdef PRODUCT_MAX_PAGE_SIZE_SUPPORTED else ifeq ($(strip $(call is-low-mem-device)),true) # Low memory device will have 4096 binary alignment. TARGET_MAX_PAGE_SIZE_SUPPORTED := 4096 -else - # The default binary alignment for userspace is 4096. +else ifeq ($(call math_lt,$(VSR_VENDOR_API_LEVEL),34),true) TARGET_MAX_PAGE_SIZE_SUPPORTED := 4096 - # When VSR vendor API level >= 34, binary alignment will be 65536. - ifeq ($(call math_gt_or_eq,$(VSR_VENDOR_API_LEVEL),34),true) - ifeq ($(TARGET_ARCH),arm64) - TARGET_MAX_PAGE_SIZE_SUPPORTED := 65536 - endif - endif +else ifeq (,$(filter arm64 x86_64,$(TARGET_ARCH))) + # TARGET_MAX_PAGE_SIZE_SUPPORTED > 4096 is only supported in arm64 and + # x86_64 targets. + TARGET_MAX_PAGE_SIZE_SUPPORTED := 4096 +else + # The default binary alignment for userspace is 16384. + TARGET_MAX_PAGE_SIZE_SUPPORTED := 16384 endif .KATI_READONLY := TARGET_MAX_PAGE_SIZE_SUPPORTED -# Only arm64 and x86_64 archs supports TARGET_MAX_PAGE_SIZE_SUPPORTED greater than 4096. -ifneq ($(TARGET_MAX_PAGE_SIZE_SUPPORTED),4096) - ifeq (,$(filter arm64 x86_64,$(TARGET_ARCH))) - $(error TARGET_MAX_PAGE_SIZE_SUPPORTED=$(TARGET_MAX_PAGE_SIZE_SUPPORTED) is greater than 4096. Only supported in arm64 and x86_64 archs) - endif -endif - # Boolean variable determining if AOSP is page size agnostic. This means # that AOSP can use a kernel configured with 4k/16k/64k PAGE SIZES. TARGET_NO_BIONIC_PAGE_SIZE_MACRO := false ifdef PRODUCT_NO_BIONIC_PAGE_SIZE_MACRO TARGET_NO_BIONIC_PAGE_SIZE_MACRO := $(PRODUCT_NO_BIONIC_PAGE_SIZE_MACRO) ifeq ($(TARGET_NO_BIONIC_PAGE_SIZE_MACRO),true) - ifneq ($(TARGET_MAX_PAGE_SIZE_SUPPORTED),65536) - $(error TARGET_MAX_PAGE_SIZE_SUPPORTED has to be 65536 to support page size agnostic) + ifeq (,$(filter 16384 65536,$(TARGET_MAX_PAGE_SIZE_SUPPORTED))) + $(error TARGET_MAX_PAGE_SIZE_SUPPORTED has to be either 16384 or 65536 to support page size agnostic) endif endif endif @@ -680,11 +673,6 @@ MKBOOTIMG := $(HOST_OUT_EXECUTABLES)/mkbootimg$(HOST_EXECUTABLE_SUFFIX) else MKBOOTIMG := $(BOARD_CUSTOM_MKBOOTIMG) endif -ifeq (,$(strip $(BOARD_CUSTOM_BPTTOOL))) -BPTTOOL := $(HOST_OUT_EXECUTABLES)/bpttool$(HOST_EXECUTABLE_SUFFIX) -else -BPTTOOL := $(BOARD_CUSTOM_BPTTOOL) -endif ifeq (,$(strip $(BOARD_CUSTOM_AVBTOOL))) AVBTOOL := $(HOST_OUT_EXECUTABLES)/avbtool$(HOST_EXECUTABLE_SUFFIX) else diff --git a/core/release_config.mk b/core/release_config.mk index 36882bb0ce..a7b5b3ff38 100644 --- a/core/release_config.mk +++ b/core/release_config.mk @@ -133,6 +133,18 @@ $(foreach f, $(config_map_files), \ ) FLAG_DECLARATION_FILES := +# Verify that all inherited/overridden release configs are declared. +$(foreach config,$(_all_release_configs),\ + $(foreach r,$(all_release_configs.$(r).OVERRIDES),\ + $(if $(strip $(_all_release_configs.$(r).FILES)$(_all_release_configs.$(r).OVERRIDES)),,\ + $(error Release config $(config) [declared in: $(_all_release_configs.$(r).DECLARED_IN)] inherits from non-existent $(r).)\ +))) +# Verify that alias configs do not have config files. +$(foreach r,$(_all_release_configs),\ + $(if $(_all_release_configs.$(r).ALIAS),$(if $(_all_release_configs.$(r).FILES),\ + $(error Alias release config "$(r)" may not specify release config files $(_all_release_configs.$(r).FILES))\ +))) + ifeq ($(TARGET_RELEASE),) # We allow some internal paths to explicitly set TARGET_RELEASE to the # empty string. For the most part, 'make' treats unset and empty string as @@ -148,8 +160,12 @@ ifeq ($(TARGET_RELEASE),) TARGET_RELEASE = trunk_staging endif -ifeq ($(filter $(_all_release_configs), $(TARGET_RELEASE)),) - $(error No release config found for TARGET_RELEASE: $(TARGET_RELEASE). Available releases are: $(_all_release_configs)) +# During pass 1 of product config, using a non-existent release config is not an error. +# We can safely assume that we are doing pass 1 if DUMP_MANY_VARS=="PRODUCT_RELEASE_CONFIG_MAPS". +ifneq (PRODUCT_RELEASE_CONFIG_MAPS,$(DUMP_MANY_VARS)) + ifeq ($(filter $(_all_release_configs), $(TARGET_RELEASE)),) + $(error No release config found for TARGET_RELEASE: $(TARGET_RELEASE). Available releases are: $(_all_release_configs)) + endif endif # Choose flag files @@ -177,7 +193,7 @@ $(call _apply-release-config-overrides,$(TARGET_RELEASE)) define declare-release-config $(error declare-release-config can only be called from inside release_config_map.mk files) endef -define apply-release-config-overrides +define _apply-release-config-overrides $(error invalid use of apply-release-config-overrides) endef @@ -192,12 +208,6 @@ TARGET_RELEASE:= endif .KATI_READONLY := TARGET_RELEASE -# Verify that alias configs do not have config files. -$(foreach r,$(_all_release_configs),\ - $(if $(_all_release_configs.$(r).ALIAS),$(if $(_all_release_configs.$(r).FILES),\ - $(error Alias release config "$(r)" may not specify release config files $(_all_release_configs.$(r).FILES))\ -))) - $(foreach config, $(_all_release_configs), \ $(eval _all_release_configs.$(config).DECLARED_IN:= ) \ $(eval _all_release_configs.$(config).FILES:= ) \ diff --git a/core/release_config.scl b/core/release_config.scl index 629e22328a..728fc1b399 100644 --- a/core/release_config.scl +++ b/core/release_config.scl @@ -53,6 +53,7 @@ _all_flags_schema = { for t in _valid_types ], }, + "origin": {"type": "string"}, "declared_in": {"type": "string"}, }, "optional_keys": { @@ -80,13 +81,14 @@ _all_values_schema = { }, } -def flag(name, partitions, default, *, appends = False): +def flag(name, partitions, default, *, origin = "Unknown", appends = False): """Declare a flag. Args: name: name of the flag partitions: the partitions where this should be recorded. default: the default value of the flag. + origin: The origin of this flag. appends: Whether new values should be append (not replace) the old. Returns: @@ -112,6 +114,7 @@ def flag(name, partitions, default, *, appends = False): "partitions": partitions, "default": default, "appends": appends, + "origin": origin, } def value(name, value): @@ -158,7 +161,10 @@ def equal_flag_declaration(flag, other): for key in "name", "partitions", "default", "appends": if flag[key] != other[key]: return False - return True + # For now, allow Unknown to match any other origin. + if flag["origin"] == "Unknown" or other["origin"] == "Unknown": + return True + return flag["origin"] == other["origin"] def release_config(all_flags, all_values): """Return the make variables that should be set for this release config. @@ -234,5 +240,6 @@ def release_config(all_flags, all_values): result["_ALL_RELEASE_FLAGS." + flag["name"] + ".VALUE"] = val result["_ALL_RELEASE_FLAGS." + flag["name"] + ".DECLARED_IN"] = flag["declared_in"] result["_ALL_RELEASE_FLAGS." + flag["name"] + ".SET_IN"] = set_in + result["_ALL_RELEASE_FLAGS." + flag["name"] + ".ORIGIN"] = flag["origin"] return result diff --git a/envsetup.sh b/envsetup.sh index db21188967..fbe522d866 100644 --- a/envsetup.sh +++ b/envsetup.sh @@ -1084,8 +1084,23 @@ function cproj() echo "can't find Android.mk" } +# Ensure that we're always using the adb in the tree. This works around the fact +# that bash caches $PATH lookups, so if you use adb before lunching/building the +# one in your tree, you'll continue to get /usr/bin/adb or whatever even after +# you have the one from your current tree on your path. Historically this would +# cause confusion because glinux had adb in /usr/bin/ by default, though that +# doesn't appear to be the case on my rodete hosts; it is however still the case +# that my Mac has /usr/local/bin/adb installed by default and on the default +# path. function adb() { - command adb "${@}" + # We need `command which` because zsh has a built-in `which` that's more + # like `type`. + local ADB=$(command which adb) + if [ -z "$ADB" ]; then + echo "Command adb not found; try lunch (and building) first?" + return 1 + fi + $ADB "${@}" } # simplified version of ps; output in the form diff --git a/target/product/base_system.mk b/target/product/base_system.mk index cd45e2a97c..d5876a7901 100644 --- a/target/product/base_system.mk +++ b/target/product/base_system.mk @@ -237,6 +237,7 @@ PRODUCT_PACKAGES += \ PackageInstaller \ passwd_system \ perfetto \ + perfetto-extras \ ping \ ping6 \ pintool \ diff --git a/tools/aconfig/aconfig_storage_read_api/aconfig_storage_read_api.cpp b/tools/aconfig/aconfig_storage_read_api/aconfig_storage_read_api.cpp index 131dc9d98b..ea756b3962 100644 --- a/tools/aconfig/aconfig_storage_read_api/aconfig_storage_read_api.cpp +++ b/tools/aconfig/aconfig_storage_read_api/aconfig_storage_read_api.cpp @@ -18,7 +18,7 @@ namespace aconfig_storage { /// Storage location pb file static constexpr char kAvailableStorageRecordsPb[] = - "/metadata/aconfig/available_storage_file_records.pb"; + "/metadata/aconfig/boot/available_storage_file_records.pb"; /// Read aconfig storage records pb file static Result<storage_records_pb> read_storage_records_pb(std::string const& pb_file) { @@ -74,11 +74,6 @@ static Result<MappedStorageFile> map_storage_file(std::string const& file) { if (fstat(fd, &fd_stat) < 0) { return Error() << "fstat failed"; } - - if ((fd_stat.st_mode & (S_IWUSR | S_IWGRP | S_IWOTH)) != 0) { - return Error() << "cannot map writeable file"; - } - size_t file_size = fd_stat.st_size; void* const map_result = mmap(nullptr, file_size, PROT_READ, MAP_SHARED, fd, 0); diff --git a/tools/aconfig/aconfig_storage_read_api/src/lib.rs b/tools/aconfig/aconfig_storage_read_api/src/lib.rs index 647ca55dfe..8a71480134 100644 --- a/tools/aconfig/aconfig_storage_read_api/src/lib.rs +++ b/tools/aconfig/aconfig_storage_read_api/src/lib.rs @@ -57,18 +57,24 @@ use std::fs::File; use std::io::Read; /// Storage file location pb file -pub const STORAGE_LOCATION_FILE: &str = "/metadata/aconfig/available_storage_file_records.pb"; +pub const STORAGE_LOCATION_FILE: &str = "/metadata/aconfig/boot/available_storage_file_records.pb"; /// Get read only mapped storage files. /// /// \input container: the flag package container /// \input file_type: stoarge file type enum /// \return a result of read only mapped file -pub fn get_mapped_storage_file( +/// +/// # Safety +/// +/// The memory mapped file may have undefined behavior if there are writes to this +/// file after being mapped. Ensure no writes can happen to this file while this +/// mapping stays alive. +pub unsafe fn get_mapped_storage_file( container: &str, file_type: StorageFileType, ) -> Result<Mmap, AconfigStorageError> { - crate::mapped_file::get_mapped_file(STORAGE_LOCATION_FILE, container, file_type) + unsafe { crate::mapped_file::get_mapped_file(STORAGE_LOCATION_FILE, container, file_type) } } /// Get package start offset for flags. @@ -311,10 +317,10 @@ mod tests { use aconfig_storage_file::protos::storage_record_pb::write_proto_to_temp_file; use tempfile::NamedTempFile; - fn create_test_storage_files(read_only: bool) -> [NamedTempFile; 4] { - let package_map = copy_to_temp_file("./tests/package.map", read_only).unwrap(); - let flag_map = copy_to_temp_file("./tests/flag.map", read_only).unwrap(); - let flag_val = copy_to_temp_file("./tests/flag.val", read_only).unwrap(); + fn create_test_storage_files() -> [NamedTempFile; 4] { + let package_map = copy_to_temp_file("./tests/package.map").unwrap(); + let flag_map = copy_to_temp_file("./tests/flag.map").unwrap(); + let flag_val = copy_to_temp_file("./tests/flag.val").unwrap(); let text_proto = format!( r#" @@ -338,10 +344,11 @@ files {{ #[test] // this test point locks down flag package offset query fn test_package_offset_query() { - let [package_map, flag_map, flag_val, pb_file] = create_test_storage_files(true); + let [_package_map, _flag_map, _flag_val, pb_file] = create_test_storage_files(); let pb_file_path = pb_file.path().display().to_string(); - let package_mapped_file = - get_mapped_file(&pb_file_path, "system", StorageFileType::PackageMap).unwrap(); + let package_mapped_file = unsafe { + get_mapped_file(&pb_file_path, "system", StorageFileType::PackageMap).unwrap() + }; let package_offset = get_package_offset(&package_mapped_file, "com.android.aconfig.storage.test_1") @@ -368,10 +375,10 @@ files {{ #[test] // this test point locks down flag offset query fn test_flag_offset_query() { - let [package_map, flag_map, flag_val, pb_file] = create_test_storage_files(true); + let [_package_map, _flag_map, _flag_val, pb_file] = create_test_storage_files(); let pb_file_path = pb_file.path().display().to_string(); let flag_mapped_file = - get_mapped_file(&pb_file_path, "system", StorageFileType::FlagMap).unwrap(); + unsafe { get_mapped_file(&pb_file_path, "system", StorageFileType::FlagMap).unwrap() }; let baseline = vec![ (0, "enabled_ro", 1u16), @@ -393,10 +400,10 @@ files {{ #[test] // this test point locks down flag offset query fn test_flag_value_query() { - let [package_map, flag_map, flag_val, pb_file] = create_test_storage_files(true); + let [_package_map, _flag_map, _flag_val, pb_file] = create_test_storage_files(); let pb_file_path = pb_file.path().display().to_string(); let flag_value_file = - get_mapped_file(&pb_file_path, "system", StorageFileType::FlagVal).unwrap(); + unsafe { get_mapped_file(&pb_file_path, "system", StorageFileType::FlagVal).unwrap() }; let baseline: Vec<bool> = vec![false; 8]; for (offset, expected_value) in baseline.into_iter().enumerate() { let flag_value = get_boolean_flag_value(&flag_value_file, offset as u32).unwrap(); @@ -407,7 +414,6 @@ files {{ #[test] // this test point locks down flag storage file version number query api fn test_storage_version_query() { - let _ro_files = create_test_storage_files(true); assert_eq!(get_storage_file_version("./tests/package.map").unwrap(), 1); assert_eq!(get_storage_file_version("./tests/flag.map").unwrap(), 1); assert_eq!(get_storage_file_version("./tests/flag.val").unwrap(), 1); diff --git a/tools/aconfig/aconfig_storage_read_api/src/mapped_file.rs b/tools/aconfig/aconfig_storage_read_api/src/mapped_file.rs index 09ecdb643d..86c6a1b053 100644 --- a/tools/aconfig/aconfig_storage_read_api/src/mapped_file.rs +++ b/tools/aconfig/aconfig_storage_read_api/src/mapped_file.rs @@ -14,7 +14,7 @@ * limitations under the License. */ -use std::fs::{self, File}; +use std::fs::File; use std::io::{BufReader, Read}; use anyhow::anyhow; @@ -56,26 +56,16 @@ fn find_container_storage_location( Err(StorageFileNotFound(anyhow!("Storage file does not exist for {}", container))) } -/// Verify the file is read only and then map it -fn verify_read_only_and_map(file_path: &str) -> Result<Mmap, AconfigStorageError> { - // ensure file has read only permission - let perms = fs::metadata(file_path).unwrap().permissions(); - if !perms.readonly() { - return Err(MapFileFail(anyhow!("fail to map non read only storage file {}", file_path))); - } - +/// Get the read only memory mapping of a storage file +/// +/// # Safety +/// +/// The memory mapped file may have undefined behavior if there are writes to this +/// file after being mapped. Ensure no writes can happen to this file while this +/// mapping stays alive. +unsafe fn map_file(file_path: &str) -> Result<Mmap, AconfigStorageError> { let file = File::open(file_path) .map_err(|errmsg| FileReadFail(anyhow!("Failed to open file {}: {}", file_path, errmsg)))?; - - // SAFETY: - // - // Mmap constructors are unsafe as it would have undefined behaviors if the file - // is modified after mapped (https://docs.rs/memmap2/latest/memmap2/struct.Mmap.html). - // - // We either have to make this api unsafe or ensure that the file will not be modified - // which means it is read only. Here in the code, we check explicitly that the file - // being mapped must only have read permission, otherwise, error out, thus making sure - // it is safe. unsafe { let mapped_file = Mmap::map(&file).map_err(|errmsg| { MapFileFail(anyhow!("fail to map storage file {}: {}", file_path, errmsg)) @@ -85,16 +75,22 @@ fn verify_read_only_and_map(file_path: &str) -> Result<Mmap, AconfigStorageError } /// Get a mapped storage file given the container and file type -pub fn get_mapped_file( +/// +/// # Safety +/// +/// The memory mapped file may have undefined behavior if there are writes to this +/// file after being mapped. Ensure no writes can happen to this file while this +/// mapping stays alive. +pub unsafe fn get_mapped_file( location_pb_file: &str, container: &str, file_type: StorageFileType, ) -> Result<Mmap, AconfigStorageError> { let files_location = find_container_storage_location(location_pb_file, container)?; match file_type { - StorageFileType::PackageMap => verify_read_only_and_map(files_location.package_map()), - StorageFileType::FlagMap => verify_read_only_and_map(files_location.flag_map()), - StorageFileType::FlagVal => verify_read_only_and_map(files_location.flag_val()), + StorageFileType::PackageMap => unsafe { map_file(files_location.package_map()) }, + StorageFileType::FlagMap => unsafe { map_file(files_location.flag_map()) }, + StorageFileType::FlagVal => unsafe { map_file(files_location.flag_val()) }, } } @@ -155,14 +151,15 @@ files { let mut content = Vec::new(); opened_file.read_to_end(&mut content).unwrap(); - let mmaped_file = get_mapped_file(location_pb_file, "system", file_type).unwrap(); + let mmaped_file = + unsafe { get_mapped_file(location_pb_file, "system", file_type).unwrap() }; assert_eq!(mmaped_file[..], content[..]); } - fn create_test_storage_files(read_only: bool) -> [NamedTempFile; 4] { - let package_map = copy_to_temp_file("./tests/package.map", read_only).unwrap(); - let flag_map = copy_to_temp_file("./tests/flag.map", read_only).unwrap(); - let flag_val = copy_to_temp_file("./tests/package.map", read_only).unwrap(); + fn create_test_storage_files() -> [NamedTempFile; 4] { + let package_map = copy_to_temp_file("./tests/package.map").unwrap(); + let flag_map = copy_to_temp_file("./tests/flag.map").unwrap(); + let flag_val = copy_to_temp_file("./tests/package.map").unwrap(); let text_proto = format!( r#" @@ -185,7 +182,7 @@ files {{ #[test] fn test_mapped_file_contents() { - let [package_map, flag_map, flag_val, pb_file] = create_test_storage_files(true); + let [package_map, flag_map, flag_val, pb_file] = create_test_storage_files(); let pb_file_path = pb_file.path().display().to_string(); map_and_verify( &pb_file_path, @@ -203,35 +200,4 @@ files {{ &flag_val.path().display().to_string(), ); } - - #[test] - fn test_map_non_read_only_file() { - let [package_map, flag_map, flag_val, pb_file] = create_test_storage_files(false); - let pb_file_path = pb_file.path().display().to_string(); - let error = - get_mapped_file(&pb_file_path, "system", StorageFileType::PackageMap).unwrap_err(); - assert_eq!( - format!("{:?}", error), - format!( - "MapFileFail(fail to map non read only storage file {})", - package_map.path().display() - ) - ); - let error = get_mapped_file(&pb_file_path, "system", StorageFileType::FlagMap).unwrap_err(); - assert_eq!( - format!("{:?}", error), - format!( - "MapFileFail(fail to map non read only storage file {})", - flag_map.path().display() - ) - ); - let error = get_mapped_file(&pb_file_path, "system", StorageFileType::FlagVal).unwrap_err(); - assert_eq!( - format!("{:?}", error), - format!( - "MapFileFail(fail to map non read only storage file {})", - flag_val.path().display() - ) - ); - } } diff --git a/tools/aconfig/aconfig_storage_read_api/src/test_utils.rs b/tools/aconfig/aconfig_storage_read_api/src/test_utils.rs index ff724990a9..84f31aa710 100644 --- a/tools/aconfig/aconfig_storage_read_api/src/test_utils.rs +++ b/tools/aconfig/aconfig_storage_read_api/src/test_utils.rs @@ -19,14 +19,8 @@ use std::fs; use tempfile::NamedTempFile; /// Create temp file copy -pub(crate) fn copy_to_temp_file(source_file: &str, read_only: bool) -> Result<NamedTempFile> { +pub(crate) fn copy_to_temp_file(source_file: &str) -> Result<NamedTempFile> { let file = NamedTempFile::new()?; fs::copy(source_file, file.path())?; - if read_only { - let file_name = file.path().display().to_string(); - let mut perms = fs::metadata(file_name).unwrap().permissions(); - perms.set_readonly(true); - fs::set_permissions(file.path(), perms.clone()).unwrap(); - } Ok(file) } diff --git a/tools/aconfig/aconfig_storage_read_api/tests/storage_read_api_test.cpp b/tools/aconfig/aconfig_storage_read_api/tests/storage_read_api_test.cpp index 377395a419..1d36aae8cb 100644 --- a/tools/aconfig/aconfig_storage_read_api/tests/storage_read_api_test.cpp +++ b/tools/aconfig/aconfig_storage_read_api/tests/storage_read_api_test.cpp @@ -33,7 +33,7 @@ namespace private_api = aconfig_storage::private_internal_api; class AconfigStorageTest : public ::testing::Test { protected: - Result<std::string> copy_to_ro_temp_file(std::string const& source_file) { + Result<std::string> copy_to_temp_file(std::string const& source_file) { auto temp_file = std::string(std::tmpnam(nullptr)); auto content = std::string(); if (!ReadFileToString(source_file, &content)) { @@ -42,9 +42,6 @@ class AconfigStorageTest : public ::testing::Test { if (!WriteStringToFile(content, temp_file)) { return Error() << "failed to copy file: " << source_file; } - if (chmod(temp_file.c_str(), S_IRUSR | S_IRGRP | S_IROTH) == -1) { - return Error() << "failed to make file read only"; - } return temp_file; } @@ -71,9 +68,9 @@ class AconfigStorageTest : public ::testing::Test { void SetUp() override { auto const test_dir = android::base::GetExecutableDirectory(); - package_map = *copy_to_ro_temp_file(test_dir + "/package.map"); - flag_map = *copy_to_ro_temp_file(test_dir + "/flag.map"); - flag_val = *copy_to_ro_temp_file(test_dir + "/flag.val"); + package_map = *copy_to_temp_file(test_dir + "/package.map"); + flag_map = *copy_to_temp_file(test_dir + "/flag.map"); + flag_val = *copy_to_temp_file(test_dir + "/flag.val"); storage_record_pb = *write_storage_location_pb_file( package_map, flag_map, flag_val); } @@ -113,27 +110,6 @@ TEST_F(AconfigStorageTest, test_none_exist_storage_file_mapping) { "Unable to find storage files for container vendor"); } -/// Negative test to lock down the error when mapping a writeable storage file -TEST_F(AconfigStorageTest, test_writable_storage_file_mapping) { - ASSERT_TRUE(chmod(package_map.c_str(), 0666) != -1); - auto mapped_file = private_api::get_mapped_file_impl( - storage_record_pb, "system", api::StorageFileType::package_map); - ASSERT_FALSE(mapped_file.ok()); - ASSERT_EQ(mapped_file.error().message(), "cannot map writeable file"); - - ASSERT_TRUE(chmod(flag_map.c_str(), 0666) != -1); - mapped_file = private_api::get_mapped_file_impl( - storage_record_pb, "system", api::StorageFileType::flag_map); - ASSERT_FALSE(mapped_file.ok()); - ASSERT_EQ(mapped_file.error().message(), "cannot map writeable file"); - - ASSERT_TRUE(chmod(flag_val.c_str(), 0666) != -1); - mapped_file = private_api::get_mapped_file_impl( - storage_record_pb, "system", api::StorageFileType::flag_val); - ASSERT_FALSE(mapped_file.ok()); - ASSERT_EQ(mapped_file.error().message(), "cannot map writeable file"); -} - /// Test to lock down storage package offset query api TEST_F(AconfigStorageTest, test_package_offset_query) { auto mapped_file = private_api::get_mapped_file_impl( diff --git a/tools/aconfig/aconfig_storage_read_api/tests/storage_read_api_test.rs b/tools/aconfig/aconfig_storage_read_api/tests/storage_read_api_test.rs index eb4d54d77a..afcd5a7f3c 100644 --- a/tools/aconfig/aconfig_storage_read_api/tests/storage_read_api_test.rs +++ b/tools/aconfig/aconfig_storage_read_api/tests/storage_read_api_test.rs @@ -9,20 +9,16 @@ mod aconfig_storage_rust_test { use std::fs; use tempfile::NamedTempFile; - pub fn copy_to_ro_temp_file(source_file: &str) -> NamedTempFile { + pub fn copy_to_temp_file(source_file: &str) -> NamedTempFile { let file = NamedTempFile::new().unwrap(); fs::copy(source_file, file.path()).unwrap(); - let file_name = file.path().display().to_string(); - let mut perms = fs::metadata(file_name).unwrap().permissions(); - perms.set_readonly(true); - fs::set_permissions(file.path(), perms.clone()).unwrap(); file } fn create_test_storage_files() -> [NamedTempFile; 4] { - let package_map = copy_to_ro_temp_file("./package.map"); - let flag_map = copy_to_ro_temp_file("./flag.map"); - let flag_val = copy_to_ro_temp_file("./flag.val"); + let package_map = copy_to_temp_file("./package.map"); + let flag_map = copy_to_temp_file("./flag.map"); + let flag_val = copy_to_temp_file("./flag.val"); let text_proto = format!( r#" @@ -47,8 +43,11 @@ files {{ fn test_unavailable_stoarge() { let [_package_map, _flag_map, _flag_val, pb_file] = create_test_storage_files(); let pb_file_path = pb_file.path().display().to_string(); - let err = - get_mapped_file(&pb_file_path, "vendor", StorageFileType::PackageMap).unwrap_err(); + // SAFETY: + // The safety here is ensured as the test process will not write to temp storage file + let err = unsafe { + get_mapped_file(&pb_file_path, "vendor", StorageFileType::PackageMap).unwrap_err() + }; assert_eq!( format!("{:?}", err), "StorageFileNotFound(Storage file does not exist for vendor)" @@ -59,8 +58,11 @@ files {{ fn test_package_offset_query() { let [_package_map, _flag_map, _flag_val, pb_file] = create_test_storage_files(); let pb_file_path = pb_file.path().display().to_string(); - let package_mapped_file = - get_mapped_file(&pb_file_path, "system", StorageFileType::PackageMap).unwrap(); + // SAFETY: + // The safety here is ensured as the test process will not write to temp storage file + let package_mapped_file = unsafe { + get_mapped_file(&pb_file_path, "system", StorageFileType::PackageMap).unwrap() + }; let package_offset = get_package_offset(&package_mapped_file, "com.android.aconfig.storage.test_1") @@ -88,8 +90,12 @@ files {{ fn test_none_exist_package_offset_query() { let [_package_map, _flag_map, _flag_val, pb_file] = create_test_storage_files(); let pb_file_path = pb_file.path().display().to_string(); - let package_mapped_file = - get_mapped_file(&pb_file_path, "system", StorageFileType::PackageMap).unwrap(); + // SAFETY: + // The safety here is ensured as the test process will not write to temp storage file + let package_mapped_file = unsafe { + get_mapped_file(&pb_file_path, "system", StorageFileType::PackageMap).unwrap() + }; + let package_offset_option = get_package_offset(&package_mapped_file, "com.android.aconfig.storage.test_3").unwrap(); assert_eq!(package_offset_option, None); @@ -99,8 +105,10 @@ files {{ fn test_flag_offset_query() { let [_package_map, _flag_map, _flag_val, pb_file] = create_test_storage_files(); let pb_file_path = pb_file.path().display().to_string(); + // SAFETY: + // The safety here is ensured as the test process will not write to temp storage file let flag_mapped_file = - get_mapped_file(&pb_file_path, "system", StorageFileType::FlagMap).unwrap(); + unsafe { get_mapped_file(&pb_file_path, "system", StorageFileType::FlagMap).unwrap() }; let baseline = vec![ (0, "enabled_ro", 1u16), @@ -123,9 +131,10 @@ files {{ fn test_none_exist_flag_offset_query() { let [_package_map, _flag_map, _flag_val, pb_file] = create_test_storage_files(); let pb_file_path = pb_file.path().display().to_string(); + // SAFETY: + // The safety here is ensured as the test process will not write to temp storage file let flag_mapped_file = - get_mapped_file(&pb_file_path, "system", StorageFileType::FlagMap).unwrap(); - + unsafe { get_mapped_file(&pb_file_path, "system", StorageFileType::FlagMap).unwrap() }; let flag_offset_option = get_flag_offset(&flag_mapped_file, 0, "none_exist").unwrap(); assert_eq!(flag_offset_option, None); @@ -137,9 +146,10 @@ files {{ fn test_boolean_flag_value_query() { let [_package_map, _flag_map, _flag_val, pb_file] = create_test_storage_files(); let pb_file_path = pb_file.path().display().to_string(); + // SAFETY: + // The safety here is ensured as the test process will not write to temp storage file let flag_value_file = - get_mapped_file(&pb_file_path, "system", StorageFileType::FlagVal).unwrap(); - + unsafe { get_mapped_file(&pb_file_path, "system", StorageFileType::FlagVal).unwrap() }; let baseline: Vec<bool> = vec![false; 8]; for (offset, expected_value) in baseline.into_iter().enumerate() { let flag_value = get_boolean_flag_value(&flag_value_file, offset as u32).unwrap(); @@ -151,9 +161,10 @@ files {{ fn test_invalid_boolean_flag_value_query() { let [_package_map, _flag_map, _flag_val, pb_file] = create_test_storage_files(); let pb_file_path = pb_file.path().display().to_string(); + // SAFETY: + // The safety here is ensured as the test process will not write to temp storage file let flag_value_file = - get_mapped_file(&pb_file_path, "system", StorageFileType::FlagVal).unwrap(); - + unsafe { get_mapped_file(&pb_file_path, "system", StorageFileType::FlagVal).unwrap() }; let err = get_boolean_flag_value(&flag_value_file, 8u32).unwrap_err(); assert_eq!( format!("{:?}", err), diff --git a/tools/ide_query/cc_analyzer/analyzer.cc b/tools/ide_query/cc_analyzer/analyzer.cc index 8cc07e81ba..bb7ca0b5bc 100644 --- a/tools/ide_query/cc_analyzer/analyzer.cc +++ b/tools/ide_query/cc_analyzer/analyzer.cc @@ -117,7 +117,7 @@ llvm::Expected<std::unique_ptr<clang::tooling::CompilationDatabase>> LoadCompDB( result.mutable_status()->set_code(::ide_query::Status::FAILURE); result.mutable_status()->set_message("Command working dir " + working_dir.str() + - "outside repository " + repo_dir); + " outside repository " + repo_dir); continue; } working_dir = working_dir.ltrim('/'); diff --git a/tools/ide_query/ide_query.go b/tools/ide_query/ide_query.go index 9602ac8f48..50264fd180 100644 --- a/tools/ide_query/ide_query.go +++ b/tools/ide_query/ide_query.go @@ -121,7 +121,7 @@ func main() { log.Fatalf("Failed to query cc targets: %v", *status.Message) } toMake = append(toMake, ccTargets...) - fmt.Printf("Running make for modules: %v\n", strings.Join(toMake, ", ")) + fmt.Fprintf(os.Stderr, "Running make for modules: %v\n", strings.Join(toMake, ", ")) if err := runMake(ctx, env, toMake...); err != nil { log.Printf("Building deps failed: %v", err) } @@ -136,13 +136,13 @@ func main() { log.Fatalf("Failed to marshal result proto: %v", err) } - err = os.WriteFile(path.Join(env.RepoDir, env.OutDir, "ide_query.pb"), data, 0644) + _, err = os.Stdout.Write(data) if err != nil { log.Fatalf("Failed to write result proto: %v", err) } for _, s := range res.Sources { - fmt.Printf("%s: %v (Deps: %d, Generated: %d)\n", s.GetPath(), s.GetStatus(), len(s.GetDeps()), len(s.GetGenerated())) + fmt.Fprintf(os.Stderr, "%s: %v (Deps: %d, Generated: %d)\n", s.GetPath(), s.GetStatus(), len(s.GetDeps()), len(s.GetGenerated())) } } @@ -312,7 +312,7 @@ func runMake(ctx context.Context, env Env, modules ...string) error { args = append(args, modules...) cmd := exec.CommandContext(ctx, "build/soong/soong_ui.bash", args...) cmd.Dir = env.RepoDir - cmd.Stdout = os.Stdout + cmd.Stdout = os.Stderr cmd.Stderr = os.Stderr return cmd.Run() } diff --git a/tools/releasetools/build_image.py b/tools/releasetools/build_image.py index a7b1d8c37c..464ad9b4cc 100755 --- a/tools/releasetools/build_image.py +++ b/tools/releasetools/build_image.py @@ -294,7 +294,7 @@ def BuildImageMkfs(in_dir, prop_dict, out_file, target_out, fs_config): build_command = [prop_dict["ext_mkuserimg"]] if "extfs_sparse_flag" in prop_dict and not disable_sparse: build_command.append(prop_dict["extfs_sparse_flag"]) - run_e2fsck = RunE2fsck + run_fsck = RunE2fsck build_command.extend([in_dir, out_file, fs_type, prop_dict["mount_point"]]) build_command.append(prop_dict["image_size"]) diff --git a/tools/releasetools/common.py b/tools/releasetools/common.py index 8a8a613c78..88362489fd 100644 --- a/tools/releasetools/common.py +++ b/tools/releasetools/common.py @@ -1313,7 +1313,11 @@ def MergeDynamicPartitionInfoDicts(framework_dict, vendor_dict): key = "super_%s_partition_list" % partition_group merged_dict[key] = uniq_concat( framework_dict.get(key, ""), vendor_dict.get(key, "")) - + # in the case that vendor is on s build, but is taking a v3 -> v3 vabc ota, we want to fallback to v2 + if "vabc_cow_version" not in vendor_dict or "vabc_cow_version" not in framework_dict: + merged_dict["vabc_cow_version"] = '2' + else: + merged_dict["vabc_cow_version"] = min(vendor_dict["vabc_cow_version"], framework_dict["vabc_cow_version"]) # Various other flags should be copied from the vendor dict, if defined. for key in ("virtual_ab", "virtual_ab_retrofit", "lpmake", "super_metadata_device", "super_partition_error_limit", @@ -2456,7 +2460,7 @@ def GetMinSdkVersion(apk_name): m = re.match(r'(?:minSdkVersion|sdkVersion):\'([^\']*)\'', line) if m: return m.group(1) - raise ExternalError("No minSdkVersion returned by aapt2") + raise ExternalError("No minSdkVersion returned by aapt2 for apk: {}".format(apk_name)) def GetMinSdkVersionInt(apk_name, codename_to_api_level_map): diff --git a/tools/releasetools/create_brick_ota.py b/tools/releasetools/create_brick_ota.py index f290323c43..9e040a53d4 100644 --- a/tools/releasetools/create_brick_ota.py +++ b/tools/releasetools/create_brick_ota.py @@ -45,13 +45,17 @@ def CreateBrickOta(product_name: str, output_path: Path, extra_wipe_partitions: partitions_to_wipe = PARTITIONS_TO_WIPE if extra_wipe_partitions is not None: partitions_to_wipe = PARTITIONS_TO_WIPE + extra_wipe_partitions.split(",") + ota_metadata = ["ota-type=BRICK", "post-timestamp=9999999999", + "pre-device=" + product_name] + if serialno is not None: + ota_metadata.append("serialno=" + serialno) # recovery requiers product name to be a | separated list product_name = product_name.replace(",", "|") with zipfile.ZipFile(output_path, "w") as zfp: zfp.writestr("recovery.wipe", "\n".join(partitions_to_wipe)) zfp.writestr("payload.bin", "") zfp.writestr("META-INF/com/android/metadata", "\n".join( - ["ota-type=BRICK", "post-timestamp=9999999999", "pre-device=" + product_name, "serialno=" + serialno])) + ota_metadata)) def main(argv): diff --git a/tools/releasetools/test_common.py b/tools/releasetools/test_common.py index 2989338fe8..89933a00fc 100644 --- a/tools/releasetools/test_common.py +++ b/tools/releasetools/test_common.py @@ -1515,6 +1515,7 @@ class CommonUtilsTest(test_utils.ReleaseToolsTestCase): 'super_group_a_group_size': '1000', 'super_group_b_partition_list': 'product', 'super_group_b_group_size': '2000', + 'vabc_cow_version': '2', } self.assertEqual(merged_dict, expected_merged_dict) @@ -1525,6 +1526,7 @@ class CommonUtilsTest(test_utils.ReleaseToolsTestCase): 'dynamic_partition_list': 'system', 'super_group_a_partition_list': 'system', 'super_group_a_group_size': '5000', + 'vabc_cow_version': '3', } vendor_dict = { 'use_dynamic_partitions': 'true', @@ -1546,6 +1548,7 @@ class CommonUtilsTest(test_utils.ReleaseToolsTestCase): 'super_group_a_group_size': '1000', 'super_group_b_partition_list': 'product', 'super_group_b_group_size': '2000', + 'vabc_cow_version': '2', } self.assertEqual(merged_dict, expected_merged_dict) |