Merge "aconfig: adjust integration tests to correctly set flag values" into main
diff --git a/tools/aconfig/src/commands.rs b/tools/aconfig/src/commands.rs
index cd53371..0c4f543 100644
--- a/tools/aconfig/src/commands.rs
+++ b/tools/aconfig/src/commands.rs
@@ -339,7 +339,7 @@
         }
     };
     if modified_parsed_flags.is_empty() {
-        bail!("{} library contains no exported flags.", codegen_mode);
+        bail!("{codegen_mode} library contains no {codegen_mode} flags");
     }
 
     Ok(modified_parsed_flags)
@@ -653,9 +653,6 @@
         parsed_flags.parsed_flag.retain_mut(|pf| !pf.is_exported());
         let error =
             modify_parsed_flags_based_on_mode(parsed_flags, CodegenMode::Exported).unwrap_err();
-        assert_eq!(
-            format!("{} library contains no exported flags.", CodegenMode::Exported),
-            format!("{:?}", error)
-        );
+        assert_eq!("exported library contains no exported flags", format!("{:?}", error));
     }
 }
diff --git a/tools/aconfig/src/storage/mod.rs b/tools/aconfig/src/storage/mod.rs
index f81fb5c..686f9ae 100644
--- a/tools/aconfig/src/storage/mod.rs
+++ b/tools/aconfig/src/storage/mod.rs
@@ -160,6 +160,11 @@
                 "storage_test_2.aconfig",
                 include_bytes!("../../tests/storage_test_2.aconfig").as_slice(),
             ),
+            (
+                "com.android.aconfig.storage.test_4",
+                "storage_test_4.aconfig",
+                include_bytes!("../../tests/storage_test_4.aconfig").as_slice(),
+            ),
         ];
 
         aconfig_files
@@ -195,7 +200,7 @@
             }
         }
 
-        assert_eq!(packages.len(), 2);
+        assert_eq!(packages.len(), 3);
 
         assert_eq!(packages[0].package_name, "com.android.aconfig.storage.test_1");
         assert_eq!(packages[0].package_id, 0);
@@ -214,5 +219,12 @@
         assert!(packages[1].flag_names.contains("disabled_ro"));
         assert!(packages[1].flag_names.contains("enabled_fixed_ro"));
         assert_eq!(packages[1].boolean_offset, 10);
+
+        assert_eq!(packages[2].package_name, "com.android.aconfig.storage.test_4");
+        assert_eq!(packages[2].package_id, 2);
+        assert_eq!(packages[2].flag_names.len(), 2);
+        assert!(packages[2].flag_names.contains("enabled_ro"));
+        assert!(packages[2].flag_names.contains("enabled_fixed_ro"));
+        assert_eq!(packages[2].boolean_offset, 16);
     }
 }
diff --git a/tools/aconfig/src/storage/package_table.rs b/tools/aconfig/src/storage/package_table.rs
index 78102a5..940c5b2 100644
--- a/tools/aconfig/src/storage/package_table.rs
+++ b/tools/aconfig/src/storage/package_table.rs
@@ -87,6 +87,7 @@
     }
 }
 
+#[derive(PartialEq, Debug)]
 pub struct PackageTable {
     pub header: PackageTableHeader,
     pub buckets: Vec<Option<u32>>,
@@ -104,11 +105,17 @@
             nodes: packages.iter().map(|pkg| PackageTableNode::new(pkg, num_buckets)).collect(),
         };
 
+        // initialize all header fields
+        table.header.bucket_offset = table.header.as_bytes().len() as u32;
+        table.header.node_offset = table.header.bucket_offset + num_buckets * 4;
+        table.header.file_size = table.header.node_offset
+            + table.nodes.iter().map(|x| x.as_bytes().len()).sum::<usize>() as u32;
+
         // sort nodes by bucket index for efficiency
         table.nodes.sort_by(|a, b| a.bucket_index.cmp(&b.bucket_index));
 
         // fill all node offset
-        let mut offset = 0;
+        let mut offset = table.header.node_offset;
         for i in 0..table.nodes.len() {
             let node_bucket_idx = table.nodes[i].bucket_index;
             let next_node_bucket_idx = if i + 1 < table.nodes.len() {
@@ -129,12 +136,6 @@
             }
         }
 
-        // fill table region offset
-        table.header.bucket_offset = table.header.as_bytes().len() as u32;
-        table.header.node_offset = table.header.bucket_offset + num_buckets * 4;
-        table.header.file_size = table.header.node_offset
-            + table.nodes.iter().map(|x| x.as_bytes().len()).sum::<usize>() as u32;
-
         Ok(table)
     }
 
@@ -190,6 +191,32 @@
         }
     }
 
+    impl PackageTable {
+        // test only method to deserialize back into the table struct
+        fn from_bytes(bytes: &[u8]) -> Result<Self> {
+            let header = PackageTableHeader::from_bytes(bytes)?;
+            let num_packages = header.num_packages;
+            let num_buckets = storage::get_table_size(num_packages)?;
+            let mut head = header.as_bytes().len();
+            let buckets = (0..num_buckets)
+                .map(|_| match read_u32_from_bytes(bytes, &mut head).unwrap() {
+                    0 => None,
+                    val => Some(val),
+                })
+                .collect();
+            let nodes = (0..num_packages)
+                .map(|_| {
+                    let node = PackageTableNode::from_bytes(&bytes[head..], num_buckets).unwrap();
+                    head += node.as_bytes().len();
+                    node
+                })
+                .collect();
+
+            let table = Self { header, buckets, nodes };
+            Ok(table)
+        }
+    }
+
     pub fn create_test_package_table() -> Result<PackageTable> {
         let caches = parse_all_test_flags();
         let packages = group_flags_by_package(caches.iter());
@@ -206,19 +233,19 @@
         let expected_header = PackageTableHeader {
             version: storage::FILE_VERSION,
             container: String::from("system"),
-            file_size: 158,
-            num_packages: 2,
+            file_size: 208,
+            num_packages: 3,
             bucket_offset: 30,
             node_offset: 58,
         };
         assert_eq!(header, &expected_header);
 
         let buckets: &Vec<Option<u32>> = &package_table.as_ref().unwrap().buckets;
-        let expected: Vec<Option<u32>> = vec![Some(0), None, None, Some(50), None, None, None];
+        let expected: Vec<Option<u32>> = vec![Some(58), None, None, Some(108), None, None, None];
         assert_eq!(buckets, &expected);
 
         let nodes: &Vec<PackageTableNode> = &package_table.as_ref().unwrap().nodes;
-        assert_eq!(nodes.len(), 2);
+        assert_eq!(nodes.len(), 3);
         let first_node_expected = PackageTableNode {
             package_name: String::from("com.android.aconfig.storage.test_2"),
             package_id: 1,
@@ -231,10 +258,18 @@
             package_name: String::from("com.android.aconfig.storage.test_1"),
             package_id: 0,
             boolean_offset: 0,
-            next_offset: None,
+            next_offset: Some(158),
             bucket_index: 3,
         };
         assert_eq!(nodes[1], second_node_expected);
+        let third_node_expected = PackageTableNode {
+            package_name: String::from("com.android.aconfig.storage.test_4"),
+            package_id: 2,
+            boolean_offset: 16,
+            next_offset: None,
+            bucket_index: 3,
+        };
+        assert_eq!(nodes[2], third_node_expected);
     }
 
     #[test]
