From f7c2fbef9f564dc2e13f438770328381ad19b474 Mon Sep 17 00:00:00 2001 From: Ronish Kalia Date: Fri, 13 Sep 2024 23:38:46 +0000 Subject: [PATCH 1/3] Update metadata target to the correct one Bug: 326986729 (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:3fb924d1eddfe031d799f34a2f32d39b9b671a44) Merged-In: I3da3d31abd3280f85aad30f81151502ec32fc368 Change-Id: I3da3d31abd3280f85aad30f81151502ec32fc368 --- ci/build_metadata | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/build_metadata b/ci/build_metadata index 8136702d94..a8eb65dd36 100755 --- a/ci/build_metadata +++ b/ci/build_metadata @@ -21,5 +21,5 @@ export TARGET_RELEASE=trunk_staging export TARGET_BUILD_VARIANT=eng build/soong/bin/m dist \ - code_metadata + all_teams From 45b0438cc0ad3f2d68c7d7ab760099ddb9571ab3 Mon Sep 17 00:00:00 2001 From: Marybeth Fair Date: Thu, 12 Sep 2024 16:14:11 -0400 Subject: [PATCH 2/3] Add ability to manually write to binary files. I was updating the format of PackageTableHeader to add an additional field (and due to that change incremented the file version). This broke several tests under aconfig_storage_read_api and aconfig_storage_write_api that were operating on files written in the old schema. I tried to re-generate them using aconfig create-storage as explained in aosp/2933375, but was having some trouble. Figure if we can just update the files directly it will be easier to make updates in the future anyway. This isn't bypassing logic that's tested - IIUC the tests cover reading the file correctly (writing is covered in separate tests). Usage: $ aconfig-storage print --file=path/to/flag.map --type=flag_map --format=json > flag_map.json $ vim flag_map.json // Manually make updates $ aconfig-storage write-bytes --input-file=flag_map.json --output-file=path/to/flag.map --type=flag_map Change-Id: I212bf0b97483740b30130eb121acb895d350da84 Test: manual (adding debug-only tooling) + cargo t Bug: 316357686 --- tools/aconfig/aconfig_storage_file/Android.bp | 6 +- tools/aconfig/aconfig_storage_file/Cargo.toml | 2 + .../aconfig_storage_file/src/flag_info.rs | 9 +- .../aconfig_storage_file/src/flag_table.rs | 7 +- .../aconfig_storage_file/src/flag_value.rs | 5 +- tools/aconfig/aconfig_storage_file/src/lib.rs | 3 +- .../aconfig/aconfig_storage_file/src/main.rs | 111 ++++++++++++++++-- .../aconfig_storage_file/src/package_table.rs | 7 +- 8 files changed, 125 insertions(+), 25 deletions(-) diff --git a/tools/aconfig/aconfig_storage_file/Android.bp b/tools/aconfig/aconfig_storage_file/Android.bp index 40b4464167..e875c7be6a 100644 --- a/tools/aconfig/aconfig_storage_file/Android.bp +++ b/tools/aconfig/aconfig_storage_file/Android.bp @@ -14,6 +14,7 @@ rust_defaults { "libclap", "libcxx", "libaconfig_storage_protos", + "libserde", ], } @@ -36,7 +37,10 @@ rust_binary_host { name: "aconfig-storage", defaults: ["aconfig_storage_file.defaults"], srcs: ["src/main.rs"], - rustlibs: ["libaconfig_storage_file"], + rustlibs: [ + "libaconfig_storage_file", + "libserde_json", + ], } rust_test_host { diff --git a/tools/aconfig/aconfig_storage_file/Cargo.toml b/tools/aconfig/aconfig_storage_file/Cargo.toml index 192dfad40a..a40557803f 100644 --- a/tools/aconfig/aconfig_storage_file/Cargo.toml +++ b/tools/aconfig/aconfig_storage_file/Cargo.toml @@ -14,6 +14,8 @@ tempfile = "3.9.0" thiserror = "1.0.56" clap = { version = "4.1.8", features = ["derive"] } cxx = "1.0" +serde = { version = "1.0.152", features = ["derive"] } +serde_json = "1.0.93" [[bin]] name = "aconfig-storage" diff --git a/tools/aconfig/aconfig_storage_file/src/flag_info.rs b/tools/aconfig/aconfig_storage_file/src/flag_info.rs index beac38d156..f090396901 100644 --- a/tools/aconfig/aconfig_storage_file/src/flag_info.rs +++ b/tools/aconfig/aconfig_storage_file/src/flag_info.rs @@ -20,10 +20,11 @@ use crate::{read_str_from_bytes, read_u32_from_bytes, read_u8_from_bytes}; use crate::{AconfigStorageError, StorageFileType}; use anyhow::anyhow; +use serde::{Deserialize, Serialize}; use std::fmt; /// Flag info header struct -#[derive(PartialEq)] +#[derive(PartialEq, Serialize, Deserialize)] pub struct FlagInfoHeader { pub version: u32, pub container: String, @@ -89,7 +90,7 @@ impl FlagInfoHeader { } /// bit field for flag info -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub enum FlagInfoBit { HasServerOverride = 1 << 0, IsReadWrite = 1 << 1, @@ -97,7 +98,7 @@ pub enum FlagInfoBit { } /// Flag info node struct -#[derive(PartialEq, Clone)] +#[derive(PartialEq, Clone, Serialize, Deserialize)] pub struct FlagInfoNode { pub attributes: u8, } @@ -138,7 +139,7 @@ impl FlagInfoNode { } /// Flag info list struct -#[derive(PartialEq)] +#[derive(PartialEq, Serialize, Deserialize)] pub struct FlagInfoList { pub header: FlagInfoHeader, pub nodes: Vec, diff --git a/tools/aconfig/aconfig_storage_file/src/flag_table.rs b/tools/aconfig/aconfig_storage_file/src/flag_table.rs index 660edac0c7..0588fe5039 100644 --- a/tools/aconfig/aconfig_storage_file/src/flag_table.rs +++ b/tools/aconfig/aconfig_storage_file/src/flag_table.rs @@ -23,10 +23,11 @@ use crate::{ }; use crate::{AconfigStorageError, StorageFileType, StoredFlagType}; use anyhow::anyhow; +use serde::{Deserialize, Serialize}; use std::fmt; /// Flag table header struct -#[derive(PartialEq)] +#[derive(PartialEq, Serialize, Deserialize)] pub struct FlagTableHeader { pub version: u32, pub container: String, @@ -95,7 +96,7 @@ impl FlagTableHeader { } /// Flag table node struct -#[derive(PartialEq, Clone)] +#[derive(PartialEq, Clone, Serialize, Deserialize)] pub struct FlagTableNode { pub package_id: u32, pub flag_name: String, @@ -154,7 +155,7 @@ impl FlagTableNode { } } -#[derive(PartialEq)] +#[derive(PartialEq, Serialize, Deserialize)] pub struct FlagTable { pub header: FlagTableHeader, pub buckets: Vec>, diff --git a/tools/aconfig/aconfig_storage_file/src/flag_value.rs b/tools/aconfig/aconfig_storage_file/src/flag_value.rs index 506924b339..b64c10ecdd 100644 --- a/tools/aconfig/aconfig_storage_file/src/flag_value.rs +++ b/tools/aconfig/aconfig_storage_file/src/flag_value.rs @@ -20,10 +20,11 @@ use crate::{read_str_from_bytes, read_u32_from_bytes, read_u8_from_bytes}; use crate::{AconfigStorageError, StorageFileType}; use anyhow::anyhow; +use serde::{Deserialize, Serialize}; use std::fmt; /// Flag value header struct -#[derive(PartialEq)] +#[derive(PartialEq, Serialize, Deserialize)] pub struct FlagValueHeader { pub version: u32, pub container: String, @@ -89,7 +90,7 @@ impl FlagValueHeader { } /// Flag value list struct -#[derive(PartialEq)] +#[derive(PartialEq, Serialize, Deserialize)] pub struct FlagValueList { pub header: FlagValueHeader, pub booleans: Vec, diff --git a/tools/aconfig/aconfig_storage_file/src/lib.rs b/tools/aconfig/aconfig_storage_file/src/lib.rs index b6367ffa35..cf52bc017d 100644 --- a/tools/aconfig/aconfig_storage_file/src/lib.rs +++ b/tools/aconfig/aconfig_storage_file/src/lib.rs @@ -41,6 +41,7 @@ pub mod sip_hasher13; pub mod test_utils; use anyhow::anyhow; +use serde::{Deserialize, Serialize}; use std::cmp::Ordering; use std::fs::File; use std::hash::Hasher; @@ -107,7 +108,7 @@ impl TryFrom for StorageFileType { /// Flag type enum as stored by storage file /// ONLY APPEND, NEVER REMOVE FOR BACKWARD COMPATIBILITY. THE MAX IS U16. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] pub enum StoredFlagType { ReadWriteBoolean = 0, ReadOnlyBoolean = 1, diff --git a/tools/aconfig/aconfig_storage_file/src/main.rs b/tools/aconfig/aconfig_storage_file/src/main.rs index 8b9e38da02..a9cfd19066 100644 --- a/tools/aconfig/aconfig_storage_file/src/main.rs +++ b/tools/aconfig/aconfig_storage_file/src/main.rs @@ -20,9 +20,29 @@ use aconfig_storage_file::{ list_flags, list_flags_with_info, read_file_to_bytes, AconfigStorageError, FlagInfoList, FlagTable, FlagValueList, PackageTable, StorageFileType, }; - use clap::{builder::ArgAction, Arg, Command}; +use serde::Serialize; +use serde_json; +use std::fmt; +use std::fs; +use std::fs::File; +use std::io::Write; +/** + * Usage Examples + * + * Print file: + * $ aconfig-storage print --file=path/to/flag.map --type=flag_map + * + * List flags: + * $ aconfig-storage list --flag-map=path/to/flag.map \ + * --flag-val=path/to/flag.val --package-map=path/to/package.map + * + * Write binary file for testing: + * $ aconfig-storage print --file=path/to/flag.map --type=flag_map --format=json > flag_map.json + * $ vim flag_map.json // Manually make updates + * $ aconfig-storage write-bytes --input-file=flag_map.json --output-file=path/to/flag.map --type=flag_map + */ fn cli() -> Command { Command::new("aconfig-storage") .subcommand_required(true) @@ -34,7 +54,8 @@ fn cli() -> Command { .long("type") .required(true) .value_parser(|s: &str| StorageFileType::try_from(s)), - ), + ) + .arg(Arg::new("format").long("format").required(false).action(ArgAction::Set)), ) .subcommand( Command::new("list") @@ -50,41 +71,75 @@ fn cli() -> Command { Arg::new("flag-info").long("flag-info").required(false).action(ArgAction::Set), ), ) + .subcommand( + Command::new("write-bytes") + // Where to write the output bytes. Suggest to use the StorageFileType names (e.g. flag.map). + .arg( + Arg::new("output-file") + .long("output-file") + .required(true) + .action(ArgAction::Set), + ) + // Input file should be json. + .arg( + Arg::new("input-file").long("input-file").required(true).action(ArgAction::Set), + ) + .arg( + Arg::new("type") + .long("type") + .required(true) + .value_parser(|s: &str| StorageFileType::try_from(s)), + ), + ) } fn print_storage_file( file_path: &str, file_type: &StorageFileType, + as_json: bool, ) -> Result<(), AconfigStorageError> { let bytes = read_file_to_bytes(file_path)?; match file_type { StorageFileType::PackageMap => { let package_table = PackageTable::from_bytes(&bytes)?; - println!("{:?}", package_table); + println!("{}", to_print_format(package_table, as_json)); } StorageFileType::FlagMap => { let flag_table = FlagTable::from_bytes(&bytes)?; - println!("{:?}", flag_table); + println!("{}", to_print_format(flag_table, as_json)); } StorageFileType::FlagVal => { let flag_value = FlagValueList::from_bytes(&bytes)?; - println!("{:?}", flag_value); + println!("{}", to_print_format(flag_value, as_json)); } StorageFileType::FlagInfo => { let flag_info = FlagInfoList::from_bytes(&bytes)?; - println!("{:?}", flag_info); + println!("{}", to_print_format(flag_info, as_json)); } } Ok(()) } +fn to_print_format(file_contents: T, as_json: bool) -> String +where + T: Serialize + fmt::Debug, +{ + if as_json { + serde_json::to_string(&file_contents).unwrap() + } else { + format!("{:?}", file_contents) + } +} + fn main() -> Result<(), AconfigStorageError> { let matches = cli().get_matches(); match matches.subcommand() { Some(("print", sub_matches)) => { let file_path = sub_matches.get_one::("file").unwrap(); let file_type = sub_matches.get_one::("type").unwrap(); - print_storage_file(file_path, file_type)? + let format = sub_matches.get_one::("format"); + let as_json: bool = format == Some(&"json".to_string()); + print_storage_file(file_path, file_type, as_json)? } Some(("list", sub_matches)) => { let package_map = sub_matches.get_one::("package-map").unwrap(); @@ -96,10 +151,10 @@ fn main() -> Result<(), AconfigStorageError> { let flags = list_flags_with_info(package_map, flag_map, flag_val, info_file)?; for flag in flags.iter() { println!( - "{} {} {} {:?} IsReadWrite: {}, HasServerOverride: {}, HasLocalOverride: {}", - flag.package_name, flag.flag_name, flag.flag_value, flag.value_type, - flag.is_readwrite, flag.has_server_override, flag.has_local_override, - ); + "{} {} {} {:?} IsReadWrite: {}, HasServerOverride: {}, HasLocalOverride: {}", + flag.package_name, flag.flag_name, flag.flag_value, flag.value_type, + flag.is_readwrite, flag.has_server_override, flag.has_local_override, + ); } } None => { @@ -113,6 +168,40 @@ fn main() -> Result<(), AconfigStorageError> { } } } + // Converts JSON of the file into raw bytes (as is used on-device). + // Intended to generate/easily update these files for testing. + Some(("write-bytes", sub_matches)) => { + let input_file_path = sub_matches.get_one::("input-file").unwrap(); + let input_json = fs::read_to_string(input_file_path).unwrap(); + + let file_type = sub_matches.get_one::("type").unwrap(); + let output_bytes: Vec; + match file_type { + StorageFileType::FlagVal => { + let list: FlagValueList = serde_json::from_str(&input_json).unwrap(); + output_bytes = list.into_bytes(); + } + StorageFileType::FlagInfo => { + let list: FlagInfoList = serde_json::from_str(&input_json).unwrap(); + output_bytes = list.into_bytes(); + } + StorageFileType::FlagMap => { + let table: FlagTable = serde_json::from_str(&input_json).unwrap(); + output_bytes = table.into_bytes(); + } + StorageFileType::PackageMap => { + let table: PackageTable = serde_json::from_str(&input_json).unwrap(); + output_bytes = table.into_bytes(); + } + } + + let output_file_path = sub_matches.get_one::("output-file").unwrap(); + let file = File::create(output_file_path); + if file.is_err() { + panic!("can't make file"); + } + let _ = file.unwrap().write_all(&output_bytes); + } _ => unreachable!(), } Ok(()) diff --git a/tools/aconfig/aconfig_storage_file/src/package_table.rs b/tools/aconfig/aconfig_storage_file/src/package_table.rs index 007f86ed1a..a5bd9e6446 100644 --- a/tools/aconfig/aconfig_storage_file/src/package_table.rs +++ b/tools/aconfig/aconfig_storage_file/src/package_table.rs @@ -20,10 +20,11 @@ use crate::{get_bucket_index, read_str_from_bytes, read_u32_from_bytes, read_u8_from_bytes}; use crate::{AconfigStorageError, StorageFileType}; use anyhow::anyhow; +use serde::{Deserialize, Serialize}; use std::fmt; /// Package table header struct -#[derive(PartialEq)] +#[derive(PartialEq, Serialize, Deserialize)] pub struct PackageTableHeader { pub version: u32, pub container: String, @@ -92,7 +93,7 @@ impl PackageTableHeader { } /// Package table node struct -#[derive(PartialEq)] +#[derive(PartialEq, Serialize, Deserialize)] pub struct PackageTableNode { pub package_name: String, pub package_id: u32, @@ -151,7 +152,7 @@ impl PackageTableNode { } /// Package table struct -#[derive(PartialEq)] +#[derive(PartialEq, Serialize, Deserialize)] pub struct PackageTable { pub header: PackageTableHeader, pub buckets: Vec>, From 431f44a31481014e339086a34c7369813d1fb8a7 Mon Sep 17 00:00:00 2001 From: Ted Bauer Date: Fri, 13 Sep 2024 20:59:34 +0000 Subject: [PATCH 3/3] Don't show staged value if flag is RO Bug: 324436145 Change-Id: I493421f29d6e7f5e844dd47cc6417b3d5fe9cdbb Test: m aflags && aflags list --- tools/aconfig/aflags/src/main.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tools/aconfig/aflags/src/main.rs b/tools/aconfig/aflags/src/main.rs index a726cc0369..07b7243ab4 100644 --- a/tools/aconfig/aflags/src/main.rs +++ b/tools/aconfig/aflags/src/main.rs @@ -116,9 +116,10 @@ impl Flag { } fn display_staged_value(&self) -> String { - match self.staged_value { - Some(v) => format!("(->{})", v), - None => "-".to_string(), + match (&self.permission, self.staged_value) { + (FlagPermission::ReadOnly, _) => "-".to_string(), + (FlagPermission::ReadWrite, None) => "-".to_string(), + (FlagPermission::ReadWrite, Some(v)) => format!("(->{})", v), } } }