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

Commit 3431f944 authored by Dennis Shen's avatar Dennis Shen
Browse files

Update how aconfig cache is used for codegen 

The storage backend now supports DEVICE_CONFIG. Thus needs to update
utility function like should_include_flag to also exclude flags that
have the storage backend set to DEVICE_CONFIG. This also implicitly
implies assign_flag_ids to exclude DEVICE_CONFIG backed flags.

Moved some common logic to src/codegen/mod.rs. Added ensure statements
in each codegen module to ensure that flag ids are assigned properly.

In native codegen module cpp.rs and rust.rs, ensure that no DEVICE_CONFIG
backed flag can be used in codegen. In java codegen module java.rs, for
each flag, add a field to indicate if using device config or not.

The actual codegen change will come after this change.

Bug: b/406508083
Test: atest -c
Change-Id: I3c6692b7453f87e2ee7f51a10faf8131e9020afc
parent 17018f40

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]