Refactor ProductConfigProperties to use a struct key instead of an

string key with hardcoded patterns.

This fixes a bug with label list conditions_default attrs where the
attribute values get clobbered in a map with the keys
"conditions_default" (with a default empty list) and
"acme__feature__conditions_default" (with a non-empty list) when
generating the LabelListAttribute.

Test: CI
Change-Id: I5429e40f747b7a0ed559f8a468a4831cd32df2c0
This commit is contained in:
Jingwen Chen 2021-11-15 12:28:43 +00:00
parent 7b5fa4277f
commit 25825ca08d
4 changed files with 146 additions and 49 deletions

View file

@ -32,6 +32,10 @@ type properties struct {
Bazel_module bazel.BazelModuleProperties
}
// namespacedVariableProperties is a map from a string representing a Soong
// config variable namespace, like "android" or "vendor_name" to a struct
// pointer representing the soong_config_variables property of a module created
// by a soong_config_module_type or soong_config_module_type_import.
type namespacedVariableProperties map[string]interface{}
// BazelModuleBase contains the property structs with metadata for modules which can be converted to

View file

@ -491,11 +491,11 @@ type ProductConfigContext interface {
type ProductConfigProperty struct {
// The name of the product variable, e.g. "safestack", "malloc_not_svelte",
// "board"
ProductConfigVariable string
Name string
// Namespace of the variable, if this is a soong_config_module_type variable
// e.g. "acme", "ANDROID", "vendor_nae"
Namespace string // for soong config variables
// e.g. "acme", "ANDROID", "vendor_name"
Namespace string
// Unique configuration to identify this product config property (i.e. a
// primary key), as just using the product variable name is not sufficient.
@ -562,9 +562,6 @@ type ProductConfigProperty struct {
// "acme__board__soc_a", "acme__board__soc_b", and
// "acme__board__conditions_default"
FullConfig string
// The actual property value: list, bool, string..
Property interface{}
}
func (p *ProductConfigProperty) ConfigurationAxis() bazel.ConfigurationAxis {
@ -573,14 +570,43 @@ func (p *ProductConfigProperty) ConfigurationAxis() bazel.ConfigurationAxis {
} else {
// Soong config variables can be uniquely identified by the namespace
// (e.g. acme, android) and the product variable name (e.g. board, size)
return bazel.ProductVariableConfigurationAxis(p.Namespace + "__" + p.ProductConfigVariable)
return bazel.ProductVariableConfigurationAxis(p.Namespace + "__" + p.Name)
}
}
// ProductConfigProperties is a map of property name to a slice of
// ProductConfigProperty such that all product variable-specific versions of a
// property are easily accessed together
type ProductConfigProperties map[string]map[string]ProductConfigProperty
// SelectKey returns the literal string that represents this variable in a BUILD
// select statement.
func (p *ProductConfigProperty) SelectKey() string {
if p.Namespace == "" {
return strings.ToLower(p.FullConfig)
}
if p.FullConfig == bazel.ConditionsDefaultConfigKey {
return bazel.ConditionsDefaultConfigKey
}
value := p.FullConfig
if value == p.Name {
value = "enabled"
}
// e.g. acme__feature1__enabled, android__board__soc_a
return strings.ToLower(strings.Join([]string{p.Namespace, p.Name, value}, "__"))
}
// ProductConfigProperties is a map of maps to group property values according
// their property name and the product config variable they're set under.
//
// The outer map key is the name of the property, like "cflags".
//
// The inner map key is a ProductConfigProperty, which is a struct of product
// variable name, namespace, and the "full configuration" of the product
// variable.
//
// e.g. product variable name: board, namespace: acme, full config: vendor_chip_foo
//
// The value of the map is the interface{} representing the value of the
// property, like ["-DDEFINES"] for cflags.
type ProductConfigProperties map[string]map[ProductConfigProperty]interface{}
// ProductVariableProperties returns a ProductConfigProperties containing only the properties which
// have been set for the module in the given context.
@ -630,19 +656,16 @@ func ProductVariableProperties(ctx BazelConversionPathContext) ProductConfigProp
func (p *ProductConfigProperties) AddProductConfigProperty(
propertyName, namespace, productVariableName, config string, property interface{}) {
if (*p)[propertyName] == nil {
(*p)[propertyName] = make(map[string]ProductConfigProperty)
(*p)[propertyName] = make(map[ProductConfigProperty]interface{})
}
// Normalize config to be all lowercase. It's the "primary key" of this
// unique property value. This can be the conditions_default value of the
// product variable as well.
config = strings.ToLower(config)
(*p)[propertyName][config] = ProductConfigProperty{
Namespace: namespace, // e.g. acme, android
ProductConfigVariable: productVariableName, // e.g. size, feature1, feature2, FEATURE3, board
FullConfig: config, // e.g. size, feature1-x86, size__conditions_default
Property: property, // e.g. ["-O3"]
productConfigProp := ProductConfigProperty{
Namespace: namespace, // e.g. acme, android
Name: productVariableName, // e.g. size, feature1, feature2, FEATURE3, board
FullConfig: config, // e.g. size, feature1-x86, size__conditions_default
}
(*p)[propertyName][productConfigProp] = property
}
var (
@ -783,8 +806,8 @@ func productVariableValues(
if field.Field(k).IsZero() {
continue
}
productVariableValue := proptools.PropertyNameForField(propertyName)
config := strings.Join([]string{namespace, productVariableName, productVariableValue}, "__")
// config can also be "conditions_default".
config := proptools.PropertyNameForField(propertyName)
actualPropertyName := field.Type().Field(k).Name
productConfigProperties.AddProductConfigProperty(
@ -799,9 +822,6 @@ func productVariableValues(
// Not a conditions_default or a struct prop, i.e. regular
// product variables, or not a string-typed config var.
config := productVariableName + suffix
if namespace != "" {
config = namespace + "__" + config
}
productConfigProperties.AddProductConfigProperty(
propertyName,
namespace,

View file

@ -68,7 +68,7 @@ custom_cc_library_static {
expectedBazelTargets: []string{`cc_library_static(
name = "foo",
copts = select({
"//build/bazel/product_variables:acme__feature1": ["-DFEATURE1"],
"//build/bazel/product_variables:acme__feature1__enabled": ["-DFEATURE1"],
"//conditions:default": ["-DDEFAULT1"],
}),
local_includes = ["."],
@ -118,7 +118,7 @@ custom_cc_library_static {
expectedBazelTargets: []string{`cc_library_static(
name = "foo",
copts = select({
"//build/bazel/product_variables:acme__feature1": ["-DFEATURE1"],
"//build/bazel/product_variables:acme__feature1__enabled": ["-DFEATURE1"],
"//conditions:default": ["-DDEFAULT1"],
}),
local_includes = ["."],
@ -246,12 +246,81 @@ custom_cc_library_static {
"//build/bazel/product_variables:acme__board__soc_b": ["-DSOC_B"],
"//conditions:default": ["-DSOC_DEFAULT"],
}) + select({
"//build/bazel/product_variables:acme__feature1": ["-DFEATURE1"],
"//build/bazel/product_variables:acme__feature1__enabled": ["-DFEATURE1"],
"//conditions:default": ["-DDEFAULT1"],
}) + select({
"//build/bazel/product_variables:acme__feature2": ["-DFEATURE2"],
"//build/bazel/product_variables:acme__feature2__enabled": ["-DFEATURE2"],
"//conditions:default": ["-DDEFAULT2"],
}),
local_includes = ["."],
)`}})
}
func TestSoongConfigModuleType_StringVar_LabelListDeps(t *testing.T) {
bp := `
soong_config_string_variable {
name: "board",
values: ["soc_a", "soc_b", "soc_c"],
}
soong_config_module_type {
name: "custom_cc_library_static",
module_type: "cc_library_static",
config_namespace: "acme",
variables: ["board"],
properties: ["cflags", "static_libs"],
bazel_module: { bp2build_available: true },
}
custom_cc_library_static {
name: "foo",
bazel_module: { bp2build_available: true },
soong_config_variables: {
board: {
soc_a: {
cflags: ["-DSOC_A"],
static_libs: ["soc_a_dep"],
},
soc_b: {
cflags: ["-DSOC_B"],
static_libs: ["soc_b_dep"],
},
soc_c: {},
conditions_default: {
cflags: ["-DSOC_DEFAULT"],
static_libs: ["soc_default_static_dep"],
},
},
},
}`
otherDeps := `
cc_library_static { name: "soc_a_dep", bazel_module: { bp2build_available: false } }
cc_library_static { name: "soc_b_dep", bazel_module: { bp2build_available: false } }
cc_library_static { name: "soc_default_static_dep", bazel_module: { bp2build_available: false } }
`
runSoongConfigModuleTypeTest(t, bp2buildTestCase{
description: "soong config variables - generates selects for label list attributes",
moduleTypeUnderTest: "cc_library_static",
moduleTypeUnderTestFactory: cc.LibraryStaticFactory,
moduleTypeUnderTestBp2BuildMutator: cc.CcLibraryStaticBp2Build,
blueprint: bp,
filesystem: map[string]string{
"foo/bar/Android.bp": otherDeps,
},
expectedBazelTargets: []string{`cc_library_static(
name = "foo",
copts = select({
"//build/bazel/product_variables:acme__board__soc_a": ["-DSOC_A"],
"//build/bazel/product_variables:acme__board__soc_b": ["-DSOC_B"],
"//conditions:default": ["-DSOC_DEFAULT"],
}),
implementation_deps = select({
"//build/bazel/product_variables:acme__board__soc_a": ["//foo/bar:soc_a_dep"],
"//build/bazel/product_variables:acme__board__soc_b": ["//foo/bar:soc_b_dep"],
"//conditions:default": ["//foo/bar:soc_default_static_dep"],
}),
local_includes = ["."],
)`}})
}

View file

@ -320,14 +320,14 @@ func (ca *compilerAttributes) convertProductVariables(ctx android.BazelConversio
"CppFlags": &ca.cppFlags,
}
for propName, attr := range productVarPropNameToAttribute {
if props, exists := productVariableProps[propName]; exists {
for _, prop := range props {
flags, ok := prop.Property.([]string)
if productConfigProps, exists := productVariableProps[propName]; exists {
for productConfigProp, prop := range productConfigProps {
flags, ok := prop.([]string)
if !ok {
ctx.ModuleErrorf("Could not convert product variable %s property", proptools.PropertyNameForField(propName))
}
newFlags, _ := bazel.TryVariableSubstitutions(flags, prop.ProductConfigVariable)
attr.SetSelectValue(prop.ConfigurationAxis(), prop.FullConfig, newFlags)
newFlags, _ := bazel.TryVariableSubstitutions(flags, productConfigProp.Name)
attr.SetSelectValue(productConfigProp.ConfigurationAxis(), productConfigProp.SelectKey(), newFlags)
}
}
}
@ -587,31 +587,35 @@ func (la *linkerAttributes) convertProductVariables(ctx android.BazelConversionP
if !exists && !excludesExists {
continue
}
// collect all the configurations that an include or exclude property exists for.
// we want to iterate all configurations rather than either the include or exclude because for a
// particular configuration we may have only and include or only an exclude to handle
configs := make(map[string]bool, len(props)+len(excludeProps))
for config := range props {
configs[config] = true
// Collect all the configurations that an include or exclude property exists for.
// We want to iterate all configurations rather than either the include or exclude because, for a
// particular configuration, we may have either only an include or an exclude to handle.
productConfigProps := make(map[android.ProductConfigProperty]bool, len(props)+len(excludeProps))
for p := range props {
productConfigProps[p] = true
}
for config := range excludeProps {
configs[config] = true
for p := range excludeProps {
productConfigProps[p] = true
}
for config := range configs {
prop, includesExists := props[config]
excludesProp, excludesExists := excludeProps[config]
for productConfigProp := range productConfigProps {
prop, includesExists := props[productConfigProp]
excludesProp, excludesExists := excludeProps[productConfigProp]
var includes, excludes []string
var ok bool
// if there was no includes/excludes property, casting fails and that's expected
if includes, ok = prop.Property.([]string); includesExists && !ok {
if includes, ok = prop.([]string); includesExists && !ok {
ctx.ModuleErrorf("Could not convert product variable %s property", name)
}
if excludes, ok = excludesProp.Property.([]string); excludesExists && !ok {
if excludes, ok = excludesProp.([]string); excludesExists && !ok {
ctx.ModuleErrorf("Could not convert product variable %s property", dep.excludesField)
}
dep.attribute.SetSelectValue(prop.ConfigurationAxis(), config, dep.depResolutionFunc(ctx, android.FirstUniqueStrings(includes), excludes))
dep.attribute.SetSelectValue(
productConfigProp.ConfigurationAxis(),
productConfigProp.SelectKey(),
dep.depResolutionFunc(ctx, android.FirstUniqueStrings(includes), excludes),
)
}
}
}