Merge "Filter read-only + disabled flags from aconfig_flags.pb" into main am: 59de3af2d6 (63e08329) · Commits · e / os / android_build

core/packaging/flags.mk

+2 −1

Original line number	Diff line number	Diff line
		@@ -36,7 +36,8 @@ $(strip $(1)): $(ACONFIG) $(strip $(4))
		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) \
		)

tools/aconfig/aconfig/src/codegen/cpp.rs

+21 −1

Original line number	Diff line number	Diff line
		@@ -127,6 +127,26 @@ fn create_class_element(
		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 @@ fn create_class_element(
		},
		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"),

tools/aconfig/aconfig/src/codegen/java.rs

+26 −5

Original line number	Diff line number	Diff line
		@@ -158,6 +158,27 @@ fn create_flag_element(
		) -> 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 @@ fn create_flag_element(
		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 @@ mod tests {
		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);

tools/aconfig/aconfig/src/codegen/rust.rs

+26 −5

Original line number	Diff line number	Diff line
		@@ -88,6 +88,27 @@ struct TemplateParsedFlag {
		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 @@ impl TemplateParsedFlag {
		},
		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 @@ static CACHED_disabled_rw: LazyLock<bool> = LazyLock::new(\|\| {
		.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 @@ static CACHED_disabled_rw_exported: LazyLock<bool> = LazyLock::new(\|\| {
		.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 @@ static CACHED_disabled_rw_in_other_namespace: LazyLock<bool> = LazyLock::new(\|\|
		.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 @@ static CACHED_enabled_rw: LazyLock<bool> = LazyLock::new(\|\| {
		.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 => {

tools/aconfig/aconfig/src/commands.rs

+46 −12

Original line number	Diff line number	Diff line
		@@ -423,17 +423,28 @@ where
		{
		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)
		}
		@@ -890,6 +901,30 @@ mod tests {
		assert!(text.contains("com.android.aconfig.test.disabled_ro"));
		}

		#[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();
		@@ -952,15 +987,14 @@ mod tests {
		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);
		}