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

Commit d877f3ee authored by Dennis Shen's avatar Dennis Shen
Browse files

Revert^2 "Add flag_storage_backend field to aconfig declaration and parsed_flag" 

This reverts commit 66b89975.

Reason for revert: The underlying issue on google3 has been identified and fixed, reintroduce the change.

Change-Id: I0d18034d9392161c9f1f3cdd237701f9a5833a9f
parent 66b89975

tools/aconfig/aconfig/Android.bp

+1 −0
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ rust_defaults { 
        "libanyhow",

        "libclap",

        "libitertools",

        "liblazy_static",

        "libprotobuf",

        "libserde",

        "libserde_json",

tools/aconfig/aconfig/Cargo.toml

+1 −0
Original line number Diff line number Diff line
@@ -15,6 +15,7 @@ protobuf = "3.2.0" 
serde = { version = "1.0.152", features = ["derive"] }

serde_json = "1.0.93"

tinytemplate = "1.2.1"

lazy_static = "1.5.0"

aconfig_protos = { path = "../aconfig_protos" }

aconfig_storage_file = { path = "../aconfig_storage_file", features = ["test_utils"] }

convert_finalized_flags = { path = "../convert_finalized_flags" }

tools/aconfig/aconfig/src/commands.rs

+138 −5
Original line number Diff line number Diff line
@@ -14,9 +14,10 @@ 
 * limitations under the License.

 */



use anyhow::{bail, ensure, Context, Result};

use anyhow::{anyhow, bail, ensure, Context, Result};

use convert_finalized_flags::FinalizedFlagMap;

use itertools::Itertools;

use lazy_static::lazy_static;

use protobuf::Message;

use std::collections::HashMap;

use std::hash::Hasher;
@@ -30,8 +31,8 @@ use crate::codegen::CodegenMode; 
use crate::dump::{DumpFormat, DumpPredicate};

use crate::storage::generate_storage_file;

use aconfig_protos::{

    ParsedFlagExt, ProtoFlagMetadata, ProtoFlagPermission, ProtoFlagState, ProtoParsedFlag,

    ProtoParsedFlags, ProtoTracepoint,

    ParsedFlagExt, ProtoFlagMetadata, ProtoFlagPermission, ProtoFlagState, ProtoFlagStorageBackend,

    ProtoParsedFlag, ProtoParsedFlags, ProtoTracepoint,

};

use aconfig_storage_file::sip_hasher13::SipHasher13;

use aconfig_storage_file::StorageFileType;
@@ -64,6 +65,31 @@ pub struct OutputFile { 
pub const DEFAULT_FLAG_STATE: ProtoFlagState = ProtoFlagState::DISABLED;

pub const DEFAULT_FLAG_PERMISSION: ProtoFlagPermission = ProtoFlagPermission::READ_WRITE;



// A hash map from beta namespace name to the corresponding containers. This is used to

// determine if an RW flag is a mainline beta flag which is defined as:

// 1, defined inside a beta namespace

// 2, has the corresponding mainline module as container

// TODO(b/328657418): Populate the map with mainline beta namespaces

lazy_static! {

    static ref MAINLINE_BETA_NAMESPACES: HashMap<&'static str, &'static str> =

        vec![].into_iter().collect();

}



fn assign_storage_backend(pf: &mut ProtoParsedFlag) -> Result<()> {

    let is_mainline_beta = MAINLINE_BETA_NAMESPACES.get(pf.namespace()) == Some(&pf.container());

    let is_read_only = pf.permission() == ProtoFlagPermission::READ_ONLY;

    let storage = if is_read_only {

        ProtoFlagStorageBackend::NONE

    } else if is_mainline_beta {

        ProtoFlagStorageBackend::DEVICE_CONFIG

    } else {

        ProtoFlagStorageBackend::ACONFIGD

    };

    let m = pf.metadata.as_mut().ok_or(anyhow!("missing metadata"))?;

    m.set_storage(storage);

    Ok(())

}



pub fn parse_flags(

    package: &str,

    container: Option<&str>,
@@ -132,6 +158,7 @@ pub fn parse_flags( 
            let purpose = flag_declaration.metadata.purpose();

            metadata.set_purpose(purpose);

            parsed_flag.metadata = Some(metadata).into();

            assign_storage_backend(&mut parsed_flag)?;



            // verify ParsedFlag looks reasonable

            aconfig_protos::parsed_flag::verify_fields(&parsed_flag)?;
@@ -178,7 +205,10 @@ pub fn parse_flags( 
            );



            parsed_flag.set_state(flag_value.state());

            if parsed_flag.permission() != flag_value.permission() {

                parsed_flag.set_permission(flag_value.permission());

                assign_storage_backend(parsed_flag)?;

            }

            let mut tracepoint = ProtoTracepoint::new();

            tracepoint.set_source(input.source.clone());

            tracepoint.set_state(flag_value.state());
@@ -834,7 +864,7 @@ mod tests { 
    }



    #[test]

    fn test_parse_flags_metadata() {

    fn test_parse_flags_metadata_purpose() {

        let metadata_flag = r#"

        package: "com.first"

        flag {
@@ -869,6 +899,109 @@ mod tests { 
        assert_eq!(ProtoFlagPurpose::PURPOSE_FEATURE, parsed_flag.metadata.purpose());

    }



    #[test]

    fn test_parse_flags_metadata_storage() {

        let metadata_flag = r#"

        package: "com.first"

        flag {

            name: "first"

            namespace: "first_ns"

            description: "This is the description of this feature flag."

            bug: "123"

        }

        "#;



        // Case 1, regular RW flag without value file override

        let declaration = vec![Input {

            source: "memory".to_string(),

            reader: Box::new(metadata_flag.as_bytes()),

        }];

        let value: Vec<Input> = vec![];



        let flags_bytes = crate::commands::parse_flags(

            "com.first",

            None,

            declaration,

            value,

            ProtoFlagPermission::READ_WRITE,

            true,

        )

        .unwrap();

        let parsed_flags =

            aconfig_protos::parsed_flags::try_from_binary_proto(&flags_bytes).unwrap();

        assert_eq!(1, parsed_flags.parsed_flag.len());

        let parsed_flag = parsed_flags.parsed_flag.first().unwrap();

        assert_eq!(ProtoFlagStorageBackend::ACONFIGD, parsed_flag.metadata.storage());



        // Case 2, regular RW flag with value file override to RO

        let declaration = vec![Input {

            source: "memory".to_string(),

            reader: Box::new(metadata_flag.as_bytes()),

        }];

        let first_flag_value = r#"

        flag_value {

            package: "com.first"

            name: "first"

            state: DISABLED

            permission: READ_ONLY

        }

        "#;

        let value = vec![Input {

            source: "memory".to_string(),

            reader: Box::new(first_flag_value.as_bytes()),

        }];

        let flags_bytes = crate::commands::parse_flags(

            "com.first",

            None,

            declaration,

            value,

            ProtoFlagPermission::READ_WRITE,

            true,

        )

        .unwrap();

        let parsed_flags =

            aconfig_protos::parsed_flags::try_from_binary_proto(&flags_bytes).unwrap();

        assert_eq!(1, parsed_flags.parsed_flag.len());

        let parsed_flag = parsed_flags.parsed_flag.first().unwrap();

        assert_eq!(ProtoFlagStorageBackend::NONE, parsed_flag.metadata.storage());



        // Case 3, fixed read only flag

        let metadata_flag = r#"

        package: "com.first"

        flag {

            name: "first"

            namespace: "first_ns"

            description: "This is the description of this feature flag."

            bug: "123"

            is_fixed_read_only: true

        }

        "#;

        let declaration = vec![Input {

            source: "memory".to_string(),

            reader: Box::new(metadata_flag.as_bytes()),

        }];

        let value: Vec<Input> = vec![];



        let flags_bytes = crate::commands::parse_flags(

            "com.first",

            None,

            declaration,

            value,

            ProtoFlagPermission::READ_WRITE,

            true,

        )

        .unwrap();

        let parsed_flags =

            aconfig_protos::parsed_flags::try_from_binary_proto(&flags_bytes).unwrap();

        assert_eq!(1, parsed_flags.parsed_flag.len());

        let parsed_flag = parsed_flags.parsed_flag.first().unwrap();

        assert_eq!(ProtoFlagStorageBackend::NONE, parsed_flag.metadata.storage());



        // TODO case 4, mainline beta namespace fixed read only flag

        // TODO case 5, mainline beta namespace platform flag

        // TODO case 6, mainline beta namespace mainline flag

    }



    #[test]

    fn test_create_device_config_defaults() {

        let input = parse_test_flags_as_input();

tools/aconfig/aconfig/src/test.rs

+9 −0
Original line number Diff line number Diff line
@@ -49,6 +49,7 @@ parsed_flag { 
  container: "system"

  metadata {

    purpose: PURPOSE_UNSPECIFIED

    storage: NONE

  }

}

parsed_flag {
@@ -69,6 +70,7 @@ parsed_flag { 
  container: "system"

  metadata {

    purpose: PURPOSE_UNSPECIFIED

    storage: ACONFIGD

  }

}

parsed_flag {
@@ -94,6 +96,7 @@ parsed_flag { 
  container: "system"

  metadata {

    purpose: PURPOSE_UNSPECIFIED

    storage: ACONFIGD

  }

}

parsed_flag {
@@ -119,6 +122,7 @@ parsed_flag { 
  container: "system"

  metadata {

    purpose: PURPOSE_UNSPECIFIED

    storage: ACONFIGD

  }

}

parsed_flag {
@@ -144,6 +148,7 @@ parsed_flag { 
  container: "system"

  metadata {

    purpose: PURPOSE_UNSPECIFIED

    storage: NONE

  }

}

parsed_flag {
@@ -169,6 +174,7 @@ parsed_flag { 
  container: "system"

  metadata {

    purpose: PURPOSE_UNSPECIFIED

    storage: NONE

  }

}

parsed_flag {
@@ -199,6 +205,7 @@ parsed_flag { 
  container: "system"

  metadata {

    purpose: PURPOSE_BUGFIX

    storage: NONE

  }

}

parsed_flag {
@@ -224,6 +231,7 @@ parsed_flag { 
  container: "system"

  metadata {

    purpose: PURPOSE_UNSPECIFIED

    storage: NONE

  }

}

parsed_flag {
@@ -249,6 +257,7 @@ parsed_flag { 
  container: "system"

  metadata {

    purpose: PURPOSE_UNSPECIFIED

    storage: ACONFIGD

  }

}

"#;

tools/aconfig/aconfig_protos/protos/aconfig.proto

+10 −0
Original line number Diff line number Diff line
@@ -106,8 +106,18 @@ message flag_metadata { 
    PURPOSE_BUGFIX = 2;

  }



  // storage backend for flag

  enum flag_storage_backend {

    UNSPECIFIED = 0;    // unspecified, let aconfig choose one

    ACONFIGD = 1;       // aconfigd based new storage

    DEVICE_CONFIG = 2;  // device config based legacy storage

    NONE = 3;           // no need for flag storage

  }



  optional flag_purpose purpose = 1;



  optional flag_storage_backend storage = 2;



  // TODO(b/315025930): Add field to designate intended target device form factor(s), such as phone, watch or other.

}