Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6344fdf9 authored by Oriol Prieto Gascó's avatar Oriol Prieto Gascó Committed by Gerrit Code Review
Browse files

Merge "Add container field to flag_declarations" into main

parents e47f6ffa 7afc7e7b

tools/aconfig/protos/aconfig.proto

+2 −1
Original line number Diff line number Diff line
@@ -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 {

tools/aconfig/src/codegen.rs

+27 −0
Original line number Diff line number Diff line
@@ -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!(

tools/aconfig/src/commands.rs

+85 −3
Original line number Diff line number Diff line
@@ -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]

tools/aconfig/src/main.rs

+19 −2
Original line number Diff line number Diff line
@@ -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,11 +176,19 @@ 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)

            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)?;

tools/aconfig/src/protos.rs

+67 −0
Original line number Diff line number Diff line
@@ -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();