Fix bp2build select generation for inter-attribute soong config am: 58ff6801f4 am: d4c0c1fc12

Original change: https://android-review.googlesource.com/c/platform/build/soong/+/1894968

Change-Id: Ic81dce3ae9743354a7499e0e2aca5d11f8e928db
This commit is contained in:
Jingwen Chen
2021-11-23 12:14:34 +00:00
committed by Automerger Merge Worker
6 changed files with 418 additions and 21 deletions

View File

@@ -564,6 +564,10 @@ type ProductConfigProperty struct {
FullConfig string
}
func (p *ProductConfigProperty) AlwaysEmit() bool {
return p.Namespace != ""
}
func (p *ProductConfigProperty) ConfigurationAxis() bazel.ConfigurationAxis {
if p.Namespace == "" {
return bazel.ProductVariableConfigurationAxis(p.FullConfig)
@@ -652,9 +656,123 @@ func ProductVariableProperties(ctx BazelConversionPathContext) ProductConfigProp
}
}
productConfigProperties.zeroValuesForNamespacedVariables()
return productConfigProperties
}
// zeroValuesForNamespacedVariables ensures that selects that contain __only__
// conditions default values have zero values set for the other non-default
// values for that select statement.
//
// If the ProductConfigProperties map contains these items, as parsed from the .bp file:
//
// library_linking_strategy: {
// prefer_static: {
// static_libs: [
// "lib_a",
// "lib_b",
// ],
// },
// conditions_default: {
// shared_libs: [
// "lib_a",
// "lib_b",
// ],
// },
// },
//
// Static_libs {Library_linking_strategy ANDROID prefer_static} [lib_a lib_b]
// Shared_libs {Library_linking_strategy ANDROID conditions_default} [lib_a lib_b]
//
// We need to add this:
//
// Shared_libs {Library_linking_strategy ANDROID prefer_static} []
//
// so that the following gets generated for the "dynamic_deps" attribute,
// instead of putting lib_a and lib_b directly into dynamic_deps without a
// select:
//
// dynamic_deps = select({
// "//build/bazel/product_variables:android__library_linking_strategy__prefer_static": [],
// "//conditions:default": [
// "//foo/bar:lib_a",
// "//foo/bar:lib_b",
// ],
// }),
func (props *ProductConfigProperties) zeroValuesForNamespacedVariables() {
// A map of product config properties to the zero values of their respective
// property value.
zeroValues := make(map[ProductConfigProperty]interface{})
// A map of prop names (e.g. cflags) to product config properties where the
// (prop name, ProductConfigProperty) tuple contains a non-conditions_default key.
//
// e.g.
//
// prefer_static: {
// static_libs: [
// "lib_a",
// "lib_b",
// ],
// },
// conditions_default: {
// shared_libs: [
// "lib_a",
// "lib_b",
// ],
// },
//
// The tuple of ("static_libs", prefer_static) would be in this map.
hasNonDefaultValue := make(map[string]map[ProductConfigProperty]bool)
// Iterate over all added soong config variables.
for propName, v := range *props {
for p, intf := range v {
if p.Namespace == "" {
// If there's no namespace, this isn't a soong config variable,
// i.e. this is a product variable. product variables have no
// conditions_defaults, so skip them.
continue
}
if p.FullConfig == bazel.ConditionsDefaultConfigKey {
// Skip conditions_defaults.
continue
}
if hasNonDefaultValue[propName] == nil {
hasNonDefaultValue[propName] = make(map[ProductConfigProperty]bool)
hasNonDefaultValue[propName][p] = false
}
// Create the zero value of the variable.
if _, exists := zeroValues[p]; !exists {
zeroValue := reflect.Zero(reflect.ValueOf(intf).Type()).Interface()
if zeroValue == nil {
panic(fmt.Errorf("Expected non-nil zero value for product/config variable %+v\n", intf))
}
zeroValues[p] = zeroValue
}
hasNonDefaultValue[propName][p] = true
}
}
for propName := range *props {
for p, zeroValue := range zeroValues {
// Ignore variables that already have a non-default value for that axis
if exists, _ := hasNonDefaultValue[propName][p]; !exists {
// fmt.Println(propName, p.Namespace, p.Name, p.FullConfig, zeroValue)
// Insert the zero value for this propname + product config value.
props.AddProductConfigProperty(
propName,
p.Namespace,
p.Name,
p.FullConfig,
zeroValue,
)
}
}
}
}
func (p *ProductConfigProperties) AddProductConfigProperty(
propertyName, namespace, productVariableName, config string, property interface{}) {
if (*p)[propertyName] == nil {
@@ -675,7 +793,7 @@ func (p *ProductConfigProperties) AddProductConfigProperty(
(*p)[propertyName][productConfigProp] = dst
}
default:
// TODO(jingwen): Add support for more types.
panic(fmt.Errorf("TODO: handle merging value %s", existing))
}
} else {
(*p)[propertyName][productConfigProp] = property
@@ -823,9 +941,10 @@ func (productConfigProperties *ProductConfigProperties) AddProductConfigProperti
field.Field(k).Interface(), // e.g. ["-DDEFAULT"], ["foo", "bar"]
)
}
} else {
// Not a conditions_default or a struct prop, i.e. regular
// product variables, or not a string-typed config var.
} else if property.Kind() != reflect.Interface {
// If not an interface, then this is not a conditions_default or
// a struct prop. That is, this is a regular product variable,
// or a bool/value config variable.
config := productVariableName + suffix
productConfigProperties.AddProductConfigProperty(
propertyName,

View File

@@ -417,6 +417,12 @@ type LabelListAttribute struct {
// This mode facilitates use of attribute defaults: an empty list should
// override the default.
ForceSpecifyEmptyList bool
// If true, signal the intent to the code generator to emit all select keys,
// even if the Includes list for that key is empty. This mode facilitates
// specific select statements where an empty list for a non-default select
// key has a meaning.
EmitEmptyList bool
}
type configurableLabelLists map[ConfigurationAxis]labelListSelectValues

View File

@@ -532,8 +532,8 @@ func isStructPtr(t reflect.Type) bool {
// prettyPrint a property value into the equivalent Starlark representation
// recursively.
func prettyPrint(propertyValue reflect.Value, indent int) (string, error) {
if isZero(propertyValue) {
func prettyPrint(propertyValue reflect.Value, indent int, emitZeroValues bool) (string, error) {
if !emitZeroValues && isZero(propertyValue) {
// A property value being set or unset actually matters -- Soong does set default
// values for unset properties, like system_shared_libs = ["libc", "libm", "libdl"] at
// https://cs.android.com/android/platform/superproject/+/master:build/soong/cc/linker.go;l=281-287;drc=f70926eef0b9b57faf04c17a1062ce50d209e480
@@ -556,7 +556,7 @@ func prettyPrint(propertyValue reflect.Value, indent int) (string, error) {
case reflect.Int, reflect.Uint, reflect.Int64:
ret = fmt.Sprintf("%v", propertyValue.Interface())
case reflect.Ptr:
return prettyPrint(propertyValue.Elem(), indent)
return prettyPrint(propertyValue.Elem(), indent, emitZeroValues)
case reflect.Slice:
if propertyValue.Len() == 0 {
return "[]", nil
@@ -565,7 +565,7 @@ func prettyPrint(propertyValue reflect.Value, indent int) (string, error) {
if propertyValue.Len() == 1 {
// Single-line list for list with only 1 element
ret += "["
indexedValue, err := prettyPrint(propertyValue.Index(0), indent)
indexedValue, err := prettyPrint(propertyValue.Index(0), indent, emitZeroValues)
if err != nil {
return "", err
}
@@ -575,7 +575,7 @@ func prettyPrint(propertyValue reflect.Value, indent int) (string, error) {
// otherwise, use a multiline list.
ret += "[\n"
for i := 0; i < propertyValue.Len(); i++ {
indexedValue, err := prettyPrint(propertyValue.Index(i), indent+1)
indexedValue, err := prettyPrint(propertyValue.Index(i), indent+1, emitZeroValues)
if err != nil {
return "", err
}
@@ -660,7 +660,7 @@ func extractStructProperties(structValue reflect.Value, indent int) map[string]s
}
propertyName := proptools.PropertyNameForField(field.Name)
prettyPrintedValue, err := prettyPrint(fieldValue, indent+1)
prettyPrintedValue, err := prettyPrint(fieldValue, indent+1, false)
if err != nil {
panic(
fmt.Errorf(

View File

@@ -95,7 +95,7 @@ func getLabelListValues(list bazel.LabelListAttribute) (reflect.Value, []selects
continue
}
selectKey := axis.SelectKey(config)
if use, value := labelListSelectValue(selectKey, labels); use {
if use, value := labelListSelectValue(selectKey, labels, list.EmitEmptyList); use {
archSelects[selectKey] = value
}
}
@@ -107,8 +107,8 @@ func getLabelListValues(list bazel.LabelListAttribute) (reflect.Value, []selects
return value, ret
}
func labelListSelectValue(selectKey string, list bazel.LabelList) (bool, reflect.Value) {
if selectKey == bazel.ConditionsDefaultSelectKey || len(list.Includes) > 0 {
func labelListSelectValue(selectKey string, list bazel.LabelList, emitEmptyList bool) (bool, reflect.Value) {
if selectKey == bazel.ConditionsDefaultSelectKey || emitEmptyList || len(list.Includes) > 0 {
return true, reflect.ValueOf(list.Includes)
} else if len(list.Excludes) > 0 {
// if there is still an excludes -- we need to have an empty list for this select & use the
@@ -129,6 +129,7 @@ func prettyPrintAttribute(v bazel.Attribute, indent int) (string, error) {
var value reflect.Value
var configurableAttrs []selects
var defaultSelectValue *string
var emitZeroValues bool
// If true, print the default attribute value, even if the attribute is zero.
shouldPrintDefault := false
switch list := v.(type) {
@@ -137,6 +138,7 @@ func prettyPrintAttribute(v bazel.Attribute, indent int) (string, error) {
defaultSelectValue = &emptyBazelList
case bazel.LabelListAttribute:
value, configurableAttrs = getLabelListValues(list)
emitZeroValues = list.EmitEmptyList
defaultSelectValue = &emptyBazelList
if list.ForceSpecifyEmptyList && (!value.IsNil() || list.HasConfigurableValues()) {
shouldPrintDefault = true
@@ -154,7 +156,7 @@ func prettyPrintAttribute(v bazel.Attribute, indent int) (string, error) {
var err error
ret := ""
if value.Kind() != reflect.Invalid {
s, err := prettyPrint(value, indent)
s, err := prettyPrint(value, indent, false) // never emit zero values for the base value
if err != nil {
return ret, err
}
@@ -163,7 +165,7 @@ func prettyPrintAttribute(v bazel.Attribute, indent int) (string, error) {
}
// Convenience function to append selects components to an attribute value.
appendSelects := func(selectsData selects, defaultValue *string, s string) (string, error) {
selectMap, err := prettyPrintSelectMap(selectsData, defaultValue, indent)
selectMap, err := prettyPrintSelectMap(selectsData, defaultValue, indent, emitZeroValues)
if err != nil {
return "", err
}
@@ -190,7 +192,7 @@ func prettyPrintAttribute(v bazel.Attribute, indent int) (string, error) {
// prettyPrintSelectMap converts a map of select keys to reflected Values as a generic way
// to construct a select map for any kind of attribute type.
func prettyPrintSelectMap(selectMap map[string]reflect.Value, defaultValue *string, indent int) (string, error) {
func prettyPrintSelectMap(selectMap map[string]reflect.Value, defaultValue *string, indent int, emitZeroValues bool) (string, error) {
if selectMap == nil {
return "", nil
}
@@ -202,11 +204,11 @@ func prettyPrintSelectMap(selectMap map[string]reflect.Value, defaultValue *stri
continue
}
value := selectMap[selectKey]
if isZero(value) {
if isZero(value) && !emitZeroValues {
// Ignore zero values to not generate empty lists.
continue
}
s, err := prettyPrintSelectEntry(value, selectKey, indent)
s, err := prettyPrintSelectEntry(value, selectKey, indent, emitZeroValues)
if err != nil {
return "", err
}
@@ -227,7 +229,7 @@ func prettyPrintSelectMap(selectMap map[string]reflect.Value, defaultValue *stri
ret += selects
// Handle the default condition
s, err := prettyPrintSelectEntry(selectMap[bazel.ConditionsDefaultSelectKey], bazel.ConditionsDefaultSelectKey, indent)
s, err := prettyPrintSelectEntry(selectMap[bazel.ConditionsDefaultSelectKey], bazel.ConditionsDefaultSelectKey, indent, emitZeroValues)
if err != nil {
return "", err
}
@@ -249,9 +251,9 @@ func prettyPrintSelectMap(selectMap map[string]reflect.Value, defaultValue *stri
// prettyPrintSelectEntry converts a reflect.Value into an entry in a select map
// with a provided key.
func prettyPrintSelectEntry(value reflect.Value, key string, indent int) (string, error) {
func prettyPrintSelectEntry(value reflect.Value, key string, indent int, emitZeroValues bool) (string, error) {
s := makeIndent(indent + 1)
v, err := prettyPrint(value, indent+1)
v, err := prettyPrint(value, indent+1, emitZeroValues)
if err != nil {
return "", err
}

View File

@@ -32,6 +32,8 @@ func registerSoongConfigModuleTypes(ctx android.RegistrationContext) {
ctx.RegisterModuleType("soong_config_module_type", android.SoongConfigModuleTypeFactory)
ctx.RegisterModuleType("soong_config_string_variable", android.SoongConfigStringVariableDummyFactory)
ctx.RegisterModuleType("soong_config_bool_variable", android.SoongConfigBoolVariableDummyFactory)
ctx.RegisterModuleType("cc_library", cc.LibraryFactory)
}
func TestSoongConfigModuleType(t *testing.T) {
@@ -590,3 +592,270 @@ cc_library_static {
local_includes = ["."],
)`}})
}
func TestSoongConfigModuleType_Defaults(t *testing.T) {
bp := `
soong_config_string_variable {
name: "library_linking_strategy",
values: [
"prefer_static",
],
}
soong_config_module_type {
name: "library_linking_strategy_cc_defaults",
module_type: "cc_defaults",
config_namespace: "ANDROID",
variables: ["library_linking_strategy"],
properties: [
"shared_libs",
"static_libs",
],
bazel_module: { bp2build_available: true },
}
library_linking_strategy_cc_defaults {
name: "library_linking_strategy_lib_a_defaults",
soong_config_variables: {
library_linking_strategy: {
prefer_static: {
static_libs: [
"lib_a",
],
},
conditions_default: {
shared_libs: [
"lib_a",
],
},
},
},
}
library_linking_strategy_cc_defaults {
name: "library_linking_strategy_merged_defaults",
defaults: ["library_linking_strategy_lib_a_defaults"],
soong_config_variables: {
library_linking_strategy: {
prefer_static: {
static_libs: [
"lib_b",
],
},
conditions_default: {
shared_libs: [
"lib_b",
],
},
},
},
}
cc_binary {
name: "library_linking_strategy_sample_binary",
srcs: ["library_linking_strategy.cc"],
defaults: ["library_linking_strategy_merged_defaults"],
}`
otherDeps := `
cc_library { name: "lib_a", bazel_module: { bp2build_available: false } }
cc_library { name: "lib_b", bazel_module: { bp2build_available: false } }
cc_library { name: "lib_default", bazel_module: { bp2build_available: false } }
`
runSoongConfigModuleTypeTest(t, bp2buildTestCase{
description: "soong config variables - generates selects for library_linking_strategy",
moduleTypeUnderTest: "cc_binary",
moduleTypeUnderTestFactory: cc.BinaryFactory,
moduleTypeUnderTestBp2BuildMutator: cc.BinaryBp2build,
blueprint: bp,
filesystem: map[string]string{
"foo/bar/Android.bp": otherDeps,
},
expectedBazelTargets: []string{`cc_binary(
name = "library_linking_strategy_sample_binary",
deps = select({
"//build/bazel/product_variables:android__library_linking_strategy__prefer_static": [
"//foo/bar:lib_b_bp2build_cc_library_static",
"//foo/bar:lib_a_bp2build_cc_library_static",
],
"//conditions:default": [],
}),
dynamic_deps = select({
"//build/bazel/product_variables:android__library_linking_strategy__prefer_static": [],
"//conditions:default": [
"//foo/bar:lib_b",
"//foo/bar:lib_a",
],
}),
local_includes = ["."],
srcs = ["library_linking_strategy.cc"],
)`}})
}
func TestSoongConfigModuleType_Defaults_Another(t *testing.T) {
bp := `
soong_config_string_variable {
name: "library_linking_strategy",
values: [
"prefer_static",
],
}
soong_config_module_type {
name: "library_linking_strategy_cc_defaults",
module_type: "cc_defaults",
config_namespace: "ANDROID",
variables: ["library_linking_strategy"],
properties: [
"shared_libs",
"static_libs",
],
bazel_module: { bp2build_available: true },
}
library_linking_strategy_cc_defaults {
name: "library_linking_strategy_sample_defaults",
soong_config_variables: {
library_linking_strategy: {
prefer_static: {
static_libs: [
"lib_a",
"lib_b",
],
},
conditions_default: {
shared_libs: [
"lib_a",
"lib_b",
],
},
},
},
}
cc_binary {
name: "library_linking_strategy_sample_binary",
srcs: ["library_linking_strategy.cc"],
defaults: ["library_linking_strategy_sample_defaults"],
}`
otherDeps := `
cc_library { name: "lib_a", bazel_module: { bp2build_available: false } }
cc_library { name: "lib_b", bazel_module: { bp2build_available: false } }
`
runSoongConfigModuleTypeTest(t, bp2buildTestCase{
description: "soong config variables - generates selects for library_linking_strategy",
moduleTypeUnderTest: "cc_binary",
moduleTypeUnderTestFactory: cc.BinaryFactory,
moduleTypeUnderTestBp2BuildMutator: cc.BinaryBp2build,
blueprint: bp,
filesystem: map[string]string{
"foo/bar/Android.bp": otherDeps,
},
expectedBazelTargets: []string{`cc_binary(
name = "library_linking_strategy_sample_binary",
deps = select({
"//build/bazel/product_variables:android__library_linking_strategy__prefer_static": [
"//foo/bar:lib_a_bp2build_cc_library_static",
"//foo/bar:lib_b_bp2build_cc_library_static",
],
"//conditions:default": [],
}),
dynamic_deps = select({
"//build/bazel/product_variables:android__library_linking_strategy__prefer_static": [],
"//conditions:default": [
"//foo/bar:lib_a",
"//foo/bar:lib_b",
],
}),
local_includes = ["."],
srcs = ["library_linking_strategy.cc"],
)`}})
}
func TestSoongConfigModuleType_Defaults_UnusedProps(t *testing.T) {
bp := `
soong_config_string_variable {
name: "alphabet",
values: [
"a",
"b",
"c", // unused
],
}
soong_config_module_type {
name: "alphabet_cc_defaults",
module_type: "cc_defaults",
config_namespace: "ANDROID",
variables: ["alphabet"],
properties: [
"cflags", // unused
"shared_libs",
"static_libs",
],
bazel_module: { bp2build_available: true },
}
alphabet_cc_defaults {
name: "alphabet_sample_cc_defaults",
soong_config_variables: {
alphabet: {
a: {
shared_libs: [
"lib_a",
],
},
b: {
shared_libs: [
"lib_b",
],
},
conditions_default: {
static_libs: [
"lib_default",
],
},
},
},
}
cc_binary {
name: "alphabet_binary",
srcs: ["main.cc"],
defaults: ["alphabet_sample_cc_defaults"],
}`
otherDeps := `
cc_library { name: "lib_a", bazel_module: { bp2build_available: false } }
cc_library { name: "lib_b", bazel_module: { bp2build_available: false } }
cc_library { name: "lib_default", bazel_module: { bp2build_available: false } }
`
runSoongConfigModuleTypeTest(t, bp2buildTestCase{
description: "soong config variables - generates selects for library_linking_strategy",
moduleTypeUnderTest: "cc_binary",
moduleTypeUnderTestFactory: cc.BinaryFactory,
moduleTypeUnderTestBp2BuildMutator: cc.BinaryBp2build,
blueprint: bp,
filesystem: map[string]string{
"foo/bar/Android.bp": otherDeps,
},
expectedBazelTargets: []string{`cc_binary(
name = "alphabet_binary",
deps = select({
"//build/bazel/product_variables:android__alphabet__a": [],
"//build/bazel/product_variables:android__alphabet__b": [],
"//conditions:default": ["//foo/bar:lib_default_bp2build_cc_library_static"],
}),
dynamic_deps = select({
"//build/bazel/product_variables:android__alphabet__a": ["//foo/bar:lib_a"],
"//build/bazel/product_variables:android__alphabet__b": ["//foo/bar:lib_b"],
"//conditions:default": [],
}),
local_includes = ["."],
srcs = ["main.cc"],
)`}})
}

View File

@@ -611,6 +611,7 @@ func (la *linkerAttributes) convertProductVariables(ctx android.BazelConversionP
ctx.ModuleErrorf("Could not convert product variable %s property", dep.excludesField)
}
dep.attribute.EmitEmptyList = productConfigProp.AlwaysEmit()
dep.attribute.SetSelectValue(
productConfigProp.ConfigurationAxis(),
productConfigProp.SelectKey(),