Merge changes from topic "aconfig-stable-flag-order" am: f469fd6cc6 am: 1ad09e0bfa

Original change: https://android-review.googlesource.com/c/platform/build/+/2595206

Change-Id: I9e812cef63f8786585db3154626f76ab1d1d5f64
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
This commit is contained in:
Treehugger Robot 2023-05-22 12:56:59 +00:00 committed by Automerger Merge Worker
commit 90ea531bb2
5 changed files with 251 additions and 143 deletions

View file

@ -51,72 +51,42 @@ 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 add_flag_declaration(
&mut self,
source: Source,
declaration: FlagDeclaration,
) -> Result<()> {
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),
"failed to declare flag {} from {}: flag already declared",
declaration.name,
source
);
self.items.push(Item {
namespace: self.namespace.clone(),
name: declaration.name.clone(),
description: declaration.description,
state: DEFAULT_FLAG_STATE,
permission: DEFAULT_FLAG_PERMISSION,
trace: vec![Tracepoint {
source,
state: DEFAULT_FLAG_STATE,
permission: DEFAULT_FLAG_PERMISSION,
}],
});
Ok(())
}
pub fn add_flag_value(&mut self, source: Source, value: FlagValue) -> Result<()> {
ensure!(!value.namespace.is_empty(), "empty flag namespace");
ensure!(!value.name.is_empty(), "empty flag name");
ensure!(
value.namespace == self.namespace,
"failed to set values for flag {}/{} from {}: expected namespace {}",
value.namespace,
value.name,
source,
self.namespace
);
let Some(existing_item) = self.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;
existing_item.permission = value.permission;
existing_item.trace.push(Tracepoint {
source,
state: value.state,
permission: value.permission,
});
Ok(())
}
pub fn iter(&self) -> impl Iterator<Item = &Item> {
self.items.iter()
}
@ -131,6 +101,80 @@ impl Cache {
}
}
#[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<&mut CacheBuilder> {
ensure!(!declaration.name.is_empty(), "empty flag name");
ensure!(!declaration.description.is_empty(), "empty flag description");
ensure!(
self.cache.items.iter().all(|item| item.name != declaration.name),
"failed to declare flag {} from {}: flag already declared",
declaration.name,
source
);
self.cache.items.push(Item {
namespace: self.cache.namespace.clone(),
name: declaration.name.clone(),
description: declaration.description,
state: DEFAULT_FLAG_STATE,
permission: DEFAULT_FLAG_PERMISSION,
trace: vec![Tracepoint {
source,
state: DEFAULT_FLAG_STATE,
permission: DEFAULT_FLAG_PERMISSION,
}],
});
Ok(self)
}
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.cache.namespace,
"failed to set values for flag {}/{} from {}: expected namespace {}",
value.namespace,
value.name,
source,
self.cache.namespace
);
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;
existing_item.permission = value.permission;
existing_item.trace.push(Tracepoint {
source,
state: value.state,
permission: value.permission,
});
Ok(self)
}
pub fn build(mut self) -> Cache {
self.cache.items.sort_by_cached_key(|item| item.name.clone());
self.cache
}
}
#[cfg(test)]
mod tests {
use super::*;
@ -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));
}
}

View file

@ -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"

View file

@ -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;

View file

@ -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,29 +107,35 @@ pub enum DumpFormat {
Protobuf,
}
pub fn dump_cache(cache: Cache, format: DumpFormat) -> Result<Vec<u8>> {
match format {
DumpFormat::Text => {
let mut lines = vec![];
for item in cache.iter() {
lines.push(format!("{}: {:?}\n", item.name, item.state));
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.namespace, item.name, item.state, item.permission
));
}
output.append(&mut lines.concat().into());
}
Ok(lines.concat().into())
}
DumpFormat::Debug => {
let mut lines = vec![];
for item in cache.iter() {
lines.push(format!("{:?}\n", item));
DumpFormat::Debug => {
let mut lines = vec![];
for item in cache.iter() {
lines.push(format!("{:?}\n", item));
}
output.append(&mut lines.concat().into());
}
DumpFormat::Protobuf => {
let parsed_flags: ProtoParsedFlags = cache.into();
parsed_flags.write_to_vec(&mut output)?;
}
Ok(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)]
@ -137,9 +143,9 @@ 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);
}
}

View file

@ -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 file = fs::File::open(path)?;
let cache = Cache::read_from_reader(file)?;
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())