Merge "Add metadata to aconfig" into main
diff --git a/tools/aconfig/protos/aconfig.proto b/tools/aconfig/protos/aconfig.proto
index 1a80b04..ed4b24c 100644
--- a/tools/aconfig/protos/aconfig.proto
+++ b/tools/aconfig/protos/aconfig.proto
@@ -41,8 +41,23 @@
repeated string bug = 4;
optional bool is_fixed_read_only = 5;
optional bool is_exported = 6;
+ optional flag_metadata metadata = 7;
};
+// Optional metadata about the flag, such as its purpose and its intended form factors.
+// Can influence the applied policies and testing strategy.
+message flag_metadata {
+ enum flag_purpose {
+ PURPOSE_UNSPECIFIED = 0;
+ PURPOSE_FEATURE = 1;
+ PURPOSE_BUGFIX = 2;
+ }
+
+ optional flag_purpose purpose = 1;
+
+ // TODO(b/315025930): Add field to designate intended target device form factor(s), such as phone, watch or other.
+}
+
message flag_declarations {
optional string package = 1;
repeated flag_declaration flag = 2;
@@ -81,6 +96,7 @@
optional bool is_fixed_read_only = 9;
optional bool is_exported = 10;
optional string container = 11;
+ optional flag_metadata metadata = 12;
}
message parsed_flags {
diff --git a/tools/aconfig/src/commands.rs b/tools/aconfig/src/commands.rs
index 37ee79e..1e80d26 100644
--- a/tools/aconfig/src/commands.rs
+++ b/tools/aconfig/src/commands.rs
@@ -24,7 +24,8 @@
use crate::codegen_java::generate_java_code;
use crate::codegen_rust::generate_rust_code;
use crate::protos::{
- ProtoFlagPermission, ProtoFlagState, ProtoParsedFlag, ProtoParsedFlags, ProtoTracepoint,
+ ProtoFlagMetadata, ProtoFlagPermission, ProtoFlagState, ProtoParsedFlag, ProtoParsedFlags,
+ ProtoTracepoint,
};
pub struct Input {
@@ -118,6 +119,11 @@
tracepoint.set_permission(flag_permission);
parsed_flag.trace.push(tracepoint);
+ let mut metadata = ProtoFlagMetadata::new();
+ let purpose = flag_declaration.metadata.purpose();
+ metadata.set_purpose(purpose);
+ parsed_flag.metadata = Some(metadata).into();
+
// verify ParsedFlag looks reasonable
crate::protos::parsed_flag::verify_fields(&parsed_flag)?;
@@ -264,7 +270,11 @@
Textproto,
}
-pub fn dump_parsed_flags(mut input: Vec<Input>, format: DumpFormat, dedup: bool) -> 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 =
@@ -325,6 +335,7 @@
#[cfg(test)]
mod tests {
use super::*;
+ use crate::protos::ProtoFlagPurpose;
#[test]
fn test_parse_flags() {
@@ -339,6 +350,7 @@
assert_eq!("This flag is ENABLED + READ_ONLY", enabled_ro.description());
assert_eq!(ProtoFlagState::ENABLED, enabled_ro.state());
assert_eq!(ProtoFlagPermission::READ_ONLY, enabled_ro.permission());
+ assert_eq!(ProtoFlagPurpose::PURPOSE_BUGFIX, enabled_ro.metadata.purpose());
assert_eq!(3, enabled_ro.trace.len());
assert!(!enabled_ro.is_fixed_read_only());
assert_eq!("tests/test.aconfig", enabled_ro.trace[0].source());
@@ -511,6 +523,41 @@
}
#[test]
+ fn test_parse_flags_metadata() {
+ let metadata_flag = r#"
+ package: "com.first"
+ flag {
+ name: "first"
+ namespace: "first_ns"
+ description: "This is the description of this feature flag."
+ bug: "123"
+ metadata {
+ purpose: PURPOSE_FEATURE
+ }
+ }
+ "#;
+ let declaration = vec![Input {
+ source: "memory".to_string(),
+ reader: Box::new(metadata_flag.as_bytes()),
+ }];
+ let value: Vec<Input> = vec![];
+
+ let flags_bytes = crate::commands::parse_flags(
+ "com.first",
+ None,
+ declaration,
+ value,
+ ProtoFlagPermission::READ_ONLY,
+ )
+ .unwrap();
+ let parsed_flags =
+ crate::protos::parsed_flags::try_from_binary_proto(&flags_bytes).unwrap();
+ assert_eq!(1, parsed_flags.parsed_flag.len());
+ let parsed_flag = parsed_flags.parsed_flag.first().unwrap();
+ assert_eq!(ProtoFlagPurpose::PURPOSE_FEATURE, parsed_flag.metadata.purpose());
+ }
+
+ #[test]
fn test_create_device_config_defaults() {
let input = parse_test_flags_as_input();
let bytes = create_device_config_defaults(input).unwrap();
diff --git a/tools/aconfig/src/protos.rs b/tools/aconfig/src/protos.rs
index 3d9089c..37eb67d 100644
--- a/tools/aconfig/src/protos.rs
+++ b/tools/aconfig/src/protos.rs
@@ -29,8 +29,10 @@
// ---- When building with the Android tool-chain ----
#[cfg(not(feature = "cargo"))]
mod auto_generated {
+ pub use aconfig_protos::aconfig::flag_metadata::Flag_purpose as ProtoFlagPurpose;
pub use aconfig_protos::aconfig::Flag_declaration as ProtoFlagDeclaration;
pub use aconfig_protos::aconfig::Flag_declarations as ProtoFlagDeclarations;
+ pub use aconfig_protos::aconfig::Flag_metadata as ProtoFlagMetadata;
pub use aconfig_protos::aconfig::Flag_permission as ProtoFlagPermission;
pub use aconfig_protos::aconfig::Flag_state as ProtoFlagState;
pub use aconfig_protos::aconfig::Flag_value as ProtoFlagValue;
@@ -47,8 +49,10 @@
// because this is only used during local development, and only if using cargo instead of the
// Android tool-chain, we allow it
include!(concat!(env!("OUT_DIR"), "/aconfig_proto/mod.rs"));
+ pub use aconfig::flag_metadata::Flag_purpose as ProtoFlagPurpose;
pub use aconfig::Flag_declaration as ProtoFlagDeclaration;
pub use aconfig::Flag_declarations as ProtoFlagDeclarations;
+ pub use aconfig::Flag_metadata as ProtoFlagMetadata;
pub use aconfig::Flag_permission as ProtoFlagPermission;
pub use aconfig::Flag_state as ProtoFlagState;
pub use aconfig::Flag_value as ProtoFlagValue;
@@ -938,11 +942,13 @@
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();
+ 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();
+ 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
@@ -950,10 +956,22 @@
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());
+ 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());
diff --git a/tools/aconfig/src/test.rs b/tools/aconfig/src/test.rs
index 8df4493..2c63fef 100644
--- a/tools/aconfig/src/test.rs
+++ b/tools/aconfig/src/test.rs
@@ -44,6 +44,9 @@
is_fixed_read_only: false
is_exported: false
container: "system"
+ metadata {
+ purpose: PURPOSE_UNSPECIFIED
+ }
}
parsed_flag {
package: "com.android.aconfig.test"
@@ -61,6 +64,9 @@
is_fixed_read_only: false
is_exported: true
container: "system"
+ metadata {
+ purpose: PURPOSE_UNSPECIFIED
+ }
}
parsed_flag {
package: "com.android.aconfig.test"
@@ -83,6 +89,9 @@
is_fixed_read_only: false
is_exported: true
container: "system"
+ metadata {
+ purpose: PURPOSE_UNSPECIFIED
+ }
}
parsed_flag {
package: "com.android.aconfig.test"
@@ -105,6 +114,9 @@
is_fixed_read_only: false
is_exported: false
container: "system"
+ metadata {
+ purpose: PURPOSE_UNSPECIFIED
+ }
}
parsed_flag {
package: "com.android.aconfig.test"
@@ -127,6 +139,9 @@
is_fixed_read_only: true
is_exported: false
container: "system"
+ metadata {
+ purpose: PURPOSE_UNSPECIFIED
+ }
}
parsed_flag {
package: "com.android.aconfig.test"
@@ -154,6 +169,9 @@
is_fixed_read_only: false
is_exported: false
container: "system"
+ metadata {
+ purpose: PURPOSE_BUGFIX
+ }
}
parsed_flag {
package: "com.android.aconfig.test"
@@ -176,6 +194,9 @@
is_fixed_read_only: false
is_exported: false
container: "system"
+ metadata {
+ purpose: PURPOSE_UNSPECIFIED
+ }
}
"#;
diff --git a/tools/aconfig/tests/test.aconfig b/tools/aconfig/tests/test.aconfig
index e44d154..310d0a6 100644
--- a/tools/aconfig/tests/test.aconfig
+++ b/tools/aconfig/tests/test.aconfig
@@ -10,6 +10,9 @@
namespace: "aconfig_test"
description: "This flag is ENABLED + READ_ONLY"
bug: "abc"
+ metadata {
+ purpose: PURPOSE_BUGFIX
+ }
}
# This flag's final value is calculated from: