From 1934adc0774668d1392fa7e780957e63f938790d Mon Sep 17 00:00:00 2001 From: Dan Willemsen Date: Mon, 23 Mar 2020 19:39:34 -0700 Subject: [PATCH] soong_config: bool_variables shortcut Using a lot of boolean variables can become very verbose without adding really any new information: variables: ["a", "b", "c"], } soong_config_bool_variable { name: "a", } soong_config_bool_variable { name: "b", } soong_config_bool_variable { name: "c", } Now turns into: bool_variables: ["a", "b", "c"], } Bug: 153161144 Test: built-in tests Change-Id: If5455a38433431c7ecbce1e5b32cfbb47f42602a Merged-In: If5455a38433431c7ecbce1e5b32cfbb47f42602a (cherry picked from commit 2b8b89cfa2f3678ec01c6ef576bd9c88b1e8a248) --- README.md | 7 +-- android/soong_config_modules.go | 14 ++--- android/soong_config_modules_test.go | 7 +-- android/soongconfig/modules.go | 15 ++++++ bpfix/bpfix/bpfix.go | 76 ++++++++++++++++++++++++++++ bpfix/bpfix/bpfix_test.go | 64 +++++++++++++++++++++++ 6 files changed, 163 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index b1bb42527..3eac87b1f 100644 --- a/README.md +++ b/README.md @@ -419,7 +419,8 @@ soong_config_module_type { name: "acme_cc_defaults", module_type: "cc_defaults", config_namespace: "acme", - variables: ["board", "feature"], + variables: ["board"], + bool_variables: ["feature"], properties: ["cflags", "srcs"], } @@ -427,10 +428,6 @@ soong_config_string_variable { name: "board", values: ["soc_a", "soc_b"], } - -soong_config_bool_variable { - name: "feature", -} ``` This example describes a new `acme_cc_defaults` module type that extends the diff --git a/android/soong_config_modules.go b/android/soong_config_modules.go index 198108d65..fa1e20448 100644 --- a/android/soong_config_modules.go +++ b/android/soong_config_modules.go @@ -88,7 +88,8 @@ type soongConfigModuleTypeImportProperties struct { // name: "acme_cc_defaults", // module_type: "cc_defaults", // config_namespace: "acme", -// variables: ["board", "feature"], +// variables: ["board"], +// bool_variables: ["feature"], // properties: ["cflags", "srcs"], // } // @@ -97,10 +98,6 @@ type soongConfigModuleTypeImportProperties struct { // values: ["soc_a", "soc_b"], // } // -// soong_config_bool_variable { -// name: "feature", -// } -// // If an acme BoardConfig.mk file contained: // // SOONG_CONFIG_NAMESPACES += acme @@ -149,7 +146,8 @@ type soongConfigModuleTypeModule struct { // name: "acme_cc_defaults", // module_type: "cc_defaults", // config_namespace: "acme", -// variables: ["board", "feature"], +// variables: ["board"], +// bool_variables: ["feature"], // properties: ["cflags", "srcs"], // } // @@ -158,10 +156,6 @@ type soongConfigModuleTypeModule struct { // values: ["soc_a", "soc_b"], // } // -// soong_config_bool_variable { -// name: "feature", -// } -// // acme_cc_defaults { // name: "acme_defaults", // cflags: ["-DGENERIC"], diff --git a/android/soong_config_modules_test.go b/android/soong_config_modules_test.go index 6ad88a2ee..1cf060dc0 100644 --- a/android/soong_config_modules_test.go +++ b/android/soong_config_modules_test.go @@ -43,7 +43,8 @@ func TestSoongConfigModule(t *testing.T) { name: "acme_test_defaults", module_type: "test_defaults", config_namespace: "acme", - variables: ["board", "feature1", "feature2", "FEATURE3"], + variables: ["board", "feature1", "FEATURE3"], + bool_variables: ["feature2"], properties: ["cflags", "srcs"], } @@ -56,10 +57,6 @@ func TestSoongConfigModule(t *testing.T) { name: "feature1", } - soong_config_bool_variable { - name: "feature2", - } - soong_config_bool_variable { name: "FEATURE3", } diff --git a/android/soongconfig/modules.go b/android/soongconfig/modules.go index aa4f5c5f1..2d6063d79 100644 --- a/android/soongconfig/modules.go +++ b/android/soongconfig/modules.go @@ -109,6 +109,9 @@ type ModuleTypeProperties struct { // the list of SOONG_CONFIG variables that this module type will read Variables []string + // the list of boolean SOONG_CONFIG variables that this module type will read + Bool_variables []string + // the list of properties that this module type will extend. Properties []string } @@ -146,6 +149,18 @@ func processModuleTypeDef(v *SoongConfigDefinition, def *parser.Module) (errs [] } v.ModuleTypes[props.Name] = mt + for _, name := range props.Bool_variables { + if name == "" { + return []error{fmt.Errorf("bool_variable name must not be blank")} + } + + mt.Variables = append(mt.Variables, &boolVariable{ + baseVariable: baseVariable{ + variable: name, + }, + }) + } + return nil } diff --git a/bpfix/bpfix/bpfix.go b/bpfix/bpfix/bpfix.go index 051627977..a1c5de122 100644 --- a/bpfix/bpfix/bpfix.go +++ b/bpfix/bpfix/bpfix.go @@ -124,6 +124,10 @@ var fixSteps = []FixStep{ Name: "removeHidlInterfaceTypes", Fix: removeHidlInterfaceTypes, }, + { + Name: "removeSoongConfigBoolVariable", + Fix: removeSoongConfigBoolVariable, + }, } func NewFixRequest() FixRequest { @@ -714,6 +718,78 @@ func removeHidlInterfaceTypes(f *Fixer) error { return nil } +func removeSoongConfigBoolVariable(f *Fixer) error { + found := map[string]bool{} + newDefs := make([]parser.Definition, 0, len(f.tree.Defs)) + for _, def := range f.tree.Defs { + if mod, ok := def.(*parser.Module); ok && mod.Type == "soong_config_bool_variable" { + if name, ok := getLiteralStringPropertyValue(mod, "name"); ok { + found[name] = true + } else { + return fmt.Errorf("Found soong_config_bool_variable without a name") + } + } else { + newDefs = append(newDefs, def) + } + } + f.tree.Defs = newDefs + + if len(found) == 0 { + return nil + } + + return runPatchListMod(func(mod *parser.Module, buf []byte, patchList *parser.PatchList) error { + if mod.Type != "soong_config_module_type" { + return nil + } + + variables, ok := getLiteralListProperty(mod, "variables") + if !ok { + return nil + } + + boolValues := strings.Builder{} + empty := true + for _, item := range variables.Values { + nameValue, ok := item.(*parser.String) + if !ok { + empty = false + continue + } + if found[nameValue.Value] { + patchList.Add(item.Pos().Offset, item.End().Offset+2, "") + + boolValues.WriteString(`"`) + boolValues.WriteString(nameValue.Value) + boolValues.WriteString(`",`) + } else { + empty = false + } + } + if empty { + *patchList = parser.PatchList{} + + prop, _ := mod.GetProperty("variables") + patchList.Add(prop.Pos().Offset, prop.End().Offset+2, "") + } + if boolValues.Len() == 0 { + return nil + } + + bool_variables, ok := getLiteralListProperty(mod, "bool_variables") + if ok { + patchList.Add(bool_variables.RBracePos.Offset, bool_variables.RBracePos.Offset, ","+boolValues.String()) + } else { + patchList.Add(variables.RBracePos.Offset+2, variables.RBracePos.Offset+2, + fmt.Sprintf(`bool_variables: [%s],`, boolValues.String())) + } + + return nil + })(f) + + return nil +} + // Converts the default source list property, 'srcs', to a single source property with a given name. // "LOCAL_MODULE" reference is also resolved during the conversion process. func convertToSingleSource(mod *parser.Module, srcPropertyName string) { diff --git a/bpfix/bpfix/bpfix_test.go b/bpfix/bpfix/bpfix_test.go index 38cefdd6b..64a7b93b2 100644 --- a/bpfix/bpfix/bpfix_test.go +++ b/bpfix/bpfix/bpfix_test.go @@ -918,3 +918,67 @@ func TestRemoveHidlInterfaceTypes(t *testing.T) { }) } } + +func TestRemoveSoongConfigBoolVariable(t *testing.T) { + tests := []struct { + name string + in string + out string + }{ + { + name: "remove bool", + in: ` + soong_config_module_type { + name: "foo", + variables: ["bar", "baz"], + } + + soong_config_bool_variable { + name: "bar", + } + + soong_config_string_variable { + name: "baz", + } + `, + out: ` + soong_config_module_type { + name: "foo", + variables: [ + "baz" + ], + bool_variables: ["bar"], + } + + soong_config_string_variable { + name: "baz", + } + `, + }, + { + name: "existing bool_variables", + in: ` + soong_config_module_type { + name: "foo", + variables: ["baz"], + bool_variables: ["bar"], + } + + soong_config_bool_variable { + name: "baz", + } + `, + out: ` + soong_config_module_type { + name: "foo", + bool_variables: ["bar", "baz"], + } + `, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + runPass(t, test.in, test.out, removeSoongConfigBoolVariable) + }) + } +}