Merge changes from topic "aconfig-rename-namespace-to-package" (ec5da68d) · Commits · e / os / android_build

tools/aconfig/Android.bp

+3 −0

Original line number	Diff line number	Diff line
		@@ -35,4 +35,7 @@ rust_binary_host {
		rust_test_host {
		name: "aconfig.test",
		defaults: ["aconfig.defaults"],
		rustlibs: [
		"libitertools",
		],
		}

tools/aconfig/Cargo.toml

+3 −0

Original line number	Diff line number	Diff line
		@@ -18,3 +18,6 @@ tinytemplate = "1.2.1"

		[build-dependencies]
		protobuf-codegen = "3.2.0"

		[dev-dependencies]
		itertools = "0.10.5"

tools/aconfig/protos/aconfig.proto

+10 −8

Original line number	Diff line number	Diff line
		@@ -36,16 +36,17 @@ enum flag_permission {

		message flag_declaration {
		required string name = 1;
		required string description = 2;
		required string namespace = 2;
		required string description = 3;
		};

		message flag_declarations {
		required string namespace = 1;
		required string package = 1;
		repeated flag_declaration flag = 2;
		};

		message flag_value {
		required string namespace = 1;
		required string package = 1;
		required string name = 2;
		required flag_state state = 3;
		required flag_permission permission = 4;
		@@ -65,12 +66,13 @@ message tracepoint {
		}

		message parsed_flag {
		required string namespace = 1;
		required string package = 1;
		required string name = 2;
		required string description = 3;
		required flag_state state = 4;
		required flag_permission permission = 5;
		repeated tracepoint trace = 6;
		required string namespace = 3;
		required string description = 4;
		required flag_state state = 5;
		required flag_permission permission = 6;
		repeated tracepoint trace = 7;
		}

		message parsed_flags {

tools/aconfig/src/aconfig.rs

+34 −17

Original line number	Diff line number	Diff line
		@@ -81,6 +81,7 @@ impl From<Permission> for ProtoFlagPermission {
		#[derive(Debug, PartialEq, Eq)]
		pub struct FlagDeclaration {
		pub name: String,
		pub namespace: String,
		pub description: String,
		}

		@@ -100,16 +101,19 @@ impl TryFrom<ProtoFlagDeclaration> for FlagDeclaration {
		let Some(name) = proto.name else {
		bail!("missing 'name' field");
		};
		let Some(namespace) = proto.namespace else {
		bail!("missing 'namespace' field");
		};
		let Some(description) = proto.description else {
		bail!("missing 'description' field");
		};
		Ok(FlagDeclaration { name, description })
		Ok(FlagDeclaration { name, namespace, description })
		}
		}

		#[derive(Debug, PartialEq, Eq)]
		pub struct FlagDeclarations {
		pub namespace: String,
		pub package: String,
		pub flags: Vec<FlagDeclaration>,
		}

		@@ -117,20 +121,20 @@ impl FlagDeclarations {
		pub fn try_from_text_proto(text_proto: &str) -> Result<FlagDeclarations> {
		let proto: ProtoFlagDeclarations = crate::protos::try_from_text_proto(text_proto)
		.with_context(\|\| text_proto.to_owned())?;
		let Some(namespace) = proto.namespace else {
		bail!("missing 'namespace' field");
		let Some(package) = proto.package else {
		bail!("missing 'package' field");
		};
		let mut flags = vec![];
		for proto_flag in proto.flag.into_iter() {
		flags.push(proto_flag.try_into()?);
		}
		Ok(FlagDeclarations { namespace, flags })
		Ok(FlagDeclarations { package, flags })
		}
		}

		#[derive(Debug, PartialEq, Eq)]
		pub struct FlagValue {
		pub namespace: String,
		pub package: String,
		pub name: String,
		pub state: FlagState,
		pub permission: Permission,
		@@ -153,8 +157,8 @@ impl TryFrom<ProtoFlagValue> for FlagValue {
		type Error = Error;

		fn try_from(proto: ProtoFlagValue) -> Result<Self, Self::Error> {
		let Some(namespace) = proto.namespace else {
		bail!("missing 'namespace' field");
		let Some(package) = proto.package else {
		bail!("missing 'package' field");
		};
		let Some(name) = proto.name else {
		bail!("missing 'name' field");
		@@ -167,7 +171,7 @@ impl TryFrom<ProtoFlagValue> for FlagValue {
		bail!("missing 'permission' field");
		};
		let permission = proto_permission.try_into()?;
		Ok(FlagValue { namespace, name, state, permission })
		Ok(FlagValue { package, name, state, permission })
		}
		}

		@@ -184,8 +188,9 @@ impl From<Cache> for ProtoParsedFlags {
		impl From<Item> for ProtoParsedFlag {
		fn from(item: Item) -> Self {
		let mut proto = crate::protos::ProtoParsedFlag::new();
		proto.set_namespace(item.namespace.to_owned());
		proto.set_package(item.package.to_owned());
		proto.set_name(item.name.clone());
		proto.set_namespace(item.namespace.clone());
		proto.set_description(item.description.clone());
		proto.set_state(item.state.into());
		proto.set_permission(item.permission.into());
		@@ -214,11 +219,13 @@ mod tests {
		fn test_flag_try_from_text_proto() {
		let expected = FlagDeclaration {
		name: "1234".to_owned(),
		namespace: "ns".to_owned(),
		description: "Description of the flag".to_owned(),
		};

		let s = r#"
		name: "1234"
		namespace: "ns"
		description: "Description of the flag"
		"#;
		let actual = FlagDeclaration::try_from_text_proto(s).unwrap();
		@@ -242,23 +249,33 @@ mod tests {
		}

		#[test]
		fn test_namespace_try_from_text_proto() {
		fn test_package_try_from_text_proto() {
		let expected = FlagDeclarations {
		namespace: "ns".to_owned(),
		package: "com.example".to_owned(),
		flags: vec![
		FlagDeclaration { name: "a".to_owned(), description: "A".to_owned() },
		FlagDeclaration { name: "b".to_owned(), description: "B".to_owned() },
		FlagDeclaration {
		name: "a".to_owned(),
		namespace: "ns".to_owned(),
		description: "A".to_owned(),
		},
		FlagDeclaration {
		name: "b".to_owned(),
		namespace: "ns".to_owned(),
		description: "B".to_owned(),
		},
		],
		};

		let s = r#"
		namespace: "ns"
		package: "com.example"
		flag {
		name: "a"
		namespace: "ns"
		description: "A"
		}
		flag {
		name: "b"
		namespace: "ns"
		description: "B"
		}
		"#;
		@@ -270,14 +287,14 @@ mod tests {
		#[test]
		fn test_flag_declaration_try_from_text_proto_list() {
		let expected = FlagValue {
		namespace: "ns".to_owned(),
		package: "com.example".to_owned(),
		name: "1234".to_owned(),
		state: FlagState::Enabled,
		permission: Permission::ReadOnly,
		};

		let s = r#"
		namespace: "ns"
		package: "com.example"
		name: "1234"
		state: ENABLED
		permission: READ_ONLY

tools/aconfig/src/cache.rs

+72 −41

Original line number	Diff line number	Diff line
		@@ -34,12 +34,13 @@ pub struct Tracepoint {

		#[derive(Serialize, Deserialize, Debug)]
		pub struct Item {
		// TODO: duplicating the Cache.namespace as Item.namespace makes the internal representation
		// TODO: duplicating the Cache.package as Item.package makes the internal representation
		// closer to the proto message `parsed_flag`; hopefully this will enable us to replace the Item
		// struct and use a newtype instead once aconfig has matured. Until then, namespace should
		// struct and use a newtype instead once aconfig has matured. Until then, package should
		// really be a Cow<String>.
		pub namespace: String,
		pub package: String,
		pub name: String,
		pub namespace: String,
		pub description: String,
		pub state: FlagState,
		pub permission: Permission,
		@@ -48,7 +49,7 @@ pub struct Item {

		#[derive(Serialize, Deserialize, Debug)]
		pub struct Cache {
		namespace: String,
		package: String,
		items: Vec<Item>,
		}

		@@ -96,9 +97,9 @@ impl Cache {
		self.items.into_iter()
		}

		pub fn namespace(&self) -> &str {
		debug_assert!(!self.namespace.is_empty());
		&self.namespace
		pub fn package(&self) -> &str {
		debug_assert!(!self.package.is_empty());
		&self.package
		}
		}

		@@ -108,9 +109,9 @@ pub struct CacheBuilder {
		}

		impl CacheBuilder {
		pub fn new(namespace: String) -> Result<CacheBuilder> {
		ensure!(codegen::is_valid_identifier(&namespace), "bad namespace");
		let cache = Cache { namespace, items: vec![] };
		pub fn new(package: String) -> Result<CacheBuilder> {
		ensure!(codegen::is_valid_package_ident(&package), "bad package");
		let cache = Cache { package, items: vec![] };
		Ok(CacheBuilder { cache })
		}

		@@ -119,7 +120,8 @@ impl CacheBuilder {
		source: Source,
		declaration: FlagDeclaration,
		) -> Result<&mut CacheBuilder> {
		ensure!(codegen::is_valid_identifier(&declaration.name), "bad flag name");
		ensure!(codegen::is_valid_name_ident(&declaration.name), "bad flag name");
		ensure!(codegen::is_valid_name_ident(&declaration.namespace), "bad namespace");
		ensure!(!declaration.description.is_empty(), "empty flag description");
		ensure!(
		self.cache.items.iter().all(\|item\| item.name != declaration.name),
		@@ -128,8 +130,9 @@ impl CacheBuilder {
		source
		);
		self.cache.items.push(Item {
		namespace: self.cache.namespace.clone(),
		package: self.cache.package.clone(),
		name: declaration.name.clone(),
		namespace: declaration.namespace.clone(),
		description: declaration.description,
		state: DEFAULT_FLAG_STATE,
		permission: DEFAULT_FLAG_PERMISSION,
		@@ -147,18 +150,18 @@ impl CacheBuilder {
		source: Source,
		value: FlagValue,
		) -> Result<&mut CacheBuilder> {
		ensure!(codegen::is_valid_identifier(&value.namespace), "bad flag namespace");
		ensure!(codegen::is_valid_identifier(&value.name), "bad flag name");
		ensure!(codegen::is_valid_package_ident(&value.package), "bad flag package");
		ensure!(codegen::is_valid_name_ident(&value.name), "bad flag name");
		ensure!(
		value.namespace == self.cache.namespace,
		"failed to set values for flag {}/{} from {}: expected namespace {}",
		value.namespace,
		value.package == self.cache.package,
		"failed to set values for flag {}/{} from {}: expected package {}",
		value.package,
		value.name,
		source,
		self.cache.namespace
		self.cache.package
		);
		let Some(existing_item) = self.cache.items.iter_mut().find(\|item\| item.name == value.name) else {
		bail!("failed to set values for flag {}/{} from {}: flag not declared", value.namespace, value.name, source);
		bail!("failed to set values for flag {}/{} from {}: flag not declared", value.package, value.name, source);
		};
		existing_item.state = value.state;
		existing_item.permission = value.permission;
		@@ -182,17 +185,25 @@ mod tests {

		#[test]
		fn test_add_flag_declaration() {
		let mut builder = CacheBuilder::new("ns".to_string()).unwrap();
		let mut builder = CacheBuilder::new("com.example".to_string()).unwrap();
		builder
		.add_flag_declaration(
		Source::File("first.txt".to_string()),
		FlagDeclaration { name: "foo".to_string(), description: "desc".to_string() },
		FlagDeclaration {
		name: "foo".to_string(),
		namespace: "ns".to_string(),
		description: "desc".to_string(),
		},
		)
		.unwrap();
		let error = builder
		.add_flag_declaration(
		Source::File("second.txt".to_string()),
		FlagDeclaration { name: "foo".to_string(), description: "desc".to_string() },
		FlagDeclaration {
		name: "foo".to_string(),
		namespace: "ns".to_string(),
		description: "desc".to_string(),
		},
		)
		.unwrap_err();
		assert_eq!(
		@@ -202,7 +213,11 @@ mod tests {
		builder
		.add_flag_declaration(
		Source::File("first.txt".to_string()),
		FlagDeclaration { name: "bar".to_string(), description: "desc".to_string() },
		FlagDeclaration {
		name: "bar".to_string(),
		namespace: "ns".to_string(),
		description: "desc".to_string(),
		},
		)
		.unwrap();

		@@ -217,12 +232,12 @@ mod tests {

		#[test]
		fn test_add_flag_value() {
		let mut builder = CacheBuilder::new("ns".to_string()).unwrap();
		let mut builder = CacheBuilder::new("com.example".to_string()).unwrap();
		let error = builder
		.add_flag_value(
		Source::Memory,
		FlagValue {
		namespace: "ns".to_string(),
		package: "com.example".to_string(),
		name: "foo".to_string(),
		state: FlagState::Enabled,
		permission: Permission::ReadOnly,
		@@ -231,13 +246,17 @@ mod tests {
		.unwrap_err();
		assert_eq!(
		&format!("{:?}", error),
		"failed to set values for flag ns/foo from <memory>: flag not declared"
		"failed to set values for flag com.example/foo from <memory>: flag not declared"
		);

		builder
		.add_flag_declaration(
		Source::File("first.txt".to_string()),
		FlagDeclaration { name: "foo".to_string(), description: "desc".to_string() },
		FlagDeclaration {
		name: "foo".to_string(),
		namespace: "ns".to_string(),
		description: "desc".to_string(),
		},
		)
		.unwrap();

		@@ -245,7 +264,7 @@ mod tests {
		.add_flag_value(
		Source::Memory,
		FlagValue {
		namespace: "ns".to_string(),
		package: "com.example".to_string(),
		name: "foo".to_string(),
		state: FlagState::Disabled,
		permission: Permission::ReadOnly,
		@@ -257,7 +276,7 @@ mod tests {
		.add_flag_value(
		Source::Memory,
		FlagValue {
		namespace: "ns".to_string(),
		package: "com.example".to_string(),
		name: "foo".to_string(),
		state: FlagState::Enabled,
		permission: Permission::ReadWrite,
		@@ -265,19 +284,19 @@ mod tests {
		)
		.unwrap();

		// different namespace -> no-op
		// different package -> no-op
		let error = builder
		.add_flag_value(
		Source::Memory,
		FlagValue {
		namespace: "some_other_namespace".to_string(),
		package: "some_other_package".to_string(),
		name: "foo".to_string(),
		state: FlagState::Enabled,
		permission: Permission::ReadOnly,
		},
		)
		.unwrap_err();
		assert_eq!(&format!("{:?}", error), "failed to set values for flag some_other_namespace/foo from <memory>: expected namespace ns");
		assert_eq!(&format!("{:?}", error), "failed to set values for flag some_other_package/foo from <memory>: expected package com.example");

		let cache = builder.build();
		let item = cache.iter().find(\|&item\| item.name == "foo").unwrap();
		@@ -286,18 +305,22 @@ mod tests {
		}

		#[test]
		fn test_reject_empty_cache_namespace() {
		fn test_reject_empty_cache_package() {
		CacheBuilder::new("".to_string()).unwrap_err();
		}

		#[test]
		fn test_reject_empty_flag_declaration_fields() {
		let mut builder = CacheBuilder::new("ns".to_string()).unwrap();
		let mut builder = CacheBuilder::new("com.example".to_string()).unwrap();

		let error = builder
		.add_flag_declaration(
		Source::Memory,
		FlagDeclaration { name: "".to_string(), description: "Description".to_string() },
		FlagDeclaration {
		name: "".to_string(),
		namespace: "ns".to_string(),
		description: "Description".to_string(),
		},
		)
		.unwrap_err();
		assert_eq!(&format!("{:?}", error), "bad flag name");
		@@ -305,7 +328,11 @@ mod tests {
		let error = builder
		.add_flag_declaration(
		Source::Memory,
		FlagDeclaration { name: "foo".to_string(), description: "".to_string() },
		FlagDeclaration {
		name: "foo".to_string(),
		namespace: "ns".to_string(),
		description: "".to_string(),
		},
		)
		.unwrap_err();
		assert_eq!(&format!("{:?}", error), "empty flag description");
		@@ -313,11 +340,15 @@ mod tests {

		#[test]
		fn test_reject_empty_flag_value_files() {
		let mut builder = CacheBuilder::new("ns".to_string()).unwrap();
		let mut builder = CacheBuilder::new("com.example".to_string()).unwrap();
		builder
		.add_flag_declaration(
		Source::Memory,
		FlagDeclaration { name: "foo".to_string(), description: "desc".to_string() },
		FlagDeclaration {
		name: "foo".to_string(),
		namespace: "ns".to_string(),
		description: "desc".to_string(),
		},
		)
		.unwrap();

		@@ -325,20 +356,20 @@ mod tests {
		.add_flag_value(
		Source::Memory,
		FlagValue {
		namespace: "".to_string(),
		package: "".to_string(),
		name: "foo".to_string(),
		state: FlagState::Enabled,
		permission: Permission::ReadOnly,
		},
		)
		.unwrap_err();
		assert_eq!(&format!("{:?}", error), "bad flag namespace");
		assert_eq!(&format!("{:?}", error), "bad flag package");

		let error = builder
		.add_flag_value(
		Source::Memory,
		FlagValue {
		namespace: "ns".to_string(),
		package: "com.example".to_string(),
		name: "".to_string(),
		state: FlagState::Enabled,
		permission: Permission::ReadOnly,