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

Commit 2f42c766 authored by Marybeth Fair's avatar Marybeth Fair
Browse files

aconfig: Add API level check to exported mode. 

API level check means that for exported flags guarding a finalized API,
we consider the flag always true on Android builds where the API is
finalized (so, the SDK version is high enough). In other words, we check
the SDK level in the flag accessor compared against the SDK level in
which the API the flag was guarding is finalized. If it's high enough,
we return true without actually performing the flag lookup because the
guarantee of finalization is that this API will now always be available
(so the flag should always be true). This is more performant.

Changes:
-add flag to aconfig binary (sdk-check) to toggle the API level check in
codegen, default to false and can be toggled via build flag
-pipe bool through to the codegen
-add api level check in new exported codegen

Next steps:
-add build flag
-generate proto <flag, API level> map from text list of finalized flags
-read artifact to set finalized_sdk_value in FlagElement

Flag: see above, guarding from binary, build flag TODO
Test: atest aconfig.test
Change-Id: I9bd4ee3e4d005acb812bd4010dd5cb301fa8de58
parent fbcc6337

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

+72 −37
Original line number Diff line number Diff line
@@ -26,25 +26,34 @@ use crate::commands::{should_include_flag, OutputFile}; 
use aconfig_protos::{ProtoFlagPermission, ProtoFlagState, ProtoParsedFlag};

use std::collections::HashMap;



// Arguments to configure codegen for generate_java_code.

pub struct JavaCodegenConfig {

    pub codegen_mode: CodegenMode,

    pub flag_ids: HashMap<String, u16>,

    pub allow_instrumentation: bool,

    pub package_fingerprint: u64,

    pub new_exported: bool,

    pub check_api_level: bool,

}



pub fn generate_java_code<I>(

    package: &str,

    parsed_flags_iter: I,

    codegen_mode: CodegenMode,

    flag_ids: HashMap<String, u16>,

    allow_instrumentation: bool,

    package_fingerprint: u64,

    new_exported: bool,

    config: JavaCodegenConfig,

) -> Result<Vec<OutputFile>>

where

    I: Iterator<Item = ProtoParsedFlag>,

{

    let flag_elements: Vec<FlagElement> =

        parsed_flags_iter.map(|pf| create_flag_element(package, &pf, flag_ids.clone())).collect();

    let flag_elements: Vec<FlagElement> = parsed_flags_iter

        .map(|pf| {

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

        })

        .collect();

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

    let is_test_mode = codegen_mode == CodegenMode::Test;

    let library_exported = codegen_mode == CodegenMode::Exported;

    let is_test_mode = config.codegen_mode == CodegenMode::Test;

    let library_exported = config.codegen_mode == CodegenMode::Exported;

    let runtime_lookup_required =

        flag_elements.iter().any(|elem| elem.is_read_write) || library_exported;

    let container = (flag_elements.first().expect("zero template flags").container).to_string();
@@ -57,11 +66,11 @@ where 
        properties_set,

        package_name: package.to_string(),

        library_exported,

        allow_instrumentation,

        allow_instrumentation: config.allow_instrumentation,

        container,

        is_platform_container,

        package_fingerprint: format!("0x{:X}L", package_fingerprint),

        new_exported,

        package_fingerprint: format!("0x{:X}L", config.package_fingerprint),

        new_exported: config.new_exported,

    };

    let mut template = TinyTemplate::new();

    template.add_template("Flags.java", include_str!("../../templates/Flags.java.template"))?;
@@ -152,12 +161,15 @@ struct FlagElement { 
    pub is_read_write: bool,

    pub method_name: String,

    pub properties: String,

    pub finalized_sdk_present: bool,

    pub finalized_sdk_value: i32,

}



