aconfig: add proto `bug` field
Add a `bug` field on the flag_declaration and parsed_flag proto
messages. This field is optional in the sense that it can occur zero or
more times, and aconfig will simply pass any value through.
Bug fields are included in the aconfig dump format, which can be
processed by other tools.
Also unify how protos.rs checks that fields marked 'optional' in the
proto file, but in practice are 'required', are actually set.
Test: atest aconfig.test aconfig.test.java
Bug: 288261336
Change-Id: I93de0005674822c6ff4d699bdc2c6509763a7f7f
diff --git a/tools/aconfig/Android.bp b/tools/aconfig/Android.bp
index 25424c5..3e34d9d 100644
--- a/tools/aconfig/Android.bp
+++ b/tools/aconfig/Android.bp
@@ -27,6 +27,9 @@
"libserde_json",
"libtinytemplate",
],
+ proc_macros: [
+ "libpaste",
+ ]
}
rust_binary_host {
diff --git a/tools/aconfig/Cargo.toml b/tools/aconfig/Cargo.toml
index b3c73b8..941b30d 100644
--- a/tools/aconfig/Cargo.toml
+++ b/tools/aconfig/Cargo.toml
@@ -11,6 +11,7 @@
[dependencies]
anyhow = "1.0.69"
clap = { version = "4.1.8", features = ["derive"] }
+paste = "1.0.11"
protobuf = "3.2.0"
serde = { version = "1.0.152", features = ["derive"] }
serde_json = "1.0.93"
diff --git a/tools/aconfig/protos/aconfig.proto b/tools/aconfig/protos/aconfig.proto
index b59fdfc..4cad69a 100644
--- a/tools/aconfig/protos/aconfig.proto
+++ b/tools/aconfig/protos/aconfig.proto
@@ -38,6 +38,7 @@
optional string name = 1;
optional string namespace = 2;
optional string description = 3;
+ repeated string bug = 4;
};
message flag_declarations {
@@ -70,9 +71,10 @@
optional string name = 2;
optional string namespace = 3;
optional string description = 4;
- optional flag_state state = 5;
- optional flag_permission permission = 6;
- repeated tracepoint trace = 7;
+ repeated string bug = 5;
+ optional flag_state state = 6;
+ optional flag_permission permission = 7;
+ repeated tracepoint trace = 8;
}
message parsed_flags {
diff --git a/tools/aconfig/src/commands.rs b/tools/aconfig/src/commands.rs
index f295697..bd5b49c 100644
--- a/tools/aconfig/src/commands.rs
+++ b/tools/aconfig/src/commands.rs
@@ -74,6 +74,7 @@
parsed_flag.set_name(flag_declaration.take_name());
parsed_flag.set_namespace(flag_declaration.take_namespace());
parsed_flag.set_description(flag_declaration.take_description());
+ parsed_flag.bug.append(&mut flag_declaration.bug);
parsed_flag.set_state(DEFAULT_FLAG_STATE);
parsed_flag.set_permission(DEFAULT_FLAG_PERMISSION);
let mut tracepoint = ProtoTracepoint::new();
@@ -311,6 +312,97 @@
assert!(text.contains("com.android.aconfig.test/disabled_ro: DISABLED READ_ONLY"));
}
+ #[test]
+ fn test_dump_protobuf_format() {
+ let text_proto = r#"
+parsed_flag {
+ package: "com.android.aconfig.test"
+ name: "disabled_ro"
+ namespace: "aconfig_test"
+ description: "This flag is DISABLED + READ_ONLY"
+ state: DISABLED
+ permission: READ_ONLY
+ trace {
+ source: "tests/test.aconfig"
+ state: DISABLED
+ permission: READ_WRITE
+ }
+ trace {
+ source: "tests/first.values"
+ state: DISABLED
+ permission: READ_ONLY
+ }
+ bug: "123"
+}
+parsed_flag {
+ package: "com.android.aconfig.test"
+ name: "disabled_rw"
+ namespace: "aconfig_test"
+ description: "This flag is DISABLED + READ_WRITE"
+ state: DISABLED
+ permission: READ_WRITE
+ trace {
+ source: "tests/test.aconfig"
+ state: DISABLED
+ permission: READ_WRITE
+ }
+ bug: "456"
+}
+parsed_flag {
+ package: "com.android.aconfig.test"
+ name: "enabled_ro"
+ namespace: "aconfig_test"
+ description: "This flag is ENABLED + READ_ONLY"
+ state: ENABLED
+ permission: READ_ONLY
+ trace {
+ source: "tests/test.aconfig"
+ state: DISABLED
+ permission: READ_WRITE
+ }
+ trace {
+ source: "tests/first.values"
+ state: DISABLED
+ permission: READ_WRITE
+ }
+ trace {
+ source: "tests/second.values"
+ state: ENABLED
+ permission: READ_ONLY
+ }
+ bug: "789"
+ bug: "abc"
+}
+parsed_flag {
+ package: "com.android.aconfig.test"
+ name: "enabled_rw"
+ namespace: "aconfig_test"
+ description: "This flag is ENABLED + READ_WRITE"
+ state: ENABLED
+ permission: READ_WRITE
+ trace {
+ source: "tests/test.aconfig"
+ state: DISABLED
+ permission: READ_WRITE
+ }
+ trace {
+ source: "tests/first.values"
+ state: ENABLED
+ permission: READ_WRITE
+ }
+}
+"#;
+ let expected = protobuf::text_format::parse_from_str::<ProtoParsedFlags>(text_proto)
+ .unwrap()
+ .write_to_bytes()
+ .unwrap();
+
+ let input = parse_test_flags_as_input();
+ let actual = dump_parsed_flags(vec![input], DumpFormat::Protobuf).unwrap();
+
+ assert_eq!(expected, actual);
+ }
+
fn parse_test_flags_as_input() -> Input {
let parsed_flags = crate::test::parse_test_flags();
let binary_proto = parsed_flags.write_to_bytes().unwrap();
diff --git a/tools/aconfig/src/protos.rs b/tools/aconfig/src/protos.rs
index beebd93..4d824f2 100644
--- a/tools/aconfig/src/protos.rs
+++ b/tools/aconfig/src/protos.rs
@@ -62,29 +62,39 @@
pub use auto_generated::*;
use anyhow::Result;
+use paste::paste;
fn try_from_text_proto<T>(s: &str) -> Result<T>
where
T: protobuf::MessageFull,
{
- // warning: parse_from_str does not check if required fields are set
protobuf::text_format::parse_from_str(s).map_err(|e| e.into())
}
+macro_rules! ensure_required_fields {
+ ($type:expr, $struct:expr, $($field:expr),+) => {
+ $(
+ paste! {
+ ensure!($struct.[<has_ $field>](), "bad {}: missing {}", $type, $field);
+ }
+ )+
+ };
+}
+
pub mod flag_declaration {
use super::*;
use crate::codegen;
use anyhow::ensure;
pub fn verify_fields(pdf: &ProtoFlagDeclaration) -> Result<()> {
- ensure!(pdf.has_name(), "bad flag declaration: missing name");
- ensure!(pdf.has_namespace(), "bad flag declaration: missing namespace");
- ensure!(pdf.has_description(), "bad flag declaration: missing description");
+ ensure_required_fields!("flag declaration", pdf, "name", "namespace", "description");
ensure!(codegen::is_valid_name_ident(pdf.name()), "bad flag declaration: bad name");
ensure!(codegen::is_valid_name_ident(pdf.namespace()), "bad flag declaration: bad name");
ensure!(!pdf.description().is_empty(), "bad flag declaration: empty description");
+ // ProtoFlagDeclaration.bug: Vec<String>: may be empty, no checks needed
+
Ok(())
}
}
@@ -101,7 +111,7 @@
}
pub fn verify_fields(pdf: &ProtoFlagDeclarations) -> Result<()> {
- ensure!(pdf.has_package(), "bad flag declarations: missing package");
+ ensure_required_fields!("flag declarations", pdf, "package");
ensure!(
codegen::is_valid_package_ident(pdf.package()),
@@ -121,10 +131,7 @@
use anyhow::ensure;
pub fn verify_fields(fv: &ProtoFlagValue) -> Result<()> {
- ensure!(fv.has_package(), "bad flag value: missing package");
- ensure!(fv.has_name(), "bad flag value: missing name");
- ensure!(fv.has_state(), "bad flag value: missing state");
- ensure!(fv.has_permission(), "bad flag value: missing permission");
+ ensure_required_fields!("flag value", fv, "package", "name", "state", "permission");
ensure!(codegen::is_valid_package_ident(fv.package()), "bad flag value: bad package");
ensure!(codegen::is_valid_name_ident(fv.name()), "bad flag value: bad name");
@@ -155,9 +162,7 @@
use anyhow::ensure;
pub fn verify_fields(tp: &ProtoTracepoint) -> Result<()> {
- ensure!(tp.has_source(), "bad tracepoint: missing source");
- ensure!(tp.has_state(), "bad tracepoint: missing state");
- ensure!(tp.has_permission(), "bad tracepoint: missing permission");
+ ensure_required_fields!("tracepoint", tp, "source", "state", "permission");
ensure!(!tp.source().is_empty(), "bad tracepoint: empty source");
@@ -171,12 +176,16 @@
use anyhow::ensure;
pub fn verify_fields(pf: &ProtoParsedFlag) -> Result<()> {
- ensure!(pf.has_package(), "bad parsed flag: missing package");
- ensure!(pf.has_name(), "bad parsed flag: missing name");
- ensure!(pf.has_namespace(), "bad parsed flag: missing namespace");
- ensure!(pf.has_description(), "bad parsed flag: missing description");
- ensure!(pf.has_state(), "bad parsed flag: missing state");
- ensure!(pf.has_permission(), "bad parsed flag: missing permission");
+ ensure_required_fields!(
+ "parsed flag",
+ pf,
+ "package",
+ "name",
+ "namespace",
+ "description",
+ "state",
+ "permission"
+ );
ensure!(codegen::is_valid_package_ident(pf.package()), "bad parsed flag: bad package");
ensure!(codegen::is_valid_name_ident(pf.name()), "bad parsed flag: bad name");
@@ -187,6 +196,8 @@
super::tracepoint::verify_fields(tp)?;
}
+ // ProtoParsedFlag.bug: Vec<String>: may be empty, no checks needed
+
Ok(())
}
}
@@ -251,6 +262,8 @@
name: "first"
namespace: "first_ns"
description: "This is the description of the first flag."
+ bug: "123"
+ bug: "abc"
}
flag {
name: "second"
@@ -265,10 +278,14 @@
assert_eq!(first.name(), "first");
assert_eq!(first.namespace(), "first_ns");
assert_eq!(first.description(), "This is the description of the first flag.");
+ assert_eq!(first.bug.len(), 2);
+ assert_eq!(first.bug[0], "123");
+ assert_eq!(first.bug[1], "abc");
let second = flag_declarations.flag.iter().find(|pf| pf.name() == "second").unwrap();
assert_eq!(second.name(), "second");
assert_eq!(second.namespace(), "second_ns");
assert_eq!(second.description(), "This is the description of the second flag.");
+ assert_eq!(second.bug.len(), 0);
// bad input: missing package in flag declarations
let error = flag_declarations::try_from_text_proto(
diff --git a/tools/aconfig/tests/test.aconfig b/tools/aconfig/tests/test.aconfig
index d09396a..a8f6652 100644
--- a/tools/aconfig/tests/test.aconfig
+++ b/tools/aconfig/tests/test.aconfig
@@ -7,6 +7,7 @@
name: "disabled_ro"
namespace: "aconfig_test"
description: "This flag is DISABLED + READ_ONLY"
+ bug: "123"
}
# This flag's final value is calculated from:
@@ -15,6 +16,7 @@
name: "disabled_rw"
namespace: "aconfig_test"
description: "This flag is DISABLED + READ_WRITE"
+ bug: "456"
}
# This flag's final value is calculated from:
@@ -25,6 +27,8 @@
name: "enabled_ro"
namespace: "aconfig_test"
description: "This flag is ENABLED + READ_ONLY"
+ bug: "789"
+ bug: "abc"
}
# This flag's final value is calculated from:
@@ -34,4 +38,5 @@
name: "enabled_rw"
namespace: "aconfig_test"
description: "This flag is ENABLED + READ_WRITE"
+ # no bug field: bug is not mandatory
}