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

Commit f25d2e3b authored by Marybeth Fair's avatar Marybeth Fair
Browse files

Add ability to read both v1/v2 package map. 

Note we're not actually updating the struct or anything to be v2 /
contain the fingerprint. All we're doing is adding methods to read v2
files.

This will ensure that any file, v1 or v2, can be read. I think we should
think a bit differently about where the version is set; ideally, it
would come from the build arg (from aconfig/src/main) and we'd use that
value within the system for all writes. For reads, we'd use whatever
version it's written as. For updates, similarly, we should maintain the
version  we have (though separately we can update files).

I also added "build flagging" to version increment. The reason I use quotes is because I added the ability to set the write version
via an optional arg to the create-storage CLI.
If the arg is not set, the default value is the existing version (1).
Since I haven't added the build flag or updated Soong to set the
version, it will always be unset, so this is equivalent to flagging by writing if (false) and
changing it to if (flagEnabled()) later. The reasons I would like to
start with this change are:
1) It lays the infra to be able to have different versions without actually
   changing the versions of anything.
2) If somehow anyone has the v2 file, at least now we can read it
   without crashing. This
   part of the change is not guarded by a flag because the check is the
   version code of the file that's on the device - so, moving forward,
   the updated reads will only be invoked if we've written the newer
   format, which is build flag guarded. If anyone has other ideas of how
   to approach this to ensure we cover any v2 files which may be in
   place, please let me know!

Test: atest, manual in-progress
Change-Id: Ib17ba44339c1d0d457c9cdd73047c891ab712774
parent cc0bb805

tools/aconfig/aconfig/src/commands.rs

+2 −1
Original line number Diff line number Diff line
@@ -280,10 +280,11 @@ pub fn create_storage( 
    caches: Vec<Input>,

    container: &str,

    file: &StorageFileType,

    version: u32,

) -> Result<Vec<u8>> {

    let parsed_flags_vec: Vec<ProtoParsedFlags> =

        caches.into_iter().map(|mut input| input.try_parse_flags()).collect::<Result<Vec<_>>>()?;

    generate_storage_file(container, parsed_flags_vec.iter(), file)

    generate_storage_file(container, parsed_flags_vec.iter(), file, version)

}



pub fn create_device_config_defaults(mut input: Input) -> Result<Vec<u8>> {

tools/aconfig/aconfig/src/main.rs

+16 −2
Original line number Diff line number Diff line
@@ -16,6 +16,8 @@ 


//! `aconfig` is a build time tool to manage build time configurations, such as feature flags.



use aconfig_storage_file::DEFAULT_FILE_VERSION;

use aconfig_storage_file::MAX_SUPPORTED_FILE_VERSION;

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

use clap::{builder::ArgAction, builder::EnumValueParser, Arg, ArgMatches, Command};

use core::any::Any;
@@ -159,7 +161,13 @@ fn cli() -> Command { 
                        .value_parser(|s: &str| StorageFileType::try_from(s)),

                )

                .arg(Arg::new("cache").long("cache").action(ArgAction::Append).required(true))

                .arg(Arg::new("out").long("out").required(true)),

                .arg(Arg::new("out").long("out").required(true))

                .arg(

                    Arg::new("version")

                        .long("version")

                        .required(false)

                        .value_parser(|s: &str| s.parse::<u32>()),

                ),

        )

}
@@ -309,12 +317,18 @@ fn main() -> Result<()> { 
            write_output_to_file_or_stdout(path, &output)?;

        }

        Some(("create-storage", sub_matches)) => {

            let version =

                get_optional_arg::<u32>(sub_matches, "version").unwrap_or(&DEFAULT_FILE_VERSION);

            if *version > MAX_SUPPORTED_FILE_VERSION {

                bail!("Invalid version selected ({})", version);

            }

            let file = get_required_arg::<StorageFileType>(sub_matches, "file")

                .context("Invalid storage file selection")?;

            let cache = open_zero_or_more_files(sub_matches, "cache")?;

            let container = get_required_arg::<String>(sub_matches, "container")?;

            let path = get_required_arg::<String>(sub_matches, "out")?;

            let output = commands::create_storage(cache, container, file)



            let output = commands::create_storage(cache, container, file, *version)

                .context("failed to create storage files")?;

            write_output_to_file_or_stdout(path, &output)?;

        }

tools/aconfig/aconfig/src/storage/flag_info.rs

+11 −8
Original line number Diff line number Diff line
@@ -17,14 +17,12 @@ 
use crate::commands::assign_flag_ids;

use crate::storage::FlagPackage;

use aconfig_protos::ProtoFlagPermission;

use aconfig_storage_file::{

    FlagInfoHeader, FlagInfoList, FlagInfoNode, StorageFileType, FILE_VERSION,

};

use aconfig_storage_file::{FlagInfoHeader, FlagInfoList, FlagInfoNode, StorageFileType};

use anyhow::{anyhow, Result};



