Merge changes from topic "aconfig-stable-flag-order" (f469fd6c) · Commits · e / os / android_build

tools/aconfig/src/cache.rs

+113 −52

Original line number	Diff line number	Diff line
		@@ -51,35 +51,83 @@ pub struct Cache {
		items: Vec<Item>,
		}

		impl Cache {
		pub fn new(namespace: String) -> Result<Cache> {
		ensure!(!namespace.is_empty(), "empty namespace");
		Ok(Cache { namespace, items: vec![] })
		// TODO: replace this function with Iterator.is_sorted_by_key(...)) when that API becomes stable
		fn iter_is_sorted_by_key<'a, T: 'a, F, K>(iter: impl Iterator<Item = &'a T>, f: F) -> bool
		where
		F: FnMut(&'a T) -> K,
		K: PartialOrd<K>,
		{
		let mut last: Option<K> = None;
		for current in iter.map(f) {
		if let Some(l) = last {
		if l > current {
		return false;
		}
		}
		last = Some(current);
		}
		true
		}

		impl Cache {
		pub fn read_from_reader(reader: impl Read) -> Result<Cache> {
		serde_json::from_reader(reader).map_err(\|e\| e.into())
		let cache: Cache = serde_json::from_reader(reader)?;
		ensure!(
		iter_is_sorted_by_key(cache.iter(), \|item\| &item.name),
		"internal error: flags in cache file not sorted"
		);
		Ok(cache)
		}

		pub fn write_to_writer(&self, writer: impl Write) -> Result<()> {
		ensure!(
		iter_is_sorted_by_key(self.iter(), \|item\| &item.name),
		"internal error: flags in cache file not sorted"
		);
		serde_json::to_writer(writer, self).map_err(\|e\| e.into())
		}

		pub fn iter(&self) -> impl Iterator<Item = &Item> {
		self.items.iter()
		}

		pub fn into_iter(self) -> impl Iterator<Item = Item> {
		self.items.into_iter()
		}

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

		#[derive(Debug)]
		pub struct CacheBuilder {
		cache: Cache,
		}

		impl CacheBuilder {
		pub fn new(namespace: String) -> Result<CacheBuilder> {
		ensure!(!namespace.is_empty(), "empty namespace");
		let cache = Cache { namespace, items: vec![] };
		Ok(CacheBuilder { cache })
		}

		pub fn add_flag_declaration(
		&mut self,
		source: Source,
		declaration: FlagDeclaration,
		) -> Result<()> {
		) -> Result<&mut CacheBuilder> {
		ensure!(!declaration.name.is_empty(), "empty flag name");
		ensure!(!declaration.description.is_empty(), "empty flag description");
		ensure!(
		self.items.iter().all(\|item\| item.name != declaration.name),
		self.cache.items.iter().all(\|item\| item.name != declaration.name),
		"failed to declare flag {} from {}: flag already declared",
		declaration.name,
		source
		);
		self.items.push(Item {
		namespace: self.namespace.clone(),
		self.cache.items.push(Item {
		namespace: self.cache.namespace.clone(),
		name: declaration.name.clone(),
		description: declaration.description,
		state: DEFAULT_FLAG_STATE,
		@@ -90,21 +138,25 @@ impl Cache {
		permission: DEFAULT_FLAG_PERMISSION,
		}],
		});
		Ok(())
		Ok(self)
		}

		pub fn add_flag_value(&mut self, source: Source, value: FlagValue) -> Result<()> {
		pub fn add_flag_value(
		&mut self,
		source: Source,
		value: FlagValue,
		) -> Result<&mut CacheBuilder> {
		ensure!(!value.namespace.is_empty(), "empty flag namespace");
		ensure!(!value.name.is_empty(), "empty flag name");
		ensure!(
		value.namespace == self.namespace,
		value.namespace == self.cache.namespace,
		"failed to set values for flag {}/{} from {}: expected namespace {}",
		value.namespace,
		value.name,
		source,
		self.namespace
		self.cache.namespace
		);
		let Some(existing_item) = self.items.iter_mut().find(\|item\| item.name == value.name) else {
		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);
		};
		existing_item.state = value.state;
		@@ -114,20 +166,12 @@ impl Cache {
		state: value.state,
		permission: value.permission,
		});
		Ok(())
		}

		pub fn iter(&self) -> impl Iterator<Item = &Item> {
		self.items.iter()
		Ok(self)
		}

		pub fn into_iter(self) -> impl Iterator<Item = Item> {
		self.items.into_iter()
		}

		pub fn namespace(&self) -> &str {
		debug_assert!(!self.namespace.is_empty());
		&self.namespace
		pub fn build(mut self) -> Cache {
		self.cache.items.sort_by_cached_key(\|item\| item.name.clone());
		self.cache
		}
		}

		@@ -138,14 +182,14 @@ mod tests {

		#[test]
		fn test_add_flag_declaration() {
		let mut cache = Cache::new("ns".to_string()).unwrap();
		cache
		let mut builder = CacheBuilder::new("ns".to_string()).unwrap();
		builder
		.add_flag_declaration(
		Source::File("first.txt".to_string()),
		FlagDeclaration { name: "foo".to_string(), description: "desc".to_string() },
		)
		.unwrap();
		let error = cache
		let error = builder
		.add_flag_declaration(
		Source::File("second.txt".to_string()),
		FlagDeclaration { name: "foo".to_string(), description: "desc".to_string() },
		@@ -155,17 +199,26 @@ mod tests {
		&format!("{:?}", error),
		"failed to declare flag foo from second.txt: flag already declared"
		);
		builder
		.add_flag_declaration(
		Source::File("first.txt".to_string()),
		FlagDeclaration { name: "bar".to_string(), description: "desc".to_string() },
		)
		.unwrap();

		let cache = builder.build();

		// check flags are sorted by name
		assert_eq!(
		cache.into_iter().map(\|item\| item.name).collect::<Vec<_>>(),
		vec!["bar".to_string(), "foo".to_string()]
		);
		}

		#[test]
		fn test_add_flag_value() {
		fn check(cache: &Cache, name: &str, expected: (FlagState, Permission)) -> bool {
		let item = cache.iter().find(\|&item\| item.name == name).unwrap();
		item.state == expected.0 && item.permission == expected.1
		}

		let mut cache = Cache::new("ns".to_string()).unwrap();
		let error = cache
		let mut builder = CacheBuilder::new("ns".to_string()).unwrap();
		let error = builder
		.add_flag_value(
		Source::Memory,
		FlagValue {
		@@ -181,15 +234,14 @@ mod tests {
		"failed to set values for flag ns/foo from <memory>: flag not declared"
		);

		cache
		builder
		.add_flag_declaration(
		Source::File("first.txt".to_string()),
		FlagDeclaration { name: "foo".to_string(), description: "desc".to_string() },
		)
		.unwrap();
		assert!(check(&cache, "foo", (DEFAULT_FLAG_STATE, DEFAULT_FLAG_PERMISSION)));

		cache
		builder
		.add_flag_value(
		Source::Memory,
		FlagValue {
		@@ -200,9 +252,8 @@ mod tests {
		},
		)
		.unwrap();
		assert!(check(&cache, "foo", (FlagState::Disabled, Permission::ReadOnly)));

		cache
		builder
		.add_flag_value(
		Source::Memory,
		FlagValue {
		@@ -213,10 +264,9 @@ mod tests {
		},
		)
		.unwrap();
		assert!(check(&cache, "foo", (FlagState::Enabled, Permission::ReadWrite)));

		// different namespace -> no-op
		let error = cache
		let error = builder
		.add_flag_value(
		Source::Memory,
		FlagValue {
		@@ -228,19 +278,23 @@ mod tests {
		)
		.unwrap_err();
		assert_eq!(&format!("{:?}", error), "failed to set values for flag some-other-namespace/foo from <memory>: expected namespace ns");
		assert!(check(&cache, "foo", (FlagState::Enabled, Permission::ReadWrite)));

		let cache = builder.build();
		let item = cache.iter().find(\|&item\| item.name == "foo").unwrap();
		assert_eq!(FlagState::Enabled, item.state);
		assert_eq!(Permission::ReadWrite, item.permission);
		}

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

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

		let error = cache
		let error = builder
		.add_flag_declaration(
		Source::Memory,
		FlagDeclaration { name: "".to_string(), description: "Description".to_string() },
		@@ -248,7 +302,7 @@ mod tests {
		.unwrap_err();
		assert_eq!(&format!("{:?}", error), "empty flag name");

		let error = cache
		let error = builder
		.add_flag_declaration(
		Source::Memory,
		FlagDeclaration { name: "foo".to_string(), description: "".to_string() },
		@@ -259,15 +313,15 @@ mod tests {

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

		let error = cache
		let error = builder
		.add_flag_value(
		Source::Memory,
		FlagValue {
		@@ -280,7 +334,7 @@ mod tests {
		.unwrap_err();
		assert_eq!(&format!("{:?}", error), "empty flag namespace");

		let error = cache
		let error = builder
		.add_flag_value(
		Source::Memory,
		FlagValue {
		@@ -293,4 +347,11 @@ mod tests {
		.unwrap_err();
		assert_eq!(&format!("{:?}", error), "empty flag name");
		}

		#[test]
		fn test_iter_is_sorted_by_key() {
		assert!(iter_is_sorted_by_key(["a", "b", "c"].iter(), \|s\| s));
		assert!(iter_is_sorted_by_key(Vec::<&str>::new().iter(), \|s\| s));
		assert!(!iter_is_sorted_by_key(["a", "c", "b"].iter(), \|s\| s));
		}
		}

tools/aconfig/src/codegen_cpp.rs

+12 −14

Original line number	Diff line number	Diff line
		@@ -64,13 +64,14 @@ fn create_class_element(item: &Item) -> ClassElement {
		mod tests {
		use super::*;
		use crate::aconfig::{FlagDeclaration, FlagState, FlagValue, Permission};
		use crate::cache::CacheBuilder;
		use crate::commands::Source;

		#[test]
		fn test_cpp_codegen_build_time_flag_only() {
		let namespace = "my_namespace";
		let mut cache = Cache::new(namespace.to_string()).unwrap();
		cache
		let mut builder = CacheBuilder::new(namespace.to_string()).unwrap();
		builder
		.add_flag_declaration(
		Source::File("aconfig_one.txt".to_string()),
		FlagDeclaration {
		@@ -78,8 +79,7 @@ mod tests {
		description: "buildtime disable".to_string(),
		},
		)
		.unwrap();
		cache
		.unwrap()
		.add_flag_value(
		Source::Memory,
		FlagValue {
		@@ -89,8 +89,7 @@ mod tests {
		permission: Permission::ReadOnly,
		},
		)
		.unwrap();
		cache
		.unwrap()
		.add_flag_declaration(
		Source::File("aconfig_two.txt".to_string()),
		FlagDeclaration {
		@@ -98,8 +97,7 @@ mod tests {
		description: "buildtime enable".to_string(),
		},
		)
		.unwrap();
		cache
		.unwrap()
		.add_flag_value(
		Source::Memory,
		FlagValue {
		@@ -110,6 +108,7 @@ mod tests {
		},
		)
		.unwrap();
		let cache = builder.build();
		let expect_content = r#"#ifndef my_namespace_HEADER_H
		#define my_namespace_HEADER_H
		#include "my_namespace.h"
		@@ -144,8 +143,8 @@ mod tests {
		#[test]
		fn test_cpp_codegen_runtime_flag() {
		let namespace = "my_namespace";
		let mut cache = Cache::new(namespace.to_string()).unwrap();
		cache
		let mut builder = CacheBuilder::new(namespace.to_string()).unwrap();
		builder
		.add_flag_declaration(
		Source::File("aconfig_one.txt".to_string()),
		FlagDeclaration {
		@@ -153,8 +152,7 @@ mod tests {
		description: "buildtime disable".to_string(),
		},
		)
		.unwrap();
		cache
		.unwrap()
		.add_flag_declaration(
		Source::File("aconfig_two.txt".to_string()),
		FlagDeclaration {
		@@ -162,8 +160,7 @@ mod tests {
		description: "runtime enable".to_string(),
		},
		)
		.unwrap();
		cache
		.unwrap()
		.add_flag_value(
		Source::Memory,
		FlagValue {
		@@ -174,6 +171,7 @@ mod tests {
		},
		)
		.unwrap();
		let cache = builder.build();
		let expect_content = r#"#ifndef my_namespace_HEADER_H
		#define my_namespace_HEADER_H
		#include "my_namespace.h"

tools/aconfig/src/codegen_java.rs

+6 −6

Original line number	Diff line number	Diff line
		@@ -71,13 +71,14 @@ fn create_class_element(item: &Item) -> ClassElement {
		mod tests {
		use super::*;
		use crate::aconfig::{FlagDeclaration, FlagValue};
		use crate::cache::CacheBuilder;
		use crate::commands::Source;

		#[test]
		fn test_generate_java_code() {
		let namespace = "com.example";
		let mut cache = Cache::new(namespace.to_string()).unwrap();
		cache
		let mut builder = CacheBuilder::new(namespace.to_string()).unwrap();
		builder
		.add_flag_declaration(
		Source::File("test.txt".to_string()),
		FlagDeclaration {
		@@ -85,8 +86,7 @@ mod tests {
		description: "buildtime enable".to_string(),
		},
		)
		.unwrap();
		cache
		.unwrap()
		.add_flag_declaration(
		Source::File("test2.txt".to_string()),
		FlagDeclaration {
		@@ -94,8 +94,7 @@ mod tests {
		description: "runtime disable".to_string(),
		},
		)
		.unwrap();
		cache
		.unwrap()
		.add_flag_value(
		Source::Memory,
		FlagValue {
		@@ -106,6 +105,7 @@ mod tests {
		},
		)
		.unwrap();
		let cache = builder.build();
		let expect_content = r#"package com.example;

		import android.provider.DeviceConfig;

tools/aconfig/src/commands.rs

+81 −35

Original line number	Diff line number	Diff line
		@@ -23,7 +23,7 @@ use std::io::Read;
		use std::path::PathBuf;

		use crate::aconfig::{FlagDeclarations, FlagValue};
		use crate::cache::Cache;
		use crate::cache::{Cache, CacheBuilder};
		use crate::codegen_cpp::generate_cpp_code;
		use crate::codegen_java::generate_java_code;
		use crate::protos::ProtoParsedFlags;
		@@ -59,7 +59,7 @@ pub fn create_cache(
		declarations: Vec<Input>,
		values: Vec<Input>,
		) -> Result<Cache> {
		let mut cache = Cache::new(namespace.to_owned())?;
		let mut builder = CacheBuilder::new(namespace.to_owned())?;

		for mut input in declarations {
		let mut contents = String::new();
		@@ -74,7 +74,7 @@ pub fn create_cache(
		dec_list.namespace
		);
		for d in dec_list.flags.into_iter() {
		cache.add_flag_declaration(input.source.clone(), d)?;
		builder.add_flag_declaration(input.source.clone(), d)?;
		}
		}

		@@ -85,11 +85,11 @@ pub fn create_cache(
		.with_context(\|\| format!("Failed to parse {}", input.source))?;
		for v in values_list {
		// TODO: warn about flag values that do not take effect?
		let _ = cache.add_flag_value(input.source.clone(), v);
		let _ = builder.add_flag_value(input.source.clone(), v);
		}
		}

		Ok(cache)
		Ok(builder.build())
		}

		pub fn create_java_lib(cache: &Cache) -> Result<OutputFile> {
		@@ -107,39 +107,45 @@ pub enum DumpFormat {
		Protobuf,
		}

		pub fn dump_cache(cache: Cache, format: DumpFormat) -> Result<Vec<u8>> {
		pub fn dump_cache(mut caches: Vec<Cache>, format: DumpFormat) -> Result<Vec<u8>> {
		let mut output = Vec::new();
		caches.sort_by_cached_key(\|cache\| cache.namespace().to_string());
		for cache in caches.into_iter() {
		match format {
		DumpFormat::Text => {
		let mut lines = vec![];
		for item in cache.iter() {
		lines.push(format!("{}: {:?}\n", item.name, item.state));
		lines.push(format!(
		"{}/{}: {:?} {:?}\n",
		item.namespace, item.name, item.state, item.permission
		));
		}
		Ok(lines.concat().into())
		output.append(&mut lines.concat().into());
		}
		DumpFormat::Debug => {
		let mut lines = vec![];
		for item in cache.iter() {
		lines.push(format!("{:?}\n", item));
		}
		Ok(lines.concat().into())
		output.append(&mut lines.concat().into());
		}
		DumpFormat::Protobuf => {
		let parsed_flags: ProtoParsedFlags = cache.into();
		let mut output = vec![];
		parsed_flags.write_to_vec(&mut output)?;
		Ok(output)
		}
		}
		}
		Ok(output)
		}

		#[cfg(test)]
		mod tests {
		use super::*;
		use crate::aconfig::{FlagState, Permission};

		fn create_test_cache() -> Cache {
		fn create_test_cache_ns1() -> Cache {
		let s = r#"
		namespace: "ns"
		namespace: "ns1"
		flag {
		name: "a"
		description: "Description of a"
		@@ -152,28 +158,49 @@ mod tests {
		let declarations = vec![Input { source: Source::Memory, reader: Box::new(s.as_bytes()) }];
		let o = r#"
		flag_value {
		namespace: "ns"
		namespace: "ns1"
		name: "a"
		state: DISABLED
		permission: READ_ONLY
		}
		"#;
		let values = vec![Input { source: Source::Memory, reader: Box::new(o.as_bytes()) }];
		create_cache("ns", declarations, values).unwrap()
		create_cache("ns1", declarations, values).unwrap()
		}

		fn create_test_cache_ns2() -> Cache {
		let s = r#"
		namespace: "ns2"
		flag {
		name: "c"
		description: "Description of c"
		}
		"#;
		let declarations = vec![Input { source: Source::Memory, reader: Box::new(s.as_bytes()) }];
		let o = r#"
		flag_value {
		namespace: "ns2"
		name: "c"
		state: DISABLED
		permission: READ_ONLY
		}
		"#;
		let values = vec![Input { source: Source::Memory, reader: Box::new(o.as_bytes()) }];
		create_cache("ns2", declarations, values).unwrap()
		}

		#[test]
		fn test_create_cache() {
		let cache = create_test_cache(); // calls create_cache
		let item = cache.iter().find(\|&item\| item.name == "a").unwrap();
		let caches = create_test_cache_ns1(); // calls create_cache
		let item = caches.iter().find(\|&item\| item.name == "a").unwrap();
		assert_eq!(FlagState::Disabled, item.state);
		assert_eq!(Permission::ReadOnly, item.permission);
		}

		#[test]
		fn test_dump_text_format() {
		let cache = create_test_cache();
		let bytes = dump_cache(cache, DumpFormat::Text).unwrap();
		let caches = vec![create_test_cache_ns1()];
		let bytes = dump_cache(caches, DumpFormat::Text).unwrap();
		let text = std::str::from_utf8(&bytes).unwrap();
		assert!(text.contains("a: Disabled"));
		}
		@@ -183,8 +210,8 @@ mod tests {
		use crate::protos::{ProtoFlagPermission, ProtoFlagState, ProtoTracepoint};
		use protobuf::Message;

		let cache = create_test_cache();
		let bytes = dump_cache(cache, DumpFormat::Protobuf).unwrap();
		let caches = vec![create_test_cache_ns1()];
		let bytes = dump_cache(caches, DumpFormat::Protobuf).unwrap();
		let actual = ProtoParsedFlags::parse_from_bytes(&bytes).unwrap();

		assert_eq!(
		@@ -194,7 +221,7 @@ mod tests {

		let item =
		actual.parsed_flag.iter().find(\|item\| item.name == Some("b".to_string())).unwrap();
		assert_eq!(item.namespace(), "ns");
		assert_eq!(item.namespace(), "ns1");
		assert_eq!(item.name(), "b");
		assert_eq!(item.description(), "Description of b");
		assert_eq!(item.state(), ProtoFlagState::DISABLED);
		@@ -205,4 +232,23 @@ mod tests {
		tp.set_permission(ProtoFlagPermission::READ_WRITE);
		assert_eq!(item.trace, vec![tp]);
		}

		#[test]
		fn test_dump_multiple_caches() {
		let caches = vec![create_test_cache_ns1(), create_test_cache_ns2()];
		let bytes = dump_cache(caches, DumpFormat::Protobuf).unwrap();
		let dump = ProtoParsedFlags::parse_from_bytes(&bytes).unwrap();
		assert_eq!(
		dump.parsed_flag
		.iter()
		.map(\|parsed_flag\| format!("{}/{}", parsed_flag.namespace(), parsed_flag.name()))
		.collect::<Vec<_>>(),
		vec!["ns1/a".to_string(), "ns1/b".to_string(), "ns2/c".to_string()]
		);

		let caches = vec![create_test_cache_ns2(), create_test_cache_ns1()];
		let bytes = dump_cache(caches, DumpFormat::Protobuf).unwrap();
		let dump_reversed_input = ProtoParsedFlags::parse_from_bytes(&bytes).unwrap();
		assert_eq!(dump, dump_reversed_input);
		}
		}

tools/aconfig/src/main.rs

+8 −5

Original line number	Diff line number	Diff line
		@@ -56,7 +56,7 @@ fn cli() -> Command {
		)
		.subcommand(
		Command::new("dump")
		.arg(Arg::new("cache").long("cache").required(true))
		.arg(Arg::new("cache").long("cache").action(ArgAction::Append).required(true))
		.arg(
		Arg::new("format")
		.long("format")
		@@ -130,11 +130,14 @@ fn main() -> Result<()> {
		write_output_file_realtive_to_dir(&dir, &generated_file)?;
		}
		Some(("dump", sub_matches)) => {
		let path = get_required_arg::<String>(sub_matches, "cache")?;
		let mut caches = Vec::new();
		for path in sub_matches.get_many::<String>("cache").unwrap_or_default() {
		let file = fs::File::open(path)?;
		let cache = Cache::read_from_reader(file)?;
		caches.push(cache);
		}
		let format = get_required_arg::<DumpFormat>(sub_matches, "format")?;
		let output = commands::dump_cache(cache, *format)?;
		let output = commands::dump_cache(caches, *format)?;
		let path = get_required_arg::<String>(sub_matches, "out")?;
		let mut file: Box<dyn Write> = if *path == "-" {
		Box::new(io::stdout())