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

Commit 1c4a99ad authored by Dennis Shen's avatar Dennis Shen Committed by Android (Google) Code Review
Browse files

Merge "Update how aconfig cache is used for codegen" into main

parents b5054411 3431f944

tools/aconfig/aconfig/src/codegen/cpp.rs

+16 −27
Original line number Diff line number Diff line
@@ -20,11 +20,12 @@ use std::collections::HashMap; 
use std::path::PathBuf;

use tinytemplate::TinyTemplate;



use aconfig_protos::{ProtoFlagPermission, ProtoFlagState, ProtoParsedFlag};

use aconfig_protos::{

    ParsedFlagExt, ProtoFlagPermission, ProtoFlagState, ProtoFlagStorageBackend, ProtoParsedFlag,

};



use crate::codegen;

use crate::codegen::CodegenMode;

use crate::commands::{should_include_flag, OutputFile};

use crate::codegen::{self, get_flag_offset_in_storage_file, CodegenMode};

use crate::commands::OutputFile;



pub fn generate_cpp_code<I>(

    package: &str,
@@ -36,9 +37,9 @@ where 
    I: Iterator<Item = ProtoParsedFlag>,

{

    let mut readwrite_count = 0;

    let class_elements: Vec<ClassElement> = parsed_flags_iter

    let class_elements = parsed_flags_iter

        .map(|pf| create_class_element(package, &pf, flag_ids.clone(), &mut readwrite_count))

        .collect();

        .collect::<Result<Vec<ClassElement>>>()?;

    let readwrite = readwrite_count > 0;

    let has_fixed_read_only = class_elements.iter().any(|item| item.is_fixed_read_only);

    let header = package.replace('.', "_");
@@ -123,25 +124,13 @@ fn create_class_element( 
    pf: &ProtoParsedFlag,

    flag_ids: HashMap<String, u16>,

    rw_count: &mut i32,

) -> ClassElement {

    let no_assigned_offset = !should_include_flag(pf);



    let flag_offset = match flag_ids.get(pf.name()) {

        Some(offset) => offset,

        None => {

            // System/vendor/product RO+disabled flags have no offset in storage files.

            // Assign placeholder value.

            if no_assigned_offset {

                &0

            }

            // All other flags _must_ have an offset.

            else {

                panic!("{}", format!("missing flag offset for {}", pf.name()));

            }

        }

    };



    ClassElement {

) -> Result<ClassElement> {

    ensure!(

        pf.metadata.storage() != ProtoFlagStorageBackend::DEVICE_CONFIG,

        "device config storage backend cannot be used in native codegen for flag {}",

        pf.fully_qualified_name()

    );

    Ok(ClassElement {

        readwrite_idx: if pf.permission() == ProtoFlagPermission::READ_WRITE {

            let index = *rw_count;

            *rw_count += 1;
@@ -158,12 +147,12 @@ fn create_class_element( 
        },

        flag_name: pf.name().to_string(),

        flag_macro: pf.name().to_uppercase(),

        flag_offset: *flag_offset,

        flag_offset: get_flag_offset_in_storage_file(&flag_ids, pf)?,

        device_config_namespace: pf.namespace().to_string(),

        device_config_flag: codegen::create_device_config_ident(package, pf.name())

            .expect("values checked at flag parse time"),

        container: pf.container().to_string(),

    }

    })

}



#[cfg(test)]

tools/aconfig/aconfig/src/codegen/java.rs

+13 −27
Original line number Diff line number Diff line
@@ -20,10 +20,11 @@ use std::collections::{BTreeMap, BTreeSet}; 
use std::path::PathBuf;

use tinytemplate::TinyTemplate;



use crate::codegen;

use crate::codegen::CodegenMode;

use crate::commands::{should_include_flag, OutputFile};

use aconfig_protos::{ProtoFlagPermission, ProtoFlagState, ProtoParsedFlag};

use crate::codegen::{self, get_flag_offset_in_storage_file, CodegenMode};

use crate::commands::OutputFile;

use aconfig_protos::{

    ProtoFlagPermission, ProtoFlagState, ProtoFlagStorageBackend, ProtoParsedFlag,

};

use convert_finalized_flags::{ApiLevel, FinalizedFlag, FinalizedFlagMap};

use std::collections::HashMap;
@@ -48,7 +49,7 @@ where 
        .map(|pf| {

            create_flag_element(package, &pf, config.flag_ids.clone(), &config.finalized_flags)

        })

        .collect();

        .collect::<Result<Vec<FlagElement>>>()?;

    let namespace_flags = gen_flags_by_namespace(&flag_elements);

    let properties_set: BTreeSet<String> =

        flag_elements.iter().map(|fe| format_property_name(&fe.device_config_namespace)).collect();
@@ -170,34 +171,18 @@ struct FlagElement { 
    pub properties: String,

    pub finalized_sdk_present: bool,

    pub finalized_sdk_check: String,

    pub use_device_config: bool,

}



fn create_flag_element(

    package: &str,

    pf: &ProtoParsedFlag,

    flag_offsets: HashMap<String, u16>,

    flag_ids: HashMap<String, u16>,

    finalized_flags: &FinalizedFlagMap,

) -> FlagElement {

) -> Result<FlagElement> {

    let device_config_flag = codegen::create_device_config_ident(package, pf.name())

        .expect("values checked at flag parse time");



    let no_assigned_offset = !should_include_flag(pf);



    let flag_offset = match flag_offsets.get(pf.name()) {

        Some(offset) => offset,

        None => {

            // System/vendor/product RO+disabled flags have no offset in storage files.

            // Assign placeholder value.

            if no_assigned_offset {

                &0

            }

            // All other flags _must_ have an offset.

            else {

                panic!("{}", format!("missing flag offset for {}", pf.name()));

            }

        }

    };



    // An empty map is provided if check_api_level is disabled.

    let (finalized_sdk_present, finalized_sdk_value) = if !finalized_flags.is_empty() {

        let finalized_sdk = finalized_flags.get_finalized_level(&FinalizedFlag {
@@ -210,20 +195,21 @@ fn create_flag_element( 
    };

    let finalized_sdk_check = finalized_sdk_value.conditional();



    FlagElement {

    Ok(FlagElement {

        container: pf.container().to_string(),

        default_value: pf.state() == ProtoFlagState::ENABLED,

        device_config_namespace: pf.namespace().to_string(),

        device_config_flag,

        flag_name: pf.name().to_string(),

        flag_name_constant_suffix: pf.name().to_ascii_uppercase(),

        flag_offset: *flag_offset,

        flag_offset: get_flag_offset_in_storage_file(&flag_ids, pf)?,

        is_read_write: pf.permission() == ProtoFlagPermission::READ_WRITE,

        method_name: format_java_method_name(pf.name()),

        properties: format_property_name(pf.namespace()),

        finalized_sdk_present,

        finalized_sdk_check,

    }

        use_device_config: pf.metadata.storage() == ProtoFlagStorageBackend::DEVICE_CONFIG,

    })

}



fn format_java_method_name(flag_name: &str) -> String {

tools/aconfig/aconfig/src/codegen/mod.rs

+55 −0
Original line number Diff line number Diff line
@@ -18,9 +18,12 @@ pub mod cpp; 
pub mod java;

pub mod rust;



use crate::commands::should_include_flag;

use aconfig_protos::{is_valid_name_ident, is_valid_package_ident};

use aconfig_protos::{ParsedFlagExt, ProtoParsedFlag};

use anyhow::{ensure, Result};

use clap::ValueEnum;

use std::collections::HashMap;



pub fn create_device_config_ident(package: &str, flag_name: &str) -> Result<String> {

    ensure!(is_valid_package_ident(package), "bad package");
@@ -28,6 +31,30 @@ pub fn create_device_config_ident(package: &str, flag_name: &str) -> Result<Stri 
    Ok(format!("{}.{}", package, flag_name))

}



pub(crate) fn get_flag_offset_in_storage_file(

    flag_ids: &HashMap<String, u16>,

    pf: &ProtoParsedFlag,

) -> Result<u16> {

    match flag_ids.get(pf.name()) {

        Some(offset) => {

            ensure!(

                should_include_flag(pf),

                "flag {} should not have an assigned flag id in new storage file",

                pf.fully_qualified_name()

            );

            Ok(*offset)

        }

        None => {

            ensure!(

                !should_include_flag(pf),

                "flag {} should have an assigned flag id in new storage file",

                pf.fully_qualified_name()

            );

            Ok(u16::MAX)

        }

    }

}



#[derive(Copy, Clone, Debug, PartialEq, Eq, ValueEnum)]

pub enum CodegenMode {

    Exported,
@@ -50,6 +77,8 @@ impl std::fmt::Display for CodegenMode { 
#[cfg(test)]

mod tests {

    use super::*;

    use aconfig_protos::ProtoFlagPermission;



    #[test]

    fn test_create_device_config_ident() {

        assert_eq!(
@@ -57,4 +86,30 @@ mod tests { 
            create_device_config_ident("com.foo.bar", "some_flag").unwrap()

        );

    }



    #[test]

    fn test_get_flag_offset_in_storage_file() {

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

        let pf = parsed_flags.parsed_flag.iter_mut().find(|pf| pf.name() == "disabled_rw").unwrap();

        let flag_ids = HashMap::from([(String::from("disabled_rw"), 0_u16)]);



        assert_eq!(0_u16, get_flag_offset_in_storage_file(&flag_ids, pf).unwrap());



        pf.set_permission(ProtoFlagPermission::READ_ONLY);

        let error = get_flag_offset_in_storage_file(&flag_ids, pf).unwrap_err();

        assert_eq!(

            format!("{:?}", error),

            "flag com.android.aconfig.test.disabled_rw should not have an assigned flag id in new storage file"

        );



        pf.set_name(String::from("enabled_rw"));

        assert_eq!(u16::MAX, get_flag_offset_in_storage_file(&flag_ids, pf).unwrap());



        pf.set_permission(ProtoFlagPermission::READ_WRITE);

        let error = get_flag_offset_in_storage_file(&flag_ids, pf).unwrap_err();

        assert_eq!(

            format!("{:?}", error),

            "flag com.android.aconfig.test.enabled_rw should have an assigned flag id in new storage file"

        );

    }

}

tools/aconfig/aconfig/src/codegen/rust.rs

+17 −26
Original line number Diff line number Diff line
@@ -14,17 +14,18 @@ 
 * limitations under the License.

 */



use anyhow::Result;

use anyhow::{ensure, Result};

use serde::Serialize;

use tinytemplate::TinyTemplate;



use aconfig_protos::{ProtoFlagPermission, ProtoFlagState, ProtoParsedFlag};

use aconfig_protos::{

    ParsedFlagExt, ProtoFlagPermission, ProtoFlagState, ProtoFlagStorageBackend, ProtoParsedFlag,

};



use std::collections::HashMap;



use crate::codegen;

use crate::codegen::CodegenMode;

use crate::commands::{should_include_flag, OutputFile};

use crate::codegen::{self, get_flag_offset_in_storage_file, CodegenMode};

use crate::commands::OutputFile;



pub fn generate_rust_code<I>(

    package: &str,
@@ -36,9 +37,9 @@ pub fn generate_rust_code<I>( 
where

    I: Iterator<Item = ProtoParsedFlag>,

{

    let template_flags: Vec<TemplateParsedFlag> = parsed_flags_iter

    let template_flags = parsed_flags_iter

        .map(|pf| TemplateParsedFlag::new(package, flag_ids.clone(), &pf))

        .collect();

        .collect::<Result<Vec<TemplateParsedFlag>>>()?;

    let has_readwrite = template_flags.iter().any(|item| item.readwrite);

    let container = (template_flags.first().expect("zero template flags").container).to_string();

    let use_package_fingerprint = package_fingerprint.is_some();
@@ -90,23 +91,13 @@ struct TemplateParsedFlag { 


impl TemplateParsedFlag {

    #[allow(clippy::nonminimal_bool)]

    fn new(package: &str, flag_offsets: HashMap<String, u16>, pf: &ProtoParsedFlag) -> Self {

        let flag_offset = match flag_offsets.get(pf.name()) {

            Some(offset) => offset,

            None => {

                // System/vendor/product RO+disabled flags have no offset in storage files.

                // Assign placeholder value.

                if !should_include_flag(pf) {

                    &0

                }

                // All other flags _must_ have an offset.

                else {

                    panic!("{}", format!("missing flag offset for {}", pf.name()));

                }

            }

        };



        Self {

    fn new(package: &str, flag_ids: HashMap<String, u16>, pf: &ProtoParsedFlag) -> Result<Self> {

        ensure!(

            pf.metadata.storage() != ProtoFlagStorageBackend::DEVICE_CONFIG,

            "device config storage backend cannot be used in native codegen for flag {}",

            pf.fully_qualified_name()

        );

        Ok(Self {

            readwrite: pf.permission() == ProtoFlagPermission::READ_WRITE,

            default_value: match pf.state() {

                ProtoFlagState::ENABLED => "true".to_string(),
@@ -114,11 +105,11 @@ impl TemplateParsedFlag { 
            },

            name: pf.name().to_string(),

            container: pf.container().to_string(),

            flag_offset: *flag_offset,

            flag_offset: get_flag_offset_in_storage_file(&flag_ids, pf)?,

            device_config_namespace: pf.namespace().to_string(),

            device_config_flag: codegen::create_device_config_ident(package, pf.name())

                .expect("values checked at flag parse time"),

        }

        })

    }

}

tools/aconfig/aconfig/src/commands.rs

+28 −8
Original line number Diff line number Diff line
@@ -446,9 +446,9 @@ where 
            return Err(anyhow::anyhow!("encountered a flag not in current package"));

        }



        // put a cap on how many flags a package can contain to 65535

        if flag_idx > u16::MAX as u32 {

            return Err(anyhow::anyhow!("the number of flags in a package cannot exceed 65535"));

        // put a cap on how many flags a package can contain to 65534

        if flag_idx >= u16::MAX as u32 {

            return Err(anyhow::anyhow!("the number of flags in a package cannot exceed 65534"));

        }



        if should_include_flag(pf) {
@@ -489,17 +489,18 @@ fn extract_flag_names(flags: ProtoParsedFlags) -> Result<Vec<String>> { 
        .collect::<Vec<_>>())

}



// Exclude system/vendor/product flags that are RO+disabled.

// Check if a flag should be managed by aconfigd

pub fn should_include_flag(pf: &ProtoParsedFlag) -> bool {

    let should_filter_container = pf.container == Some("vendor".to_string())

    let is_platform_container = pf.container == Some("vendor".to_string())

        || pf.container == Some("system".to_string())

        || pf.container == Some("system_ext".to_string())

        || pf.container == Some("product".to_string());



    let disabled_ro = pf.state == Some(ProtoFlagState::DISABLED.into())

    let is_disabled_ro = pf.state == Some(ProtoFlagState::DISABLED.into())

        && pf.permission == Some(ProtoFlagPermission::READ_ONLY.into());



    !should_filter_container || !disabled_ro

    !((is_platform_container && is_disabled_ro)

        || pf.metadata.storage() == ProtoFlagStorageBackend::DEVICE_CONFIG)

}



#[cfg(test)]
@@ -1110,7 +1111,7 @@ mod tests { 


    #[test]

    fn test_assign_flag_ids() {

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

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

        let package = find_unique_package(&parsed_flags.parsed_flag).unwrap().to_string();

        let flag_ids = assign_flag_ids(&package, parsed_flags.parsed_flag.iter()).unwrap();

        let expected_flag_ids = HashMap::from([
@@ -1124,6 +1125,25 @@ mod tests { 
            (String::from("enabled_rw"), 7_u16),

        ]);

        assert_eq!(flag_ids, expected_flag_ids);



        let pf = parsed_flags

            .parsed_flag

            .iter_mut()

            .find(|pf| pf.name() == "disabled_rw_in_other_namespace")

            .unwrap();

        let m = pf.metadata.as_mut().unwrap();

        m.set_storage(ProtoFlagStorageBackend::DEVICE_CONFIG);

        let flag_ids = assign_flag_ids(&package, parsed_flags.parsed_flag.iter()).unwrap();

        let expected_flag_ids = HashMap::from([

            (String::from("disabled_rw"), 0_u16),

            (String::from("disabled_rw_exported"), 1_u16),

            (String::from("enabled_fixed_ro"), 2_u16),

            (String::from("enabled_fixed_ro_exported"), 3_u16),

            (String::from("enabled_ro"), 4_u16),

            (String::from("enabled_ro_exported"), 5_u16),

            (String::from("enabled_rw"), 6_u16),

        ]);

        assert_eq!(flag_ids, expected_flag_ids);

    }



    #[test]