Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f3963f8b authored by Liz Kammer's avatar Liz Kammer Committed by Sasha Smundak
Browse files

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
parent 0ddd60fc

android/variable.go

+10 −122
Original line number Diff line number Diff line
@@ -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

				for k := 0; k < field.NumField(); k++ {

				// Iterate over fields of this struct prop.

					if field.Field(k).IsZero() {

				for k := 0; k < field.NumField(); k++ {

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

bazel/properties.go

+3 −0
Original line number Diff line number Diff line
@@ -1367,6 +1367,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/soong_config_module_type_conversion_test.go

+90 −2
Original line number Diff line number Diff line
@@ -192,6 +192,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 +211,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 +264,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 +281,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 +334,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 +608,90 @@ cc_library_static { 
)`}})

}



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 {