Merge "Support aconfig dump --dedup" into main am: 952df85c69 am: b44c99d160

Original change: https://android-review.googlesource.com/c/platform/build/+/2853485

Change-Id: Ibba72ec7ac2bdb99de5dfcc3b3964500085c86e6
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

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());
     }

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)?;
         }

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();
 
-        // bad cases
+        let text_proto = r#"
-        let error = parsed_flags::merge(vec![first.clone(), first.clone()]).unwrap_err();
+parsed_flag {
-        assert_eq!(format!("{:?}", error), "bad parsed flags: duplicate flag com.first.first (defined in flags.declarations and flags.declarations)");
+    package: "com.second"
-
+    name: "second"
-        // valid cases
+    namespace: "second_ns"
-        assert!(parsed_flags::merge(vec![]).unwrap().parsed_flag.is_empty());
+    bug: "b"
-        assert_eq!(first, parsed_flags::merge(vec![first.clone()]).unwrap());
+    description: "This is the description of the second flag."
-        assert_eq!(expected, parsed_flags::merge(vec![first.clone(), second.clone()]).unwrap());
+    state: ENABLED
-        assert_eq!(expected, parsed_flags::merge(vec![second, first]).unwrap());
+    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
+
+        // 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![], 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());
     }
 }