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

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

Change-Id: I742ff28ab3c601571d6fbe38d76e0274a8713119

android/variable.go

@@ -567,6 +567,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)
@@ -655,9 +659,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 {
@@ -678,7 +796,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
@@ -826,9 +944,10 @@ func (productConfigProperties *ProductConfigProperties) AddProductConfigProperti
 						field.Field(k).Interface(), // e.g. ["-DDEFAULT"], ["foo", "bar"]
 					)
 				}
-			} else {
+			} else if property.Kind() != reflect.Interface {
-				// Not a conditions_default or a struct prop, i.e. regular
+				// If not an interface, then this is not a conditions_default or
-				// product variables, or not a string-typed config var.
+				// a struct prop. That is, this is a regular product variable,
+				// or a bool/value config variable.
 				config := productVariableName + suffix
 				productConfigProperties.AddProductConfigProperty(
 					propertyName,

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

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) {
+func prettyPrint(propertyValue reflect.Value, indent int, emitZeroValues bool) (string, error) {
-	if isZero(propertyValue) {
+	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(

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) {
+func labelListSelectValue(selectKey string, list bazel.LabelList, emitEmptyList bool) (bool, reflect.Value) {
-	if selectKey == bazel.ConditionsDefaultSelectKey || len(list.Includes) > 0 {
+	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
 	}

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"],
+)`}})
+}

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(),