Merge "aconfig: move aconfig_storage_metadata proto and its apis to aconfig_storage_file crate" into main

tools/aconfig/Cargo.toml

@@ -5,6 +5,7 @@ members = [
     "aconfig_protos",
     "aconfig_storage_file",
     "aconfig_storage_read_api",
+    "aconfig_storage_write_api",
     "aflags",
     "printflags"
 ]

tools/aconfig/aconfig_storage_file/Android.bp

@@ -11,6 +11,9 @@ rust_defaults {
         "libanyhow",
         "libthiserror",
         "libtempfile",
+        "libprotobuf",
+        "libclap",
+        "libaconfig_storage_protos",
     ],
 }
 
@@ -26,3 +29,25 @@ rust_test_host {
     test_suites: ["general-tests"],
     defaults: ["aconfig_storage_file.defaults"],
 }
+
+rust_protobuf {
+    name: "libaconfig_storage_protos",
+    protos: ["protos/aconfig_storage_metadata.proto"],
+    crate_name: "aconfig_storage_protos",
+    source_stem: "aconfig_storage_protos",
+    host_supported: true,
+}
+
+cc_library_static {
+    name: "libaconfig_storage_protos_cc",
+    proto: {
+        export_proto_headers: true,
+        type: "lite",
+    },
+    srcs: ["protos/aconfig_storage_metadata.proto"],
+    apex_available: [
+        "//apex_available:platform",
+        "//apex_available:anyapex",
+    ],
+    host_supported: true,
+}

tools/aconfig/aconfig_storage_file/Cargo.toml

@@ -9,11 +9,8 @@ cargo = []
 
 [dependencies]
 anyhow = "1.0.69"
-memmap2 = "0.8.0"
 protobuf = "3.2.0"
-once_cell = "1.19.0"
 tempfile = "3.9.0"
-cxx = "1.0"
 thiserror = "1.0.56"
 clap = { version = "4.1.8", features = ["derive"] }
 

tools/aconfig/aconfig_storage_file/build.rs

@@ -0,0 +1,17 @@
+use protobuf_codegen::Codegen;
+
+fn main() {
+    let proto_files = vec!["protos/aconfig_storage_metadata.proto"];
+
+    // tell cargo to only re-run the build script if any of the proto files has changed
+    for path in &proto_files {
+        println!("cargo:rerun-if-changed={}", path);
+    }
+
+    Codegen::new()
+        .pure()
+        .include("protos")
+        .inputs(proto_files)
+        .cargo_out_dir("aconfig_storage_protos")
+        .run_from_script();
+}

tools/aconfig/aconfig_storage_file/src/lib.rs

@@ -35,6 +35,7 @@
 pub mod flag_table;
 pub mod flag_value;
 pub mod package_table;
+pub mod protos;
 
 #[cfg(test)]
 mod test_utils;

tools/aconfig/aconfig_storage_file/src/protos.rs