fn create_flag_element(

    package: &str,

    pf: &ProtoParsedFlag,

    flag_offsets: HashMap<String, u16>,

    check_api_level: bool,

) -> FlagElement {

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

        .expect("values checked at flag parse time");
@@ -190,6 +202,8 @@ fn create_flag_element( 
        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: check_api_level,

        finalized_sdk_value: i32::MAX, // TODO: b/378936061 - Read value from artifact.

    }

}
@@ -523,14 +537,18 @@ mod tests { 
            crate::commands::modify_parsed_flags_based_on_mode(parsed_flags, mode).unwrap();

        let flag_ids =

            assign_flag_ids(crate::test::TEST_PACKAGE, modified_parsed_flags.iter()).unwrap();

        let config = JavaCodegenConfig {

            codegen_mode: mode,

            flag_ids,

            allow_instrumentation: true,

            package_fingerprint: 5801144784618221668,

            new_exported: false,

            check_api_level: false,

        };

        let generated_files = generate_java_code(

            crate::test::TEST_PACKAGE,

            modified_parsed_flags.into_iter(),

            mode,

            flag_ids,

            true,

            5801144784618221668,

            false,

            config,

        )

        .unwrap();

        let expect_flags_content = EXPECTED_FLAG_COMMON_CONTENT.to_string()
@@ -679,14 +697,18 @@ mod tests { 
            crate::commands::modify_parsed_flags_based_on_mode(parsed_flags, mode).unwrap();

        let flag_ids =

            assign_flag_ids(crate::test::TEST_PACKAGE, modified_parsed_flags.iter()).unwrap();

        let config = JavaCodegenConfig {

            codegen_mode: mode,

            flag_ids,

            allow_instrumentation: true,

            package_fingerprint: 5801144784618221668,

            new_exported: false,

            check_api_level: false,

        };

        let generated_files = generate_java_code(

            crate::test::TEST_PACKAGE,

            modified_parsed_flags.into_iter(),

            mode,

            flag_ids,

            true,

            5801144784618221668,

            false,

            config,

        )

        .unwrap();
@@ -879,14 +901,18 @@ mod tests { 
            crate::commands::modify_parsed_flags_based_on_mode(parsed_flags, mode).unwrap();

        let flag_ids =

            assign_flag_ids(crate::test::TEST_PACKAGE, modified_parsed_flags.iter()).unwrap();

        let config = JavaCodegenConfig {

            codegen_mode: mode,

            flag_ids,

            allow_instrumentation: true,

            package_fingerprint: 5801144784618221668,

            new_exported: true,

            check_api_level: false,

        };

        let generated_files = generate_java_code(

            crate::test::TEST_PACKAGE,

            modified_parsed_flags.into_iter(),

            mode,

            flag_ids,

            true,

            5801144784618221668,

            true,

            config,

        )

        .unwrap();
@@ -925,6 +951,7 @@ mod tests { 


        let expect_feature_flags_impl_content = r#"

        package com.android.aconfig.test;

        import android.os.Build;

        import android.os.flagging.AconfigPackage;

        import android.util.Log;

        /** @hide */
@@ -1068,14 +1095,18 @@ mod tests { 
            crate::commands::modify_parsed_flags_based_on_mode(parsed_flags, mode).unwrap();

        let flag_ids =

            assign_flag_ids(crate::test::TEST_PACKAGE, modified_parsed_flags.iter()).unwrap();

        let config = JavaCodegenConfig {

            codegen_mode: mode,

            flag_ids,

            allow_instrumentation: true,

            package_fingerprint: 5801144784618221668,

            new_exported: false,

            check_api_level: false,

        };

        let generated_files = generate_java_code(

            crate::test::TEST_PACKAGE,

            modified_parsed_flags.into_iter(),

            mode,

            flag_ids,

            true,

            5801144784618221668,

            false,

            config,

        )

        .unwrap();
@@ -1191,14 +1222,18 @@ mod tests { 
            crate::commands::modify_parsed_flags_based_on_mode(parsed_flags, mode).unwrap();

        let flag_ids =

            assign_flag_ids(crate::test::TEST_PACKAGE, modified_parsed_flags.iter()).unwrap();

        let config = JavaCodegenConfig {

            codegen_mode: mode,

            flag_ids,

            allow_instrumentation: true,

            package_fingerprint: 5801144784618221668,

            new_exported: false,

            check_api_level: false,

        };

        let generated_files = generate_java_code(

            crate::test::TEST_PACKAGE,

            modified_parsed_flags.into_iter(),

            mode,

            flag_ids,

            true,

            5801144784618221668,

            false,

            config,

        )

        .unwrap();

        let expect_featureflags_content = r#"

tools/aconfig/aconfig/src/commands.rs

+6 −5
Original line number Diff line number Diff line
@@ -23,7 +23,7 @@ use std::io::Read; 
use std::path::PathBuf;



use crate::codegen::cpp::generate_cpp_code;

use crate::codegen::java::generate_java_code;

use crate::codegen::java::{generate_java_code, JavaCodegenConfig};

use crate::codegen::rust::generate_rust_code;

use crate::codegen::CodegenMode;

use crate::dump::{DumpFormat, DumpPredicate};
@@ -219,6 +219,7 @@ pub fn create_java_lib( 
    codegen_mode: CodegenMode,

    allow_instrumentation: bool,

    new_exported: bool,

    check_api_level: bool,

) -> Result<Vec<OutputFile>> {

    let parsed_flags = input.try_parse_flags()?;

    let modified_parsed_flags =
@@ -230,15 +231,15 @@ pub fn create_java_lib( 
    let mut flag_names = extract_flag_names(parsed_flags)?;

    let package_fingerprint = compute_flags_fingerprint(&mut flag_names);

    let flag_ids = assign_flag_ids(&package, modified_parsed_flags.iter())?;

    generate_java_code(

        &package,

        modified_parsed_flags.into_iter(),

    let config = JavaCodegenConfig {

        codegen_mode,

        flag_ids,

        allow_instrumentation,

        package_fingerprint,

        new_exported,

    )

        check_api_level,

    };

    generate_java_code(&package, modified_parsed_flags.into_iter(), config)

}



pub fn create_cpp_lib(

tools/aconfig/aconfig/src/main.rs

+19 −3
Original line number Diff line number Diff line
@@ -91,6 +91,16 @@ fn cli() -> Command { 
                        .long("new-exported")

                        .value_parser(clap::value_parser!(bool))

                        .default_value("false"),

                )

                // Allows build flag toggling of checking API level in exported

                // flag lib for finalized API flags.

                // TODO: b/378936061 - Remove once build flag for API level

                // check is fully enabled.

                .arg(

                    Arg::new("check-api-level")

                        .long("check-api-level")

                        .value_parser(clap::value_parser!(bool))

                        .default_value("false"),

                ),

        )

        .subcommand(
@@ -274,8 +284,14 @@ fn main() -> Result<()> { 
            let allow_instrumentation =

                get_required_arg::<bool>(sub_matches, "allow-instrumentation")?;

            let new_exported = get_required_arg::<bool>(sub_matches, "new-exported")?;

            let generated_files =

                commands::create_java_lib(cache, *mode, *allow_instrumentation, *new_exported)

            let check_api_level = get_required_arg::<bool>(sub_matches, "check-api-level")?;

            let generated_files = commands::create_java_lib(

                cache,

                *mode,

                *allow_instrumentation,

                *new_exported,

                *check_api_level,

            )

            .context("failed to create java lib")?;

            let dir = PathBuf::from(get_required_arg::<String>(sub_matches, "out")?);

            generated_files

tools/aconfig/aconfig/templates/FeatureFlagsImpl.java.template

+5 −0
Original line number Diff line number Diff line
@@ -66,6 +66,7 @@ public final class FeatureFlagsImpl implements FeatureFlags \{ 
}

{{ -else- }}{#- device config for exproted mode #}

{{ -if new_exported }}

import android.os.Build;

import android.os.flagging.AconfigPackage;

import android.util.Log;

/** @hide */
@@ -80,7 +81,11 @@ public final class FeatureFlagsImpl implements FeatureFlags \{ 
            AconfigPackage reader = AconfigPackage.load("{package_name}");

            {{ -for namespace_with_flags in namespace_flags }}

            {{ -for flag in namespace_with_flags.flags }}

            {{ -if flag.finalized_sdk_present }}

            {flag.method_name} = Build.VERSION.SDK_INT >= {flag.finalized_sdk_value} ? true : reader.getBooleanFlagValue("{flag.flag_name}", {flag.default_value});

            {{ - else }}

            {flag.method_name} = reader.getBooleanFlagValue("{flag.flag_name}", {flag.default_value});

            {{ -endif}}

            {{ -endfor }}

            {{ -endfor }}

        } catch (Exception e) \{