From 1f6555151ec2565585abfd36376df3d1cbbccd2a Mon Sep 17 00:00:00 2001 From: Joe Onorato Date: Mon, 12 Jun 2023 23:29:25 -0700 Subject: [PATCH] Allow and merge multiple release configs with the same name For now (to prevent build breakages when this is submitted), duplicate flag values are allowed, but that will be disallowed once this change automerges everywhere. Bug: 286689485 Test: Treehugger Change-Id: I723340ff9751b61d33c4108b0cc2f90702a116c9 --- core/release_config.bzl | 1 + core/release_config.mk | 25 ++++++++++++++++++------- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/core/release_config.bzl b/core/release_config.bzl index 134650851c..805106f4e7 100644 --- a/core/release_config.bzl +++ b/core/release_config.bzl @@ -88,6 +88,7 @@ def release_config(all_flags, all_values): partitions.setdefault(partition, []).append(flag["name"]) # Validate values + # TODO(joeo): Disallow duplicate values after we've split AOSP and vendor flags. values = {} for value in all_values: if value["name"] not in flag_names: diff --git a/core/release_config.mk b/core/release_config.mk index 3cd8b4103b..1a2d4804fd 100644 --- a/core/release_config.mk +++ b/core/release_config.mk @@ -16,6 +16,22 @@ # ----------------------------------------------------------------- # Choose the flag files # ----------------------------------------------------------------- +# Release configs are defined in reflease_config_map files, which map +# the short name (e.g. -next) used in lunch to the starlark files +# defining the build flag values. +# +# (If you're thinking about aconfig flags, there is one build flag, +# RELEASE_DEVICE_CONFIG_VALUE_SETS, that sets which device_config_value_set +# module to use to set the aconfig flag values.) +# +# The short release config names *can* appear multiple times, to allow +# for AOSP and vendor specific flags under the same name, but the +# individual flag values must appear in exactly one config. Vendor +# does not override AOSP, or anything like that. This is because +# vendor code usually includes prebuilts, and having vendor compile +# with different flags from AOSP increases the likelihood of flag +# mismatch. + # Do this first, because we're going to unset TARGET_RELEASE before # including anyone, so they don't start making conditionals based on it. # This logic is in make because starlark doesn't understand optional @@ -39,17 +55,12 @@ config_map_files := $(wildcard build/release/release_config_map.mk) \ # $1 config name # $2 release config files define declare-release-config - $(eval # No duplicates) - $(if $(filter $(_all_release_configs), $(strip $(1))), \ - $(error declare-release-config: config $(strip $(1)) declared in: $(_included) Previously declared here: $(_all_release_configs.$(strip $(1)).DECLARED_IN)) \ - ) - $(eval # Must have release config files) $(if $(strip $(2)),, \ $(error declare-release-config: config $(strip $(1)) must have release config files) \ ) $(eval _all_release_configs := $(sort $(_all_release_configs) $(strip $(1)))) - $(eval _all_release_configs.$(strip $(1)).DECLARED_IN := $(_included)) - $(eval _all_release_configs.$(strip $(1)).FILES := $(strip $(2))) + $(eval _all_release_configs.$(strip $(1)).DECLARED_IN := $(_included) $(_all_release_configs.$(strip $(1)).DECLARED_IN)) + $(eval _all_release_configs.$(strip $(1)).FILES := $(_all_release_configs.$(strip $(1)).FILES) $(strip $(2))) endef # Include the config map files