From 9d2d41065c3686a02c7dca51ee658d913b78656f Mon Sep 17 00:00:00 2001 From: Liz Kammer Date: Wed, 21 Dec 2022 14:51:37 -0500 Subject: [PATCH] Handle soong config vars for string attrs Previously if there was a value set outside of product variables and one inside a conditions default soong config var, we would ignore the conditions default. Now, we maintain the conditions default and use the root value in cases where there is not a value for a product variable. Test: go test soong tests Change-Id: Ic991e3aebe5bb6039353f4e3d25625e7c5190f96 --- bazel/properties.go | 34 +++++++++---- bp2build/configurability.go | 7 ++- ...oong_config_module_type_conversion_test.go | 50 +++++++++++++++++++ bp2build/testing.go | 8 +++ 4 files changed, 88 insertions(+), 11 deletions(-) diff --git a/bazel/properties.go b/bazel/properties.go index f56a61387..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 } 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 5cc4983c4..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) { @@ -608,6 +609,55 @@ 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 { 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()