fn new_header(container: &str, num_flags: u32) -> FlagInfoHeader {

fn new_header(container: &str, num_flags: u32, version: u32) -> FlagInfoHeader {

    FlagInfoHeader {

        version: FILE_VERSION,

        version,

        container: String::from(container),

        file_type: StorageFileType::FlagInfo as u8,

        file_size: 0,
@@ -33,7 +31,11 @@ fn new_header(container: &str, num_flags: u32) -> FlagInfoHeader { 
    }

}



pub fn create_flag_info(container: &str, packages: &[FlagPackage]) -> Result<FlagInfoList> {

pub fn create_flag_info(

    container: &str,

    packages: &[FlagPackage],

    version: u32,

) -> Result<FlagInfoList> {

    // create list

    let num_flags = packages.iter().map(|pkg| pkg.boolean_flags.len() as u32).sum();
@@ -51,7 +53,7 @@ pub fn create_flag_info(container: &str, packages: &[FlagPackage]) -> Result<Fla 
    }



    let mut list = FlagInfoList {

        header: new_header(container, num_flags),

        header: new_header(container, num_flags, version),

        nodes: is_flag_rw.iter().map(|&rw| FlagInfoNode::create(rw)).collect(),

    };
@@ -67,11 +69,12 @@ pub fn create_flag_info(container: &str, packages: &[FlagPackage]) -> Result<Fla 
mod tests {

    use super::*;

    use crate::storage::{group_flags_by_package, tests::parse_all_test_flags};

    use aconfig_storage_file::DEFAULT_FILE_VERSION;



    pub fn create_test_flag_info_list_from_source() -> Result<FlagInfoList> {

        let caches = parse_all_test_flags();

        let packages = group_flags_by_package(caches.iter());

        create_flag_info("mockup", &packages)

        create_flag_info("mockup", &packages, DEFAULT_FILE_VERSION)

    }



    #[test]

tools/aconfig/aconfig/src/storage/flag_table.rs

+11 −6
Original line number Diff line number Diff line
@@ -19,13 +19,12 @@ use crate::storage::FlagPackage; 
use aconfig_protos::ProtoFlagPermission;

use aconfig_storage_file::{

    get_table_size, FlagTable, FlagTableHeader, FlagTableNode, StorageFileType, StoredFlagType,

    FILE_VERSION,

};

use anyhow::{anyhow, Result};



fn new_header(container: &str, num_flags: u32) -> FlagTableHeader {

fn new_header(container: &str, num_flags: u32, version: u32) -> FlagTableHeader {

    FlagTableHeader {

        version: FILE_VERSION,

        version,

        container: String::from(container),

        file_type: StorageFileType::FlagMap as u8,

        file_size: 0,
@@ -86,12 +85,16 @@ impl FlagTableNodeWrapper { 
    }

}



pub fn create_flag_table(container: &str, packages: &[FlagPackage]) -> Result<FlagTable> {

pub fn create_flag_table(

    container: &str,

    packages: &[FlagPackage],

    version: u32,

) -> Result<FlagTable> {

    // create table

    let num_flags = packages.iter().map(|pkg| pkg.boolean_flags.len() as u32).sum();

    let num_buckets = get_table_size(num_flags)?;



    let mut header = new_header(container, num_flags);

    let mut header = new_header(container, num_flags, version);

    let mut buckets = vec![None; num_buckets as usize];

    let mut node_wrappers = packages

        .iter()
@@ -138,13 +141,15 @@ pub fn create_flag_table(container: &str, packages: &[FlagPackage]) -> Result<Fl 


#[cfg(test)]

mod tests {

    use aconfig_storage_file::DEFAULT_FILE_VERSION;



    use super::*;

    use crate::storage::{group_flags_by_package, tests::parse_all_test_flags};



    fn create_test_flag_table_from_source() -> Result<FlagTable> {

        let caches = parse_all_test_flags();

        let packages = group_flags_by_package(caches.iter());

        create_flag_table("mockup", &packages)

        create_flag_table("mockup", &packages, DEFAULT_FILE_VERSION)

    }



    #[test]

tools/aconfig/aconfig/src/storage/flag_value.rs

+12 −6
Original line number Diff line number Diff line
@@ -17,12 +17,12 @@ 
use crate::commands::assign_flag_ids;

use crate::storage::FlagPackage;

use aconfig_protos::ProtoFlagState;

use aconfig_storage_file::{FlagValueHeader, FlagValueList, StorageFileType, FILE_VERSION};

use aconfig_storage_file::{FlagValueHeader, FlagValueList, StorageFileType};

use anyhow::{anyhow, Result};



fn new_header(container: &str, num_flags: u32) -> FlagValueHeader {

fn new_header(container: &str, num_flags: u32, version: u32) -> FlagValueHeader {

    FlagValueHeader {

        version: FILE_VERSION,

        version,

        container: String::from(container),

        file_type: StorageFileType::FlagVal as u8,

        file_size: 0,
@@ -31,12 +31,16 @@ fn new_header(container: &str, num_flags: u32) -> FlagValueHeader { 
    }

}



pub fn create_flag_value(container: &str, packages: &[FlagPackage]) -> Result<FlagValueList> {

pub fn create_flag_value(

    container: &str,

    packages: &[FlagPackage],

    version: u32,

) -> Result<FlagValueList> {

    // create list

    let num_flags = packages.iter().map(|pkg| pkg.boolean_flags.len() as u32).sum();



    let mut list = FlagValueList {

        header: new_header(container, num_flags),

        header: new_header(container, num_flags, version),

        booleans: vec![false; num_flags as usize],

    };
@@ -61,13 +65,15 @@ pub fn create_flag_value(container: &str, packages: &[FlagPackage]) -> Result<Fl 


#[cfg(test)]

mod tests {

    use aconfig_storage_file::DEFAULT_FILE_VERSION;



    use super::*;

    use crate::storage::{group_flags_by_package, tests::parse_all_test_flags};



    pub fn create_test_flag_value_list_from_source() -> Result<FlagValueList> {

        let caches = parse_all_test_flags();

        let packages = group_flags_by_package(caches.iter());

        create_flag_value("mockup", &packages)

        create_flag_value("mockup", &packages, DEFAULT_FILE_VERSION)

    }



    #[test]