diff --git a/android/variable.go b/android/variable.go index 97258955a..e838b7cbd 100644 --- a/android/variable.go +++ b/android/variable.go @@ -702,124 +702,9 @@ 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, - bazel.NoConfigAxis, - ) - } - } - } -} - func (p *ProductConfigProperties) AddProductConfigProperty( propertyName, namespace, productVariableName, config string, property interface{}, outerAxis bazel.ConfigurationAxis) { if (*p)[propertyName] == nil { @@ -945,14 +830,16 @@ func (productConfigProperties *ProductConfigProperties) AddProductConfigProperti for j := 0; j < variableValue.NumField(); j++ { property := variableValue.Field(j) + // e.g. Asflags, Cflags, Enabled, etc. + propertyName := variableValue.Type().Field(j).Name + // config can also be "conditions_default". + config := proptools.PropertyNameForField(propertyName) + // If the property wasn't set, no need to pass it along if property.IsZero() { continue } - // e.g. Asflags, Cflags, Enabled, etc. - propertyName := variableValue.Type().Field(j).Name - if v, ok := maybeExtractConfigVarProp(property); ok { // The field is a struct, which is used by: // 1) soong_config_string_variables @@ -972,13 +859,14 @@ func (productConfigProperties *ProductConfigProperties) AddProductConfigProperti // static_libs: ... // } field := v + // Iterate over fields of this struct prop. for k := 0; k < field.NumField(); k++ { - // Iterate over fields of this struct prop. - if field.Field(k).IsZero() { + // For product variables, zero values are irrelevant; however, for soong config variables, + // empty values are relevant because there can also be a conditions default which is not + // applied for empty variables. + if field.Field(k).IsZero() && namespace == "" { continue } - // config can also be "conditions_default". - config := proptools.PropertyNameForField(propertyName) actualPropertyName := field.Type().Field(k).Name productConfigProperties.AddProductConfigProperty( diff --git a/apex/apex.go b/apex/apex.go index e99823bf0..9485a4b0b 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -3426,7 +3426,7 @@ type bazelApexBundleAttributes struct { Key bazel.LabelAttribute Certificate bazel.LabelAttribute // used when the certificate prop is a module Certificate_name bazel.StringAttribute // used when the certificate prop is a string - Min_sdk_version *string + Min_sdk_version bazel.StringAttribute Updatable bazel.BoolAttribute Installable bazel.BoolAttribute Binaries bazel.LabelListAttribute @@ -3445,6 +3445,10 @@ type convertedNativeSharedLibs struct { Native_shared_libs_64 bazel.LabelListAttribute } +const ( + minSdkVersionPropName = "Min_sdk_version" +) + // ConvertWithBp2build performs bp2build conversion of an apex func (a *apexBundle) ConvertWithBp2build(ctx android.TopDownMutatorContext) { // We only convert apex and apex_test modules at this time @@ -3483,11 +3487,19 @@ func convertWithBp2build(a *apexBundle, ctx android.TopDownMutatorContext) (baze fileContextsLabelAttribute.SetValue(android.BazelLabelForModuleSrcSingle(ctx, *a.properties.File_contexts)) } + productVariableProps := android.ProductVariableProperties(ctx) // TODO(b/219503907) this would need to be set to a.MinSdkVersionValue(ctx) but // given it's coming via config, we probably don't want to put it in here. - var minSdkVersion *string + var minSdkVersion bazel.StringAttribute if a.properties.Min_sdk_version != nil { - minSdkVersion = a.properties.Min_sdk_version + minSdkVersion.SetValue(*a.properties.Min_sdk_version) + } + if props, ok := productVariableProps[minSdkVersionPropName]; ok { + for c, p := range props { + if val, ok := p.(*string); ok { + minSdkVersion.SetSelectValue(c.ConfigurationAxis(), c.SelectKey(), val) + } + } } var keyLabelAttribute bazel.LabelAttribute diff --git a/bazel/properties.go b/bazel/properties.go index 76450dc7b..8a6d1b065 100644 --- a/bazel/properties.go +++ b/bazel/properties.go @@ -1080,14 +1080,10 @@ func (cs configurableStrings) setValueForAxis(axis ConfigurationAxis, config str if cs[axis] == nil { cs[axis] = make(stringSelectValues) } - var v = "" - if str != nil { - v = *str - } - cs[axis][config] = v + cs[axis][config] = str } -type stringSelectValues map[string]string +type stringSelectValues map[string]*string // HasConfigurableValues returns true if the attribute contains axis-specific string values. func (sa StringAttribute) HasConfigurableValues() bool { @@ -1099,6 +1095,11 @@ func (sa StringAttribute) HasConfigurableValues() bool { return false } +// SetValue sets the base, non-configured value for the Label +func (sa *StringAttribute) SetValue(value string) { + sa.SetSelectValue(NoConfigAxis, "", &value) +} + // SetSelectValue set a value for a bazel select for the given axis, config and value. func (sa *StringAttribute) SetSelectValue(axis ConfigurationAxis, config string, str *string) { axis.validateConfig(config) @@ -1123,7 +1124,7 @@ func (sa *StringAttribute) SelectValue(axis ConfigurationAxis, config string) *s return sa.Value case arch, os, osArch, productVariables: if v, ok := sa.ConfigurableValues[axis][config]; ok { - return &v + return v } else { return nil } @@ -1154,7 +1155,7 @@ func (sa *StringAttribute) Collapse() error { _, containsProductVariables := axisTypes[productVariables] if containsProductVariables { if containsOs || containsArch || containsOsArch { - return fmt.Errorf("boolean attribute could not be collapsed as it has two or more unrelated axes") + return fmt.Errorf("string attribute could not be collapsed as it has two or more unrelated axes") } } if (containsOs && containsArch) || (containsOsArch && (containsOs || containsArch)) { @@ -1181,7 +1182,7 @@ func (sa *StringAttribute) Collapse() error { } } } - // All os_arch values are now set. Clear os and arch axes. + /// All os_arch values are now set. Clear os and arch axes. delete(sa.ConfigurableValues, ArchConfigurationAxis) delete(sa.ConfigurableValues, OsConfigurationAxis) // Verify post-condition; this should never fail, provided no additional @@ -1189,6 +1190,21 @@ func (sa *StringAttribute) Collapse() error { if len(sa.ConfigurableValues) > 1 { panic(fmt.Errorf("error in collapsing attribute: %#v", sa)) } + } else if containsProductVariables { + usedBaseValue := false + for a, configToProp := range sa.ConfigurableValues { + if a.configurationType == productVariables { + for c, p := range configToProp { + if p == nil { + sa.SetSelectValue(a, c, sa.Value) + usedBaseValue = true + } + } + } + } + if usedBaseValue { + sa.Value = nil + } } return nil } @@ -1367,6 +1383,9 @@ func (sla *StringListAttribute) DeduplicateAxesFromBase() { // TryVariableSubstitution, replace string substitution formatting within each string in slice with // Starlark string.format compatible tag for productVariable. func TryVariableSubstitutions(slice []string, productVariable string) ([]string, bool) { + if len(slice) == 0 { + return slice, false + } ret := make([]string, 0, len(slice)) changesMade := false for _, s := range slice { diff --git a/bp2build/configurability.go b/bp2build/configurability.go index 987c90343..424495692 100644 --- a/bp2build/configurability.go +++ b/bp2build/configurability.go @@ -28,10 +28,13 @@ func getStringValue(str bazel.StringAttribute) (reflect.Value, []selects) { ret[selectKey] = reflect.ValueOf(strs) } } + // if there is a select, use the base value as the conditions default value if len(ret) > 0 { - ret[bazel.ConditionsDefaultSelectKey] = value - value = reflect.Zero(value.Type()) + if _, ok := ret[bazel.ConditionsDefaultSelectKey]; !ok { + ret[bazel.ConditionsDefaultSelectKey] = value + value = reflect.Zero(value.Type()) + } } return value, []selects{ret} diff --git a/bp2build/soong_config_module_type_conversion_test.go b/bp2build/soong_config_module_type_conversion_test.go index dcd1f856c..89be767dc 100644 --- a/bp2build/soong_config_module_type_conversion_test.go +++ b/bp2build/soong_config_module_type_conversion_test.go @@ -32,6 +32,7 @@ func registerSoongConfigModuleTypes(ctx android.RegistrationContext) { android.RegisterSoongConfigModuleBuildComponents(ctx) ctx.RegisterModuleType("cc_library", cc.LibraryFactory) + ctx.RegisterModuleType("custom", customModuleFactoryHostAndDevice) } func TestErrorInBpFileDoesNotPanic(t *testing.T) { @@ -192,6 +193,7 @@ custom_cc_library_static { copts = select({ "//build/bazel/product_variables:acme__board__soc_a": ["-DSOC_A"], "//build/bazel/product_variables:acme__board__soc_b": ["-DSOC_B"], + "//build/bazel/product_variables:acme__board__soc_c": [], "//conditions:default": ["-DSOC_DEFAULT"], }), local_includes = ["."], @@ -210,7 +212,7 @@ soong_config_bool_variable { soong_config_string_variable { name: "board", - values: ["soc_a", "soc_b", "soc_c"], + values: ["soc_a", "soc_b", "soc_c", "soc_d"], } soong_config_module_type { @@ -263,6 +265,7 @@ custom_cc_library_static { copts = select({ "//build/bazel/product_variables:acme__board__soc_a": ["-DSOC_A"], "//build/bazel/product_variables:acme__board__soc_b": ["-DSOC_B"], + "//build/bazel/product_variables:acme__board__soc_c": [], "//conditions:default": ["-DSOC_DEFAULT"], }) + select({ "//build/bazel/product_variables:acme__feature1": ["-DFEATURE1"], @@ -279,7 +282,7 @@ func TestSoongConfigModuleType_StringVar_LabelListDeps(t *testing.T) { bp := ` soong_config_string_variable { name: "board", - values: ["soc_a", "soc_b", "soc_c"], + values: ["soc_a", "soc_b", "soc_c", "soc_d"], } soong_config_module_type { @@ -332,11 +335,13 @@ cc_library_static { name: "soc_default_static_dep", bazel_module: { bp2build_ava copts = select({ "//build/bazel/product_variables:acme__board__soc_a": ["-DSOC_A"], "//build/bazel/product_variables:acme__board__soc_b": ["-DSOC_B"], + "//build/bazel/product_variables:acme__board__soc_c": [], "//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"], + "//build/bazel/product_variables:acme__board__soc_c": [], "//conditions:default": ["//foo/bar:soc_default_static_dep"], }), local_includes = ["."], @@ -604,6 +609,139 @@ cc_library_static { )`}}) } +func TestSoongConfigModuleType_Defaults_UseBaselineValueForStringProp(t *testing.T) { + bp := ` +soong_config_string_variable { + name: "library_linking_strategy", + values: [ + "prefer_static", + ], +} + +soong_config_module_type { + name: "library_linking_strategy_custom", + module_type: "custom", + config_namespace: "ANDROID", + variables: ["library_linking_strategy"], + properties: [ + "string_literal_prop", + ], +} + +library_linking_strategy_custom { + name: "foo", + string_literal_prop: "29", + soong_config_variables: { + library_linking_strategy: { + prefer_static: {}, + conditions_default: { + string_literal_prop: "30", + }, + }, + }, +}` + + runSoongConfigModuleTypeTest(t, Bp2buildTestCase{ + Description: "soong config variables - generates selects for library_linking_strategy", + ModuleTypeUnderTest: "cc_binary", + ModuleTypeUnderTestFactory: cc.BinaryFactory, + Blueprint: bp, + Filesystem: map[string]string{}, + ExpectedBazelTargets: []string{ + MakeBazelTarget("custom", "foo", AttrNameToString{ + "string_literal_prop": `select({ + "//build/bazel/product_variables:android__library_linking_strategy__prefer_static": "29", + "//conditions:default": "30", + })`, + }), + }, + }) +} + +func TestSoongConfigModuleType_UnsetConditions(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", + ], +} + +library_linking_strategy_cc_defaults { + name: "library_linking_strategy_lib_a_defaults", + soong_config_variables: { + library_linking_strategy: { + prefer_static: {}, + conditions_default: { + shared_libs: [ + "lib_a", + ], + }, + }, + }, +} + +library_linking_strategy_cc_defaults { + name: "library_linking_strategy_merged_defaults", + defaults: ["library_linking_strategy_lib_a_defaults"], + host_supported: true, + soong_config_variables: { + library_linking_strategy: { + prefer_static: {}, + 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"], + include_build_directory: false, +}` + + 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, + Blueprint: bp, + Filesystem: map[string]string{ + "foo/bar/Android.bp": otherDeps, + }, + ExpectedBazelTargets: []string{`cc_binary( + name = "library_linking_strategy_sample_binary", + dynamic_deps = select({ + "//build/bazel/product_variables:android__library_linking_strategy__prefer_static": [], + "//conditions:default": [ + "//foo/bar:lib_b", + "//foo/bar:lib_a", + ], + }), + srcs = ["library_linking_strategy.cc"], +)`}}) +} + func TestSoongConfigModuleType_Defaults(t *testing.T) { bp := ` soong_config_string_variable { diff --git a/bp2build/testing.go b/bp2build/testing.go index 1f9874c21..c340a8f72 100644 --- a/bp2build/testing.go +++ b/bp2build/testing.go @@ -453,6 +453,14 @@ func (m *customModule) ConvertWithBp2build(ctx android.TopDownMutatorContext) { } } } + productVariableProps := android.ProductVariableProperties(ctx) + if props, ok := productVariableProps["String_literal_prop"]; ok { + for c, p := range props { + if val, ok := p.(*string); ok { + strAttr.SetSelectValue(c.ConfigurationAxis(), c.SelectKey(), val) + } + } + } paths.ResolveExcludes()