tequilaOS/platform_build
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Add container field to flag_declarations

Browse source 
A container is software which is always built in its entirety using the
same build environment. In particular, all of its parts are built using
the same build-time default flag values. In addition, containers are
always installed as a single unit.

Bug: 312696545
Test: atest aconfig.test && m all_aconfig_declarations
Change-Id: I2ef3db836c4456f4f4fb5c066edf9094e38f89cc
This commit is contained in:
Oriol Prieto Gasco 2023-11-22 13:26:02 +00:00
parent 60bb7b0d75
commit 7afc7e7b1c
7 changed files with 209 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
tools/aconfig/protos/aconfig.proto
View file

@ -46,6 +46,7 @@ message flag_declaration {
message flag_declarations {
optional string package = 1;
repeated flag_declaration flag = 2;
optional string container = 3;
};
message flag_value {
@ -79,7 +80,7 @@ message parsed_flag {
repeated tracepoint trace = 8;
optional bool is_fixed_read_only = 9;
optional bool is_exported = 10;
optional string container = 11;
}
message parsed_flags {

27
tools/aconfig/src/codegen.rs
View file

@ -38,6 +38,10 @@ pub fn is_valid_package_ident(s: &str) -> bool {
s.split('.').all(is_valid_name_ident)
}
pub fn is_valid_container_ident(s: &str) -> bool {
is_valid_name_ident(s) || s.split('.').all(is_valid_name_ident)
}
pub fn create_device_config_ident(package: &str, flag_name: &str) -> Result<String> {
ensure!(is_valid_package_ident(package), "bad package");
ensure!(is_valid_name_ident(flag_name), "bad flag name");
@ -84,6 +88,29 @@ mod tests {
assert!(!is_valid_package_ident("foo.__bar"));
}
#[test]
fn test_is_valid_container_ident() {
assert!(is_valid_container_ident("foo.bar"));
assert!(is_valid_container_ident("foo.bar_baz"));
assert!(is_valid_container_ident("foo.bar.a123"));
assert!(is_valid_container_ident("foo"));
assert!(is_valid_container_ident("foo_bar_123"));
assert!(!is_valid_container_ident(""));
assert!(!is_valid_container_ident("foo._bar"));
assert!(!is_valid_container_ident("_foo"));
assert!(!is_valid_container_ident("123_foo"));
assert!(!is_valid_container_ident("foo-bar"));
assert!(!is_valid_container_ident("foo-b\u{00e5}r"));
assert!(!is_valid_container_ident("foo.bar.123"));
assert!(!is_valid_container_ident(".foo.bar"));
assert!(!is_valid_container_ident("foo.bar."));
assert!(!is_valid_container_ident("."));
assert!(!is_valid_container_ident(".."));
assert!(!is_valid_container_ident("foo..bar"));
assert!(!is_valid_container_ident("foo.__bar"));
}
#[test]
fn test_create_device_config_ident() {
assert_eq!(

88
tools/aconfig/src/commands.rs
View file

@ -57,6 +57,7 @@ pub const DEFAULT_FLAG_PERMISSION: ProtoFlagPermission = ProtoFlagPermission::RE
pub fn parse_flags(
package: &str,
container: Option<&str>,
declarations: Vec<Input>,
values: Vec<Input>,
default_permission: ProtoFlagPermission,
@ -79,12 +80,24 @@ pub fn parse_flags(
package,
flag_declarations.package()
);
if let Some(c) = container {
ensure!(
c == flag_declarations.container(),
"failed to parse {}: expected container {}, got {}",
input.source,
c,
flag_declarations.container()
);
}
for mut flag_declaration in flag_declarations.flag.into_iter() {
crate::protos::flag_declaration::verify_fields(&flag_declaration)
.with_context(|| input.error_context())?;
// create ParsedFlag using FlagDeclaration and default values
let mut parsed_flag = ProtoParsedFlag::new();
if let Some(c) = container {
parsed_flag.set_container(c.to_string());
}
parsed_flag.set_package(package.to_string());
parsed_flag.set_name(flag_declaration.take_name());
parsed_flag.set_namespace(flag_declaration.take_namespace());
@ -262,9 +275,10 @@ pub fn dump_parsed_flags(mut input: Vec<Input>, format: DumpFormat) -> Result<Ve
DumpFormat::Text => {
for parsed_flag in parsed_flags.parsed_flag.into_iter() {
let line = format!(
"{}.{}: {:?} + {:?}\n",
"{}.{} [{}]: {:?} + {:?}\n",
parsed_flag.package(),
parsed_flag.name(),
parsed_flag.container(),
parsed_flag.permission(),
parsed_flag.state()
);
@ -276,9 +290,10 @@ pub fn dump_parsed_flags(mut input: Vec<Input>, format: DumpFormat) -> Result<Ve
let sources: Vec<_> =
parsed_flag.trace.iter().map(|tracepoint| tracepoint.source()).collect();
let line = format!(
"{}.{}: {:?} + {:?} ({})\n",
"{}.{} [{}]: {:?} + {:?} ({})\n",
parsed_flag.package(),
parsed_flag.name(),
parsed_flag.container(),
parsed_flag.permission(),
parsed_flag.state(),
sources.join(", ")
@ -377,6 +392,7 @@ mod tests {
let flags_bytes = crate::commands::parse_flags(
"com.first",
None,
declaration,
value,
ProtoFlagPermission::READ_ONLY,
@ -390,10 +406,73 @@ mod tests {
assert_eq!(ProtoFlagPermission::READ_ONLY, parsed_flag.permission());
}
#[test]
fn test_parse_flags_package_mismatch_between_declaration_and_command_line() {
let first_flag = r#"
package: "com.declaration.package"
container: "first.container"
flag {
name: "first"
namespace: "first_ns"
description: "This is the description of the first flag."
bug: "123"
}
"#;
let declaration =
vec![Input { source: "memory".to_string(), reader: Box::new(first_flag.as_bytes()) }];
let value: Vec<Input> = vec![];
let error = crate::commands::parse_flags(
"com.argument.package",
Some("first.container"),
declaration,
value,
ProtoFlagPermission::READ_WRITE,
)
.unwrap_err();
assert_eq!(
format!("{:?}", error),
"failed to parse memory: expected package com.argument.package, got com.declaration.package"
);
}
#[test]
fn test_parse_flags_container_mismatch_between_declaration_and_command_line() {
let first_flag = r#"
package: "com.first"
container: "declaration.container"
flag {
name: "first"
namespace: "first_ns"
description: "This is the description of the first flag."
bug: "123"
}
"#;
let declaration =
vec![Input { source: "memory".to_string(), reader: Box::new(first_flag.as_bytes()) }];
let value: Vec<Input> = vec![];
let error = crate::commands::parse_flags(
"com.first",
Some("argument.container"),
declaration,
value,
ProtoFlagPermission::READ_WRITE,
)
.unwrap_err();
assert_eq!(
format!("{:?}", error),
"failed to parse memory: expected container argument.container, got declaration.container"
);
}
#[test]
fn test_parse_flags_override_fixed_read_only() {
let first_flag = r#"
package: "com.first"
container: "com.first.container"
flag {
name: "first"
namespace: "first_ns"
@ -419,6 +498,7 @@ mod tests {
}];
let error = crate::commands::parse_flags(
"com.first",
Some("com.first.container"),
declaration,
value,
ProtoFlagPermission::READ_WRITE,
@ -451,7 +531,9 @@ mod tests {
let input = parse_test_flags_as_input();
let bytes = dump_parsed_flags(vec![input], DumpFormat::Text).unwrap();
let text = std::str::from_utf8(&bytes).unwrap();
assert!(text.contains("com.android.aconfig.test.disabled_ro: READ_ONLY + DISABLED"));
assert!(
text.contains("com.android.aconfig.test.disabled_ro [system]: READ_ONLY + DISABLED")
);
}
#[test]

21
tools/aconfig/src/main.rs
View file

@ -42,6 +42,8 @@ fn cli() -> Command {
.subcommand(
Command::new("create-cache")
.arg(Arg::new("package").long("package").required(true))
// TODO(b/312769710): Make this argument required.
.arg(Arg::new("container").long("container"))
.arg(Arg::new("declarations").long("declarations").action(ArgAction::Append))
.arg(Arg::new("values").long("values").action(ArgAction::Append))
.arg(
@ -119,6 +121,13 @@ where
.ok_or(anyhow!("internal error: required argument '{}' not found", arg_name))
}
fn get_optional_arg<'a, T>(matches: &'a ArgMatches, arg_name: &str) -> Option<&'a T>
where
T: Any + Clone + Send + Sync + 'static,
{
matches.get_one::<T>(arg_name)
}
fn open_zero_or_more_files(matches: &ArgMatches, arg_name: &str) -> Result<Vec<Input>> {
let mut opened_files = vec![];
for path in matches.get_many::<String>(arg_name).unwrap_or_default() {
@ -167,12 +176,20 @@ fn main() -> Result<()> {
match matches.subcommand() {
Some(("create-cache", sub_matches)) => {
let package = get_required_arg::<String>(sub_matches, "package")?;
let container =
get_optional_arg::<String>(sub_matches, "container").map(|c| c.as_str());
let declarations = open_zero_or_more_files(sub_matches, "declarations")?;
let values = open_zero_or_more_files(sub_matches, "values")?;
let default_permission =
get_required_arg::<protos::ProtoFlagPermission>(sub_matches, "default-permission")?;
let output = commands::parse_flags(package, declarations, values, *default_permission)
.context("failed to create cache")?;
let output = commands::parse_flags(
package,
container,
declarations,
values,
*default_permission,
)
.context("failed to create cache")?;
let path = get_required_arg::<String>(sub_matches, "cache")?;
write_output_to_file_or_stdout(path, &output)?;
}

67
tools/aconfig/src/protos.rs
View file

@ -111,11 +111,16 @@ pub mod flag_declarations {
pub fn verify_fields(pdf: &ProtoFlagDeclarations) -> Result<()> {
ensure_required_fields!("flag declarations", pdf, "package");
// TODO(b/312769710): Make the container field required.
ensure!(
codegen::is_valid_package_ident(pdf.package()),
"bad flag declarations: bad package"
);
ensure!(
!pdf.has_container() || codegen::is_valid_container_ident(pdf.container()),
"bad flag declarations: bad container"
);
for flag_declaration in pdf.flag.iter() {
super::flag_declaration::verify_fields(flag_declaration)?;
}
@ -207,6 +212,10 @@ pub mod parsed_flag {
);
ensure!(codegen::is_valid_package_ident(pf.package()), "bad parsed flag: bad package");
ensure!(
!pf.has_container() || codegen::is_valid_container_ident(pf.container()),
"bad parsed flag: bad container"
);
ensure!(codegen::is_valid_name_ident(pf.name()), "bad parsed flag: bad name");
ensure!(codegen::is_valid_name_ident(pf.namespace()), "bad parsed flag: bad namespace");
ensure!(!pf.description().is_empty(), "bad parsed flag: empty description");
@ -303,6 +312,7 @@ mod tests {
let flag_declarations = flag_declarations::try_from_text_proto(
r#"
package: "com.foo.bar"
container: "system"
flag {
name: "first"
namespace: "first_ns"
@ -321,6 +331,7 @@ flag {
)
.unwrap();
assert_eq!(flag_declarations.package(), "com.foo.bar");
assert_eq!(flag_declarations.container(), "system");
let first = flag_declarations.flag.iter().find(|pf| pf.name() == "first").unwrap();
assert_eq!(first.name(), "first");
assert_eq!(first.namespace(), "first_ns");
@ -336,9 +347,26 @@ flag {
assert!(second.is_fixed_read_only());
assert!(!second.is_exported());
// valid input: missing container in flag declarations is supported
let flag_declarations = flag_declarations::try_from_text_proto(
r#"
package: "com.foo.bar"
flag {
name: "first"
namespace: "first_ns"
description: "This is the description of the first flag."
bug: "123"
}
"#,
)
.unwrap();
assert_eq!(flag_declarations.container(), "");
assert!(!flag_declarations.has_container());
// bad input: missing package in flag declarations
let error = flag_declarations::try_from_text_proto(
r#"
container: "system"
flag {
name: "first"
namespace: "first_ns"
@ -358,6 +386,7 @@ flag {
let error = flag_declarations::try_from_text_proto(
r#"
package: "com.foo.bar"
container: "system"
flag {
name: "first"
description: "This is the description of the first flag."
@ -376,6 +405,7 @@ flag {
let error = flag_declarations::try_from_text_proto(
r#"
package: "_com.FOO__BAR"
container: "system"
flag {
name: "first"
namespace: "first_ns"
@ -395,6 +425,7 @@ flag {
let error = flag_declarations::try_from_text_proto(
r#"
package: "com.foo.bar"
container: "system"
flag {
name: "FIRST"
namespace: "first_ns"
@ -414,6 +445,7 @@ flag {
let error = flag_declarations::try_from_text_proto(
r#"
package: "com.foo.bar"
container: "system"
flag {
name: "first"
namespace: "first_ns"
@ -428,6 +460,7 @@ flag {
let error = flag_declarations::try_from_text_proto(
r#"
package: "com.foo.bar"
container: "system"
flag {
name: "first"
namespace: "first_ns"
@ -439,6 +472,25 @@ flag {
)
.unwrap_err();
assert!(format!("{:?}", error).contains("bad flag declaration: exactly one bug required"));
// bad input: invalid container name in flag declaration
let error = flag_declarations::try_from_text_proto(
r#"
package: "com.foo.bar"
container: "__bad_bad_container.com"
flag {
name: "first"
namespace: "first_ns"
description: "This is the description of the first flag."
bug: "123"
bug: "abc"
}
"#,
)
.unwrap_err();
assert!(format!("{:?}", error).contains("bad flag declarations: bad container"));
// TODO(b/312769710): Verify error when container is missing.
}
#[test]
@ -553,6 +605,7 @@ parsed_flag {
state: DISABLED
permission: READ_ONLY
}
container: "system"
}
parsed_flag {
package: "com.second"
@ -573,6 +626,7 @@ parsed_flag {
permission: READ_ONLY
}
is_fixed_read_only: true
container: "system"
}
"#;
let parsed_flags = try_from_binary_proto_from_text_proto(text_proto).unwrap();
@ -607,6 +661,7 @@ parsed_flag {
description: "This is the description of the first flag."
state: DISABLED
permission: READ_ONLY
container: "system"
}
"#;
let error = try_from_binary_proto_from_text_proto(text_proto).unwrap_err();
@ -625,6 +680,7 @@ parsed_flag {
state: DISABLED
permission: READ_ONLY
}
container: "system"
}
"#;
let error = try_from_binary_proto_from_text_proto(text_proto).unwrap_err();
@ -645,6 +701,7 @@ parsed_flag {
state: DISABLED
permission: READ_ONLY
}
container: "system"
}
parsed_flag {
package: "aaa.aaa"
@ -659,6 +716,7 @@ parsed_flag {
state: DISABLED
permission: READ_ONLY
}
container: "system"
}
"#;
let error = try_from_binary_proto_from_text_proto(text_proto).unwrap_err();
@ -682,6 +740,7 @@ parsed_flag {
state: DISABLED
permission: READ_ONLY
}
container: "system"
}
parsed_flag {
package: "com.foo"
@ -696,6 +755,7 @@ parsed_flag {
state: DISABLED
permission: READ_ONLY
}
container: "system"
}
"#;
let error = try_from_binary_proto_from_text_proto(text_proto).unwrap_err();
@ -719,6 +779,7 @@ parsed_flag {
state: DISABLED
permission: READ_ONLY
}
container: "system"
}
parsed_flag {
package: "com.foo"
@ -733,6 +794,7 @@ parsed_flag {
state: DISABLED
permission: READ_ONLY
}
container: "system"
}
"#;
let error = try_from_binary_proto_from_text_proto(text_proto).unwrap_err();
@ -760,6 +822,7 @@ parsed_flag {
state: ENABLED
permission: READ_ONLY
}
container: "system"
}
"#;
let parsed_flags = try_from_binary_proto_from_text_proto(text_proto).unwrap();
@ -786,6 +849,7 @@ parsed_flag {
state: DISABLED
permission: READ_ONLY
}
container: "system"
}
parsed_flag {
package: "com.second"
@ -800,6 +864,7 @@ parsed_flag {
state: DISABLED
permission: READ_ONLY
}
container: "system"
}
"#;
let expected = try_from_binary_proto_from_text_proto(text_proto).unwrap();
@ -818,6 +883,7 @@ parsed_flag {
state: DISABLED
permission: READ_ONLY
}
container: "system"
}
"#;
let first = try_from_binary_proto_from_text_proto(text_proto).unwrap();
@ -836,6 +902,7 @@ parsed_flag {
state: DISABLED
permission: READ_ONLY
}
container: "system"
}
"#;
let second = try_from_binary_proto_from_text_proto(text_proto).unwrap();

8
tools/aconfig/src/test.rs
View file

@ -43,6 +43,7 @@ parsed_flag {
}
is_fixed_read_only: false
is_exported: false
container: "system"
}
parsed_flag {
package: "com.android.aconfig.test"
@ -59,6 +60,7 @@ parsed_flag {
}
is_fixed_read_only: false
is_exported: true
container: "system"
}
parsed_flag {
package: "com.android.aconfig.test"
@ -80,6 +82,7 @@ parsed_flag {
}
is_fixed_read_only: false
is_exported: true
container: "system"
}
parsed_flag {
package: "com.android.aconfig.test"
@ -101,6 +104,7 @@ parsed_flag {
}
is_fixed_read_only: false
is_exported: false
container: "system"
}
parsed_flag {
package: "com.android.aconfig.test"
@ -122,6 +126,7 @@ parsed_flag {
}
is_fixed_read_only: true
is_exported: false
container: "system"
}
parsed_flag {
package: "com.android.aconfig.test"
@ -148,6 +153,7 @@ parsed_flag {
}
is_fixed_read_only: false
is_exported: false
container: "system"
}
parsed_flag {
package: "com.android.aconfig.test"
@ -169,12 +175,14 @@ parsed_flag {
}
is_fixed_read_only: false
is_exported: false
container: "system"
}
"#;
pub fn parse_test_flags() -> ProtoParsedFlags {
let bytes = crate::commands::parse_flags(
"com.android.aconfig.test",
Some("system"),
vec![Input {
source: "tests/test.aconfig".to_string(),
reader: Box::new(include_bytes!("../tests/test.aconfig").as_slice()),

1
tools/aconfig/tests/test.aconfig
View file

@ -1,4 +1,5 @@
package: "com.android.aconfig.test"
container: "system"
# This flag's final value is calculated from:
# - test.aconfig: DISABLED + READ_WRITE (default)