@@ -242,18 +277,23 @@
     fn test_serialization() {
         let package_table = create_test_package_table();
         assert!(package_table.is_ok());
+        let package_table = package_table.unwrap();
 
-        let header: &PackageTableHeader = &package_table.as_ref().unwrap().header;
+        let header: &PackageTableHeader = &package_table.header;
         let reinterpreted_header = PackageTableHeader::from_bytes(&header.as_bytes());
         assert!(reinterpreted_header.is_ok());
         assert_eq!(header, &reinterpreted_header.unwrap());
 
-        let nodes: &Vec<PackageTableNode> = &package_table.as_ref().unwrap().nodes;
+        let nodes: &Vec<PackageTableNode> = &package_table.nodes;
         let num_buckets = storage::get_table_size(header.num_packages).unwrap();
         for node in nodes.iter() {
             let reinterpreted_node = PackageTableNode::from_bytes(&node.as_bytes(), num_buckets);
             assert!(reinterpreted_node.is_ok());
             assert_eq!(node, &reinterpreted_node.unwrap());
         }
+
+        let reinterpreted_table = PackageTable::from_bytes(&package_table.as_bytes());
+        assert!(reinterpreted_table.is_ok());
+        assert_eq!(&package_table, &reinterpreted_table.unwrap());
     }
 }
diff --git a/tools/aconfig/templates/FeatureFlags.java.template b/tools/aconfig/templates/FeatureFlags.java.template
index d6af62c..5e67b13 100644
--- a/tools/aconfig/templates/FeatureFlags.java.template
+++ b/tools/aconfig/templates/FeatureFlags.java.template
@@ -5,10 +5,10 @@
 /** @hide */
 public interface FeatureFlags \{
 {{ for item in flag_elements }}
-{{ if library_exported }}
+{{ -if library_exported }}
     @UnsupportedAppUsage
     boolean {item.method_name}();
-{{ else }}
+{{ -else }}
 {{ -if not item.is_read_write }}
 {{ -if item.default_value }}
     @com.android.aconfig.annotations.AssumeTrueForR8
@@ -19,5 +19,5 @@
     @UnsupportedAppUsage
     boolean {item.method_name}();
 {{ endif }}
-{{ endfor }}
+{{ -endfor }}
 }
diff --git a/tools/aconfig/templates/FeatureFlagsImpl.java.template b/tools/aconfig/templates/FeatureFlagsImpl.java.template
index cf49c17..28baa41 100644
--- a/tools/aconfig/templates/FeatureFlagsImpl.java.template
+++ b/tools/aconfig/templates/FeatureFlagsImpl.java.template
@@ -1,44 +1,42 @@
 package {package_name};
 // TODO(b/303773055): Remove the annotation after access issue is resolved.
 import android.compat.annotation.UnsupportedAppUsage;
