Merge changes I48426a8e,Ic991e3ae,I9768241f

* changes:
  bp2build apex min_sdk_version w/ soong config var
  Handle soong config vars for string attrs
  bp2build correctly handle empty soong_config_var

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.
+					// For product variables, zero values are irrelevant; however, for soong config variables,
-					if field.Field(k).IsZero() {
+					// 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(

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

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 = ""
+	cs[axis][config] = str
-	if str != nil {
-		v = *str
-	}
-	cs[axis][config] = v
 }
 
-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 {

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
+		if _, ok := ret[bazel.ConditionsDefaultSelectKey]; !ok {
-		value = reflect.Zero(value.Type())
+			ret[bazel.ConditionsDefaultSelectKey] = value
+			value = reflect.Zero(value.Type())
+		}
 	}
 
 	return value, []selects{ret}

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 {

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