diff --git a/android/soong_config_modules.go b/android/soong_config_modules.go index 9f5440d22..5fa60124e 100644 --- a/android/soong_config_modules.go +++ b/android/soong_config_modules.go @@ -396,7 +396,7 @@ func loadSoongConfigModuleTypeDefinition(ctx LoadHookContext, from string) map[s } if ctx.Config().BuildMode == Bp2build { - ctx.Config().Bp2buildSoongConfigDefinitions.AddVars(*mtDef) + ctx.Config().Bp2buildSoongConfigDefinitions.AddVars(mtDef) } globalModuleTypes := ctx.moduleFactories() diff --git a/android/soongconfig/modules.go b/android/soongconfig/modules.go index 9239ca950..23c8afa0b 100644 --- a/android/soongconfig/modules.go +++ b/android/soongconfig/modules.go @@ -240,12 +240,7 @@ type SoongConfigDefinition struct { // string vars, bool vars and value vars created by every // soong_config_module_type in this build. type Bp2BuildSoongConfigDefinitions struct { - // varCache contains a cache of string variables namespace + property - // The same variable may be used in multiple module types (for example, if need support - // for cc_default and java_default), only need to process once - varCache map[string]bool - - StringVars map[string][]string + StringVars map[string]map[string]bool BoolVars map[string]bool ValueVars map[string]bool } @@ -255,7 +250,7 @@ var bp2buildSoongConfigVarsLock sync.Mutex // SoongConfigVariablesForBp2build extracts information from a // SoongConfigDefinition that bp2build needs to generate constraint settings and // values for, in order to migrate soong_config_module_type usages to Bazel. -func (defs *Bp2BuildSoongConfigDefinitions) AddVars(mtDef SoongConfigDefinition) { +func (defs *Bp2BuildSoongConfigDefinitions) AddVars(mtDef *SoongConfigDefinition) { // In bp2build mode, this method is called concurrently in goroutines from // loadhooks while parsing soong_config_module_type, so add a mutex to // prevent concurrent map writes. See b/207572723 @@ -263,7 +258,7 @@ func (defs *Bp2BuildSoongConfigDefinitions) AddVars(mtDef SoongConfigDefinition) defer bp2buildSoongConfigVarsLock.Unlock() if defs.StringVars == nil { - defs.StringVars = make(map[string][]string) + defs.StringVars = make(map[string]map[string]bool) } if defs.BoolVars == nil { defs.BoolVars = make(map[string]bool) @@ -271,24 +266,29 @@ func (defs *Bp2BuildSoongConfigDefinitions) AddVars(mtDef SoongConfigDefinition) if defs.ValueVars == nil { defs.ValueVars = make(map[string]bool) } - if defs.varCache == nil { - defs.varCache = make(map[string]bool) - } + // varCache contains a cache of string variables namespace + property + // The same variable may be used in multiple module types (for example, if need support + // for cc_default and java_default), only need to process once + varCache := map[string]bool{} + for _, moduleType := range mtDef.ModuleTypes { for _, v := range moduleType.Variables { key := strings.Join([]string{moduleType.ConfigNamespace, v.variableProperty()}, "__") // The same variable may be used in multiple module types (for example, if need support // for cc_default and java_default), only need to process once - if _, keyInCache := defs.varCache[key]; keyInCache { + if _, keyInCache := varCache[key]; keyInCache { continue } else { - defs.varCache[key] = true + varCache[key] = true } if strVar, ok := v.(*stringVariable); ok { + if _, ok := defs.StringVars[key]; !ok { + defs.StringVars[key] = make(map[string]bool, len(strVar.values)) + } for _, value := range strVar.values { - defs.StringVars[key] = append(defs.StringVars[key], value) + defs.StringVars[key][value] = true } } else if _, ok := v.(*boolVariable); ok { defs.BoolVars[key] = true @@ -301,6 +301,23 @@ func (defs *Bp2BuildSoongConfigDefinitions) AddVars(mtDef SoongConfigDefinition) } } +// This is a copy of the one available in soong/android/util.go, but depending +// on the android package causes a cyclic dependency. A refactoring here is to +// extract common utils out from android/utils.go for other packages like this. +func sortedStringKeys(m interface{}) []string { + v := reflect.ValueOf(m) + if v.Kind() != reflect.Map { + panic(fmt.Sprintf("%#v is not a map", m)) + } + keys := v.MapKeys() + s := make([]string, 0, len(keys)) + for _, key := range keys { + s = append(s, key.String()) + } + sort.Strings(s) + return s +} + // String emits the Soong config variable definitions as Starlark dictionaries. func (defs Bp2BuildSoongConfigDefinitions) String() string { ret := "" @@ -312,8 +329,13 @@ func (defs Bp2BuildSoongConfigDefinitions) String() string { ret += starlark_fmt.PrintBoolDict(defs.ValueVars, 0) ret += "\n\n" + stringVars := make(map[string][]string, len(defs.StringVars)) + for k, v := range defs.StringVars { + stringVars[k] = sortedStringKeys(v) + } + ret += "soong_config_string_variables = " - ret += starlark_fmt.PrintStringListDict(defs.StringVars, 0) + ret += starlark_fmt.PrintStringListDict(stringVars, 0) return ret } diff --git a/android/soongconfig/modules_test.go b/android/soongconfig/modules_test.go index d5d87efc0..a5fa34938 100644 --- a/android/soongconfig/modules_test.go +++ b/android/soongconfig/modules_test.go @@ -414,6 +414,110 @@ func Test_PropertiesToApply_String_Error(t *testing.T) { } } +func Test_Bp2BuildSoongConfigDefinitionsAddVars(t *testing.T) { + testCases := []struct { + desc string + defs []*SoongConfigDefinition + expected Bp2BuildSoongConfigDefinitions + }{ + { + desc: "non-overlapping", + defs: []*SoongConfigDefinition{ + &SoongConfigDefinition{ + ModuleTypes: map[string]*ModuleType{ + "a": &ModuleType{ + ConfigNamespace: "foo", + Variables: []soongConfigVariable{ + &stringVariable{ + baseVariable: baseVariable{"string_var"}, + values: []string{"a", "b", "c"}, + }, + }, + }, + }, + }, + &SoongConfigDefinition{ + ModuleTypes: map[string]*ModuleType{ + "b": &ModuleType{ + ConfigNamespace: "foo", + Variables: []soongConfigVariable{ + &stringVariable{ + baseVariable: baseVariable{"string_var"}, + values: []string{"a", "b", "c"}, + }, + &boolVariable{baseVariable: baseVariable{"bool_var"}}, + &valueVariable{baseVariable: baseVariable{"variable_var"}}, + }, + }, + }, + }, + }, + expected: Bp2BuildSoongConfigDefinitions{ + StringVars: map[string]map[string]bool{ + "foo__string_var": map[string]bool{"a": true, "b": true, "c": true}, + }, + BoolVars: map[string]bool{"foo__bool_var": true}, + ValueVars: map[string]bool{"foo__variable_var": true}, + }, + }, + { + desc: "overlapping", + defs: []*SoongConfigDefinition{ + &SoongConfigDefinition{ + ModuleTypes: map[string]*ModuleType{ + "a": &ModuleType{ + ConfigNamespace: "foo", + Variables: []soongConfigVariable{ + &stringVariable{ + baseVariable: baseVariable{"string_var"}, + values: []string{"a", "b", "c"}, + }, + }, + }, + }, + }, + &SoongConfigDefinition{ + ModuleTypes: map[string]*ModuleType{ + "b": &ModuleType{ + ConfigNamespace: "foo", + Variables: []soongConfigVariable{ + &stringVariable{ + baseVariable: baseVariable{"string_var"}, + values: []string{"b", "c", "d"}, + }, + &boolVariable{baseVariable: baseVariable{"bool_var"}}, + &valueVariable{baseVariable: baseVariable{"variable_var"}}, + }, + }, + }, + }, + }, + expected: Bp2BuildSoongConfigDefinitions{ + StringVars: map[string]map[string]bool{ + "foo__string_var": map[string]bool{"a": true, "b": true, "c": true, "d": true}, + }, + BoolVars: map[string]bool{"foo__bool_var": true}, + ValueVars: map[string]bool{"foo__variable_var": true}, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + actual := &Bp2BuildSoongConfigDefinitions{} + for _, d := range tc.defs { + func(def *SoongConfigDefinition) { + actual.AddVars(def) + }(d) + } + if !reflect.DeepEqual(*actual, tc.expected) { + t.Errorf("Expected %#v, got %#v", tc.expected, *actual) + } + }) + } + +} + func Test_Bp2BuildSoongConfigDefinitions(t *testing.T) { testCases := []struct { desc string @@ -456,11 +560,11 @@ soong_config_value_variables = { soong_config_string_variables = {}`}, { desc: "only string vars", defs: Bp2BuildSoongConfigDefinitions{ - StringVars: map[string][]string{ - "string_var": []string{ - "choice1", - "choice2", - "choice3", + StringVars: map[string]map[string]bool{ + "string_var": map[string]bool{ + "choice1": true, + "choice2": true, + "choice3": true, }, }, }, @@ -484,15 +588,15 @@ soong_config_string_variables = { "value_var_one": true, "value_var_two": true, }, - StringVars: map[string][]string{ - "string_var_one": []string{ - "choice1", - "choice2", - "choice3", + StringVars: map[string]map[string]bool{ + "string_var_one": map[string]bool{ + "choice1": true, + "choice2": true, + "choice3": true, }, - "string_var_two": []string{ - "foo", - "bar", + "string_var_two": map[string]bool{ + "foo": true, + "bar": true, }, }, }, @@ -512,8 +616,8 @@ soong_config_string_variables = { "choice3", ], "string_var_two": [ - "foo", "bar", + "foo", ], }`}, }