Skip to content

Commit

Permalink
soong_config: bool_variables shortcut
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
danw committed Apr 3, 2020
1 parent b92ae27 commit 1934adc
Show file tree
Hide file tree
Showing 6 changed files with 163 additions and 20 deletions.
7 changes: 2 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -419,18 +419,15 @@ 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"],
}
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
Expand Down
14 changes: 4 additions & 10 deletions android/soong_config_modules.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
// }
//
Expand All @@ -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
Expand Down Expand Up @@ -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"],
// }
//
Expand All @@ -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"],
Expand Down
7 changes: 2 additions & 5 deletions android/soong_config_modules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
}
Expand All @@ -56,10 +57,6 @@ func TestSoongConfigModule(t *testing.T) {
name: "feature1",
}
soong_config_bool_variable {
name: "feature2",
}
soong_config_bool_variable {
name: "FEATURE3",
}
Expand Down
15 changes: 15 additions & 0 deletions android/soongconfig/modules.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}

Expand Down
76 changes: 76 additions & 0 deletions bpfix/bpfix/bpfix.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ var fixSteps = []FixStep{
Name: "removeHidlInterfaceTypes",
Fix: removeHidlInterfaceTypes,
},
{
Name: "removeSoongConfigBoolVariable",
Fix: removeSoongConfigBoolVariable,
},
}

func NewFixRequest() FixRequest {
Expand Down Expand Up @@ -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) {
Expand Down
64 changes: 64 additions & 0 deletions bpfix/bpfix/bpfix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}

0 comments on commit 1934adc

Please sign in to comment.