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

Commit 1b8636bd authored by Mårten Kongstad's avatar Mårten Kongstad
Browse files

aconfig: add proto `bug` field 

Add a `bug` field on the flag_declaration and parsed_flag proto
messages. This field is optional in the sense that it can occur zero or
more times, and aconfig will simply pass any value through.

Bug fields are included in the aconfig dump format, which can be
processed by other tools.

Also unify how protos.rs checks that fields marked 'optional' in the
proto file, but in practice are 'required', are actually set.

Test: atest aconfig.test aconfig.test.java
Bug: 288261336
Change-Id: I93de0005674822c6ff4d699bdc2c6509763a7f7f
parent 0e5e1749

tools/aconfig/Android.bp

+3 −0
Original line number Diff line number Diff line
@@ -27,6 +27,9 @@ rust_defaults { 
        "libserde_json",

        "libtinytemplate",

    ],

    proc_macros: [

        "libpaste",

    ]

}



rust_binary_host {

tools/aconfig/Cargo.toml

+1 −0
Original line number Diff line number Diff line
@@ -11,6 +11,7 @@ cargo = [] 
[dependencies]

anyhow = "1.0.69"

clap = { version = "4.1.8", features = ["derive"] }

paste = "1.0.11"

protobuf = "3.2.0"

serde = { version = "1.0.152", features = ["derive"] }

serde_json = "1.0.93"

tools/aconfig/protos/aconfig.proto

+5 −3
Original line number Diff line number Diff line
@@ -38,6 +38,7 @@ message flag_declaration { 
  optional string name = 1;

  optional string namespace = 2;

  optional string description = 3;

  repeated string bug = 4;

};



message flag_declarations {
@@ -70,9 +71,10 @@ message parsed_flag { 
  optional string name = 2;

  optional string namespace = 3;

  optional string description = 4;

  optional flag_state state = 5;

  optional flag_permission permission = 6;

  repeated tracepoint trace = 7;

  repeated string bug = 5;

  optional flag_state state = 6;

  optional flag_permission permission = 7;

  repeated tracepoint trace = 8;

}



message parsed_flags {

tools/aconfig/src/commands.rs

+92 −0
Original line number Diff line number Diff line
@@ -74,6 +74,7 @@ pub fn parse_flags(package: &str, declarations: Vec<Input>, values: Vec<Input>) 
            parsed_flag.set_name(flag_declaration.take_name());

            parsed_flag.set_namespace(flag_declaration.take_namespace());

            parsed_flag.set_description(flag_declaration.take_description());

            parsed_flag.bug.append(&mut flag_declaration.bug);

            parsed_flag.set_state(DEFAULT_FLAG_STATE);

            parsed_flag.set_permission(DEFAULT_FLAG_PERMISSION);

            let mut tracepoint = ProtoTracepoint::new();
@@ -311,6 +312,97 @@ mod tests { 
        assert!(text.contains("com.android.aconfig.test/disabled_ro: DISABLED READ_ONLY"));

    }



    #[test]

    fn test_dump_protobuf_format() {

        let text_proto = r#"

parsed_flag {

  package: "com.android.aconfig.test"

  name: "disabled_ro"

  namespace: "aconfig_test"

  description: "This flag is DISABLED + READ_ONLY"

  state: DISABLED

  permission: READ_ONLY

  trace {

    source: "tests/test.aconfig"

    state: DISABLED

    permission: READ_WRITE

  }

  trace {

    source: "tests/first.values"

    state: DISABLED

    permission: READ_ONLY

  }

  bug: "123"

}

parsed_flag {

  package: "com.android.aconfig.test"

  name: "disabled_rw"

  namespace: "aconfig_test"

  description: "This flag is DISABLED + READ_WRITE"

  state: DISABLED

  permission: READ_WRITE

  trace {

    source: "tests/test.aconfig"

    state: DISABLED

    permission: READ_WRITE

  }

  bug: "456"

}

parsed_flag {

  package: "com.android.aconfig.test"

  name: "enabled_ro"

  namespace: "aconfig_test"

  description: "This flag is ENABLED + READ_ONLY"

  state: ENABLED

  permission: READ_ONLY

  trace {

    source: "tests/test.aconfig"

    state: DISABLED

    permission: READ_WRITE

  }

  trace {

    source: "tests/first.values"

    state: DISABLED

    permission: READ_WRITE

  }

  trace {

    source: "tests/second.values"

    state: ENABLED

    permission: READ_ONLY

  }

  bug: "789"

  bug: "abc"

}

parsed_flag {

  package: "com.android.aconfig.test"

  name: "enabled_rw"

  namespace: "aconfig_test"

  description: "This flag is ENABLED + READ_WRITE"

  state: ENABLED

  permission: READ_WRITE

  trace {

    source: "tests/test.aconfig"

    state: DISABLED

    permission: READ_WRITE

  }

  trace {

    source: "tests/first.values"

    state: ENABLED

    permission: READ_WRITE

  }

}

"#;

        let expected = protobuf::text_format::parse_from_str::<ProtoParsedFlags>(text_proto)

            .unwrap()

            .write_to_bytes()

            .unwrap();



        let input = parse_test_flags_as_input();

        let actual = dump_parsed_flags(vec![input], DumpFormat::Protobuf).unwrap();



        assert_eq!(expected, actual);

    }



    fn parse_test_flags_as_input() -> Input {

        let parsed_flags = crate::test::parse_test_flags();

        let binary_proto = parsed_flags.write_to_bytes().unwrap();

tools/aconfig/src/protos.rs

+35 −18
Original line number Diff line number Diff line
@@ -62,29 +62,39 @@ mod auto_generated { 
pub use auto_generated::*;



use anyhow::Result;

use paste::paste;



fn try_from_text_proto<T>(s: &str) -> Result<T>

where

    T: protobuf::MessageFull,

{

    // warning: parse_from_str does not check if required fields are set

    protobuf::text_format::parse_from_str(s).map_err(|e| e.into())

}



macro_rules! ensure_required_fields {

    ($type:expr, $struct:expr, $($field:expr),+) => {

        $(

        paste! {

            ensure!($struct.[<has_ $field>](), "bad {}: missing {}", $type, $field);

        }

        )+

    };

}



pub mod flag_declaration {

    use super::*;

    use crate::codegen;

    use anyhow::ensure;



    pub fn verify_fields(pdf: &ProtoFlagDeclaration) -> Result<()> {

        ensure!(pdf.has_name(), "bad flag declaration: missing name");

        ensure!(pdf.has_namespace(), "bad flag declaration: missing namespace");

        ensure!(pdf.has_description(), "bad flag declaration: missing description");

        ensure_required_fields!("flag declaration", pdf, "name", "namespace", "description");



        ensure!(codegen::is_valid_name_ident(pdf.name()), "bad flag declaration: bad name");

        ensure!(codegen::is_valid_name_ident(pdf.namespace()), "bad flag declaration: bad name");

        ensure!(!pdf.description().is_empty(), "bad flag declaration: empty description");



        // ProtoFlagDeclaration.bug: Vec<String>: may be empty, no checks needed



        Ok(())

    }

}
@@ -101,7 +111,7 @@ pub mod flag_declarations { 
    }



    pub fn verify_fields(pdf: &ProtoFlagDeclarations) -> Result<()> {

        ensure!(pdf.has_package(), "bad flag declarations: missing package");

        ensure_required_fields!("flag declarations", pdf, "package");



        ensure!(

            codegen::is_valid_package_ident(pdf.package()),
@@ -121,10 +131,7 @@ pub mod flag_value { 
    use anyhow::ensure;



    pub fn verify_fields(fv: &ProtoFlagValue) -> Result<()> {

        ensure!(fv.has_package(), "bad flag value: missing package");

        ensure!(fv.has_name(), "bad flag value: missing name");

        ensure!(fv.has_state(), "bad flag value: missing state");

        ensure!(fv.has_permission(), "bad flag value: missing permission");

        ensure_required_fields!("flag value", fv, "package", "name", "state", "permission");



        ensure!(codegen::is_valid_package_ident(fv.package()), "bad flag value: bad package");

        ensure!(codegen::is_valid_name_ident(fv.name()), "bad flag value: bad name");
@@ -155,9 +162,7 @@ pub mod tracepoint { 
    use anyhow::ensure;



    pub fn verify_fields(tp: &ProtoTracepoint) -> Result<()> {

        ensure!(tp.has_source(), "bad tracepoint: missing source");

        ensure!(tp.has_state(), "bad tracepoint: missing state");

        ensure!(tp.has_permission(), "bad tracepoint: missing permission");

        ensure_required_fields!("tracepoint", tp, "source", "state", "permission");



        ensure!(!tp.source().is_empty(), "bad tracepoint: empty source");
@@ -171,12 +176,16 @@ pub mod parsed_flag { 
    use anyhow::ensure;



    pub fn verify_fields(pf: &ProtoParsedFlag) -> Result<()> {

        ensure!(pf.has_package(), "bad parsed flag: missing package");

        ensure!(pf.has_name(), "bad parsed flag: missing name");

        ensure!(pf.has_namespace(), "bad parsed flag: missing namespace");

        ensure!(pf.has_description(), "bad parsed flag: missing description");

        ensure!(pf.has_state(), "bad parsed flag: missing state");

        ensure!(pf.has_permission(), "bad parsed flag: missing permission");

        ensure_required_fields!(

            "parsed flag",

            pf,

            "package",

            "name",

            "namespace",

            "description",

            "state",

            "permission"

        );



        ensure!(codegen::is_valid_package_ident(pf.package()), "bad parsed flag: bad package");

        ensure!(codegen::is_valid_name_ident(pf.name()), "bad parsed flag: bad name");
@@ -187,6 +196,8 @@ pub mod parsed_flag { 
            super::tracepoint::verify_fields(tp)?;

        }



        // ProtoParsedFlag.bug: Vec<String>: may be empty, no checks needed



        Ok(())

    }

}
@@ -251,6 +262,8 @@ flag { 
    name: "first"

    namespace: "first_ns"

    description: "This is the description of the first flag."

    bug: "123"

    bug: "abc"

}

flag {

    name: "second"
@@ -265,10 +278,14 @@ flag { 
        assert_eq!(first.name(), "first");

        assert_eq!(first.namespace(), "first_ns");

        assert_eq!(first.description(), "This is the description of the first flag.");

        assert_eq!(first.bug.len(), 2);

        assert_eq!(first.bug[0], "123");

        assert_eq!(first.bug[1], "abc");

        let second = flag_declarations.flag.iter().find(|pf| pf.name() == "second").unwrap();

        assert_eq!(second.name(), "second");

        assert_eq!(second.namespace(), "second_ns");

        assert_eq!(second.description(), "This is the description of the second flag.");

        assert_eq!(second.bug.len(), 0);



        // bad input: missing package in flag declarations

        let error = flag_declarations::try_from_text_proto(