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
diff --git a/android/bazel.go b/android/bazel.go
index 26d70e4..8864db0 100644
--- a/android/bazel.go
+++ b/android/bazel.go
@@ -32,6 +32,10 @@
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
diff --git a/android/variable.go b/android/variable.go
index 6ad58c3..89cd59e 100644
--- a/android/variable.go
+++ b/android/variable.go
@@ -491,11 +491,11 @@
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 @@
// "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 @@
} 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 (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 @@
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 @@
// 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,
diff --git a/bp2build/soong_config_module_type_conversion_test.go b/bp2build/soong_config_module_type_conversion_test.go
index 4118479..2e6f54c 100644
--- a/bp2build/soong_config_module_type_conversion_test.go
+++ b/bp2build/soong_config_module_type_conversion_test.go
@@ -68,7 +68,7 @@
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 @@
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 @@
"//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 = ["."],
+)`}})
+}
diff --git a/cc/bp2build.go b/cc/bp2build.go
index 2059f5e..dde8dd4 100644
--- a/cc/bp2build.go
+++ b/cc/bp2build.go
@@ -320,14 +320,14 @@
"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 @@
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),
+ )
}
}
}