diff options
author | Colin Cross <ccross@android.com> | 2023-12-05 20:01:48 +0000 |
---|---|---|
committer | Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com> | 2023-12-05 20:01:48 +0000 |
commit | b44c99d16013b481728ac4b858c8f9c6976c7531 (patch) | |
tree | 4b8b08cea73738ff70c461ef4937d695a4fd74a7 | |
parent | ed97f851207bcf2e38431b0f921f6724a43676ad (diff) | |
parent | 952df85c69c65943acf3962e5edf24c2a060ed5c (diff) | |
download | build-b44c99d16013b481728ac4b858c8f9c6976c7531.tar.gz |
Merge "Support aconfig dump --dedup" into main am: 952df85c69
Original change: https://android-review.googlesource.com/c/platform/build/+/2853485
Change-Id: I69fa404f17390677812908e99c3c0c8d7203463a
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
-rw-r--r-- | tools/aconfig/src/commands.rs | 19 | ||||
-rw-r--r-- | tools/aconfig/src/main.rs | 6 | ||||
-rw-r--r-- | tools/aconfig/src/protos.rs | 53 |
3 files changed, 65 insertions, 13 deletions
diff --git a/tools/aconfig/src/commands.rs b/tools/aconfig/src/commands.rs index d04c0cfd29..37ee79e4da 100644 --- a/tools/aconfig/src/commands.rs +++ b/tools/aconfig/src/commands.rs @@ -264,11 +264,11 @@ pub enum DumpFormat { 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 { @@ -529,7 +529,7 @@ mod tests { #[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 [system]: READ_ONLY + DISABLED") @@ -546,7 +546,7 @@ mod tests { .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); } @@ -554,7 +554,16 @@ mod tests { #[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 af0368a545..90464c5e8a 100644 --- a/tools/aconfig/src/main.rs +++ b/tools/aconfig/src/main.rs @@ -101,13 +101,14 @@ fn cli() -> Command { ) .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("-")), ) } @@ -239,7 +240,8 @@ fn main() -> Result<()> { 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 c11596ba57..3d9089cb42 100644 --- a/tools/aconfig/src/protos.rs +++ b/tools/aconfig/src/protos.rs @@ -283,12 +283,18 @@ pub mod parsed_flags { 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) } @@ -907,14 +913,49 @@ parsed_flag { "#; 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()); } } |