Merge "aconfig: add proto `bug` field" (37cb74b3) · Commits · e / os / android_build

tools/aconfig/Android.bp

+3 −0

Original line number	Diff line number	Diff line
		@@ -27,6 +27,9 @@ rust_defaults {
		"libserde_json",
		"libtinytemplate",
		],
		proc_macros: [
		"libpaste",
		]
		}

		rust_binary_host {

tools/aconfig/Cargo.toml

+1 −0

Original line number	Diff line number	Diff line
		@@ -11,6 +11,7 @@ cargo = []
		[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"

tools/aconfig/protos/aconfig.proto

+5 −3

Original line number	Diff line number	Diff line
		@@ -38,6 +38,7 @@ message flag_declaration {
		optional string name = 1;
		optional string namespace = 2;
		optional string description = 3;
		repeated string bug = 4;
		};

		message flag_declarations {
		@@ -70,9 +71,10 @@ message parsed_flag {
		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 {

tools/aconfig/src/commands.rs

+92 −0

Original line number	Diff line number	Diff line
		@@ -74,6 +74,7 @@ pub fn parse_flags(package: &str, declarations: Vec<Input>, values: Vec<Input>)
		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 @@ mod tests {
		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();

tools/aconfig/src/protos.rs

+35 −18

Original line number	Diff line number	Diff line
		@@ -62,29 +62,39 @@ mod auto_generated {
		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 mod flag_declarations {
		}

		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 @@ pub mod flag_value {
		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 @@ pub mod tracepoint {
		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 @@ pub mod parsed_flag {
		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 @@ pub mod parsed_flag {
		super::tracepoint::verify_fields(tp)?;
		}

		// ProtoParsedFlag.bug: Vec<String>: may be empty, no checks needed

		Ok(())
		}
		}
		@@ -251,6 +262,8 @@ flag {
		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 @@ flag {
		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(