From ab5457c038aee6ef785e02ef8d19236208f74e3d Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Fri, 6 Sep 2024 01:38:58 +0000 Subject: [PATCH 1/9] Export USE_CAMERA_V4L2_HAL to soong Export the variable for hardware/libhardware/modules/camera/3_4:camera.v4l2 Bug: 349300092 Test: lunch aosp_cheetah-trunk_staging-userdebug; \ USE_CAMERA_V4L2_HAL=true m camera.v4l2 camera.v4l2_test Change-Id: I590e5218dbe51af004a15d2da4456da7515e72dc --- core/android_soong_config_vars.mk | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/android_soong_config_vars.mk b/core/android_soong_config_vars.mk index 5fc8fd4027..314f89f7d4 100644 --- a/core/android_soong_config_vars.mk +++ b/core/android_soong_config_vars.mk @@ -182,3 +182,6 @@ endif # Add target_use_pan_display flag for hardware/libhardware:gralloc.default $(call soong_config_set_bool,gralloc,target_use_pan_display,$(if $(filter true,$(TARGET_USE_PAN_DISPLAY)),true,false)) + +# Add use_camera_v4l2_hal flag for hardware/libhardware/modules/camera/3_4:camera.v4l2 +$(call soong_config_set_bool,camera,use_camera_v4l2_hal,$(if $(filter true,$(USE_CAMERA_V4L2_HAL)),true,false)) From 1e9faa2bc926ed9ee4cb9e6c3cb61a5887ac135e Mon Sep 17 00:00:00 2001 From: Shreshta Manu Date: Wed, 11 Sep 2024 20:35:01 +0000 Subject: [PATCH 2/9] [Ranging] Add ranging to build Bug: 364930449 Test: compiles Change-Id: I35af9f96715be1366fba7041b65094e5a8c04eb0 --- target/product/base_system.mk | 5 +++++ target/product/default_art_config.mk | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/target/product/base_system.mk b/target/product/base_system.mk index 5b54051303..ab23397cc4 100644 --- a/target/product/base_system.mk +++ b/target/product/base_system.mk @@ -346,6 +346,11 @@ ifeq ($(RELEASE_USE_WEBVIEW_BOOTSTRAP_MODULE),true) com.android.webview.bootstrap endif +ifneq (,$(RELEASE_RANGING_STACK)) + PRODUCT_PACKAGES += \ + com.android.ranging +endif + # VINTF data for system image PRODUCT_PACKAGES += \ system_manifest.xml \ diff --git a/target/product/default_art_config.mk b/target/product/default_art_config.mk index 1a3f2cf0e8..4c9eda8f8a 100644 --- a/target/product/default_art_config.mk +++ b/target/product/default_art_config.mk @@ -113,6 +113,12 @@ ifeq ($(RELEASE_PACKAGE_PROFILING_MODULE),true) endif +ifneq (,$(RELEASE_RANGING_STACK)) + PRODUCT_APEX_BOOT_JARS += \ + com.android.uwb:framework-ranging \ + $(call soong_config_set,bootclasspath,release_ranging_stack,true) +endif + # List of system_server classpath jars delivered via apex. # Keep the list sorted by module names and then library names. # Note: For modules available in Q, DO NOT add new entries here. From 420237dc41767e5a54654a406fdcce456cf7a93e Mon Sep 17 00:00:00 2001 From: Shreshta Manu Date: Wed, 18 Sep 2024 03:27:54 +0000 Subject: [PATCH 3/9] [Ranging] Add service-ranging to build Bug: 364930449 Test: compiles Change-Id: Ib14d0434a3e40e8dee76eea647051816dc8a740c --- target/product/default_art_config.mk | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/target/product/default_art_config.mk b/target/product/default_art_config.mk index 4c9eda8f8a..a50d9d6956 100644 --- a/target/product/default_art_config.mk +++ b/target/product/default_art_config.mk @@ -174,6 +174,11 @@ ifeq ($(RELEASE_PACKAGE_PROFILING_MODULE),true) endif +ifneq (,$(RELEASE_RANGING_STACK)) + PRODUCT_APEX_STANDALONE_SYSTEM_SERVER_JARS += \ + com.android.uwb:service-ranging +endif + # Overrides the (apex, jar) pairs above when determining the on-device location. The format is: # ::: PRODUCT_CONFIGURED_JAR_LOCATION_OVERRIDES := \ From e4c1ec94664761d546a0ae12a487e04afd400dab Mon Sep 17 00:00:00 2001 From: Wei Li Date: Wed, 18 Sep 2024 20:15:31 +0000 Subject: [PATCH 4/9] Add command line tool that generates NOTICE.xml.gz for partitions. The tool currently generates a XML file with the root element only and its content will be filled in in following CLs. Also disable the generation of NOTICE.xml.gz in make when USE_SOONG_DEFINED_SYSTEM_IMAGE is true, so the Soong module could be used without conflict. Bug: 330949782 Bug: 338342381 Test: lunch aosp_cf_x86_64_phone-trunk_staging-eng && m, and check that system/etc/NOTICE.xml.gz have all the XML elements. Test: lunch aosp_cf_x86_64_phone_soong_system-trunk_staging-eng && m, and check that system/etc/NOTICE.xml.gz has root element only. Change-Id: I82e90bd9aa3dabc605acfe8da697ab1f7e7ecf9b --- core/Makefile | 4 +- core/os_licensing.mk | 4 ++ tools/sbom/Android.bp | 14 +++++++ tools/sbom/gen_notice_xml.py | 81 ++++++++++++++++++++++++++++++++++++ 4 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 tools/sbom/gen_notice_xml.py diff --git a/core/Makefile b/core/Makefile index b0392cdc8b..0bc6f1fc9e 100644 --- a/core/Makefile +++ b/core/Makefile @@ -1964,7 +1964,7 @@ target_system_dlkm_notice_file_xml_gz := $(TARGET_OUT_INTERMEDIATES)/NOTICE_SYST installed_system_dlkm_notice_xml_gz := $(TARGET_OUT_SYSTEM_DLKM)/etc/NOTICE.xml.gz ALL_INSTALLED_NOTICE_FILES := \ - $(installed_notice_html_or_xml_gz) \ + $(if $(USE_SOONG_DEFINED_SYSTEM_IMAGE),,$(installed_notice_html_or_xml_gz)) \ $(installed_vendor_notice_xml_gz) \ $(installed_product_notice_xml_gz) \ $(installed_system_ext_notice_xml_gz) \ @@ -2051,7 +2051,9 @@ endif endif # PRODUCT_NOTICE_SPLIT +ifneq ($(USE_SOONG_DEFINED_SYSTEM_IMAGE),true) ALL_DEFAULT_INSTALLED_MODULES += $(installed_notice_html_or_xml_gz) +endif need_vendor_notice:=false ifeq ($(BUILDING_VENDOR_BOOT_IMAGE),true) diff --git a/core/os_licensing.mk b/core/os_licensing.mk index 1e1b7df7a9..d15a3d0715 100644 --- a/core/os_licensing.mk +++ b/core/os_licensing.mk @@ -17,13 +17,17 @@ $(eval $(call xml-notice-rule,$(target_notice_file_xml_gz),"System image",$(syst $(eval $(call text-notice-rule,$(target_notice_file_txt),"System image",$(system_notice_file_message),$(SYSTEM_NOTICE_DEPS),$(SYSTEM_NOTICE_DEPS))) +ifneq ($(USE_SOONG_DEFINED_SYSTEM_IMAGE),true) $(installed_notice_html_or_xml_gz): $(target_notice_file_xml_gz) $(copy-file-to-target) endif +endif $(call declare-1p-target,$(target_notice_file_xml_gz)) +ifneq ($(USE_SOONG_DEFINED_SYSTEM_IMAGE),true) $(call declare-1p-target,$(installed_notice_html_or_xml_gz)) endif +endif .PHONY: vendorlicense vendorlicense: $(call corresponding-license-metadata, $(VENDOR_NOTICE_DEPS)) reportmissinglicenses diff --git a/tools/sbom/Android.bp b/tools/sbom/Android.bp index 6901b06720..74b3d626f6 100644 --- a/tools/sbom/Android.bp +++ b/tools/sbom/Android.bp @@ -109,3 +109,17 @@ python_binary_host { "sbom_lib", ], } + +python_binary_host { + name: "gen_notice_xml", + srcs: [ + "gen_notice_xml.py", + ], + version: { + py3: { + embedded_launcher: true, + }, + }, + libs: [ + ], +} diff --git a/tools/sbom/gen_notice_xml.py b/tools/sbom/gen_notice_xml.py new file mode 100644 index 0000000000..eaa6e5a74d --- /dev/null +++ b/tools/sbom/gen_notice_xml.py @@ -0,0 +1,81 @@ +# !/usr/bin/env python3 +# +# Copyright (C) 2024 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. + +""" +Generate NOTICE.xml.gz of a partition. +Usage example: + gen_notice_xml.py --output_file out/soong/.intermediate/.../NOTICE.xml.gz \ + --metadata out/soong/compliance-metadata/aosp_cf_x86_64_phone/compliance-metadata.db \ + --partition system \ + --product_out out/target/vsoc_x86_64 \ + --soong_out out/soong +""" + +import argparse + + +FILE_HEADER = '''\ + + +''' +FILE_FOOTER = '''\ + +''' + + +def get_args(): + parser = argparse.ArgumentParser() + parser.add_argument('-v', '--verbose', action='store_true', default=False, help='Print more information.') + parser.add_argument('-d', '--debug', action='store_true', default=True, help='Debug mode') + parser.add_argument('--output_file', required=True, help='The path of the generated NOTICE.xml.gz file.') + parser.add_argument('--partition', required=True, help='The name of partition for which the NOTICE.xml.gz is generated.') + parser.add_argument('--metadata', required=True, help='The path of compliance metadata DB file.') + parser.add_argument('--product_out', required=True, help='The path of PRODUCT_OUT, e.g. out/target/product/vsoc_x86_64.') + parser.add_argument('--soong_out', required=True, help='The path of Soong output directory, e.g. out/soong') + + return parser.parse_args() + + +def log(*info): + if args.verbose: + for i in info: + print(i) + + +def new_file_name_tag(file_metadata, package_name): + file_path = file_metadata['installed_file'].removeprefix(args.product_out) + lib = 'Android' + if package_name: + lib = package_name + return f'{file_path}\n' + + +def new_file_content_tag(): + pass + + +def main(): + global args + args = get_args() + log('Args:', vars(args)) + + with open(args.output_file, 'w', encoding="utf-8") as notice_xml_file: + notice_xml_file.write(FILE_HEADER) + notice_xml_file.write(FILE_FOOTER) + + +if __name__ == '__main__': + main() From 01d6bb3aae26bce1a65a5de4f06cc3215b2964c7 Mon Sep 17 00:00:00 2001 From: William Escande Date: Wed, 18 Sep 2024 18:01:20 -0700 Subject: [PATCH 5/9] aconfig: cpp test-mode lib Incompatible interface between test-mode and normal mode. Bluetooth is linking the normal flag for the code that is shipped, and it is linking the test-mode library for unit testing. While trying to move some test to use the test-mode, we noticed some failure as the flag returned were not the one expected. It appear the code and the test are built against 2 different library, but linked against one. The provide interface only contains getter in normal mode, and it is alternating between getter/setter in test-mode. By moving the setter in test-mode toward the end of the interface, we are not modifying the assumed order for the code built against the normal mode. This CL is a short term fix, to make the test goes green and prevent immediat failure. An refactor of the cpp generated code should be done to avoid using 2 different version of the same symbol in 2 different libs. Bug: 311772251 Test: atest aconfig.test Change-Id: I97f6523452c451d005d2e92bfa1ef861611ab840 --- tools/aconfig/aconfig/src/codegen/cpp.rs | 32 +++++-------------- .../templates/cpp_exported_header.template | 7 ++-- 2 files changed, 12 insertions(+), 27 deletions(-) diff --git a/tools/aconfig/aconfig/src/codegen/cpp.rs b/tools/aconfig/aconfig/src/codegen/cpp.rs index 2c569da8f6..7a9c382bc7 100644 --- a/tools/aconfig/aconfig/src/codegen/cpp.rs +++ b/tools/aconfig/aconfig/src/codegen/cpp.rs @@ -283,39 +283,23 @@ public: virtual ~flag_provider_interface() = default; virtual bool disabled_ro() = 0; - - virtual void disabled_ro(bool val) = 0; - virtual bool disabled_rw() = 0; - - virtual void disabled_rw(bool val) = 0; - virtual bool disabled_rw_exported() = 0; - - virtual void disabled_rw_exported(bool val) = 0; - virtual bool disabled_rw_in_other_namespace() = 0; - - virtual void disabled_rw_in_other_namespace(bool val) = 0; - virtual bool enabled_fixed_ro() = 0; - - virtual void enabled_fixed_ro(bool val) = 0; - virtual bool enabled_fixed_ro_exported() = 0; - - virtual void enabled_fixed_ro_exported(bool val) = 0; - virtual bool enabled_ro() = 0; - - virtual void enabled_ro(bool val) = 0; - virtual bool enabled_ro_exported() = 0; - - virtual void enabled_ro_exported(bool val) = 0; - virtual bool enabled_rw() = 0; + virtual void disabled_ro(bool val) = 0; + virtual void disabled_rw(bool val) = 0; + virtual void disabled_rw_exported(bool val) = 0; + virtual void disabled_rw_in_other_namespace(bool val) = 0; + virtual void enabled_fixed_ro(bool val) = 0; + virtual void enabled_fixed_ro_exported(bool val) = 0; + virtual void enabled_ro(bool val) = 0; + virtual void enabled_ro_exported(bool val) = 0; virtual void enabled_rw(bool val) = 0; virtual void reset_flags() {} diff --git a/tools/aconfig/aconfig/templates/cpp_exported_header.template b/tools/aconfig/aconfig/templates/cpp_exported_header.template index 0f7853e405..4643c9775c 100644 --- a/tools/aconfig/aconfig/templates/cpp_exported_header.template +++ b/tools/aconfig/aconfig/templates/cpp_exported_header.template @@ -27,12 +27,13 @@ public: {{ -for item in class_elements}} virtual bool {item.flag_name}() = 0; - {{ -if is_test_mode }} - virtual void {item.flag_name}(bool val) = 0; - {{ -endif }} {{ -endfor }} {{ -if is_test_mode }} + {{ -for item in class_elements}} + virtual void {item.flag_name}(bool val) = 0; + {{ -endfor }} + virtual void reset_flags() \{} {{ -endif }} }; From 769d8eed378d468be6c8d840343772510752e4f1 Mon Sep 17 00:00:00 2001 From: Marybeth Fair Date: Thu, 5 Sep 2024 11:38:50 -0400 Subject: [PATCH 6/9] Add fingerprint to packages.map. No guards to this change because we will guard actually writing the fingerprint, and right now new storage is not in trunkfood yet. This change modifies the package map file structure. Note that if the new storage was in trunkfood, this could (theoretically) cause issues if there were cross-container READ_WRITE flags (not permitted per documentation) and if the containers were built at separate aconfig versions (ie before and after this change). Adding the fingerprint will help prevent such issues in the future. Incremented the storage version number as I've changed the format. Again, fingerprint is not actually written in this CL, it always has a value of 0. Updated the test files as well to have the new version and the fingerprint. Since this changed the package node size, some of the information in the buckets there (offset) has changed as well. Also added a test util for flags from another package to test future changes. Bug: 316357686 Test: atest aconfig.test Change-Id: I09e10808492f241fe78028d2757f7d63328623c3 --- tools/aconfig/aconfig/Android.bp | 8 ++ tools/aconfig/aconfig/src/commands.rs | 74 +++++++++++++----- tools/aconfig/aconfig/src/storage/mod.rs | 9 ++- .../aconfig/src/storage/package_table.rs | 1 + tools/aconfig/aconfig/src/test.rs | 18 +++++ tools/aconfig/aconfig/tests/test.aconfig | 2 +- .../aconfig/tests/test_second_package.aconfig | 12 +++ tools/aconfig/aconfig/tests/third.values | 6 ++ .../aconfig_storage_file/src/flag_info.rs | 2 +- .../aconfig_storage_file/src/flag_table.rs | 2 +- .../aconfig_storage_file/src/flag_value.rs | 2 +- tools/aconfig/aconfig_storage_file/src/lib.rs | 12 ++- .../aconfig_storage_file/src/package_table.rs | 18 ++++- .../aconfig_storage_file/src/test_utils.rs | 17 ++-- .../aconfig/storage/ByteBufferReader.java | 4 + .../android/aconfig/storage/PackageTable.java | 7 ++ .../aconfig_storage_file/tests/flag.info | Bin 35 -> 35 bytes .../aconfig_storage_file/tests/flag.map | Bin 321 -> 321 bytes .../aconfig_storage_file/tests/flag.val | Bin 35 -> 35 bytes .../aconfig_storage_file/tests/package.map | Bin 209 -> 233 bytes .../tests/srcs/FlagTableTest.java | 2 +- .../tests/srcs/FlagValueListTest.java | 2 +- .../tests/srcs/PackageTableTest.java | 59 +++++++------- .../aconfig_storage_read_api/src/lib.rs | 8 +- .../aconfig_storage_read_api/tests/flag.info | Bin 35 -> 35 bytes .../aconfig_storage_read_api/tests/flag.map | Bin 321 -> 321 bytes .../aconfig_storage_read_api/tests/flag.val | Bin 35 -> 35 bytes .../tests/package.map | Bin 209 -> 233 bytes .../tests/storage_read_api_test.cpp | 8 +- .../tests/storage_read_api_test.rs | 8 +- 30 files changed, 201 insertions(+), 80 deletions(-) create mode 100644 tools/aconfig/aconfig/tests/test_second_package.aconfig create mode 100644 tools/aconfig/aconfig/tests/third.values diff --git a/tools/aconfig/aconfig/Android.bp b/tools/aconfig/aconfig/Android.bp index 68521af91f..5037783fb5 100644 --- a/tools/aconfig/aconfig/Android.bp +++ b/tools/aconfig/aconfig/Android.bp @@ -68,6 +68,14 @@ aconfig_values { ], } +aconfig_values { + name: "aconfig.test.flag.second_values", + package: "com.android.aconfig.test", + srcs: [ + "tests/third.values", + ], +} + aconfig_value_set { name: "aconfig.test.flag.value_set", values: [ diff --git a/tools/aconfig/aconfig/src/commands.rs b/tools/aconfig/aconfig/src/commands.rs index 797a893ff1..b5854165bc 100644 --- a/tools/aconfig/aconfig/src/commands.rs +++ b/tools/aconfig/aconfig/src/commands.rs @@ -17,7 +17,7 @@ use anyhow::{bail, ensure, Context, Result}; use itertools::Itertools; use protobuf::Message; -use std::collections::{BTreeMap, HashMap}; +use std::collections::HashMap; use std::hash::Hasher; use std::io::Read; use std::path::PathBuf; @@ -422,25 +422,32 @@ where Ok(flag_ids) } -#[allow(dead_code)] // TODO: b/316357686 - Use fingerprint in codegen to - // protect hardcoded offset reads. -pub fn compute_flag_offsets_fingerprint(flags_map: &HashMap) -> Result { +// Creates a fingerprint of the flag names. Sorts the vector. +pub fn compute_flags_fingerprint(flag_names: &mut Vec) -> Result { + flag_names.sort(); + let mut hasher = SipHasher13::new(); - - // Need to sort to ensure the data is added to the hasher in the same order - // each run. - let sorted_map: BTreeMap<&String, &u16> = flags_map.iter().collect(); - - for (flag, offset) in sorted_map { - // See https://docs.rs/siphasher/latest/siphasher/#note for use of write - // over write_i16. Similarly, use to_be_bytes rather than to_ne_bytes to - // ensure consistency. + for flag in flag_names { hasher.write(flag.as_bytes()); - hasher.write(&offset.to_be_bytes()); } Ok(hasher.finish()) } +#[allow(dead_code)] // TODO: b/316357686 - Use fingerprint in codegen to + // protect hardcoded offset reads. +fn compute_fingerprint_from_parsed_flags(flags: ProtoParsedFlags) -> Result { + let separated_flags: Vec = flags.parsed_flag.into_iter().collect::>(); + + // All flags must belong to the same package as the fingerprint is per-package. + let Some(_package) = find_unique_package(&separated_flags) else { + bail!("No parsed flags, or the parsed flags use different packages."); + }; + + let mut flag_names = + separated_flags.into_iter().map(|flag| flag.name.unwrap()).collect::>(); + compute_flags_fingerprint(&mut flag_names) +} + #[cfg(test)] mod tests { use super::*; @@ -449,15 +456,46 @@ mod tests { #[test] fn test_offset_fingerprint() { let parsed_flags = crate::test::parse_test_flags(); - let package = find_unique_package(&parsed_flags.parsed_flag).unwrap().to_string(); - let flag_ids = assign_flag_ids(&package, parsed_flags.parsed_flag.iter()).unwrap(); - let expected_fingerprint = 10709892481002252132u64; + let expected_fingerprint: u64 = 5801144784618221668; - let hash_result = compute_flag_offsets_fingerprint(&flag_ids); + let hash_result = compute_fingerprint_from_parsed_flags(parsed_flags); assert_eq!(hash_result.unwrap(), expected_fingerprint); } + #[test] + fn test_offset_fingerprint_matches_from_package() { + let parsed_flags: ProtoParsedFlags = crate::test::parse_test_flags(); + + // All test flags are in the same package, so fingerprint from all of them. + let result_from_parsed_flags = compute_fingerprint_from_parsed_flags(parsed_flags.clone()); + + let mut flag_names_vec = parsed_flags + .parsed_flag + .clone() + .into_iter() + .map(|flag| flag.name.unwrap()) + .map(String::from) + .collect::>(); + let result_from_names = compute_flags_fingerprint(&mut flag_names_vec); + + // Assert the same hash is generated for each case. + assert_eq!(result_from_parsed_flags.unwrap(), result_from_names.unwrap()); + } + + #[test] + fn test_offset_fingerprint_different_packages_does_not_match() { + // Parse flags from two packages. + let parsed_flags: ProtoParsedFlags = crate::test::parse_test_flags(); + let second_parsed_flags = crate::test::parse_second_package_flags(); + + let result_from_parsed_flags = compute_fingerprint_from_parsed_flags(parsed_flags).unwrap(); + let second_result = compute_fingerprint_from_parsed_flags(second_parsed_flags).unwrap(); + + // Different flags should have a different fingerprint. + assert_ne!(result_from_parsed_flags, second_result); + } + #[test] fn test_parse_flags() { let parsed_flags = crate::test::parse_test_flags(); // calls parse_flags diff --git a/tools/aconfig/aconfig/src/storage/mod.rs b/tools/aconfig/aconfig/src/storage/mod.rs index 73339f24b3..efce24e422 100644 --- a/tools/aconfig/aconfig/src/storage/mod.rs +++ b/tools/aconfig/aconfig/src/storage/mod.rs @@ -25,12 +25,14 @@ use crate::storage::{ flag_table::create_flag_table, flag_value::create_flag_value, package_table::create_package_table, }; -use aconfig_protos::{ProtoParsedFlag, ProtoParsedFlags}; +use aconfig_protos::ProtoParsedFlag; +use aconfig_protos::ProtoParsedFlags; use aconfig_storage_file::StorageFileType; pub struct FlagPackage<'a> { pub package_name: &'a str, pub package_id: u32, + pub fingerprint: u64, pub flag_names: HashSet<&'a str>, pub boolean_flags: Vec<&'a ProtoParsedFlag>, // The index of the first boolean flag in this aconfig package among all boolean @@ -43,6 +45,7 @@ impl<'a> FlagPackage<'a> { FlagPackage { package_name, package_id, + fingerprint: 0, flag_names: HashSet::new(), boolean_flags: vec![], boolean_start_index: 0, @@ -78,6 +81,8 @@ where for p in packages.iter_mut() { p.boolean_start_index = boolean_start_index; boolean_start_index += p.boolean_flags.len() as u32; + + // TODO: b/316357686 - Calculate fingerprint and add to package. } packages @@ -115,6 +120,8 @@ mod tests { use super::*; use crate::Input; + use aconfig_protos::ProtoParsedFlags; + pub fn parse_all_test_flags() -> Vec { let aconfig_files = [ ( diff --git a/tools/aconfig/aconfig/src/storage/package_table.rs b/tools/aconfig/aconfig/src/storage/package_table.rs index c53602f9cb..33bb0774e0 100644 --- a/tools/aconfig/aconfig/src/storage/package_table.rs +++ b/tools/aconfig/aconfig/src/storage/package_table.rs @@ -48,6 +48,7 @@ impl PackageTableNodeWrapper { let node = PackageTableNode { package_name: String::from(package.package_name), package_id: package.package_id, + fingerprint: package.fingerprint, boolean_start_index: package.boolean_start_index, next_offset: None, }; diff --git a/tools/aconfig/aconfig/src/test.rs b/tools/aconfig/aconfig/src/test.rs index 7409cda6e8..a19b372aac 100644 --- a/tools/aconfig/aconfig/src/test.rs +++ b/tools/aconfig/aconfig/src/test.rs @@ -295,6 +295,24 @@ parsed_flag { aconfig_protos::parsed_flags::try_from_binary_proto(&bytes).unwrap() } + pub fn parse_second_package_flags() -> ProtoParsedFlags { + let bytes = crate::commands::parse_flags( + "com.android.aconfig.second_test", + Some("system"), + vec![Input { + source: "tests/test_second_package.aconfig".to_string(), + reader: Box::new(include_bytes!("../tests/test_second_package.aconfig").as_slice()), + }], + vec![Input { + source: "tests/third.values".to_string(), + reader: Box::new(include_bytes!("../tests/third.values").as_slice()), + }], + crate::commands::DEFAULT_FLAG_PERMISSION, + ) + .unwrap(); + aconfig_protos::parsed_flags::try_from_binary_proto(&bytes).unwrap() + } + pub fn first_significant_code_diff(a: &str, b: &str) -> Option { let a = a.lines().map(|line| line.trim_start()).filter(|line| !line.is_empty()); let b = b.lines().map(|line| line.trim_start()).filter(|line| !line.is_empty()); diff --git a/tools/aconfig/aconfig/tests/test.aconfig b/tools/aconfig/aconfig/tests/test.aconfig index c11508aabc..a818b2332e 100644 --- a/tools/aconfig/aconfig/tests/test.aconfig +++ b/tools/aconfig/aconfig/tests/test.aconfig @@ -86,4 +86,4 @@ flag { bug: "111" is_fixed_read_only: true is_exported: true -} \ No newline at end of file +} diff --git a/tools/aconfig/aconfig/tests/test_second_package.aconfig b/tools/aconfig/aconfig/tests/test_second_package.aconfig new file mode 100644 index 0000000000..a8740b85dd --- /dev/null +++ b/tools/aconfig/aconfig/tests/test_second_package.aconfig @@ -0,0 +1,12 @@ +package: "com.android.aconfig.second_test" +container: "system" + +flag { + name: "testing_flag" + namespace: "another_namespace" + description: "This is a flag for testing." + bug: "123" + metadata { + purpose: PURPOSE_UNSPECIFIED + } +} diff --git a/tools/aconfig/aconfig/tests/third.values b/tools/aconfig/aconfig/tests/third.values new file mode 100644 index 0000000000..675832a4bc --- /dev/null +++ b/tools/aconfig/aconfig/tests/third.values @@ -0,0 +1,6 @@ +flag_value { + package: "com.android.aconfig.second_test" + name: "testing_flag" + state: DISABLED + permission: READ_WRITE +} diff --git a/tools/aconfig/aconfig_storage_file/src/flag_info.rs b/tools/aconfig/aconfig_storage_file/src/flag_info.rs index f090396901..a49756d012 100644 --- a/tools/aconfig/aconfig_storage_file/src/flag_info.rs +++ b/tools/aconfig/aconfig_storage_file/src/flag_info.rs @@ -227,7 +227,7 @@ mod tests { let bytes = &flag_info_list.into_bytes(); let mut head = 0; let version = read_u32_from_bytes(bytes, &mut head).unwrap(); - assert_eq!(version, 1); + assert_eq!(version, 2); } #[test] diff --git a/tools/aconfig/aconfig_storage_file/src/flag_table.rs b/tools/aconfig/aconfig_storage_file/src/flag_table.rs index 0588fe5039..be82c63028 100644 --- a/tools/aconfig/aconfig_storage_file/src/flag_table.rs +++ b/tools/aconfig/aconfig_storage_file/src/flag_table.rs @@ -253,7 +253,7 @@ mod tests { let bytes = &flag_table.into_bytes(); let mut head = 0; let version = read_u32_from_bytes(bytes, &mut head).unwrap(); - assert_eq!(version, 1); + assert_eq!(version, 2); } #[test] diff --git a/tools/aconfig/aconfig_storage_file/src/flag_value.rs b/tools/aconfig/aconfig_storage_file/src/flag_value.rs index b64c10ecdd..c4cf29451e 100644 --- a/tools/aconfig/aconfig_storage_file/src/flag_value.rs +++ b/tools/aconfig/aconfig_storage_file/src/flag_value.rs @@ -159,7 +159,7 @@ mod tests { let bytes = &flag_value_list.into_bytes(); let mut head = 0; let version = read_u32_from_bytes(bytes, &mut head).unwrap(); - assert_eq!(version, 1); + assert_eq!(version, 2); } #[test] diff --git a/tools/aconfig/aconfig_storage_file/src/lib.rs b/tools/aconfig/aconfig_storage_file/src/lib.rs index cf52bc017d..19d0e51e61 100644 --- a/tools/aconfig/aconfig_storage_file/src/lib.rs +++ b/tools/aconfig/aconfig_storage_file/src/lib.rs @@ -58,7 +58,7 @@ use crate::AconfigStorageError::{ }; /// Storage file version -pub const FILE_VERSION: u32 = 1; +pub const FILE_VERSION: u32 = 2; /// Good hash table prime number pub(crate) const HASH_PRIMES: [u32; 29] = [ @@ -254,6 +254,16 @@ pub fn read_u32_from_bytes(buf: &[u8], head: &mut usize) -> Result Result { + let val = + u64::from_le_bytes(buf[*head..*head + 8].try_into().map_err(|errmsg| { + BytesParseFail(anyhow!("fail to parse u64 from bytes: {}", errmsg)) + })?); + *head += 8; + Ok(val) +} + /// Read and parse bytes as string pub(crate) fn read_str_from_bytes( buf: &[u8], diff --git a/tools/aconfig/aconfig_storage_file/src/package_table.rs b/tools/aconfig/aconfig_storage_file/src/package_table.rs index a5bd9e6446..350f072df2 100644 --- a/tools/aconfig/aconfig_storage_file/src/package_table.rs +++ b/tools/aconfig/aconfig_storage_file/src/package_table.rs @@ -17,7 +17,10 @@ //! package table module defines the package table file format and methods for serialization //! and deserialization -use crate::{get_bucket_index, read_str_from_bytes, read_u32_from_bytes, read_u8_from_bytes}; +use crate::{ + get_bucket_index, read_str_from_bytes, read_u32_from_bytes, read_u64_from_bytes, + read_u8_from_bytes, +}; use crate::{AconfigStorageError, StorageFileType}; use anyhow::anyhow; use serde::{Deserialize, Serialize}; @@ -97,6 +100,7 @@ impl PackageTableHeader { pub struct PackageTableNode { pub package_name: String, pub package_id: u32, + pub fingerprint: u64, // The index of the first boolean flag in this aconfig package among all boolean // flags in this container. pub boolean_start_index: u32, @@ -108,8 +112,12 @@ impl fmt::Debug for PackageTableNode { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { writeln!( f, - "Package: {}, Id: {}, Boolean flag start index: {}, Next: {:?}", - self.package_name, self.package_id, self.boolean_start_index, self.next_offset + "Package: {}, Id: {}, Fingerprint: {}, Boolean flag start index: {}, Next: {:?}", + self.package_name, + self.package_id, + self.fingerprint, + self.boolean_start_index, + self.next_offset )?; Ok(()) } @@ -123,6 +131,7 @@ impl PackageTableNode { result.extend_from_slice(&(name_bytes.len() as u32).to_le_bytes()); result.extend_from_slice(name_bytes); result.extend_from_slice(&self.package_id.to_le_bytes()); + result.extend_from_slice(&self.fingerprint.to_le_bytes()); result.extend_from_slice(&self.boolean_start_index.to_le_bytes()); result.extend_from_slice(&self.next_offset.unwrap_or(0).to_le_bytes()); result @@ -134,6 +143,7 @@ impl PackageTableNode { let node = Self { package_name: read_str_from_bytes(bytes, &mut head)?, package_id: read_u32_from_bytes(bytes, &mut head)?, + fingerprint: read_u64_from_bytes(bytes, &mut head)?, boolean_start_index: read_u32_from_bytes(bytes, &mut head)?, next_offset: match read_u32_from_bytes(bytes, &mut head)? { 0 => None, @@ -251,7 +261,7 @@ mod tests { let bytes = &package_table.into_bytes(); let mut head = 0; let version = read_u32_from_bytes(bytes, &mut head).unwrap(); - assert_eq!(version, 1); + assert_eq!(version, 2); } #[test] diff --git a/tools/aconfig/aconfig_storage_file/src/test_utils.rs b/tools/aconfig/aconfig_storage_file/src/test_utils.rs index 106666c47f..11e2dc688e 100644 --- a/tools/aconfig/aconfig_storage_file/src/test_utils.rs +++ b/tools/aconfig/aconfig_storage_file/src/test_utils.rs @@ -26,30 +26,33 @@ use tempfile::NamedTempFile; pub fn create_test_package_table() -> PackageTable { let header = PackageTableHeader { - version: 1, + version: 2, container: String::from("mockup"), file_type: StorageFileType::PackageMap as u8, - file_size: 209, + file_size: 233, num_packages: 3, bucket_offset: 31, node_offset: 59, }; - let buckets: Vec> = vec![Some(59), None, None, Some(109), None, None, None]; + let buckets: Vec> = vec![Some(59), None, None, Some(117), None, None, None]; let first_node = PackageTableNode { package_name: String::from("com.android.aconfig.storage.test_2"), package_id: 1, + fingerprint: 0, boolean_start_index: 3, next_offset: None, }; let second_node = PackageTableNode { package_name: String::from("com.android.aconfig.storage.test_1"), package_id: 0, + fingerprint: 0, boolean_start_index: 0, - next_offset: Some(159), + next_offset: Some(175), }; let third_node = PackageTableNode { package_name: String::from("com.android.aconfig.storage.test_4"), package_id: 2, + fingerprint: 0, boolean_start_index: 6, next_offset: None, }; @@ -78,7 +81,7 @@ impl FlagTableNode { pub fn create_test_flag_table() -> FlagTable { let header = FlagTableHeader { - version: 1, + version: 2, container: String::from("mockup"), file_type: StorageFileType::FlagMap as u8, file_size: 321, @@ -120,7 +123,7 @@ pub fn create_test_flag_table() -> FlagTable { pub fn create_test_flag_value_list() -> FlagValueList { let header = FlagValueHeader { - version: 1, + version: 2, container: String::from("mockup"), file_type: StorageFileType::FlagVal as u8, file_size: 35, @@ -133,7 +136,7 @@ pub fn create_test_flag_value_list() -> FlagValueList { pub fn create_test_flag_info_list() -> FlagInfoList { let header = FlagInfoHeader { - version: 1, + version: 2, container: String::from("mockup"), file_type: StorageFileType::FlagInfo as u8, file_size: 35, diff --git a/tools/aconfig/aconfig_storage_file/srcs/android/aconfig/storage/ByteBufferReader.java b/tools/aconfig/aconfig_storage_file/srcs/android/aconfig/storage/ByteBufferReader.java index 4bea0836f0..44a82ee1c7 100644 --- a/tools/aconfig/aconfig_storage_file/srcs/android/aconfig/storage/ByteBufferReader.java +++ b/tools/aconfig/aconfig_storage_file/srcs/android/aconfig/storage/ByteBufferReader.java @@ -37,6 +37,10 @@ public class ByteBufferReader { return Short.toUnsignedInt(mByteBuffer.getShort()); } + public long readLong() { + return mByteBuffer.getLong(); + } + public int readInt() { return this.mByteBuffer.getInt(); } diff --git a/tools/aconfig/aconfig_storage_file/srcs/android/aconfig/storage/PackageTable.java b/tools/aconfig/aconfig_storage_file/srcs/android/aconfig/storage/PackageTable.java index 773b882f4a..f1288f5ea6 100644 --- a/tools/aconfig/aconfig_storage_file/srcs/android/aconfig/storage/PackageTable.java +++ b/tools/aconfig/aconfig_storage_file/srcs/android/aconfig/storage/PackageTable.java @@ -118,6 +118,7 @@ public class PackageTable { private String mPackageName; private int mPackageId; + private long mFingerprint; private int mBooleanStartIndex; private int mNextOffset; @@ -125,6 +126,7 @@ public class PackageTable { Node node = new Node(); node.mPackageName = reader.readString(); node.mPackageId = reader.readInt(); + node.mFingerprint = reader.readLong(); node.mBooleanStartIndex = reader.readInt(); node.mNextOffset = reader.readInt(); node.mNextOffset = node.mNextOffset == 0 ? -1 : node.mNextOffset; @@ -150,6 +152,7 @@ public class PackageTable { return Objects.equals(mPackageName, other.mPackageName) && mPackageId == other.mPackageId && mBooleanStartIndex == other.mBooleanStartIndex + && mFingerprint == other.mFingerprint && mNextOffset == other.mNextOffset; } @@ -165,6 +168,10 @@ public class PackageTable { return mBooleanStartIndex; } + public long getFingerprint() { + return mFingerprint; + } + public int getNextOffset() { return mNextOffset; } diff --git a/tools/aconfig/aconfig_storage_file/tests/flag.info b/tools/aconfig/aconfig_storage_file/tests/flag.info index 6223edf369cbd0635aa34de23a676b1cf8aa3667..9db7fde7aedb036210391ed5e566611f25740aaf 100644 GIT binary patch delta 7 OcmY#ZW@MVks0085vH<=7 delta 7 OcmY#ZW@Mbms0085t^oZ2 diff --git a/tools/aconfig/aconfig_storage_file/tests/flag.map b/tools/aconfig/aconfig_storage_file/tests/flag.map index e868f53d7e599f130604faac1fd20149e73cb821..cf4685ceb47f29c1f5268ceaf0247e5716127c67 100644 GIT binary patch delta 10 RcmX@ebdZUWX(OWpBLEQ;0(}4g delta 10 RcmX@ebdZUWaU-JxBLEQ(0(<}f diff --git a/tools/aconfig/aconfig_storage_file/tests/flag.val b/tools/aconfig/aconfig_storage_file/tests/flag.val index ed203d4d13d068fc22d4136d413fe1fbd82cc57d..37d4750206acbd5a3924f2aec4a75c75e4fe153f 100644 GIT binary patch delta 7 OcmY#ZW@MVks0085vH<=7 delta 7 OcmY#ZW@Mbms0085t^oZ2 diff --git a/tools/aconfig/aconfig_storage_file/tests/package.map b/tools/aconfig/aconfig_storage_file/tests/package.map index 6c46a0339cf94dde5e32e3e345835f8e893f6fba..358010cf37cd8e82691b9f8c0e5d8d0f938671ab 100644 GIT binary patch delta 70 ucmcb}_>z&4X(FQ*W9dX4OI`*jU=V%BS!bD*7`iXON1z;RDC<6d#F9)vx delta 31 icmaFKc#)BjaU!D@W9~#9%Za&WjPob1(dA%cfB*oFKnE)T diff --git a/tools/aconfig/aconfig_storage_file/tests/srcs/FlagTableTest.java b/tools/aconfig/aconfig_storage_file/tests/srcs/FlagTableTest.java index fd40d4c4ef..e3b02cd666 100644 --- a/tools/aconfig/aconfig_storage_file/tests/srcs/FlagTableTest.java +++ b/tools/aconfig/aconfig_storage_file/tests/srcs/FlagTableTest.java @@ -33,7 +33,7 @@ public class FlagTableTest { public void testFlagTable_rightHeader() throws Exception { FlagTable flagTable = FlagTable.fromBytes(TestDataUtils.getTestFlagMapByteBuffer()); FlagTable.Header header = flagTable.getHeader(); - assertEquals(1, header.getVersion()); + assertEquals(2, header.getVersion()); assertEquals("mockup", header.getContainer()); assertEquals(FileType.FLAG_MAP, header.getFileType()); assertEquals(321, header.getFileSize()); diff --git a/tools/aconfig/aconfig_storage_file/tests/srcs/FlagValueListTest.java b/tools/aconfig/aconfig_storage_file/tests/srcs/FlagValueListTest.java index 1b0de630c7..ebc231c2dd 100644 --- a/tools/aconfig/aconfig_storage_file/tests/srcs/FlagValueListTest.java +++ b/tools/aconfig/aconfig_storage_file/tests/srcs/FlagValueListTest.java @@ -36,7 +36,7 @@ public class FlagValueListTest { FlagValueList flagValueList = FlagValueList.fromBytes(TestDataUtils.getTestFlagValByteBuffer()); FlagValueList.Header header = flagValueList.getHeader(); - assertEquals(1, header.getVersion()); + assertEquals(2, header.getVersion()); assertEquals("mockup", header.getContainer()); assertEquals(FileType.FLAG_VAL, header.getFileType()); assertEquals(35, header.getFileSize()); diff --git a/tools/aconfig/aconfig_storage_file/tests/srcs/PackageTableTest.java b/tools/aconfig/aconfig_storage_file/tests/srcs/PackageTableTest.java index e7e19d8d51..6d56ceed96 100644 --- a/tools/aconfig/aconfig_storage_file/tests/srcs/PackageTableTest.java +++ b/tools/aconfig/aconfig_storage_file/tests/srcs/PackageTableTest.java @@ -20,7 +20,6 @@ import static org.junit.Assert.assertEquals; import android.aconfig.storage.FileType; import android.aconfig.storage.PackageTable; - import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -28,42 +27,40 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) public class PackageTableTest { - @Test - public void testPackageTable_rightHeader() throws Exception { - PackageTable packageTable = - PackageTable.fromBytes(TestDataUtils.getTestPackageMapByteBuffer()); - PackageTable.Header header = packageTable.getHeader(); - assertEquals(1, header.getVersion()); - assertEquals("mockup", header.getContainer()); - assertEquals(FileType.PACKAGE_MAP, header.getFileType()); - assertEquals(209, header.getFileSize()); - assertEquals(3, header.getNumPackages()); - assertEquals(31, header.getBucketOffset()); - assertEquals(59, header.getNodeOffset()); - } + @Test + public void testPackageTable_rightHeader() throws Exception { + PackageTable packageTable = PackageTable.fromBytes(TestDataUtils.getTestPackageMapByteBuffer()); + PackageTable.Header header = packageTable.getHeader(); + assertEquals(2, header.getVersion()); + assertEquals("mockup", header.getContainer()); + assertEquals(FileType.PACKAGE_MAP, header.getFileType()); + assertEquals(209, header.getFileSize()); + assertEquals(3, header.getNumPackages()); + assertEquals(31, header.getBucketOffset()); + assertEquals(59, header.getNodeOffset()); + } - @Test - public void testPackageTable_rightNode() throws Exception { - PackageTable packageTable = - PackageTable.fromBytes(TestDataUtils.getTestPackageMapByteBuffer()); + @Test + public void testPackageTable_rightNode() throws Exception { + PackageTable packageTable = PackageTable.fromBytes(TestDataUtils.getTestPackageMapByteBuffer()); - PackageTable.Node node1 = packageTable.get("com.android.aconfig.storage.test_1"); - PackageTable.Node node2 = packageTable.get("com.android.aconfig.storage.test_2"); - PackageTable.Node node4 = packageTable.get("com.android.aconfig.storage.test_4"); + PackageTable.Node node1 = packageTable.get("com.android.aconfig.storage.test_1"); + PackageTable.Node node2 = packageTable.get("com.android.aconfig.storage.test_2"); + PackageTable.Node node4 = packageTable.get("com.android.aconfig.storage.test_4"); - assertEquals("com.android.aconfig.storage.test_1", node1.getPackageName()); - assertEquals("com.android.aconfig.storage.test_2", node2.getPackageName()); - assertEquals("com.android.aconfig.storage.test_4", node4.getPackageName()); + assertEquals("com.android.aconfig.storage.test_1", node1.getPackageName()); + assertEquals("com.android.aconfig.storage.test_2", node2.getPackageName()); + assertEquals("com.android.aconfig.storage.test_4", node4.getPackageName()); - assertEquals(0, node1.getPackageId()); - assertEquals(1, node2.getPackageId()); - assertEquals(2, node4.getPackageId()); + assertEquals(0, node1.getPackageId()); + assertEquals(1, node2.getPackageId()); + assertEquals(2, node4.getPackageId()); - assertEquals(0, node1.getBooleanStartIndex()); - assertEquals(3, node2.getBooleanStartIndex()); - assertEquals(6, node4.getBooleanStartIndex()); + assertEquals(0, node1.getBooleanStartIndex()); + assertEquals(3, node2.getBooleanStartIndex()); + assertEquals(6, node4.getBooleanStartIndex()); - assertEquals(159, node1.getNextOffset()); + assertEquals(175, node1.getNextOffset()); assertEquals(-1, node2.getNextOffset()); assertEquals(-1, node4.getNextOffset()); } diff --git a/tools/aconfig/aconfig_storage_read_api/src/lib.rs b/tools/aconfig/aconfig_storage_read_api/src/lib.rs index d76cf3fe4e..59aa749c74 100644 --- a/tools/aconfig/aconfig_storage_read_api/src/lib.rs +++ b/tools/aconfig/aconfig_storage_read_api/src/lib.rs @@ -507,9 +507,9 @@ mod tests { #[test] // this test point locks down flag storage file version number query api fn test_storage_version_query() { - 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); - assert_eq!(get_storage_file_version("./tests/flag.info").unwrap(), 1); + assert_eq!(get_storage_file_version("./tests/package.map").unwrap(), 2); + assert_eq!(get_storage_file_version("./tests/flag.map").unwrap(), 2); + assert_eq!(get_storage_file_version("./tests/flag.val").unwrap(), 2); + assert_eq!(get_storage_file_version("./tests/flag.info").unwrap(), 2); } } diff --git a/tools/aconfig/aconfig_storage_read_api/tests/flag.info b/tools/aconfig/aconfig_storage_read_api/tests/flag.info index 6223edf369cbd0635aa34de23a676b1cf8aa3667..9db7fde7aedb036210391ed5e566611f25740aaf 100644 GIT binary patch delta 7 OcmY#ZW@MVks0085vH<=7 delta 7 OcmY#ZW@Mbms0085t^oZ2 diff --git a/tools/aconfig/aconfig_storage_read_api/tests/flag.map b/tools/aconfig/aconfig_storage_read_api/tests/flag.map index e868f53d7e599f130604faac1fd20149e73cb821..cf4685ceb47f29c1f5268ceaf0247e5716127c67 100644 GIT binary patch delta 10 RcmX@ebdZUWX(OWpBLEQ;0(}4g delta 10 RcmX@ebdZUWaU-JxBLEQ(0(<}f diff --git a/tools/aconfig/aconfig_storage_read_api/tests/flag.val b/tools/aconfig/aconfig_storage_read_api/tests/flag.val index ed203d4d13d068fc22d4136d413fe1fbd82cc57d..37d4750206acbd5a3924f2aec4a75c75e4fe153f 100644 GIT binary patch delta 7 OcmY#ZW@MVks0085vH<=7 delta 7 OcmY#ZW@Mbms0085t^oZ2 diff --git a/tools/aconfig/aconfig_storage_read_api/tests/package.map b/tools/aconfig/aconfig_storage_read_api/tests/package.map index 6c46a0339cf94dde5e32e3e345835f8e893f6fba..358010cf37cd8e82691b9f8c0e5d8d0f938671ab 100644 GIT binary patch delta 70 ucmcb}_>z&4X(FQ*W9dX4OI`*jU=V%BS!bD*7`iXON1z;RDC<6d#F9)vx delta 31 icmaFKc#)BjaU!D@W9~#9%Za&WjPob1(dA%cfB*oFKnE)T 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 6d29045efe..58460d1ac7 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 @@ -80,16 +80,16 @@ class AconfigStorageTest : public ::testing::Test { TEST_F(AconfigStorageTest, test_storage_version_query) { auto version = api::get_storage_file_version(package_map); ASSERT_TRUE(version.ok()); - ASSERT_EQ(*version, 1); + ASSERT_EQ(*version, 2); version = api::get_storage_file_version(flag_map); ASSERT_TRUE(version.ok()); - ASSERT_EQ(*version, 1); + ASSERT_EQ(*version, 2); version = api::get_storage_file_version(flag_val); ASSERT_TRUE(version.ok()); - ASSERT_EQ(*version, 1); + ASSERT_EQ(*version, 2); version = api::get_storage_file_version(flag_info); ASSERT_TRUE(version.ok()); - ASSERT_EQ(*version, 1); + ASSERT_EQ(*version, 2); } /// Negative test to lock down the error when mapping none exist storage files 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 afc44d4d70..bd1b5843f1 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 @@ -200,9 +200,9 @@ mod aconfig_storage_rust_test { #[test] fn test_storage_version_query() { - assert_eq!(get_storage_file_version("./package.map").unwrap(), 1); - assert_eq!(get_storage_file_version("./flag.map").unwrap(), 1); - assert_eq!(get_storage_file_version("./flag.val").unwrap(), 1); - assert_eq!(get_storage_file_version("./flag.info").unwrap(), 1); + assert_eq!(get_storage_file_version("./package.map").unwrap(), 2); + assert_eq!(get_storage_file_version("./flag.map").unwrap(), 2); + assert_eq!(get_storage_file_version("./flag.val").unwrap(), 2); + assert_eq!(get_storage_file_version("./flag.info").unwrap(), 2); } } From 392c0c954d7ef1a779b04f916f9fcd593bd62840 Mon Sep 17 00:00:00 2001 From: Jeongik Cha Date: Fri, 20 Sep 2024 00:26:22 +0900 Subject: [PATCH 7/9] Add etc/bpf/uprobestats/ProcessManagement.o in allowlist to unblock build failure Bug: 351698657 Bug: 368185982 Test: build Change-Id: I1017091dd26cd74b471d1d71c3f3b6ad80a0c50a --- tools/filelistdiff/allowlist | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/filelistdiff/allowlist b/tools/filelistdiff/allowlist index c4a464dd16..120045e3b2 100644 --- a/tools/filelistdiff/allowlist +++ b/tools/filelistdiff/allowlist @@ -43,7 +43,9 @@ etc/aconfig/flag.val etc/aconfig/package.map etc/bpf/uprobestats/BitmapAllocation.o etc/bpf/uprobestats/GenericInstrumentation.o +etc/bpf/uprobestats/ProcessManagement.o etc/init/UprobeStats.rc lib/libuprobestats_client.so lib64/libuprobestats_client.so -priv-app/DeviceDiagnostics/DeviceDiagnostics.apk \ No newline at end of file +priv-app/DeviceDiagnostics/DeviceDiagnostics.apk + From 2f37c1980c857544c90a596c5742d03bd39f7832 Mon Sep 17 00:00:00 2001 From: "Priyanka Advani (xWF)" Date: Thu, 19 Sep 2024 17:43:38 +0000 Subject: [PATCH 8/9] Revert "Add command line tool that generates NOTICE.xml.gz for p..." Revert submission 3273112-soong-notice-xml Reason for revert: Droidmonitor created revert due to b/368348129. Reverted changes: /q/submissionid:3273112-soong-notice-xml Change-Id: I55e1e93aa6d4b311c6ee461d4216a104909af842 --- core/Makefile | 4 +- core/os_licensing.mk | 4 -- tools/sbom/Android.bp | 14 ------- tools/sbom/gen_notice_xml.py | 81 ------------------------------------ 4 files changed, 1 insertion(+), 102 deletions(-) delete mode 100644 tools/sbom/gen_notice_xml.py diff --git a/core/Makefile b/core/Makefile index 0bc6f1fc9e..b0392cdc8b 100644 --- a/core/Makefile +++ b/core/Makefile @@ -1964,7 +1964,7 @@ target_system_dlkm_notice_file_xml_gz := $(TARGET_OUT_INTERMEDIATES)/NOTICE_SYST installed_system_dlkm_notice_xml_gz := $(TARGET_OUT_SYSTEM_DLKM)/etc/NOTICE.xml.gz ALL_INSTALLED_NOTICE_FILES := \ - $(if $(USE_SOONG_DEFINED_SYSTEM_IMAGE),,$(installed_notice_html_or_xml_gz)) \ + $(installed_notice_html_or_xml_gz) \ $(installed_vendor_notice_xml_gz) \ $(installed_product_notice_xml_gz) \ $(installed_system_ext_notice_xml_gz) \ @@ -2051,9 +2051,7 @@ endif endif # PRODUCT_NOTICE_SPLIT -ifneq ($(USE_SOONG_DEFINED_SYSTEM_IMAGE),true) ALL_DEFAULT_INSTALLED_MODULES += $(installed_notice_html_or_xml_gz) -endif need_vendor_notice:=false ifeq ($(BUILDING_VENDOR_BOOT_IMAGE),true) diff --git a/core/os_licensing.mk b/core/os_licensing.mk index d15a3d0715..1e1b7df7a9 100644 --- a/core/os_licensing.mk +++ b/core/os_licensing.mk @@ -17,17 +17,13 @@ $(eval $(call xml-notice-rule,$(target_notice_file_xml_gz),"System image",$(syst $(eval $(call text-notice-rule,$(target_notice_file_txt),"System image",$(system_notice_file_message),$(SYSTEM_NOTICE_DEPS),$(SYSTEM_NOTICE_DEPS))) -ifneq ($(USE_SOONG_DEFINED_SYSTEM_IMAGE),true) $(installed_notice_html_or_xml_gz): $(target_notice_file_xml_gz) $(copy-file-to-target) endif -endif $(call declare-1p-target,$(target_notice_file_xml_gz)) -ifneq ($(USE_SOONG_DEFINED_SYSTEM_IMAGE),true) $(call declare-1p-target,$(installed_notice_html_or_xml_gz)) endif -endif .PHONY: vendorlicense vendorlicense: $(call corresponding-license-metadata, $(VENDOR_NOTICE_DEPS)) reportmissinglicenses diff --git a/tools/sbom/Android.bp b/tools/sbom/Android.bp index 74b3d626f6..6901b06720 100644 --- a/tools/sbom/Android.bp +++ b/tools/sbom/Android.bp @@ -109,17 +109,3 @@ python_binary_host { "sbom_lib", ], } - -python_binary_host { - name: "gen_notice_xml", - srcs: [ - "gen_notice_xml.py", - ], - version: { - py3: { - embedded_launcher: true, - }, - }, - libs: [ - ], -} diff --git a/tools/sbom/gen_notice_xml.py b/tools/sbom/gen_notice_xml.py deleted file mode 100644 index eaa6e5a74d..0000000000 --- a/tools/sbom/gen_notice_xml.py +++ /dev/null @@ -1,81 +0,0 @@ -# !/usr/bin/env python3 -# -# Copyright (C) 2024 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. - -""" -Generate NOTICE.xml.gz of a partition. -Usage example: - gen_notice_xml.py --output_file out/soong/.intermediate/.../NOTICE.xml.gz \ - --metadata out/soong/compliance-metadata/aosp_cf_x86_64_phone/compliance-metadata.db \ - --partition system \ - --product_out out/target/vsoc_x86_64 \ - --soong_out out/soong -""" - -import argparse - - -FILE_HEADER = '''\ - - -''' -FILE_FOOTER = '''\ - -''' - - -def get_args(): - parser = argparse.ArgumentParser() - parser.add_argument('-v', '--verbose', action='store_true', default=False, help='Print more information.') - parser.add_argument('-d', '--debug', action='store_true', default=True, help='Debug mode') - parser.add_argument('--output_file', required=True, help='The path of the generated NOTICE.xml.gz file.') - parser.add_argument('--partition', required=True, help='The name of partition for which the NOTICE.xml.gz is generated.') - parser.add_argument('--metadata', required=True, help='The path of compliance metadata DB file.') - parser.add_argument('--product_out', required=True, help='The path of PRODUCT_OUT, e.g. out/target/product/vsoc_x86_64.') - parser.add_argument('--soong_out', required=True, help='The path of Soong output directory, e.g. out/soong') - - return parser.parse_args() - - -def log(*info): - if args.verbose: - for i in info: - print(i) - - -def new_file_name_tag(file_metadata, package_name): - file_path = file_metadata['installed_file'].removeprefix(args.product_out) - lib = 'Android' - if package_name: - lib = package_name - return f'{file_path}\n' - - -def new_file_content_tag(): - pass - - -def main(): - global args - args = get_args() - log('Args:', vars(args)) - - with open(args.output_file, 'w', encoding="utf-8") as notice_xml_file: - notice_xml_file.write(FILE_HEADER) - notice_xml_file.write(FILE_FOOTER) - - -if __name__ == '__main__': - main() From 8ea6742d053bb674c67a4b5c35e621a76462aa72 Mon Sep 17 00:00:00 2001 From: Luca Farsi Date: Tue, 17 Sep 2024 15:48:11 -0700 Subject: [PATCH 9/9] Fix packaging outputs commands There were a few issues with the output packaging process that were found during testing of the general-tests optimization. First and foremost is that the packaging commands were trying to be created before the build ran, when the outputs don't exist. I've changed the logic to just collect the methods themselves which will then be run during build plan execution after the build has completed. A few other smaller issues include fixing the path to the soong_zip binary, incorrect execution of the soong dumpvars command, and not building the shared libs zip. Test: atest build_test_suites_test; atest optimized_targets_test Bug: 358215235 Change-Id: I8a3f54738f8bb5d871aadf7423844076c38b54a6 --- ci/build_test_suites.py | 16 +++++---- ci/build_test_suites_test.py | 14 ++++++-- ci/optimized_targets.py | 67 ++++++++++++++++++++---------------- ci/optimized_targets_test.py | 14 +------- 4 files changed, 59 insertions(+), 52 deletions(-) diff --git a/ci/build_test_suites.py b/ci/build_test_suites.py index 933e43e387..b8c4a385e0 100644 --- a/ci/build_test_suites.py +++ b/ci/build_test_suites.py @@ -22,6 +22,7 @@ import os import pathlib import subprocess import sys +from typing import Callable from build_context import BuildContext import optimized_targets @@ -68,7 +69,7 @@ class BuildPlanner: return BuildPlan(set(self.args.extra_targets), set()) build_targets = set() - packaging_commands = [] + packaging_commands_getters = [] for target in self.args.extra_targets: if self._unused_target_exclusion_enabled( target @@ -84,9 +85,11 @@ class BuildPlanner: target, self.build_context, self.args ) build_targets.update(target_optimizer.get_build_targets()) - packaging_commands.extend(target_optimizer.get_package_outputs_commands()) + packaging_commands_getters.append( + target_optimizer.get_package_outputs_commands + ) - return BuildPlan(build_targets, packaging_commands) + return BuildPlan(build_targets, packaging_commands_getters) def _unused_target_exclusion_enabled(self, target: str) -> bool: return ( @@ -98,7 +101,7 @@ class BuildPlanner: @dataclass(frozen=True) class BuildPlan: build_targets: set[str] - packaging_commands: list[list[str]] + packaging_commands_getters: list[Callable[[], list[list[str]]]] def build_test_suites(argv: list[str]) -> int: @@ -180,9 +183,10 @@ def execute_build_plan(build_plan: BuildPlan): except subprocess.CalledProcessError as e: raise BuildFailureError(e.returncode) from e - for packaging_command in build_plan.packaging_commands: + for packaging_commands_getter in build_plan.packaging_commands_getters: try: - run_command(packaging_command) + for packaging_command in packaging_commands_getter(): + run_command(packaging_command) except subprocess.CalledProcessError as e: raise BuildFailureError(e.returncode) from e diff --git a/ci/build_test_suites_test.py b/ci/build_test_suites_test.py index fd06a3ae1e..2afaab7711 100644 --- a/ci/build_test_suites_test.py +++ b/ci/build_test_suites_test.py @@ -276,7 +276,8 @@ class BuildPlannerTest(unittest.TestCase): build_plan = build_planner.create_build_plan() - self.assertEqual(len(build_plan.packaging_commands), 0) + for packaging_command in self.run_packaging_commands(build_plan): + self.assertEqual(len(packaging_command), 0) def test_build_optimization_on_optimizes_target(self): build_targets = {'target_1', 'target_2'} @@ -306,7 +307,7 @@ class BuildPlannerTest(unittest.TestCase): build_plan = build_planner.create_build_plan() - self.assertIn([f'packaging {optimized_target_name}'], build_plan.packaging_commands) + self.assertIn(packaging_commands, self.run_packaging_commands(build_plan)) def test_individual_build_optimization_off_doesnt_optimize(self): build_targets = {'target_1', 'target_2'} @@ -328,7 +329,8 @@ class BuildPlannerTest(unittest.TestCase): build_plan = build_planner.create_build_plan() - self.assertFalse(build_plan.packaging_commands) + for packaging_command in self.run_packaging_commands(build_plan): + self.assertEqual(len(packaging_command), 0) def test_target_output_used_target_built(self): build_target = 'test_target' @@ -485,6 +487,12 @@ class BuildPlannerTest(unittest.TestCase): ], } + def run_packaging_commands(self, build_plan: build_test_suites.BuildPlan): + return [ + packaging_command_getter() + for packaging_command_getter in build_plan.packaging_commands_getters + ] + def wait_until( condition_function: Callable[[], bool], diff --git a/ci/optimized_targets.py b/ci/optimized_targets.py index 4bee401569..688bdd8370 100644 --- a/ci/optimized_targets.py +++ b/ci/optimized_targets.py @@ -121,13 +121,13 @@ class OptimizedBuildTarget(ABC): process_result = subprocess.run( args=[ f'{src_top / self._SOONG_UI_BASH_PATH}', - '--dumpvar-mode', - '--abs', - soong_vars, + '--dumpvars-mode', + f'--abs-vars={" ".join(soong_vars)}', ], env=os.environ, check=False, capture_output=True, + text=True, ) if not process_result.returncode == 0: logging.error('soong dumpvars command failed! stderr:') @@ -142,7 +142,7 @@ class OptimizedBuildTarget(ABC): try: return { line.split('=')[0]: line.split('=')[1].strip("'") - for line in process_result.stdout.split('\n') + for line in process_result.stdout.strip().split('\n') } except IndexError as e: raise RuntimeError( @@ -214,10 +214,13 @@ class GeneralTestsOptimizer(OptimizedBuildTarget): normally built. """ - # List of modules that are always required to be in general-tests.zip. - _REQUIRED_MODULES = frozenset( - ['cts-tradefed', 'vts-tradefed', 'compatibility-host-util'] - ) + # List of modules that are built alongside general-tests as dependencies. + _REQUIRED_MODULES = frozenset([ + 'cts-tradefed', + 'vts-tradefed', + 'compatibility-host-util', + 'general-tests-shared-libs', + ]) def get_build_targets_impl(self) -> set[str]: change_info_file_path = os.environ.get('CHANGE_INFO') @@ -286,6 +289,10 @@ class GeneralTestsOptimizer(OptimizedBuildTarget): host_config_files = [] target_config_files = [] for module in self.modules_to_build: + # The required modules are handled separately, no need to package. + if module in self._REQUIRED_MODULES: + continue + host_path = host_out_testcases / module if os.path.exists(host_path): host_paths.append(host_path) @@ -303,6 +310,7 @@ class GeneralTestsOptimizer(OptimizedBuildTarget): zip_commands.extend( self._get_zip_test_configs_zips_commands( + src_top, dist_dir, host_out, product_out, @@ -311,27 +319,27 @@ class GeneralTestsOptimizer(OptimizedBuildTarget): ) ) - zip_command = self._base_zip_command( - host_out, dist_dir, 'general-tests.zip' - ) + zip_command = self._base_zip_command(src_top, dist_dir, 'general-tests.zip') # Add host testcases. - zip_command.extend( - self._generate_zip_options_for_items( - prefix='host', - relative_root=f'{src_top / soong_host_out}', - directories=host_paths, - ) - ) + if host_paths: + zip_command.extend( + self._generate_zip_options_for_items( + prefix='host', + relative_root=f'{src_top / soong_host_out}', + directories=host_paths, + ) + ) # Add target testcases. - zip_command.extend( - self._generate_zip_options_for_items( - prefix='target', - relative_root=f'{src_top / product_out}', - directories=target_paths, - ) - ) + if target_paths: + zip_command.extend( + self._generate_zip_options_for_items( + prefix='target', + relative_root=f'{src_top / product_out}', + directories=target_paths, + ) + ) # TODO(lucafarsi): Push this logic into a general-tests-minimal build command # Add necessary tools. These are also hardcoded in general-tests.mk. @@ -365,6 +373,7 @@ class GeneralTestsOptimizer(OptimizedBuildTarget): def _get_zip_test_configs_zips_commands( self, + src_top: pathlib.Path, dist_dir: pathlib.Path, host_out: pathlib.Path, product_out: pathlib.Path, @@ -428,7 +437,7 @@ class GeneralTestsOptimizer(OptimizedBuildTarget): zip_commands = [] tests_config_zip_command = self._base_zip_command( - host_out, dist_dir, 'general-tests_configs.zip' + src_top, dist_dir, 'general-tests_configs.zip' ) tests_config_zip_command.extend( self._generate_zip_options_for_items( @@ -442,16 +451,14 @@ class GeneralTestsOptimizer(OptimizedBuildTarget): self._generate_zip_options_for_items( prefix='target', relative_root=str(product_out), - list_files=[ - f"{product_out / 'target_general-tests_list'}" - ], + list_files=[f"{product_out / 'target_general-tests_list'}"], ), ) zip_commands.append(tests_config_zip_command) tests_list_zip_command = self._base_zip_command( - host_out, dist_dir, 'general-tests_list.zip' + src_top, dist_dir, 'general-tests_list.zip' ) tests_list_zip_command.extend( self._generate_zip_options_for_items( diff --git a/ci/optimized_targets_test.py b/ci/optimized_targets_test.py index 762b62e664..0b0c0ec087 100644 --- a/ci/optimized_targets_test.py +++ b/ci/optimized_targets_test.py @@ -220,18 +220,6 @@ class GeneralTestsOptimizerTest(fake_filesystem_unittest.TestCase): ): package_commands = optimizer.get_package_outputs_commands() - @mock.patch('subprocess.run') - def test_no_build_outputs_packaging_fails(self, subprocess_run): - subprocess_run.return_value = self._get_soong_vars_output() - optimizer = self._create_general_tests_optimizer() - - targets = optimizer.get_build_targets() - - with self.assertRaisesRegex( - RuntimeError, 'No items specified to be added to zip' - ): - package_commands = optimizer.get_package_outputs_commands() - def _create_general_tests_optimizer(self, build_context: BuildContext = None): if not build_context: build_context = self._create_build_context() @@ -321,7 +309,7 @@ class GeneralTestsOptimizerTest(fake_filesystem_unittest.TestCase): """ for command in commands: self.assertEqual( - '/tmp/top/host_out/prebuilts/build-tools/linux-x86/bin/soong_zip', + '/tmp/top/prebuilts/build-tools/linux-x86/bin/soong_zip', command[0], ) self.assertEqual('-d', command[1])