Merge "Add metadata to aconfig" into main (56835092) · Commits · e / os / android_build

tools/aconfig/protos/aconfig.proto

+16 −0

Original line number	Diff line number	Diff line
		@@ -41,8 +41,23 @@ message flag_declaration {
		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 @@ message parsed_flag {
		optional bool is_fixed_read_only = 9;
		optional bool is_exported = 10;
		optional string container = 11;
		optional flag_metadata metadata = 12;
		}

		message parsed_flags {

tools/aconfig/src/commands.rs

+49 −2

Original line number	Diff line number	Diff line
		@@ -24,7 +24,8 @@ use crate::codegen_cpp::generate_cpp_code;
		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 @@ pub fn parse_flags(
		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 @@ pub enum DumpFormat {
		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 @@ fn find_unique_package(parsed_flags: &ProtoParsedFlags) -> Option<&str> {
		#[cfg(test)]
		mod tests {
		use super::*;
		use crate::protos::ProtoFlagPurpose;

		#[test]
		fn test_parse_flags() {
		@@ -339,6 +350,7 @@ mod tests {
		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());
		@@ -510,6 +522,41 @@ mod tests {
		);
		}

		#[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();

tools/aconfig/src/protos.rs

+24 −6

Original line number	Diff line number	Diff line
		@@ -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 @@ mod auto_generated {
		// 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 @@ parsed_flag {
		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 @@ parsed_flag {
		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());

tools/aconfig/src/test.rs

+21 −0

Original line number	Diff line number	Diff line
		@@ -44,6 +44,9 @@ parsed_flag {
		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 @@ parsed_flag {
		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 @@ parsed_flag {
		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 @@ parsed_flag {
		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 @@ parsed_flag {
		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 @@ parsed_flag {
		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 @@ parsed_flag {
		is_fixed_read_only: false
		is_exported: false
		container: "system"
		metadata {
		purpose: PURPOSE_UNSPECIFIED
		}
		}
		"#;

tools/aconfig/tests/test.aconfig

+3 −0

Original line number	Diff line number	Diff line
		@@ -10,6 +10,9 @@ flag {
		namespace: "aconfig_test"
		description: "This flag is ENABLED + READ_ONLY"
		bug: "abc"
		metadata {
		purpose: PURPOSE_BUGFIX
		}
		}

		# This flag's final value is calculated from: