From a102909e09dd61298d3ee15190105ac4951b4860 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Kongstad?= Date: Mon, 8 May 2023 11:51:59 +0200 Subject: [PATCH] aconfig: add dump protobuf format Introduce a new protobuf format to represent the all flags parsed by aconfig. This data in this new format is similar to that of the internal cache object, but the protobuf is a public API for other tools to consume and any changes to the proto spec must be backwards compatible. When aconfig has matured more, Cache can potentially be rewritten to work with proto structs directly, removing some of the hand-written wrapper structs and the logic to convert to and from the proto structs. At this point, the intermediate json format can be replaced by the protobuf dump. Also, teach `aconfig dump` to accept an --out argument (default: stdout). Also, teach `aconfig dump` to read more than once cache file. Note: the new protobuf fields refer to existing fields. It would make sense to split the .proto file in one for input and one for output formats, and import the common messages, but the Android build system and cargo will need different import paths. Keep the definitions in the same file to circumvent this problem. Bug: 279485059 Test: atest aconfig.test Change-Id: I55ee4a52c0fb3369d91d61406867ae03a15805c3 --- tools/aconfig/protos/aconfig.proto | 18 +++++++++ tools/aconfig/src/aconfig.rs | 57 +++++++++++++++++++++++++++- tools/aconfig/src/cache.rs | 6 ++- tools/aconfig/src/commands.rs | 60 ++++++++++++++++++++++++++---- tools/aconfig/src/main.rs | 26 +++++++++---- tools/aconfig/src/protos.rs | 18 +++++++++ 6 files changed, 167 insertions(+), 18 deletions(-) diff --git a/tools/aconfig/protos/aconfig.proto b/tools/aconfig/protos/aconfig.proto index 6eac414b1e..f3d61487bc 100644 --- a/tools/aconfig/protos/aconfig.proto +++ b/tools/aconfig/protos/aconfig.proto @@ -55,3 +55,21 @@ message override { message override_config { repeated override override = 1; }; + +message dump { + repeated dump_item item = 1; +} + +message dump_trace { + required string source = 1; + required flag_state state = 2; + required permission permission = 3; +} + +message dump_item { + required string id = 1; + required string description = 2; + required flag_state state = 3; + required permission permission = 4; + repeated dump_trace trace = 5; +} diff --git a/tools/aconfig/src/aconfig.rs b/tools/aconfig/src/aconfig.rs index f10ca1ff8b..edab36c781 100644 --- a/tools/aconfig/src/aconfig.rs +++ b/tools/aconfig/src/aconfig.rs @@ -18,9 +18,10 @@ use anyhow::{anyhow, Context, Error, Result}; use protobuf::{Enum, EnumOrUnknown}; use serde::{Deserialize, Serialize}; +use crate::cache::{Cache, Item, TracePoint}; use crate::protos::{ - ProtoAndroidConfig, ProtoFlag, ProtoFlagState, ProtoOverride, ProtoOverrideConfig, - ProtoPermission, ProtoValue, + ProtoAndroidConfig, ProtoDump, ProtoDumpItem, ProtoDumpTracePoint, ProtoFlag, ProtoFlagState, + ProtoOverride, ProtoOverrideConfig, ProtoPermission, ProtoValue, }; #[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Clone, Copy)] @@ -41,6 +42,15 @@ impl TryFrom> for FlagState { } } +impl From for ProtoFlagState { + fn from(state: FlagState) -> Self { + match state { + FlagState::Enabled => ProtoFlagState::ENABLED, + FlagState::Disabled => ProtoFlagState::DISABLED, + } + } +} + #[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Clone, Copy)] pub enum Permission { ReadOnly, @@ -59,6 +69,15 @@ impl TryFrom> for Permission { } } +impl From for ProtoPermission { + fn from(permission: Permission) -> Self { + match permission { + Permission::ReadOnly => ProtoPermission::READ_ONLY, + Permission::ReadWrite => ProtoPermission::READ_WRITE, + } + } +} + #[derive(Debug, PartialEq, Eq)] pub struct Value { state: FlagState, @@ -202,6 +221,40 @@ impl TryFrom for Override { } } +impl From for ProtoDump { + fn from(cache: Cache) -> Self { + let mut dump = ProtoDump::new(); + for item in cache.into_iter() { + dump.item.push(item.into()); + } + dump + } +} + +impl From for ProtoDumpItem { + fn from(item: Item) -> Self { + let mut dump_item = crate::protos::ProtoDumpItem::new(); + dump_item.set_id(item.id.clone()); + dump_item.set_description(item.description.clone()); + dump_item.set_state(item.state.into()); + dump_item.set_permission(item.permission.into()); + for trace in item.trace.into_iter() { + dump_item.trace.push(trace.into()); + } + dump_item + } +} + +impl From for ProtoDumpTracePoint { + fn from(tracepoint: TracePoint) -> Self { + let mut dump_tracepoint = ProtoDumpTracePoint::new(); + dump_tracepoint.set_source(format!("{}", tracepoint.source)); + dump_tracepoint.set_state(tracepoint.state.into()); + dump_tracepoint.set_permission(tracepoint.permission.into()); + dump_tracepoint + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/tools/aconfig/src/cache.rs b/tools/aconfig/src/cache.rs index 94443d7309..fffeb9b49b 100644 --- a/tools/aconfig/src/cache.rs +++ b/tools/aconfig/src/cache.rs @@ -92,9 +92,11 @@ impl Cache { pub fn iter(&self) -> impl Iterator { self.items.iter() } -} -impl Item {} + pub fn into_iter(self) -> impl Iterator { + self.items.into_iter() + } +} #[cfg(test)] mod tests { diff --git a/tools/aconfig/src/commands.rs b/tools/aconfig/src/commands.rs index 73d335763a..645d39540d 100644 --- a/tools/aconfig/src/commands.rs +++ b/tools/aconfig/src/commands.rs @@ -16,12 +16,14 @@ use anyhow::{Context, Result}; use clap::ValueEnum; +use protobuf::Message; use serde::{Deserialize, Serialize}; use std::fmt; use std::io::Read; use crate::aconfig::{Flag, Override}; use crate::cache::Cache; +use crate::protos::ProtoDump; #[derive(Serialize, Deserialize, Clone, Debug)] pub enum Source { @@ -74,22 +76,32 @@ pub fn create_cache(build_id: u32, aconfigs: Vec, overrides: Vec) pub enum Format { Text, Debug, + Protobuf, } -pub fn dump_cache(cache: Cache, format: Format) -> Result<()> { +pub fn dump_cache(cache: Cache, format: Format) -> Result> { match format { Format::Text => { + let mut lines = vec![]; for item in cache.iter() { - println!("{}: {:?}", item.id, item.state); + lines.push(format!("{}: {:?}\n", item.id, item.state)); } + Ok(lines.concat().into()) } Format::Debug => { + let mut lines = vec![]; for item in cache.iter() { - println!("{:?}", item); + lines.push(format!("{:?}\n", item)); } + Ok(lines.concat().into()) + } + Format::Protobuf => { + let dump: ProtoDump = cache.into(); + let mut output = vec![]; + dump.write_to_vec(&mut output)?; + Ok(output) } } - Ok(()) } #[cfg(test)] @@ -97,8 +109,7 @@ mod tests { use super::*; use crate::aconfig::{FlagState, Permission}; - #[test] - fn test_create_cache() { + fn create_test_cache() -> Cache { let s = r#" flag { id: "a" @@ -108,6 +119,14 @@ mod tests { permission: READ_WRITE } } + flag { + id: "b" + description: "Description of b" + value { + state: ENABLED + permission: READ_ONLY + } + } "#; let aconfigs = vec![Input { source: Source::Memory, reader: Box::new(s.as_bytes()) }]; let o = r#" @@ -118,9 +137,36 @@ mod tests { } "#; let overrides = vec![Input { source: Source::Memory, reader: Box::new(o.as_bytes()) }]; - let cache = create_cache(1, aconfigs, overrides).unwrap(); + create_cache(1, aconfigs, overrides).unwrap() + } + + #[test] + fn test_create_cache() { + let cache = create_test_cache(); // calls create_cache let item = cache.iter().find(|&item| item.id == "a").unwrap(); assert_eq!(FlagState::Disabled, item.state); assert_eq!(Permission::ReadOnly, item.permission); } + + #[test] + fn test_dump_text_format() { + let cache = create_test_cache(); + let bytes = dump_cache(cache, Format::Text).unwrap(); + let text = std::str::from_utf8(&bytes).unwrap(); + assert!(text.contains("a: Disabled")); + } + + #[test] + fn test_dump_protobuf_format() { + use protobuf::Message; + + let cache = create_test_cache(); + let bytes = dump_cache(cache, Format::Protobuf).unwrap(); + let actual = ProtoDump::parse_from_bytes(&bytes).unwrap(); + + assert_eq!( + vec!["a".to_string(), "b".to_string()], + actual.item.iter().map(|item| item.id.clone().unwrap()).collect::>() + ); + } } diff --git a/tools/aconfig/src/main.rs b/tools/aconfig/src/main.rs index 62750ae9a1..dbff86e065 100644 --- a/tools/aconfig/src/main.rs +++ b/tools/aconfig/src/main.rs @@ -19,6 +19,8 @@ use anyhow::Result; use clap::{builder::ArgAction, builder::EnumValueParser, Arg, ArgMatches, Command}; use std::fs; +use std::io; +use std::io::Write; mod aconfig; mod cache; @@ -44,12 +46,15 @@ fn cli() -> Command { .arg(Arg::new("cache").long("cache").required(true)), ) .subcommand( - Command::new("dump").arg(Arg::new("cache").long("cache").required(true)).arg( - Arg::new("format") - .long("format") - .value_parser(EnumValueParser::::new()) - .default_value("text"), - ), + Command::new("dump") + .arg(Arg::new("cache").long("cache").required(true)) + .arg( + Arg::new("format") + .long("format") + .value_parser(EnumValueParser::::new()) + .default_value("text"), + ) + .arg(Arg::new("out").long("out").default_value("-")), ) } @@ -79,7 +84,14 @@ fn main() -> Result<()> { let file = fs::File::open(path)?; let cache = Cache::read_from_reader(file)?; let format = sub_matches.get_one("format").unwrap(); - commands::dump_cache(cache, *format)?; + let output = commands::dump_cache(cache, *format)?; + let path = sub_matches.get_one::("out").unwrap(); + let mut file: Box = if path == "-" { + Box::new(io::stdout()) + } else { + Box::new(fs::File::create(path)?) + }; + file.write_all(&output)?; } _ => unreachable!(), } diff --git a/tools/aconfig/src/protos.rs b/tools/aconfig/src/protos.rs index 604eca4424..f18a91db0c 100644 --- a/tools/aconfig/src/protos.rs +++ b/tools/aconfig/src/protos.rs @@ -48,6 +48,15 @@ pub use aconfig_protos::aconfig::Permission as ProtoPermission; #[cfg(not(feature = "cargo"))] pub use aconfig_protos::aconfig::Flag_state as ProtoFlagState; +#[cfg(not(feature = "cargo"))] +pub use aconfig_protos::aconfig::Dump as ProtoDump; + +#[cfg(not(feature = "cargo"))] +pub use aconfig_protos::aconfig::Dump_item as ProtoDumpItem; + +#[cfg(not(feature = "cargo"))] +pub use aconfig_protos::aconfig::Dump_trace as ProtoDumpTracePoint; + // ---- When building with cargo ---- #[cfg(feature = "cargo")] include!(concat!(env!("OUT_DIR"), "/aconfig_proto/mod.rs")); @@ -73,6 +82,15 @@ pub use aconfig::Permission as ProtoPermission; #[cfg(feature = "cargo")] pub use aconfig::Flag_state as ProtoFlagState; +#[cfg(feature = "cargo")] +pub use aconfig::Dump as ProtoDump; + +#[cfg(feature = "cargo")] +pub use aconfig::Dump_item as ProtoDumpItem; + +#[cfg(feature = "cargo")] +pub use aconfig::Dump_trace as ProtoDumpTracePoint; + // ---- Common for both the Android tool-chain and cargo ---- use anyhow::Result;