Support aconfig dump --dedup
Add a flag to aconfig dump that will allow identical flags to be merged
without erroring. This will allow merging the aconfig cache files from
dependencies for each module, which requires passing less data to Make
from Soong, and thus reduces the percentage of builds require Kati
analysis.
Bug: 313698230
Test: aconfig.test
Change-Id: Id2d9b78225720ce01defad4b13ea9395f6ab1033
diff --git a/tools/aconfig/src/commands.rs b/tools/aconfig/src/commands.rs
index ff0df1f..ed7489f 100644
--- a/tools/aconfig/src/commands.rs
+++ b/tools/aconfig/src/commands.rs
@@ -250,11 +250,11 @@
Textproto,
}
-pub fn dump_parsed_flags(mut input: Vec<Input>, format: DumpFormat) -> Result<Vec<u8>> {
+pub fn dump_parsed_flags(mut input: Vec<Input>, format: DumpFormat, dedup: bool) -> Result<Vec<u8>> {
let individually_parsed_flags: Result<Vec<ProtoParsedFlags>> =
input.iter_mut().map(|i| i.try_parse_flags()).collect();
let parsed_flags: ProtoParsedFlags =
- crate::protos::parsed_flags::merge(individually_parsed_flags?)?;
+ crate::protos::parsed_flags::merge(individually_parsed_flags?, dedup)?;
let mut output = Vec::new();
match format {
@@ -448,7 +448,7 @@
#[test]
fn test_dump_text_format() {
let input = parse_test_flags_as_input();
- let bytes = dump_parsed_flags(vec![input], DumpFormat::Text).unwrap();
+ let bytes = dump_parsed_flags(vec![input], DumpFormat::Text, false).unwrap();
let text = std::str::from_utf8(&bytes).unwrap();
assert!(text.contains("com.android.aconfig.test.disabled_ro: READ_ONLY + DISABLED"));
}
@@ -463,7 +463,7 @@
.unwrap();
let input = parse_test_flags_as_input();
- let actual = dump_parsed_flags(vec![input], DumpFormat::Protobuf).unwrap();
+ let actual = dump_parsed_flags(vec![input], DumpFormat::Protobuf, false).unwrap();
assert_eq!(expected, actual);
}
@@ -471,7 +471,16 @@
#[test]
fn test_dump_textproto_format() {
let input = parse_test_flags_as_input();
- let bytes = dump_parsed_flags(vec![input], DumpFormat::Textproto).unwrap();
+ let bytes = dump_parsed_flags(vec![input], DumpFormat::Textproto, false).unwrap();
+ let text = std::str::from_utf8(&bytes).unwrap();
+ assert_eq!(crate::test::TEST_FLAGS_TEXTPROTO.trim(), text.trim());
+ }
+
+ #[test]
+ fn test_dump_textproto_format_dedup() {
+ let input = parse_test_flags_as_input();
+ let input2 = parse_test_flags_as_input();
+ let bytes = dump_parsed_flags(vec![input, input2], DumpFormat::Textproto, true).unwrap();
let text = std::str::from_utf8(&bytes).unwrap();
assert_eq!(crate::test::TEST_FLAGS_TEXTPROTO.trim(), text.trim());
}
diff --git a/tools/aconfig/src/main.rs b/tools/aconfig/src/main.rs
index 7e44baf..17c388d 100644
--- a/tools/aconfig/src/main.rs
+++ b/tools/aconfig/src/main.rs
@@ -99,13 +99,14 @@
)
.subcommand(
Command::new("dump")
- .arg(Arg::new("cache").long("cache").action(ArgAction::Append).required(true))
+ .arg(Arg::new("cache").long("cache").action(ArgAction::Append))
.arg(
Arg::new("format")
.long("format")
.value_parser(EnumValueParser::<commands::DumpFormat>::new())
.default_value("text"),
)
+ .arg(Arg::new("dedup").long("dedup").num_args(0).action(ArgAction::SetTrue))
.arg(Arg::new("out").long("out").default_value("-")),
)
}
@@ -222,7 +223,8 @@
let input = open_zero_or_more_files(sub_matches, "cache")?;
let format = get_required_arg::<DumpFormat>(sub_matches, "format")
.context("failed to dump previously parsed flags")?;
- let output = commands::dump_parsed_flags(input, *format)?;
+ let dedup = get_required_arg::<bool>(sub_matches, "dedup")?;
+ let output = commands::dump_parsed_flags(input, *format, *dedup)?;
let path = get_required_arg::<String>(sub_matches, "out")?;
write_output_to_file_or_stdout(path, &output)?;
}
diff --git a/tools/aconfig/src/protos.rs b/tools/aconfig/src/protos.rs
index a5a5342..8e76468 100644
--- a/tools/aconfig/src/protos.rs
+++ b/tools/aconfig/src/protos.rs
@@ -274,12 +274,18 @@
Ok(())
}
- pub fn merge(parsed_flags: Vec<ProtoParsedFlags>) -> Result<ProtoParsedFlags> {
+ pub fn merge(parsed_flags: Vec<ProtoParsedFlags>, dedup: bool) -> Result<ProtoParsedFlags> {
let mut merged = ProtoParsedFlags::new();
for mut pfs in parsed_flags.into_iter() {
merged.parsed_flag.append(&mut pfs.parsed_flag);
}
merged.parsed_flag.sort_by_cached_key(create_sorting_key);
+ if dedup {
+ // Deduplicate identical protobuf messages. Messages with the same sorting key but
+ // different fields (including the path to the original source file) will not be
+ // deduplicated and trigger an error in verify_fields.
+ merged.parsed_flag.dedup();
+ }
verify_fields(&merged)?;
Ok(merged)
}
@@ -840,14 +846,49 @@
"#;
let second = try_from_binary_proto_from_text_proto(text_proto).unwrap();
+ let text_proto = r#"
+parsed_flag {
+ package: "com.second"
+ name: "second"
+ namespace: "second_ns"
+ bug: "b"
+ description: "This is the description of the second flag."
+ state: ENABLED
+ permission: READ_WRITE
+ trace {
+ source: "duplicate/flags.declarations"
+ state: DISABLED
+ permission: READ_ONLY
+ }
+}
+"#;
+ let second_duplicate = try_from_binary_proto_from_text_proto(text_proto).unwrap();
+
// bad cases
- let error = parsed_flags::merge(vec![first.clone(), first.clone()]).unwrap_err();
+
+ // two of the same flag with dedup disabled
+ let error = parsed_flags::merge(vec![first.clone(), first.clone()], false).unwrap_err();
assert_eq!(format!("{:?}", error), "bad parsed flags: duplicate flag com.first.first (defined in flags.declarations and flags.declarations)");
+ // two conflicting flags with dedup disabled
+ let error = parsed_flags::merge(vec![second.clone(), second_duplicate.clone()], false).unwrap_err();
+ assert_eq!(format!("{:?}", error), "bad parsed flags: duplicate flag com.second.second (defined in flags.declarations and duplicate/flags.declarations)");
+
+ // two conflicting flags with dedup enabled
+ let error = parsed_flags::merge(vec![second.clone(), second_duplicate.clone()], true).unwrap_err();
+ assert_eq!(format!("{:?}", error), "bad parsed flags: duplicate flag com.second.second (defined in flags.declarations and duplicate/flags.declarations)");
+
// valid cases
- assert!(parsed_flags::merge(vec![]).unwrap().parsed_flag.is_empty());
- assert_eq!(first, parsed_flags::merge(vec![first.clone()]).unwrap());
- assert_eq!(expected, parsed_flags::merge(vec![first.clone(), second.clone()]).unwrap());
- assert_eq!(expected, parsed_flags::merge(vec![second, first]).unwrap());
+ assert!(parsed_flags::merge(vec![], false).unwrap().parsed_flag.is_empty());
+ assert!(parsed_flags::merge(vec![], true).unwrap().parsed_flag.is_empty());
+ assert_eq!(first, parsed_flags::merge(vec![first.clone()], false).unwrap());
+ assert_eq!(first, parsed_flags::merge(vec![first.clone()], true).unwrap());
+ assert_eq!(expected, parsed_flags::merge(vec![first.clone(), second.clone()], false).unwrap());
+ assert_eq!(expected, parsed_flags::merge(vec![first.clone(), second.clone()], true).unwrap());
+ assert_eq!(expected, parsed_flags::merge(vec![second.clone(), first.clone()], false).unwrap());
+ assert_eq!(expected, parsed_flags::merge(vec![second.clone(), first.clone()], true).unwrap());
+
+ // two identical flags with dedup enabled
+ assert_eq!(first, parsed_flags::merge(vec![first.clone(), first.clone()], true).unwrap());
}
}