-{{ if not is_test_mode }}
-{{ if runtime_lookup_required- }}
+{{ -if not is_test_mode }}
+{{ -if runtime_lookup_required }}
 import android.provider.DeviceConfig;
 import android.provider.DeviceConfig.Properties;
 {{ endif }}
 /** @hide */
 public final class FeatureFlagsImpl implements FeatureFlags \{
-{{- if runtime_lookup_required }}
-{{- for namespace_with_flags in namespace_flags }}
+{{ -if runtime_lookup_required }}
+{{ -for namespace_with_flags in namespace_flags }}
     private static boolean {namespace_with_flags.namespace}_is_cached = false;
-{{- endfor- }}
+{{ -endfor- }}
 
 {{ for flag in flag_elements }}
-{{ if library_exported }}
+{{ -if library_exported }}
     private static boolean {flag.method_name} = false;
-{{ else }}
+{{ -else }}
 {{- if flag.is_read_write }}
     private static boolean {flag.method_name} = {flag.default_value};
 {{- endif- }}
-{{ endif }}
-{{ endfor }}
-
+{{ -endif }}
+{{ -endfor }}
 {{ for namespace_with_flags in namespace_flags }}
     private void load_overrides_{namespace_with_flags.namespace}() \{
         try \{
             Properties properties = DeviceConfig.getProperties("{namespace_with_flags.namespace}");
-
-            {{- for flag in namespace_with_flags.flags }}
-            {{ if library_exported }}
+{{ -for flag in namespace_with_flags.flags }}
+{{ -if library_exported }}
             {flag.method_name} =
                 properties.getBoolean("{flag.device_config_flag}", false);
-            {{ else }}
-            {{ if flag.is_read_write }}
+{{ -else }}
+{{ -if flag.is_read_write }}
             {flag.method_name} =
                 properties.getBoolean("{flag.device_config_flag}", {flag.default_value});
-            {{ endif }}
-            {{ endif }}
-            {{ endfor }}
+{{ -endif }}
+{{ -endif }}
+{{ -endfor }}
         } catch (NullPointerException e) \{
             throw new RuntimeException(
                 "Cannot read value from namespace {namespace_with_flags.namespace} "
@@ -52,27 +50,26 @@
         {namespace_with_flags.namespace}_is_cached = true;
     }
 {{ endfor- }}
-{{ endif- }}
-
-{{ for flag in flag_elements }}
+{{ -endif }}{#- end of runtime_lookup_required #}
+{{ -for flag in flag_elements }}
     @Override
     @UnsupportedAppUsage
     public boolean {flag.method_name}() \{
-    {{ -if library_exported }}
+{{ -if library_exported }}
         if (!{flag.device_config_namespace}_is_cached) \{
             load_overrides_{flag.device_config_namespace}();
         }
         return {flag.method_name};
-    {{ else }}
-    {{ -if flag.is_read_write }}
+{{ -else }}
+{{ -if flag.is_read_write }}
         if (!{flag.device_config_namespace}_is_cached) \{
             load_overrides_{flag.device_config_namespace}();
         }
         return {flag.method_name};
-    {{ else }}
+{{ -else }}
         return {flag.default_value};
-    {{ -endif- }}
-    {{ -endif }}
+{{ -endif- }}
+{{ -endif }}
     }
 {{ endfor }}
 }
diff --git a/tools/aconfig/templates/Flags.java.template b/tools/aconfig/templates/Flags.java.template
index ff942e5..34b8189 100644
--- a/tools/aconfig/templates/Flags.java.template
+++ b/tools/aconfig/templates/Flags.java.template
@@ -5,12 +5,12 @@
 
 /** @hide */
 public final class Flags \{
-{{- for item in flag_elements}}
+{{ -for item in flag_elements}}
     /** @hide */
     public static final String FLAG_{item.flag_name_constant_suffix} = "{item.device_config_flag}";
 {{- endfor }}
-{{ for item in flag_elements}}
-{{ -if library_exported }}
+{{ -for item in flag_elements}}
+{{ if library_exported }}
     @UnsupportedAppUsage
     public static boolean {item.method_name}() \{
         return FEATURE_FLAGS.{item.method_name}();
@@ -21,14 +21,14 @@
     @com.android.aconfig.annotations.AssumeTrueForR8
 {{ -else }}
     @com.android.aconfig.annotations.AssumeFalseForR8
-{{ -endif- }}
-{{ endif }}
+{{ -endif }}
+{{ -endif }}
     @UnsupportedAppUsage
     public static boolean {item.method_name}() \{
         return FEATURE_FLAGS.{item.method_name}();
     }
-{{ endif }}
-{{ endfor }}
+{{ -endif }}
+{{ -endfor }}
 {{ -if is_test_mode }}
     public static void setFeatureFlags(FeatureFlags featureFlags) \{
         Flags.FEATURE_FLAGS = featureFlags;
@@ -37,7 +37,8 @@
     public static void unsetFeatureFlags() \{
         Flags.FEATURE_FLAGS = null;
     }
-{{ endif }}
+{{ -endif }}
+
     private static FeatureFlags FEATURE_FLAGS{{ -if not is_test_mode }} = new FeatureFlagsImpl(){{ -endif- }};
 
 }
diff --git a/tools/aconfig/templates/cpp_exported_header.template b/tools/aconfig/templates/cpp_exported_header.template
index 8db9ec4..6b6daa7 100644
--- a/tools/aconfig/templates/cpp_exported_header.template
+++ b/tools/aconfig/templates/cpp_exported_header.template
@@ -6,13 +6,11 @@
 #define {package_macro}(FLAG) {package_macro}_##FLAG
 #endif
 {{ for item in class_elements }}
-
-{{ if item.is_fixed_read_only- }}
+{{ -if item.is_fixed_read_only }}
 #ifndef {package_macro}_{item.flag_macro}
 #define {package_macro}_{item.flag_macro} {item.default_value}
 #endif
 {{ -endif }}
-
 {{ -endfor }}
 {{ -endif }}
 {{ -endif }}
@@ -26,15 +24,15 @@
 class flag_provider_interface \{
 public:
     virtual ~flag_provider_interface() = default;
-    {{ for item in class_elements}}
+    {{ -for item in class_elements}}
     virtual bool {item.flag_name}() = 0;
 
-    {{ if is_test_mode }}
+    {{ -if is_test_mode }}
     virtual void {item.flag_name}(bool val) = 0;
     {{ -endif }}
     {{ -endfor }}
 
-    {{ if is_test_mode }}
+    {{ -if is_test_mode }}
     virtual void reset_flags() \{}
     {{ -endif }}
 };
@@ -43,35 +41,35 @@
 
 {{ for item in class_elements}}
 inline bool {item.flag_name}() \{
-    {{ if is_test_mode }}
+    {{ -if is_test_mode }}
     return provider_->{item.flag_name}();
-    {{ -else- }}
-    {{ if is_prod_mode- }}
-    {{ if item.readwrite- }}
+    {{ -else }}
+    {{ -if is_prod_mode }}
+    {{ -if item.readwrite }}
     return provider_->{item.flag_name}();
-    {{ -else- }}
-    {{ if item.is_fixed_read_only- }}
+    {{ -else }}
+    {{ -if item.is_fixed_read_only }}
     return {package_macro}_{item.flag_macro};
-    {{ -else- }}
+    {{ -else }}
     return {item.default_value};
     {{ -endif }}
     {{ -endif }}
-    {{ -else- }}
-    {{ if is_exported_mode- }}
+    {{ -else }}
+    {{ -if is_exported_mode }}
     return provider_->{item.flag_name}();
     {{ -endif }}
     {{ -endif }}
     {{ -endif }}
 }
 
-{{ if is_test_mode }}
+{{ -if is_test_mode }}
 inline void {item.flag_name}(bool val) \{
     provider_->{item.flag_name}(val);
 }
 {{ -endif }}
 {{ -endfor }}
 
-{{ if is_test_mode }}
+{{ -if is_test_mode }}
 inline void reset_flags() \{
     return provider_->reset_flags();
 }
@@ -85,12 +83,12 @@
 {{ for item in class_elements }}
 bool {header}_{item.flag_name}();
 
-{{ if is_test_mode }}
+{{ -if is_test_mode }}
 void set_{header}_{item.flag_name}(bool val);
 {{ -endif }}
 {{ -endfor }}
 
-{{ if is_test_mode }}
+{{ -if is_test_mode }}
 void {header}_reset_flags();
 {{ -endif }}
 
diff --git a/tools/aconfig/templates/cpp_source_file.template b/tools/aconfig/templates/cpp_source_file.template
index a10d7cb..4aec540 100644
--- a/tools/aconfig/templates/cpp_source_file.template
+++ b/tools/aconfig/templates/cpp_source_file.template
@@ -24,7 +24,7 @@
             : overrides_()
         \{}
 
-        {{ for item in class_elements }}
+{{ for item in class_elements }}
         virtual bool {item.flag_name}() override \{
             auto it = overrides_.find("{item.flag_name}");
               if (it != overrides_.end()) \{
@@ -35,7 +35,7 @@
                   "aconfig_flags.{item.device_config_namespace}",
                   "{item.device_config_flag}",
                   "{item.default_value}") == "true";
-              {{ -else- }}
+              {{ -else }}
                   return {item.default_value};
               {{ -endif }}
             }
@@ -44,7 +44,7 @@
         virtual void {item.flag_name}(bool val) override \{
             overrides_["{item.flag_name}"] = val;
         }
-        {{ -endfor }}
+{{ endfor }}
 
         virtual void reset_flags() override \{
             overrides_.clear();
@@ -56,10 +56,11 @@
     class flag_provider : public flag_provider_interface \{
     public:
 
-        {{ for item in class_elements }}
+        {{ -for item in class_elements }}
+
         virtual bool {item.flag_name}() override \{
-            {{ if is_prod_mode- }}
-            {{ if item.readwrite- }}
+            {{ -if is_prod_mode }}
+            {{ -if item.readwrite }}
             if (cache_[{item.readwrite_idx}] == -1) \{
                 cache_[{item.readwrite_idx}] = server_configurable_flags::GetServerConfigurableFlag(
                     "aconfig_flags.{item.device_config_namespace}",
@@ -67,15 +68,15 @@
                     "{item.default_value}") == "true";
             }
             return cache_[{item.readwrite_idx}];
-            {{ -else- }}
-            {{ if item.is_fixed_read_only }}
+            {{ -else }}
+            {{ -if item.is_fixed_read_only }}
             return {package_macro}_{item.flag_macro};
-            {{ -else- }}
+            {{ -else }}
             return {item.default_value};
             {{ -endif }}
             {{ -endif }}
             {{ -else- }}
-            {{ if is_exported_mode-}}
+            {{ -if is_exported_mode }}
             if (cache_[{item.readwrite_idx}] == -1) \{
                 cache_[{item.readwrite_idx}] = server_configurable_flags::GetServerConfigurableFlag(
                     "aconfig_flags.{item.device_config_namespace}",
@@ -86,7 +87,7 @@
             {{ -endif }}
             {{ -endif }}
         }
-        {{ endfor }}
+        {{ -endfor }}
     {{ if readwrite- }}
     private:
         std::vector<int8_t> cache_ = std::vector<int8_t>({readwrite_count}, -1);
@@ -102,35 +103,35 @@
 
 {{ for item in class_elements }}
 bool {header}_{item.flag_name}() \{
-    {{ if is_test_mode }}
+    {{ -if is_test_mode }}
     return {cpp_namespace}::{item.flag_name}();
-    {{ -else- }}
-    {{ if is_prod_mode- }}
-    {{ if item.readwrite- }}
+    {{ -else }}
+    {{ -if is_prod_mode }}
+    {{ -if item.readwrite }}
     return {cpp_namespace}::{item.flag_name}();
-    {{ -else- }}
-    {{ if item.is_fixed_read_only }}
+    {{ -else }}
+    {{ -if item.is_fixed_read_only }}
     return {package_macro}_{item.flag_macro};
-    {{ -else- }}
+    {{ -else }}
     return {item.default_value};
     {{ -endif }}
     {{ -endif }}
-    {{ -else- }}
-    {{ if is_exported_mode- }}
+    {{ -else }}
+    {{ -if is_exported_mode }}
     return {cpp_namespace}::{item.flag_name}();
     {{ -endif }}
     {{ -endif }}
     {{ -endif }}
 }
 
-{{ if is_test_mode }}
+{{ -if is_test_mode }}
 void set_{header}_{item.flag_name}(bool val) \{
     {cpp_namespace}::{item.flag_name}(val);
 }
 {{ -endif }}
-{{ endfor-}}
+{{ endfor }}
 
-{{ if is_test_mode }}
+{{ -if is_test_mode }}
 void {header}_reset_flags() \{
      {cpp_namespace}::reset_flags();
 }
diff --git a/tools/aconfig/templates/rust_exported.template b/tools/aconfig/templates/rust_exported.template
index b31bcef..110f2d4 100644
--- a/tools/aconfig/templates/rust_exported.template
+++ b/tools/aconfig/templates/rust_exported.template
@@ -4,23 +4,22 @@
 pub struct FlagProvider;
 
 lazy_static::lazy_static! \{
-    {{ for flag in template_flags }}
+{{ for flag in template_flags }}
     /// flag value cache for {flag.name}
     static ref CACHED_{flag.name}: bool = flags_rust::GetServerConfigurableFlag(
         "aconfig_flags.{flag.device_config_namespace}",
         "{flag.device_config_flag}",
         "false") == "true";
-    {{ endfor }}
+{{ endfor }}
 }
 
 impl FlagProvider \{
-
-    {{ for flag in template_flags }}
+{{ for flag in template_flags }}
     /// query flag {flag.name}
     pub fn {flag.name}(&self) -> bool \{
         *CACHED_{flag.name}
     }
-    {{ endfor }}
+{{ endfor }}
 
 }
 
diff --git a/tools/aconfig/templates/rust_prod.template b/tools/aconfig/templates/rust_prod.template
index 30ea646..f9a2829 100644
--- a/tools/aconfig/templates/rust_prod.template
+++ b/tools/aconfig/templates/rust_prod.template
@@ -3,32 +3,32 @@
 /// flag provider
 pub struct FlagProvider;
 
-{{ if has_readwrite - }}
+{{ if has_readwrite- }}
 lazy_static::lazy_static! \{
-    {{ for flag in template_flags }}
-    {{ if flag.readwrite -}}
+{{ -for flag in template_flags }}
+    {{ -if flag.readwrite }}
     /// flag value cache for {flag.name}
     static ref CACHED_{flag.name}: bool = flags_rust::GetServerConfigurableFlag(
         "aconfig_flags.{flag.device_config_namespace}",
         "{flag.device_config_flag}",
         "{flag.default_value}") == "true";
     {{ -endif }}
-    {{ endfor }}
+{{ -endfor }}
 }
 {{ -endif }}
 
 impl FlagProvider \{
 
-    {{ for flag in template_flags }}
+{{ for flag in template_flags }}
     /// query flag {flag.name}
     pub fn {flag.name}(&self) -> bool \{
-    {{ if flag.readwrite -}}
+    {{ -if flag.readwrite }}
         *CACHED_{flag.name}
-    {{ -else- }}
+    {{ -else }}
         {flag.default_value}
     {{ -endif }}
     }
-    {{ endfor }}
+{{ endfor }}
 
 }
 
@@ -38,10 +38,10 @@
 {{ for flag in template_flags }}
 /// query flag {flag.name}
 #[inline(always)]
-{{ if flag.readwrite -}}
+{{ -if flag.readwrite }}
 pub fn {flag.name}() -> bool \{
     PROVIDER.{flag.name}()
-{{ -else- }}
+{{ -else }}
 pub fn {flag.name}() -> bool \{
     {flag.default_value}
 {{ -endif }}
diff --git a/tools/aconfig/templates/rust_test.template b/tools/aconfig/templates/rust_test.template
index fd1229b..d01f40a 100644
--- a/tools/aconfig/templates/rust_test.template
+++ b/tools/aconfig/templates/rust_test.template
@@ -9,7 +9,7 @@
 }
 
 impl FlagProvider \{
-    {{ for flag in template_flags }}
+{{ for flag in template_flags }}
     /// query flag {flag.name}
     pub fn {flag.name}(&self) -> bool \{
         self.overrides.get("{flag.name}").copied().unwrap_or(
@@ -28,7 +28,7 @@
     pub fn set_{flag.name}(&mut self, val: bool) \{
         self.overrides.insert("{flag.name}", val);
     }
-    {{ endfor }}
+{{ endfor }}
 
     /// clear all flag overrides
     pub fn reset_flags(&mut self) \{
diff --git a/tools/aconfig/tests/storage_test_4.aconfig b/tools/aconfig/tests/storage_test_4.aconfig
new file mode 100644
index 0000000..333fe09
--- /dev/null
+++ b/tools/aconfig/tests/storage_test_4.aconfig
@@ -0,0 +1,17 @@
+package: "com.android.aconfig.storage.test_4"
+container: "system"
+
+flag {
+    name: "enabled_ro"
+    namespace: "aconfig_test"
+    description: "This flag is ENABLED + READ_ONLY"
+    bug: "abc"
+}
+
+flag {
+    name: "enabled_fixed_ro"
+    namespace: "aconfig_test"
+    description: "This flag is fixed READ_ONLY + ENABLED"
+    bug: ""
+    is_fixed_read_only: true
+}
diff --git a/tools/perf/benchmarks b/tools/perf/benchmarks
index c42a2d8..f46b920 100755
--- a/tools/perf/benchmarks
+++ b/tools/perf/benchmarks
@@ -23,9 +23,12 @@
 import json
 import os
 import pathlib
+import random
+import re
 import shutil
 import subprocess
 import time
+import uuid
 
 import pretty
 import utils
@@ -137,9 +140,23 @@
     return Change(label="No change", change=lambda: None, undo=lambda: None)
 
 
+def Create(filename):
+    "Create an action to create `filename`. The parent directory must exist."
+    def create():
+        with open(filename, "w") as f:
+            pass
+    def delete():
+        os.remove(filename)
+    return Change(
+                label=f"Create {filename}",
+                change=create,
+                undo=delete,
+            )
+
+
 def Modify(filename, contents, before=None):
-    """Create an action to modify `filename` by appending `contents` before the last instances
-    of `before` in the file.
+    """Create an action to modify `filename` by appending the result of `contents`
+    before the last instances of `before` in the file.
 
     Raises an error if `before` doesn't appear in the file.
     """
@@ -151,13 +168,29 @@
             raise FatalError()
     else:
         index = len(orig.contents)
-    modified = FileSnapshot(filename, orig.contents[:index] + contents + orig.contents[index:])
+    modified = FileSnapshot(filename, orig.contents[:index] + contents() + orig.contents[index:])
+    if False:
+        print(f"Modify: {filename}")
+        x = orig.contents.replace("\n", "\n   ORIG")
+        print(f"   ORIG {x}")
+        x = modified.contents.replace("\n", "\n   MODIFIED")
+        print(f"   MODIFIED {x}")
+
     return Change(
             label="Modify " + filename,
             change=lambda: modified.write(),
             undo=lambda: orig.write()
         )
 
+def AddJavaField(filename, prefix):
+    return Modify(filename,
+                  lambda: f"{prefix} static final int BENCHMARK = {random.randint(0, 1000000)};\n",
+                  before="}")
+
+
+def Comment(prefix, suffix=""):
+    return lambda: prefix + " " + str(uuid.uuid4()) + suffix
+
 
 class BenchmarkReport():
     "Information about a run of the benchmark"
@@ -262,11 +295,16 @@
             ns = self._run_build(lunch, benchmark_log_dir.joinpath("measured"), benchmark.modules)
             report.duration_ns = ns
 
-            # Postroll builds
-            for i in range(benchmark.preroll):
-                ns = self._run_build(lunch, benchmark_log_dir.joinpath(f"post_{i}"),
-                                     benchmark.modules)
-                report.postroll_duration_ns.append(ns)
+            dist_one = self._options.DistOne()
+            if dist_one:
+                # If we're disting just one benchmark, save the logs and we can stop here.
+                self._dist(dist_one)
+            else:
+                # Postroll builds
+                for i in range(benchmark.preroll):
+                    ns = self._run_build(lunch, benchmark_log_dir.joinpath(f"post_{i}"),
+                                         benchmark.modules)
+                    report.postroll_duration_ns.append(ns)
 
         finally:
             # Always undo, even if we crashed or the build failed and we stopped.
@@ -315,6 +353,22 @@
 
         return after_ns - before_ns
 
+    def _dist(self, dist_dir):
+        out_dir = pathlib.Path("out")
+        dest_dir = pathlib.Path(dist_dir).joinpath("logs")
+        os.makedirs(dest_dir, exist_ok=True)
+        basenames = [
+            "build.trace.gz",
+            "soong.log",
+            "soong_build_metrics.pb",
+            "soong_metrics",
+        ]
+        for base in basenames:
+            src = out_dir.joinpath(base)
+            if src.exists():
+                sys.stderr.write(f"DIST: copied {src} to {dest_dir}\n")
+                shutil.copy(src, dest_dir)
+
     def _write_summary(self):
         # Write the results, even if the build failed or we crashed, including
         # whether we finished all of the benchmarks.
@@ -406,6 +460,9 @@
         parser.add_argument("--benchmark", nargs="*", default=[b.id for b in self._benchmarks],
                             metavar="BENCHMARKS",
                             help="Benchmarks to run.  Default suite will be run if omitted.")
+        parser.add_argument("--dist-one", type=str,
+                            help="Copy logs and metrics to the given dist dir. Requires that only"
+                                + " one benchmark be supplied. Postroll steps will be skipped.")
 
         self._args = parser.parse_args()
 
@@ -420,6 +477,10 @@
             for id in bad_ids:
                 self._error(f"Invalid benchmark: {id}")
 
+        # --dist-one requires that only one benchmark be supplied
+        if len(self.Benchmarks()) != 1:
+            self._error("--dist-one requires that exactly one --benchmark.")
+
         if self._had_error:
             raise FatalError()
 
@@ -501,31 +562,129 @@
     def Iterations(self):
         return self._args.iterations
 
+    def DistOne(self):
+        return self._args.dist_one
+
     def _init_benchmarks(self):
         """Initialize the list of benchmarks."""
         # Assumes that we've already chdired to the root of the tree.
         self._benchmarks = [
             Benchmark(id="full",
-                title="Full build",
-                change=Clean(),
-                modules=["droid"],
-                preroll=0,
-                postroll=3
-            ),
+                      title="Full build",
+                      change=Clean(),
+                      modules=["droid"],
+                      preroll=0,
+                      postroll=3,
+                      ),
             Benchmark(id="nochange",
-                title="No change",
-                change=NoChange(),
-                modules=["droid"],
-                preroll=2,
-                postroll=3
-            ),
+                      title="No change",
+                      change=NoChange(),
+                      modules=["droid"],
+                      preroll=2,
+                      postroll=3,
+                      ),
+            Benchmark(id="unreferenced",
+                      title="Create unreferenced file",
+                      change=Create("bionic/unreferenced.txt"),
+                      modules=["droid"],
+                      preroll=1,
+                      postroll=2,
+                      ),
             Benchmark(id="modify_bp",
-                title="Modify Android.bp",
-                change=Modify("bionic/libc/Android.bp", "// Comment"),
-                modules=["droid"],
-                preroll=1,
-                postroll=3
-            ),
+                      title="Modify Android.bp",
+                      change=Modify("bionic/libc/Android.bp", Comment("//")),
+                      modules=["droid"],
+                      preroll=1,
+                      postroll=3,
+                      ),
+            Benchmark(id="modify_stdio",
+                      title="Modify stdio.cpp",
+                      change=Modify("bionic/libc/stdio/stdio.cpp", Comment("//")),
+                      modules=["libc"],
+                      preroll=1,
+                      postroll=2,
+                      ),
+            Benchmark(id="modify_adbd",
+                      title="Modify adbd",
+                      change=Modify("packages/modules/adb/daemon/main.cpp", Comment("//")),
+                      modules=["adbd"],
+                      preroll=1,
+                      postroll=2,
+                      ),
+            Benchmark(id="services_private_field",
+                      title="Add private field to ActivityManagerService.java",
+                      change=AddJavaField("frameworks/base/services/core/java/com/android/server/am/ActivityManagerService.java",
+                                          "private"),
+                      modules=["services"],
+                      preroll=1,
+                      postroll=2,
+                      ),
+            Benchmark(id="services_public_field",
+                      title="Add public field to ActivityManagerService.java",
+                      change=AddJavaField("frameworks/base/services/core/java/com/android/server/am/ActivityManagerService.java",
+                                          "/** @hide */ public"),
+                      modules=["services"],
+                      preroll=1,
+                      postroll=2,
+                      ),
+            Benchmark(id="services_api",
+                      title="Add API to ActivityManagerService.javaa",
+                      change=AddJavaField("frameworks/base/services/core/java/com/android/server/am/ActivityManagerService.java",
+                                          "@android.annotation.SuppressLint(\"UnflaggedApi\") public"),
+                      modules=["services"],
+                      preroll=1,
+                      postroll=2,
+                      ),
+            Benchmark(id="framework_private_field",
+                      title="Add private field to Settings.java",
+                      change=AddJavaField("frameworks/base/core/java/android/provider/Settings.java",
+                                          "private"),
+                      modules=["framework-minus-apex"],
+                      preroll=1,
+                      postroll=2,
+                      ),
+            Benchmark(id="framework_public_field",
+                      title="Add public field to Settings.java",
+                      change=AddJavaField("frameworks/base/core/java/android/provider/Settings.java",
+                                          "/** @hide */ public"),
+                      modules=["framework-minus-apex"],
+                      preroll=1,
+                      postroll=2,
+                      ),
+            Benchmark(id="framework_api",
+                      title="Add API to Settings.java",
+                      change=AddJavaField("frameworks/base/core/java/android/provider/Settings.java",
+                                          "@android.annotation.SuppressLint(\"UnflaggedApi\") public"),
+                      modules=["framework-minus-apex"],
+                      preroll=1,
+                      postroll=2,
+                      ),
+            Benchmark(id="modify_framework_resource",
+                      title="Modify framework resource",
+                      change=Modify("frameworks/base/core/res/res/values/config.xml",
+                                    lambda: str(uuid.uuid4()),
+                                    before="</string>"),
+                      modules=["framework-minus-apex"],
+                      preroll=1,
+                      postroll=2,
+                      ),
+            Benchmark(id="add_framework_resource",
+                      title="Add framework resource",
+                      change=Modify("frameworks/base/core/res/res/values/config.xml",
+                                    lambda: f"<string name=\"BENCHMARK\">{uuid.uuid4()}</string>",
+                                    before="</resources>"),
+                      modules=["framework-minus-apex"],
+                      preroll=1,
+                      postroll=2,
+                      ),
+            Benchmark(id="add_systemui_field",
+                      title="Add SystemUI field",
+                      change=AddJavaField("frameworks/base/packages/SystemUI/src/com/android/systemui/wmshell/WMShell.java",
+                                    "public"),
+                      modules=["SystemUI"],
+                      preroll=1,
+                      postroll=2,
+                      ),
         ]
 
     def _error(self, message):
diff --git a/tools/releasetools/ota_from_target_files.py b/tools/releasetools/ota_from_target_files.py
index 4a5facd..b65764b 100755
--- a/tools/releasetools/ota_from_target_files.py
+++ b/tools/releasetools/ota_from_target_files.py
@@ -1038,7 +1038,11 @@
   partition_timestamps_flags = []
   # Enforce a max timestamp this payload can be applied on top of.
   if OPTIONS.downgrade:
-    max_timestamp = source_info.GetBuildProp("ro.build.date.utc")
+    # When generating ota between merged target-files, partition build date can
+    # decrease in target, at the same time as ro.build.date.utc increases,
+    # so always pick largest value.
+    max_timestamp = max(source_info.GetBuildProp("ro.build.date.utc"),
+        str(metadata.postcondition.timestamp))
     partition_timestamps_flags = GeneratePartitionTimestampFlagsDowngrade(
         metadata.precondition.partition_state,
         metadata.postcondition.partition_state
diff --git a/tools/releasetools/ota_utils.py b/tools/releasetools/ota_utils.py
index ddd2d36..048a497 100644
--- a/tools/releasetools/ota_utils.py
+++ b/tools/releasetools/ota_utils.py
@@ -364,26 +364,66 @@
   # Only incremental OTAs are allowed to reach here.
   assert OPTIONS.incremental_source is not None
 
+  # used for logging upon errors
+  log_downgrades = []
+  log_upgrades = []
+
   post_timestamp = target_info.GetBuildProp("ro.build.date.utc")
   pre_timestamp = source_info.GetBuildProp("ro.build.date.utc")
-  is_downgrade = int(post_timestamp) < int(pre_timestamp)
+  if int(post_timestamp) < int(pre_timestamp):
+    logger.info(f"ro.build.date.utc pre timestamp: {pre_timestamp}, "
+                f"post timestamp: {post_timestamp}. Downgrade detected.")
+    log_downgrades.append(f"ro.build.date.utc pre: {pre_timestamp} post: {post_timestamp}")
+  else:
+    logger.info(f"ro.build.date.utc pre timestamp: {pre_timestamp}, "
+                f"post timestamp: {post_timestamp}.")
+    log_upgrades.append(f"ro.build.date.utc pre: {pre_timestamp} post: {post_timestamp}")
+
+  # When merging system and vendor target files, it is not enough
+  # to check ro.build.date.utc, the timestamp for each partition must
+  # be checked.
+  if source_info.is_ab:
+    ab_partitions = set(source_info.get("ab_partitions"))
+    for partition in sorted(set(PARTITIONS_WITH_BUILD_PROP) & ab_partitions):
+
+      partition_prop = source_info.get('{}.build.prop'.format(partition))
+      # Skip if the partition is missing, or it doesn't have a build.prop
+      if not partition_prop or not partition_prop.build_props:
+        continue
+      partition_prop = target_info.get('{}.build.prop'.format(partition))
+      # Skip if the partition is missing, or it doesn't have a build.prop
+      if not partition_prop or not partition_prop.build_props:
+        continue
+
+      post_timestamp = target_info.GetPartitionBuildProp(
+        'ro.build.date.utc', partition)
+      pre_timestamp = source_info.GetPartitionBuildProp(
+        'ro.build.date.utc', partition)
+      if int(post_timestamp) < int(pre_timestamp):
+        logger.info(f"Partition {partition} pre timestamp: {pre_timestamp}, "
+                    f"post time: {post_timestamp}. Downgrade detected.")
+        log_downgrades.append(f"{partition} pre: {pre_timestamp} post: {post_timestamp}")
+      else:
+        logger.info(f"Partition {partition} pre timestamp: {pre_timestamp}, "
+                    f"post timestamp: {post_timestamp}.")
+        log_upgrades.append(f"{partition} pre: {pre_timestamp} post: {post_timestamp}")
 
   if OPTIONS.spl_downgrade:
     metadata_proto.spl_downgrade = True
 
   if OPTIONS.downgrade:
-    if not is_downgrade:
+    if len(log_downgrades) == 0:
       raise RuntimeError(
           "--downgrade or --override_timestamp specified but no downgrade "
-          "detected: pre: %s, post: %s" % (pre_timestamp, post_timestamp))
+          "detected. Current values for ro.build.date.utc: " + ', '.join(log_upgrades))
     metadata_proto.downgrade = True
   else:
-    if is_downgrade:
+    if len(log_downgrades) != 0:
       raise RuntimeError(
-          "Downgrade detected based on timestamp check: pre: %s, post: %s. "
+          "Downgrade detected based on timestamp check in ro.build.date.utc. "
           "Need to specify --override_timestamp OR --downgrade to allow "
-          "building the incremental." % (pre_timestamp, post_timestamp))
-
+          "building the incremental. Downgrades detected for: "
+          + ', '.join(log_downgrades))
 
 def ComputeRuntimeBuildInfos(default_build_info, boot_variable_values):
   """Returns a set of build info objects that may exist during runtime."""
diff --git a/tools/releasetools/test_ota_from_target_files.py b/tools/releasetools/test_ota_from_target_files.py
index ad0f7a8..d1e76b9 100644
--- a/tools/releasetools/test_ota_from_target_files.py
+++ b/tools/releasetools/test_ota_from_target_files.py
@@ -163,6 +163,20 @@
       'oem_fingerprint_properties': 'ro.product.device ro.product.brand',
   }
 
+  TEST_TARGET_VENDOR_INFO_DICT = common.PartitionBuildProps.FromDictionary(
+    'vendor', {
+      'ro.vendor.build.date.utc' : '87654321',
+      'ro.product.vendor.device':'vendor-device',
+      'ro.vendor.build.fingerprint': 'build-fingerprint-vendor'}
+  )
+
+  TEST_SOURCE_VENDOR_INFO_DICT = common.PartitionBuildProps.FromDictionary(
+    'vendor', {
+      'ro.vendor.build.date.utc' : '12345678',
+      'ro.product.vendor.device':'vendor-device',
+      'ro.vendor.build.fingerprint': 'build-fingerprint-vendor'}
+  )
+
   def setUp(self):
     self.testdata_dir = test_utils.get_testdata_dir()
     self.assertTrue(os.path.exists(self.testdata_dir))
@@ -351,6 +365,13 @@
          source_info['build.prop'].build_props['ro.build.date.utc'],
          target_info['build.prop'].build_props['ro.build.date.utc'])
 
+  @staticmethod
+  def _test_GetPackageMetadata_swapVendorBuildTimestamps(target_info, source_info):
+    (target_info['vendor.build.prop'].build_props['ro.vendor.build.date.utc'],
+     source_info['vendor.build.prop'].build_props['ro.vendor.build.date.utc']) = (
+         source_info['vendor.build.prop'].build_props['ro.vendor.build.date.utc'],
+         target_info['vendor.build.prop'].build_props['ro.vendor.build.date.utc'])
+
   def test_GetPackageMetadata_unintentionalDowngradeDetected(self):
     target_info_dict = copy.deepcopy(self.TEST_TARGET_INFO_DICT)
     source_info_dict = copy.deepcopy(self.TEST_SOURCE_INFO_DICT)
@@ -363,6 +384,24 @@
     self.assertRaises(RuntimeError, self.GetLegacyOtaMetadata, target_info,
                       source_info)
 
+  def test_GetPackageMetadata_unintentionalVendorDowngradeDetected(self):
+    target_info_dict = copy.deepcopy(self.TEST_TARGET_INFO_DICT)
+    target_info_dict['ab_update'] = 'true'
+    target_info_dict['ab_partitions'] = ['vendor']
+    target_info_dict["vendor.build.prop"] = copy.deepcopy(self.TEST_TARGET_VENDOR_INFO_DICT)
+    source_info_dict = copy.deepcopy(self.TEST_SOURCE_INFO_DICT)
+    source_info_dict['ab_update'] = 'true'
+    source_info_dict['ab_partitions'] = ['vendor']
+    source_info_dict["vendor.build.prop"] = copy.deepcopy(self.TEST_SOURCE_VENDOR_INFO_DICT)
+    self._test_GetPackageMetadata_swapVendorBuildTimestamps(
+        target_info_dict, source_info_dict)
+
+    target_info = common.BuildInfo(target_info_dict, None)
+    source_info = common.BuildInfo(source_info_dict, None)
+    common.OPTIONS.incremental_source = ''
+    self.assertRaises(RuntimeError, self.GetLegacyOtaMetadata, target_info,
+                      source_info)
+
   def test_GetPackageMetadata_downgrade(self):
     target_info_dict = copy.deepcopy(self.TEST_TARGET_INFO_DICT)
     source_info_dict = copy.deepcopy(self.TEST_SOURCE_INFO_DICT)
@@ -397,6 +436,55 @@
         },
         metadata)
 
+  def test_GetPackageMetadata_vendorDowngrade(self):
+    target_info_dict = copy.deepcopy(self.TEST_TARGET_INFO_DICT)
+    target_info_dict['ab_update'] = 'true'
+    target_info_dict['ab_partitions'] = ['vendor']
+    target_info_dict["vendor.build.prop"] = copy.deepcopy(self.TEST_TARGET_VENDOR_INFO_DICT)
+    source_info_dict = copy.deepcopy(self.TEST_SOURCE_INFO_DICT)
+    source_info_dict['ab_update'] = 'true'
+    source_info_dict['ab_partitions'] = ['vendor']
+    source_info_dict["vendor.build.prop"] = copy.deepcopy(self.TEST_SOURCE_VENDOR_INFO_DICT)
+    self._test_GetPackageMetadata_swapVendorBuildTimestamps(
+        target_info_dict, source_info_dict)
+
+    target_info = common.BuildInfo(target_info_dict, None)
+    source_info = common.BuildInfo(source_info_dict, None)
+    common.OPTIONS.incremental_source = ''
+    common.OPTIONS.downgrade = True
+    common.OPTIONS.wipe_user_data = True
+    common.OPTIONS.spl_downgrade = True
+    metadata = self.GetLegacyOtaMetadata(target_info, source_info)
+    # Reset spl_downgrade so other tests are unaffected
+    common.OPTIONS.spl_downgrade = False
+
+    self.assertDictEqual(
+        {
+            'ota-downgrade': 'yes',
+            'ota-type': 'AB',
+            'ota-required-cache': '0',
+            'ota-wipe': 'yes',
+            'post-build': 'build-fingerprint-target',
+            'post-build-incremental': 'build-version-incremental-target',
+            'post-sdk-level': '27',
+            'post-security-patch-level': '2017-12-01',
+            'post-timestamp': '1500000000',
+            'pre-device': 'product-device',
+            'pre-build': 'build-fingerprint-source',
+            'pre-build-incremental': 'build-version-incremental-source',
+            'spl-downgrade': 'yes',
+        },
+        metadata)
+
+    post_build = GetPackageMetadata(target_info, source_info).postcondition
+    self.assertEqual('vendor', post_build.partition_state[0].partition_name)
+    self.assertEqual('12345678', post_build.partition_state[0].version)
+
+    pre_build = GetPackageMetadata(target_info, source_info).precondition
+    self.assertEqual('vendor', pre_build.partition_state[0].partition_name)
+    self.assertEqual('87654321', pre_build.partition_state[0].version)
+
+
   @test_utils.SkipIfExternalToolsUnavailable()
   def test_GetTargetFilesZipForSecondaryImages(self):
     input_file = construct_target_files(secondary=True)