Merge "Remove RO disabled flags before calc. fingerprint." into main (826dbcba) · Commits · e / os / android_build

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

+2 −5

Original line number	Diff line number	Diff line
		@@ -24,7 +24,7 @@ use aconfig_protos::{ProtoFlagPermission, ProtoFlagState, ProtoParsedFlag};

		use crate::codegen;
		use crate::codegen::CodegenMode;
		use crate::commands::OutputFile;
		use crate::commands::{should_include_flag, OutputFile};

		pub fn generate_cpp_code<I>(
		package: &str,
		@@ -127,10 +127,7 @@ 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 no_assigned_offset = !should_include_flag(pf);

		let flag_offset = match flag_ids.get(pf.name()) {
		Some(offset) => offset,

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

+2 −5

Original line number	Diff line number	Diff line
		@@ -22,7 +22,7 @@ use tinytemplate::TinyTemplate;

		use crate::codegen;
		use crate::codegen::CodegenMode;
		use crate::commands::OutputFile;
		use crate::commands::{should_include_flag, OutputFile};
		use aconfig_protos::{ProtoFlagPermission, ProtoFlagState, ProtoParsedFlag};
		use std::collections::HashMap;

		@@ -162,10 +162,7 @@ fn create_flag_element(
		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 no_assigned_offset = !should_include_flag(pf);

		let flag_offset = match flag_offsets.get(pf.name()) {
		Some(offset) => offset,

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

+2 −8

Original line number	Diff line number	Diff line
		@@ -24,7 +24,7 @@ use std::collections::HashMap;

		use crate::codegen;
		use crate::codegen::CodegenMode;
		use crate::commands::OutputFile;
		use crate::commands::{should_include_flag, OutputFile};

		pub fn generate_rust_code<I>(
		package: &str,
		@@ -88,18 +88,12 @@ 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 {
		if !should_include_flag(pf) {
		&0
		}
		// All other flags _must_ have an offset.

tools/aconfig/aconfig/src/commands.rs

+28 −22

Original line number	Diff line number	Diff line
		@@ -221,13 +221,13 @@ pub fn create_java_lib(
		new_exported: bool,
		) -> Result<Vec<OutputFile>> {
		let parsed_flags = input.try_parse_flags()?;
		let modified_parsed_flags = modify_parsed_flags_based_on_mode(parsed_flags, codegen_mode)?;
		let modified_parsed_flags =
		modify_parsed_flags_based_on_mode(parsed_flags.clone(), codegen_mode)?;
		let Some(package) = find_unique_package(&modified_parsed_flags) else {
		bail!("no parsed flags, or the parsed flags use different packages");
		};
		let package = package.to_string();
		let mut flag_names =
		modified_parsed_flags.iter().map(\|pf\| pf.name().to_string()).collect::<Vec<_>>();
		let mut flag_names = extract_flag_names(parsed_flags)?;
		let package_fingerprint = compute_flags_fingerprint(&mut flag_names);
		let flag_ids = assign_flag_ids(&package, modified_parsed_flags.iter())?;
		generate_java_code(
		@@ -436,14 +436,7 @@ where
		return Err(anyhow::anyhow!("the number of flags in a package cannot exceed 65535"));
		}

		// 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("product".to_string());
		if !(should_filter_container
		&& pf.state == Some(ProtoFlagState::DISABLED.into())
		&& pf.permission == Some(ProtoFlagPermission::READ_ONLY.into()))
		{
		if should_include_flag(pf) {
		flag_ids.insert(pf.name().to_string(), flag_idx as u16);
		flag_idx += 1;
		}
		@@ -451,8 +444,6 @@ where
		Ok(flag_ids)
		}

		#[allow(dead_code)] // TODO: b/316357686 - Use fingerprint in codegen to
		// protect hardcoded offset reads.
		// Creates a fingerprint of the flag names (which requires sorting the vector).
		// Fingerprint is used by both codegen and storage files.
		pub fn compute_flags_fingerprint(flag_names: &mut Vec<String>) -> u64 {
		@@ -465,8 +456,6 @@ pub fn compute_flags_fingerprint(flag_names: &mut Vec<String>) -> u64 {
		hasher.finish()
		}

		#[allow(dead_code)] // TODO: b/316357686 - Use fingerprint in codegen to
		// protect hardcoded offset reads.
		// Converts ProtoParsedFlags into a vector of strings containing all of the flag
		// names. Helper fn for creating fingerprint for codegen files. Flags must all
		// belong to the same package.
		@@ -478,7 +467,23 @@ fn extract_flag_names(flags: ProtoParsedFlags) -> Result<Vec<String>> {
		bail!("No parsed flags, or the parsed flags use different packages.");
		};

		Ok(separated_flags.into_iter().map(\|flag\| flag.name.unwrap()).collect::<Vec<_>>())
		Ok(separated_flags
		.into_iter()
		.filter(should_include_flag)
		.map(\|flag\| flag.name.unwrap())
		.collect::<Vec<_>>())
		}

		// Exclude system/vendor/product flags that are RO+disabled.
		pub fn should_include_flag(pf: &ProtoParsedFlag) -> bool {
		let should_filter_container = pf.container == Some("vendor".to_string())
		\|\| pf.container == Some("system".to_string())
		\|\| pf.container == Some("product".to_string());

		let disabled_ro = pf.state == Some(ProtoFlagState::DISABLED.into())
		&& pf.permission == Some(ProtoFlagPermission::READ_ONLY.into());

		!should_filter_container \|\| !disabled_ro
		}

		#[cfg(test)]
		@@ -489,7 +494,7 @@ mod tests {
		#[test]
		fn test_offset_fingerprint() {
		let parsed_flags = crate::test::parse_test_flags();
		let expected_fingerprint: u64 = 5801144784618221668;
		let expected_fingerprint: u64 = 11551379960324242360;

		let mut extracted_flags = extract_flag_names(parsed_flags).unwrap();
		let hash_result = compute_flags_fingerprint(&mut extracted_flags);
		@@ -509,6 +514,7 @@ mod tests {
		.parsed_flag
		.clone()
		.into_iter()
		.filter(should_include_flag)
		.map(\|flag\| flag.name.unwrap())
		.map(String::from)
		.collect::<Vec<_>>();

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

+3 −9

Original line number	Diff line number	Diff line
		@@ -14,9 +14,9 @@
		* limitations under the License.
		*/

		use crate::commands::assign_flag_ids;
		use crate::commands::{assign_flag_ids, should_include_flag};
		use crate::storage::FlagPackage;
		use aconfig_protos::{ProtoFlagPermission, ProtoFlagState};
		use aconfig_protos::ProtoFlagPermission;
		use aconfig_storage_file::{
		get_table_size, FlagTable, FlagTableHeader, FlagTableNode, StorageFileType, StoredFlagType,
		};
		@@ -64,13 +64,7 @@ impl FlagTableNodeWrapper {
		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()))
		});
		filtered_package.boolean_flags.retain(\|pf\| should_include_flag(pf));

		let flag_ids =
		assign_flag_ids(package.package_name, filtered_package.boolean_flags.iter().copied())?;