bp2build correctly handle empty soong_config_var
Previously in cases where a variable value was set but empty in a bp
file we ignored the value in bp2build; however, the correct behavior is
to take the empty value for each case. The tests for soong config vars
with empty_prop_for_string_var and unused_string_var:
https://cs.android.com/android/platform/superproject/+/master:build/soong/android/soong_config_modules_test.go;l=243-283;drc=72beb34609217f3c98623fb6795b6417c7f0fc65
highlight the difference in when conditions default should be used or
not.
Test: go test bp2build tests
Change-Id: I9768241f013a1a1bc8fa23657a066b5b3f52f09b
diff --git a/android/variable.go b/android/variable.go
index 9725895..e838b7c 100644
--- a/android/variable.go
+++ b/android/variable.go
@@ -702,124 +702,9 @@
}
}
- 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 @@
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 @@
// 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/bazel/properties.go b/bazel/properties.go
index 76450dc..f56a613 100644
--- a/bazel/properties.go
+++ b/bazel/properties.go
@@ -1367,6 +1367,9 @@
// 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/soong_config_module_type_conversion_test.go b/bp2build/soong_config_module_type_conversion_test.go
index dcd1f85..5cc4983 100644
--- a/bp2build/soong_config_module_type_conversion_test.go
+++ b/bp2build/soong_config_module_type_conversion_test.go
@@ -192,6 +192,7 @@
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 +211,7 @@
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 +264,7 @@
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 +281,7 @@
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 +334,13 @@
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 +608,90 @@
)`}})
}
+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 {