@@ -49,8 +49,11 @@ mod auto_generated {
 pub use auto_generated::*;
 
 use anyhow::Result;
+use protobuf::Message;
+use std::io::Write;
+use tempfile::NamedTempFile;
 
-pub mod storage_files {
+pub mod storage_record_pb {
     use super::*;
     use anyhow::ensure;
 
@@ -80,15 +83,28 @@ pub mod storage_files {
         }
         Ok(())
     }
+
+    pub fn get_binary_proto_from_text_proto(text_proto: &str) -> Result<Vec<u8>> {
+        let storage_files: ProtoStorageFiles = protobuf::text_format::parse_from_str(text_proto)?;
+        let mut binary_proto = Vec::new();
+        storage_files.write_to_vec(&mut binary_proto)?;
+        Ok(binary_proto)
+    }
+
+    pub fn write_proto_to_temp_file(text_proto: &str) -> Result<NamedTempFile> {
+        let bytes = get_binary_proto_from_text_proto(text_proto).unwrap();
+        let mut file = NamedTempFile::new()?;
+        let _ = file.write_all(&bytes);
+        Ok(file)
+    }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::test_utils::get_binary_storage_proto_bytes;
 
     #[test]
-    fn test_parse_storage_files() {
+    fn test_parse_storage_record_pb() {
         let text_proto = r#"
 files {
     version: 0
@@ -107,8 +123,9 @@ files {
     timestamp: 54321
 }
 "#;
-        let binary_proto_bytes = get_binary_storage_proto_bytes(text_proto).unwrap();
-        let storage_files = storage_files::try_from_binary_proto(&binary_proto_bytes).unwrap();
+        let binary_proto_bytes =
+            storage_record_pb::get_binary_proto_from_text_proto(text_proto).unwrap();
+        let storage_files = storage_record_pb::try_from_binary_proto(&binary_proto_bytes).unwrap();
         assert_eq!(storage_files.files.len(), 2);
         let system_file = &storage_files.files[0];
         assert_eq!(system_file.version(), 0);
@@ -127,7 +144,7 @@ files {
     }
 
     #[test]
-    fn test_parse_invalid_storage_files() {
+    fn test_parse_invalid_storage_record_pb() {
         let text_proto = r#"
 files {
     version: 0
@@ -138,8 +155,9 @@ files {
     timestamp: 12345
 }
 "#;
-        let binary_proto_bytes = get_binary_storage_proto_bytes(text_proto).unwrap();
-        let err = storage_files::try_from_binary_proto(&binary_proto_bytes).unwrap_err();
+        let binary_proto_bytes =
+            storage_record_pb::get_binary_proto_from_text_proto(text_proto).unwrap();
+        let err = storage_record_pb::try_from_binary_proto(&binary_proto_bytes).unwrap_err();
         assert_eq!(
             format!("{:?}", err),
             "invalid storage file record: missing package map file for container system"
@@ -155,8 +173,9 @@ files {
     timestamp: 12345
 }
 "#;
-        let binary_proto_bytes = get_binary_storage_proto_bytes(text_proto).unwrap();
-        let err = storage_files::try_from_binary_proto(&binary_proto_bytes).unwrap_err();
+        let binary_proto_bytes =
+            storage_record_pb::get_binary_proto_from_text_proto(text_proto).unwrap();
+        let err = storage_record_pb::try_from_binary_proto(&binary_proto_bytes).unwrap_err();
         assert_eq!(
             format!("{:?}", err),
             "invalid storage file record: missing flag map file for container system"
@@ -172,8 +191,9 @@ files {
     timestamp: 12345
 }
 "#;
-        let binary_proto_bytes = get_binary_storage_proto_bytes(text_proto).unwrap();
-        let err = storage_files::try_from_binary_proto(&binary_proto_bytes).unwrap_err();
+        let binary_proto_bytes =
+            storage_record_pb::get_binary_proto_from_text_proto(text_proto).unwrap();
+        let err = storage_record_pb::try_from_binary_proto(&binary_proto_bytes).unwrap_err();
         assert_eq!(
             format!("{:?}", err),
             "invalid storage file record: missing flag val file for container system"

tools/aconfig/aconfig_storage_read_api/Android.bp

@@ -9,9 +9,7 @@ rust_defaults {
     srcs: ["src/lib.rs"],
     rustlibs: [
         "libanyhow",
-        "libaconfig_storage_protos",
         "libonce_cell",
-        "libprotobuf",
         "libtempfile",
         "libmemmap2",
         "libcxx",
@@ -38,28 +36,6 @@ rust_test_host {
     ],
 }
 
-rust_protobuf {
-    name: "libaconfig_storage_protos",
-    protos: ["protos/aconfig_storage_metadata.proto"],
-    crate_name: "aconfig_storage_protos",
-    source_stem: "aconfig_storage_protos",
-    host_supported: true,
-}
-
-cc_library_static {
-    name: "libaconfig_storage_protos_cc",
-    proto: {
-        export_proto_headers: true,
-        type: "lite",
-    },
-    srcs: ["protos/aconfig_storage_metadata.proto"],
-    apex_available: [
-        "//apex_available:platform",
-        "//apex_available:anyapex",
-    ],
-    host_supported: true,
-}
-
 // cxx source codegen from rust api
 genrule {
     name: "libcxx_aconfig_storage_read_api_bridge_code",

tools/aconfig/aconfig_storage_read_api/Cargo.toml

@@ -10,7 +10,6 @@ cargo = []
 [dependencies]
 anyhow = "1.0.69"
 memmap2 = "0.8.0"
-protobuf = "3.2.0"
 once_cell = "1.19.0"
 tempfile = "3.9.0"
 cxx = "1.0"

tools/aconfig/aconfig_storage_read_api/build.rs

@@ -1,20 +1,4 @@
-use protobuf_codegen::Codegen;
-
 fn main() {
-    let proto_files = vec!["protos/aconfig_storage_metadata.proto"];
-
-    // tell cargo to only re-run the build script if any of the proto files has changed
-    for path in &proto_files {
-        println!("cargo:rerun-if-changed={}", path);
-    }
-
-    Codegen::new()
-        .pure()
-        .include("protos")
-        .inputs(proto_files)
-        .cargo_out_dir("aconfig_storage_protos")
-        .run_from_script();
-
     let _ = cxx_build::bridge("src/lib.rs");
     println!("cargo:rerun-if-changed=src/lib.rs");
 }

tools/aconfig/aconfig_storage_read_api/src/lib.rs

@@ -38,14 +38,13 @@ pub mod flag_table_query;
 pub mod flag_value_query;
 pub mod mapped_file;
 pub mod package_table_query;
-pub mod protos;
 
 #[cfg(test)]
 mod test_utils;
 
-pub use crate::protos::ProtoStorageFiles;
 pub use aconfig_storage_file::{
-    read_u32_from_bytes, AconfigStorageError, StorageFileSelection, FILE_VERSION,
+    protos::ProtoStorageFiles, read_u32_from_bytes, AconfigStorageError, StorageFileSelection,
+    FILE_VERSION,
 };
 pub use flag_table_query::FlagOffset;
 pub use package_table_query::PackageOffset;
@@ -380,7 +379,8 @@ pub fn get_boolean_flag_value_cxx(container: &str, offset: u32) -> ffi::BooleanF
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::test_utils::{write_storage_text_to_temp_file, TestStorageFileSet};
+    use crate::test_utils::TestStorageFileSet;
+    use aconfig_storage_file::protos::storage_record_pb::write_proto_to_temp_file;
 
     fn create_test_storage_files(read_only: bool) -> TestStorageFileSet {
         TestStorageFileSet::new(
@@ -410,7 +410,7 @@ files {{
             ro_files.package_map.name, ro_files.flag_map.name, ro_files.flag_val.name
         );
 
-        let file = write_storage_text_to_temp_file(&text_proto).unwrap();
+        let file = write_proto_to_temp_file(&text_proto).unwrap();
         let file_full_path = file.path().display().to_string();
         let package_offset = get_package_offset_impl(
             &file_full_path,
@@ -461,7 +461,7 @@ files {{
             ro_files.package_map.name, ro_files.flag_map.name, ro_files.flag_val.name
         );
 
-        let file = write_storage_text_to_temp_file(&text_proto).unwrap();
+        let file = write_proto_to_temp_file(&text_proto).unwrap();
         let file_full_path = file.path().display().to_string();
         let baseline = vec![
             (0, "enabled_ro", 1u16),
@@ -500,7 +500,7 @@ files {{
             ro_files.package_map.name, ro_files.flag_map.name, ro_files.flag_val.name
         );
 
-        let file = write_storage_text_to_temp_file(&text_proto).unwrap();
+        let file = write_proto_to_temp_file(&text_proto).unwrap();
         let file_full_path = file.path().display().to_string();
         let baseline: Vec<bool> = vec![false; 8];
         for (offset, expected_value) in baseline.into_iter().enumerate() {

tools/aconfig/aconfig_storage_read_api/src/mapped_file.rs

@@ -23,13 +23,13 @@ use anyhow::anyhow;
 use memmap2::Mmap;
 use once_cell::sync::Lazy;
 
-use crate::protos::{
-    storage_files::try_from_binary_proto, ProtoStorageFileInfo, ProtoStorageFiles,
-};
 use crate::AconfigStorageError::{
     self, FileReadFail, MapFileFail, ProtobufParseFail, StorageFileNotFound,
 };
 use crate::StorageFileSelection;
+use aconfig_storage_file::protos::{
+    storage_record_pb::try_from_binary_proto, ProtoStorageFileInfo, ProtoStorageFiles,
+};
 
 /// Cache for already mapped files
 static ALL_MAPPED_FILES: Lazy<Mutex<HashMap<String, MappedStorageFileSet>>> = Lazy::new(|| {
@@ -148,7 +148,8 @@ pub(crate) fn get_mapped_file(
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::test_utils::{write_storage_text_to_temp_file, TestStorageFileSet};
+    use crate::test_utils::TestStorageFileSet;
+    use aconfig_storage_file::protos::storage_record_pb::write_proto_to_temp_file;
 
     #[test]
     fn test_find_storage_file_location() {
@@ -170,7 +171,7 @@ files {
     timestamp: 54321
 }
 "#;
-        let file = write_storage_text_to_temp_file(text_proto).unwrap();
+        let file = write_proto_to_temp_file(&text_proto).unwrap();
         let file_full_path = file.path().display().to_string();
         let file_info = find_container_storage_location(&file_full_path, "system").unwrap();
         assert_eq!(file_info.version(), 0);
@@ -235,7 +236,7 @@ files {{
             ro_files.package_map.name, ro_files.flag_map.name, ro_files.flag_val.name
         );
 
-        let file = write_storage_text_to_temp_file(&text_proto).unwrap();
+        let file = write_proto_to_temp_file(&text_proto).unwrap();
         let file_full_path = file.path().display().to_string();
         map_and_verify(
             &file_full_path,
@@ -264,7 +265,7 @@ files {{
             rw_files.package_map.name, ro_files.flag_map.name, ro_files.flag_val.name
         );
 
-        let file = write_storage_text_to_temp_file(&text_proto).unwrap();
+        let file = write_proto_to_temp_file(&text_proto).unwrap();
         let file_full_path = file.path().display().to_string();
         let error = map_container_storage_files(&file_full_path, "system").unwrap_err();
         assert_eq!(
@@ -289,7 +290,7 @@ files {{
             ro_files.package_map.name, rw_files.flag_map.name, ro_files.flag_val.name
         );
 
-        let file = write_storage_text_to_temp_file(&text_proto).unwrap();
+        let file = write_proto_to_temp_file(&text_proto).unwrap();
         let file_full_path = file.path().display().to_string();
         let error = map_container_storage_files(&file_full_path, "system").unwrap_err();
         assert_eq!(
@@ -314,7 +315,7 @@ files {{
             ro_files.package_map.name, ro_files.flag_map.name, rw_files.flag_val.name
         );
 
-        let file = write_storage_text_to_temp_file(&text_proto).unwrap();
+        let file = write_proto_to_temp_file(&text_proto).unwrap();
         let file_full_path = file.path().display().to_string();
         let error = map_container_storage_files(&file_full_path, "system").unwrap_err();
         assert_eq!(

tools/aconfig/aconfig_storage_read_api/src/test_utils.rs

@@ -14,27 +14,10 @@
  * limitations under the License.
  */
 
-use crate::protos::ProtoStorageFiles;
 use anyhow::Result;
-use protobuf::Message;
 use std::fs;
-use std::io::Write;
 use tempfile::NamedTempFile;
 
-pub(crate) fn get_binary_storage_proto_bytes(text_proto: &str) -> Result<Vec<u8>> {
-    let storage_files: ProtoStorageFiles = protobuf::text_format::parse_from_str(text_proto)?;
-    let mut binary_proto = Vec::new();
-    storage_files.write_to_vec(&mut binary_proto)?;
-    Ok(binary_proto)
-}
-
-pub(crate) fn write_storage_text_to_temp_file(text_proto: &str) -> Result<NamedTempFile> {
-    let bytes = get_binary_storage_proto_bytes(text_proto).unwrap();
-    let mut file = NamedTempFile::new()?;
-    let _ = file.write_all(&bytes);
-    Ok(file)
-}
-
 fn set_file_read_only(file: &NamedTempFile) {
     let mut perms = fs::metadata(file.path()).unwrap().permissions();
     if !perms.readonly() {