diff --git a/android/variable.go b/android/variable.go index a3f8e160f..a7d287cc8 100644 --- a/android/variable.go +++ b/android/variable.go @@ -566,6 +566,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) @@ -654,9 +658,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 { @@ -677,7 +795,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 @@ -825,9 +943,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, diff --git a/bazel/properties.go b/bazel/properties.go index a438481be..76e73c588 100644 --- a/bazel/properties.go +++ b/bazel/properties.go @@ -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 diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go index aa1cf7068..10ee58294 100644 --- a/bp2build/build_conversion.go +++ b/bp2build/build_conversion.go @@ -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( diff --git a/bp2build/configurability.go b/bp2build/configurability.go index 0bcf91d58..c953259f9 100644 --- a/bp2build/configurability.go +++ b/bp2build/configurability.go @@ -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 } diff --git a/bp2build/soong_config_module_type_conversion_test.go b/bp2build/soong_config_module_type_conversion_test.go index 5669a8383..523f75031 100644 --- a/bp2build/soong_config_module_type_conversion_test.go +++ b/bp2build/soong_config_module_type_conversion_test.go @@ -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"], +)`}}) +} diff --git a/cc/bp2build.go b/cc/bp2build.go index dde8dd4c7..d9494362f 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -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(),