Merge "Fix crash when failing to parse test discovery output" into main
diff --git a/core/packaging/flags.mk b/core/packaging/flags.mk
index 06e5001..fd9dc9b 100644
--- a/core/packaging/flags.mk
+++ b/core/packaging/flags.mk
@@ -36,7 +36,8 @@
mkdir -p $$(dir $$(PRIVATE_OUT))
$$(if $$(PRIVATE_IN), \
$$(ACONFIG) dump --dedup --format protobuf --out $$(PRIVATE_OUT) \
- --filter container:$(strip $(3)) \
+ --filter container:$(strip $(3))+state:ENABLED \
+ --filter container:$(strip $(3))+permission:READ_WRITE \
$$(addprefix --cache ,$$(PRIVATE_IN)), \
echo -n > $$(PRIVATE_OUT) \
)
diff --git a/core/soong_app_prebuilt.mk b/core/soong_app_prebuilt.mk
index df1cf2d..ab9227f 100644
--- a/core/soong_app_prebuilt.mk
+++ b/core/soong_app_prebuilt.mk
@@ -224,30 +224,6 @@
include $(BUILD_SYSTEM)/link_type.mk
endif # !LOCAL_IS_HOST_MODULE
-ifeq (,$(filter tests,$(LOCAL_MODULE_TAGS)))
- ifdef LOCAL_SOONG_DEVICE_RRO_DIRS
- $(call append_enforce_rro_sources, \
- $(my_register_name), \
- false, \
- $(LOCAL_FULL_MANIFEST_FILE), \
- $(if $(LOCAL_EXPORT_PACKAGE_RESOURCES),true,false), \
- $(LOCAL_SOONG_DEVICE_RRO_DIRS), \
- vendor \
- )
- endif
-
- ifdef LOCAL_SOONG_PRODUCT_RRO_DIRS
- $(call append_enforce_rro_sources, \
- $(my_register_name), \
- false, \
- $(LOCAL_FULL_MANIFEST_FILE), \
- $(if $(LOCAL_EXPORT_PACKAGE_RESOURCES),true,false), \
- $(LOCAL_SOONG_PRODUCT_RRO_DIRS), \
- product \
- )
- endif
-endif
-
ifdef LOCAL_PREBUILT_COVERAGE_ARCHIVE
my_coverage_dir := $(TARGET_OUT_COVERAGE)/$(patsubst $(PRODUCT_OUT)/%,%,$(my_module_path))
my_coverage_copy_pairs := $(foreach f,$(LOCAL_PREBUILT_COVERAGE_ARCHIVE),$(f):$(my_coverage_dir)/$(notdir $(f)))
diff --git a/tools/aconfig/aconfig/src/codegen/cpp.rs b/tools/aconfig/aconfig/src/codegen/cpp.rs
index 7a9c382..ae18679 100644
--- a/tools/aconfig/aconfig/src/codegen/cpp.rs
+++ b/tools/aconfig/aconfig/src/codegen/cpp.rs
@@ -127,6 +127,26 @@
flag_ids: HashMap<String, u16>,
rw_count: &mut i32,
) -> ClassElement {
+ let no_assigned_offset =
+ (pf.container() == "system" || pf.container() == "vendor" || pf.container() == "product")
+ && pf.permission() == ProtoFlagPermission::READ_ONLY
+ && pf.state() == ProtoFlagState::DISABLED;
+
+ let flag_offset = match flag_ids.get(pf.name()) {
+ Some(offset) => offset,
+ None => {
+ // System/vendor/product RO+disabled flags have no offset in storage files.
+ // Assign placeholder value.
+ if no_assigned_offset {
+ &0
+ }
+ // All other flags _must_ have an offset.
+ else {
+ panic!("{}", format!("missing flag offset for {}", pf.name()));
+ }
+ }
+ };
+
ClassElement {
readwrite_idx: if pf.permission() == ProtoFlagPermission::READ_WRITE {
let index = *rw_count;
@@ -144,7 +164,7 @@
},
flag_name: pf.name().to_string(),
flag_macro: pf.name().to_uppercase(),
- flag_offset: *flag_ids.get(pf.name()).expect("values checked at flag parse time"),
+ flag_offset: *flag_offset,
device_config_namespace: pf.namespace().to_string(),
device_config_flag: codegen::create_device_config_ident(package, pf.name())
.expect("values checked at flag parse time"),
diff --git a/tools/aconfig/aconfig/src/codegen/java.rs b/tools/aconfig/aconfig/src/codegen/java.rs
index 2f2889c..81cbcf4 100644
--- a/tools/aconfig/aconfig/src/codegen/java.rs
+++ b/tools/aconfig/aconfig/src/codegen/java.rs
@@ -158,6 +158,27 @@
) -> FlagElement {
let device_config_flag = codegen::create_device_config_ident(package, pf.name())
.expect("values checked at flag parse time");
+
+ let no_assigned_offset =
+ (pf.container() == "system" || pf.container() == "vendor" || pf.container() == "product")
+ && pf.permission() == ProtoFlagPermission::READ_ONLY
+ && pf.state() == ProtoFlagState::DISABLED;
+
+ let flag_offset = match flag_offsets.get(pf.name()) {
+ Some(offset) => offset,
+ None => {
+ // System/vendor/product RO+disabled flags have no offset in storage files.
+ // Assign placeholder value.
+ if no_assigned_offset {
+ &0
+ }
+ // All other flags _must_ have an offset.
+ else {
+ panic!("{}", format!("missing flag offset for {}", pf.name()));
+ }
+ }
+ };
+
FlagElement {
container: pf.container().to_string(),
default_value: pf.state() == ProtoFlagState::ENABLED,
@@ -165,7 +186,7 @@
device_config_flag,
flag_name: pf.name().to_string(),
flag_name_constant_suffix: pf.name().to_ascii_uppercase(),
- flag_offset: *flag_offsets.get(pf.name()).expect("didnt find package offset :("),
+ flag_offset: *flag_offset,
is_read_write: pf.permission() == ProtoFlagPermission::READ_WRITE,
method_name: format_java_method_name(pf.name()),
properties: format_property_name(pf.namespace()),
@@ -537,10 +558,10 @@
PlatformAconfigPackageInternal reader = PlatformAconfigPackageInternal.load("system", "com.android.aconfig.test", 0x5081CE7221C77064L);
AconfigStorageReadException error = reader.getException();
if (error == null) {
- disabledRw = reader.getBooleanFlagValue(1);
- disabledRwExported = reader.getBooleanFlagValue(2);
- enabledRw = reader.getBooleanFlagValue(8);
- disabledRwInOtherNamespace = reader.getBooleanFlagValue(3);
+ disabledRw = reader.getBooleanFlagValue(0);
+ disabledRwExported = reader.getBooleanFlagValue(1);
+ enabledRw = reader.getBooleanFlagValue(7);
+ disabledRwInOtherNamespace = reader.getBooleanFlagValue(2);
} else if (Build.VERSION.SDK_INT > 35 && error.getErrorCode() == 5 /* fingerprint doesn't match*/) {
disabledRw = reader.getBooleanFlagValue("disabled_rw", false);
disabledRwExported = reader.getBooleanFlagValue("disabled_rw_exported", false);
diff --git a/tools/aconfig/aconfig/src/codegen/rust.rs b/tools/aconfig/aconfig/src/codegen/rust.rs
index 82a6ebc..2bf565a 100644
--- a/tools/aconfig/aconfig/src/codegen/rust.rs
+++ b/tools/aconfig/aconfig/src/codegen/rust.rs
@@ -88,6 +88,27 @@
impl TemplateParsedFlag {
#[allow(clippy::nonminimal_bool)]
fn new(package: &str, flag_offsets: HashMap<String, u16>, pf: &ProtoParsedFlag) -> Self {
+ let no_assigned_offset = (pf.container() == "system"
+ || pf.container() == "vendor"
+ || pf.container() == "product")
+ && pf.permission() == ProtoFlagPermission::READ_ONLY
+ && pf.state() == ProtoFlagState::DISABLED;
+
+ let flag_offset = match flag_offsets.get(pf.name()) {
+ Some(offset) => offset,
+ None => {
+ // System/vendor/product RO+disabled flags have no offset in storage files.
+ // Assign placeholder value.
+ if no_assigned_offset {
+ &0
+ }
+ // All other flags _must_ have an offset.
+ else {
+ panic!("{}", format!("missing flag offset for {}", pf.name()));
+ }
+ }
+ };
+
Self {
readwrite: pf.permission() == ProtoFlagPermission::READ_WRITE,
default_value: match pf.state() {
@@ -96,7 +117,7 @@
},
name: pf.name().to_string(),
container: pf.container().to_string(),
- flag_offset: *flag_offsets.get(pf.name()).expect("didnt find package offset :("),
+ flag_offset: *flag_offset,
device_config_namespace: pf.namespace().to_string(),
device_config_flag: codegen::create_device_config_ident(package, pf.name())
.expect("values checked at flag parse time"),
@@ -287,7 +308,7 @@
.and_then(|package_offset| {
match package_offset {
Some(offset) => {
- get_boolean_flag_value(&flag_val_map, offset + 1)
+ get_boolean_flag_value(&flag_val_map, offset + 0)
.map_err(|err| format!("failed to get flag: {err}"))
},
None => {
@@ -327,7 +348,7 @@
.and_then(|package_offset| {
match package_offset {
Some(offset) => {
- get_boolean_flag_value(&flag_val_map, offset + 2)
+ get_boolean_flag_value(&flag_val_map, offset + 1)
.map_err(|err| format!("failed to get flag: {err}"))
},
None => {
@@ -367,7 +388,7 @@
.and_then(|package_offset| {
match package_offset {
Some(offset) => {
- get_boolean_flag_value(&flag_val_map, offset + 3)
+ get_boolean_flag_value(&flag_val_map, offset + 2)
.map_err(|err| format!("failed to get flag: {err}"))
},
None => {
@@ -408,7 +429,7 @@
.and_then(|package_offset| {
match package_offset {
Some(offset) => {
- get_boolean_flag_value(&flag_val_map, offset + 8)
+ get_boolean_flag_value(&flag_val_map, offset + 7)
.map_err(|err| format!("failed to get flag: {err}"))
},
None => {
diff --git a/tools/aconfig/aconfig/src/commands.rs b/tools/aconfig/aconfig/src/commands.rs
index c1989a2..3f869de 100644
--- a/tools/aconfig/aconfig/src/commands.rs
+++ b/tools/aconfig/aconfig/src/commands.rs
@@ -423,17 +423,28 @@
{
assert!(parsed_flags_iter.clone().tuple_windows().all(|(a, b)| a.name() <= b.name()));
let mut flag_ids = HashMap::new();
- for (id_to_assign, pf) in (0_u32..).zip(parsed_flags_iter) {
+ let mut flag_idx = 0;
+ for pf in parsed_flags_iter {
if package != pf.package() {
return Err(anyhow::anyhow!("encountered a flag not in current package"));
}
// put a cap on how many flags a package can contain to 65535
- if id_to_assign > u16::MAX as u32 {
+ if flag_idx > u16::MAX as u32 {
return Err(anyhow::anyhow!("the number of flags in a package cannot exceed 65535"));
}
- flag_ids.insert(pf.name().to_string(), id_to_assign as u16);
+ // Exclude system/vendor/product flags that are RO+disabled.
+ let should_filter_container = pf.container == Some("vendor".to_string())
+ || pf.container == Some("system".to_string())
+ || pf.container == Some("vendor".to_string());
+ if !(should_filter_container
+ && pf.state == Some(ProtoFlagState::DISABLED.into())
+ && pf.permission == Some(ProtoFlagPermission::READ_ONLY.into()))
+ {
+ flag_ids.insert(pf.name().to_string(), flag_idx as u16);
+ flag_idx += 1;
+ }
}
Ok(flag_ids)
}
@@ -891,6 +902,30 @@
}
#[test]
+ fn test_dump_multiple_filters() {
+ let input = parse_test_flags_as_input();
+ let bytes = dump_parsed_flags(
+ vec![input],
+ DumpFormat::Custom("{fully_qualified_name}".to_string()),
+ &["container:system+state:ENABLED", "container:system+permission:READ_WRITE"],
+ false,
+ )
+ .unwrap();
+ let text = std::str::from_utf8(&bytes).unwrap();
+ let expected_flag_list = &[
+ "com.android.aconfig.test.disabled_rw",
+ "com.android.aconfig.test.disabled_rw_exported",
+ "com.android.aconfig.test.disabled_rw_in_other_namespace",
+ "com.android.aconfig.test.enabled_fixed_ro",
+ "com.android.aconfig.test.enabled_fixed_ro_exported",
+ "com.android.aconfig.test.enabled_ro",
+ "com.android.aconfig.test.enabled_ro_exported",
+ "com.android.aconfig.test.enabled_rw",
+ ];
+ assert_eq!(expected_flag_list.map(|s| format!("{}\n", s)).join(""), text);
+ }
+
+ #[test]
fn test_dump_textproto_format_dedup() {
let input = parse_test_flags_as_input();
let input2 = parse_test_flags_as_input();
@@ -952,15 +987,14 @@
let package = find_unique_package(&parsed_flags.parsed_flag).unwrap().to_string();
let flag_ids = assign_flag_ids(&package, parsed_flags.parsed_flag.iter()).unwrap();
let expected_flag_ids = HashMap::from([
- (String::from("disabled_ro"), 0_u16),
- (String::from("disabled_rw"), 1_u16),
- (String::from("disabled_rw_exported"), 2_u16),
- (String::from("disabled_rw_in_other_namespace"), 3_u16),
- (String::from("enabled_fixed_ro"), 4_u16),
- (String::from("enabled_fixed_ro_exported"), 5_u16),
- (String::from("enabled_ro"), 6_u16),
- (String::from("enabled_ro_exported"), 7_u16),
- (String::from("enabled_rw"), 8_u16),
+ (String::from("disabled_rw"), 0_u16),
+ (String::from("disabled_rw_exported"), 1_u16),
+ (String::from("disabled_rw_in_other_namespace"), 2_u16),
+ (String::from("enabled_fixed_ro"), 3_u16),
+ (String::from("enabled_fixed_ro_exported"), 4_u16),
+ (String::from("enabled_ro"), 5_u16),
+ (String::from("enabled_ro_exported"), 6_u16),
+ (String::from("enabled_rw"), 7_u16),
]);
assert_eq!(flag_ids, expected_flag_ids);
}
diff --git a/tools/aconfig/aconfig/src/storage/flag_info.rs b/tools/aconfig/aconfig/src/storage/flag_info.rs
index 0b5a67b..0943daa 100644
--- a/tools/aconfig/aconfig/src/storage/flag_info.rs
+++ b/tools/aconfig/aconfig/src/storage/flag_info.rs
@@ -16,7 +16,7 @@
use crate::commands::assign_flag_ids;
use crate::storage::FlagPackage;
-use aconfig_protos::ProtoFlagPermission;
+use aconfig_protos::{ProtoFlagPermission, ProtoFlagState};
use aconfig_storage_file::{FlagInfoHeader, FlagInfoList, FlagInfoNode, StorageFileType};
use anyhow::{anyhow, Result};
@@ -36,14 +36,24 @@
packages: &[FlagPackage],
version: u32,
) -> Result<FlagInfoList> {
- // create list
- let num_flags = packages.iter().map(|pkg| pkg.boolean_flags.len() as u32).sum();
+ // Exclude system/vendor/product flags that are RO+disabled.
+ let mut filtered_packages = packages.to_vec();
+ if container == "system" || container == "vendor" || container == "product" {
+ for package in filtered_packages.iter_mut() {
+ package.boolean_flags.retain(|b| {
+ !(b.state == Some(ProtoFlagState::DISABLED.into())
+ && b.permission == Some(ProtoFlagPermission::READ_ONLY.into()))
+ });
+ }
+ }
+
+ let num_flags = filtered_packages.iter().map(|pkg| pkg.boolean_flags.len() as u32).sum();
let mut is_flag_rw = vec![false; num_flags as usize];
- for pkg in packages.iter() {
+ for pkg in filtered_packages {
let start_index = pkg.boolean_start_index as usize;
let flag_ids = assign_flag_ids(pkg.package_name, pkg.boolean_flags.iter().copied())?;
- for pf in pkg.boolean_flags.iter() {
+ for pf in pkg.boolean_flags {
let fid = flag_ids
.get(pf.name())
.ok_or(anyhow!(format!("missing flag id for {}", pf.name())))?;
diff --git a/tools/aconfig/aconfig/src/storage/flag_table.rs b/tools/aconfig/aconfig/src/storage/flag_table.rs
index ae5a16c..3b245a7 100644
--- a/tools/aconfig/aconfig/src/storage/flag_table.rs
+++ b/tools/aconfig/aconfig/src/storage/flag_table.rs
@@ -16,7 +16,7 @@
use crate::commands::assign_flag_ids;
use crate::storage::FlagPackage;
-use aconfig_protos::ProtoFlagPermission;
+use aconfig_protos::{ProtoFlagPermission, ProtoFlagState};
use aconfig_storage_file::{
get_table_size, FlagTable, FlagTableHeader, FlagTableNode, StorageFileType, StoredFlagType,
};
@@ -62,9 +62,19 @@
}
fn create_nodes(package: &FlagPackage, num_buckets: u32) -> Result<Vec<Self>> {
+ // Exclude system/vendor/product flags that are RO+disabled.
+ let mut filtered_package = package.clone();
+ filtered_package.boolean_flags.retain(|f| {
+ !((f.container == Some("system".to_string())
+ || f.container == Some("vendor".to_string())
+ || f.container == Some("product".to_string()))
+ && f.permission == Some(ProtoFlagPermission::READ_ONLY.into())
+ && f.state == Some(ProtoFlagState::DISABLED.into()))
+ });
+
let flag_ids =
- assign_flag_ids(package.package_name, package.boolean_flags.iter().copied())?;
- package
+ assign_flag_ids(package.package_name, filtered_package.boolean_flags.iter().copied())?;
+ filtered_package
.boolean_flags
.iter()
.map(|&pf| {
diff --git a/tools/aconfig/aconfig/src/storage/flag_value.rs b/tools/aconfig/aconfig/src/storage/flag_value.rs
index 065b7e3..3cfa447 100644
--- a/tools/aconfig/aconfig/src/storage/flag_value.rs
+++ b/tools/aconfig/aconfig/src/storage/flag_value.rs
@@ -16,7 +16,7 @@
use crate::commands::assign_flag_ids;
use crate::storage::FlagPackage;
-use aconfig_protos::ProtoFlagState;
+use aconfig_protos::{ProtoFlagPermission, ProtoFlagState};
use aconfig_storage_file::{FlagValueHeader, FlagValueList, StorageFileType};
use anyhow::{anyhow, Result};
@@ -36,15 +36,22 @@
packages: &[FlagPackage],
version: u32,
) -> Result<FlagValueList> {
- // create list
- let num_flags = packages.iter().map(|pkg| pkg.boolean_flags.len() as u32).sum();
-
+ // Exclude system/vendor/product flags that are RO+disabled.
+ let mut filtered_packages = packages.to_vec();
+ if container == "system" || container == "vendor" || container == "product" {
+ for package in filtered_packages.iter_mut() {
+ package.boolean_flags.retain(|b| {
+ !(b.state == Some(ProtoFlagState::DISABLED.into())
+ && b.permission == Some(ProtoFlagPermission::READ_ONLY.into()))
+ });
+ }
+ }
+ let num_flags = filtered_packages.iter().map(|pkg| pkg.boolean_flags.len() as u32).sum();
let mut list = FlagValueList {
header: new_header(container, num_flags, version),
booleans: vec![false; num_flags as usize],
};
-
- for pkg in packages.iter() {
+ for pkg in filtered_packages {
let start_index = pkg.boolean_start_index as usize;
let flag_ids = assign_flag_ids(pkg.package_name, pkg.boolean_flags.iter().copied())?;
for pf in pkg.boolean_flags.iter() {
diff --git a/tools/aconfig/aconfig/src/storage/mod.rs b/tools/aconfig/aconfig/src/storage/mod.rs
index 4bc72f7..61e65d1 100644
--- a/tools/aconfig/aconfig/src/storage/mod.rs
+++ b/tools/aconfig/aconfig/src/storage/mod.rs
@@ -27,9 +27,10 @@
flag_info::create_flag_info, flag_table::create_flag_table, flag_value::create_flag_value,
package_table::create_package_table,
};
-use aconfig_protos::{ProtoParsedFlag, ProtoParsedFlags};
+use aconfig_protos::{ProtoFlagPermission, ProtoFlagState, ProtoParsedFlag, ProtoParsedFlags};
use aconfig_storage_file::StorageFileType;
+#[derive(Clone)]
pub struct FlagPackage<'a> {
pub package_name: &'a str,
pub package_id: u32,
@@ -73,6 +74,17 @@
if index == packages.len() {
packages.push(FlagPackage::new(parsed_flag.package(), index as u32));
}
+
+ // Exclude system/vendor/product flags that are RO+disabled.
+ if (parsed_flag.container == Some("system".to_string())
+ || parsed_flag.container == Some("vendor".to_string())
+ || parsed_flag.container == Some("product".to_string()))
+ && parsed_flag.permission == Some(ProtoFlagPermission::READ_ONLY.into())
+ && parsed_flag.state == Some(ProtoFlagState::DISABLED.into())
+ {
+ continue;
+ }
+
packages[index].insert(parsed_flag);
}
}