From b0935db8c3838e90582fdece217bc956ba951f07 Mon Sep 17 00:00:00 2001 From: Dan Willemsen Date: Mon, 23 Mar 2020 19:42:18 -0700 Subject: [PATCH] soong config: add value_variable substitution There are some cases that aren't handled with the existing variable types for booleans or known lists of strings. Similarly to our product_variables that uses %s / %d for things like PLATFORM_SDK_VERSION, allow vendors to define their own config variables to be substituted into properties. For example, some of the makefiles that I've attempted to convert had the option to pass in version numbers from the board, or the default display size: -DDISPLAY_VERSION=550 -DDISP_H=1080 These examples happen to be integers, but since our configuration language (make) doesn't support numbers, %s works just as well. This change will allow the above to be represented using: soong_config_module_type { name: "acme_cc_defaults", module_type: "cc_defaults", config_namespace: "acme", value_variables: [ "DISPLAY_VERSION", "DISP_H", ], properties: ["cflags"], } acme_cc_defaults { name: "my_defaults", soong_config_variables: { DISPLAY_VERSION: { cflags: ["-DDISPLAY_VERSION=%s"], }, DISP_H: { cflags: ["-DDISP_H=%s"], } }, } Test: built-in tests Change-Id: I18f35746b5cc39c304a136980249e886d38c6df6 --- README.md | 12 ++- android/soong_config_modules.go | 19 ++++- android/soong_config_modules_test.go | 7 +- android/soongconfig/modules.go | 108 ++++++++++++++++++++++++--- 4 files changed, 131 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 3eac87b1f..8b028a81a 100644 --- a/README.md +++ b/README.md @@ -421,6 +421,7 @@ soong_config_module_type { config_namespace: "acme", variables: ["board"], bool_variables: ["feature"], + value_variables: ["width"], properties: ["cflags", "srcs"], } @@ -431,8 +432,9 @@ soong_config_string_variable { ``` This example describes a new `acme_cc_defaults` module type that extends the -`cc_defaults` module type, with two additional conditionals based on variables -`board` and `feature`, which can affect properties `cflags` and `srcs`. +`cc_defaults` module type, with three additional conditionals based on +variables `board`, `feature` and `width`, which can affect properties `cflags` +and `srcs`. The values of the variables can be set from a product's `BoardConfig.mk` file: ``` @@ -443,6 +445,7 @@ SOONG_CONFIG_acme += \ SOONG_CONFIG_acme_board := soc_a SOONG_CONFIG_acme_feature := true +SOONG_CONFIG_acme_width := 200 ``` The `acme_cc_defaults` module type can be used anywhere after the definition in @@ -471,6 +474,9 @@ acme_cc_defaults { feature: { cflags: ["-DFEATURE"], }, + width: { + cflags: ["-DWIDTH=%s"], + }, }, } @@ -482,7 +488,7 @@ cc_library { ``` With the `BoardConfig.mk` snippet above, libacme_foo would build with -cflags "-DGENERIC -DSOC_A -DFEATURE". +cflags "-DGENERIC -DSOC_A -DFEATURE -DWIDTH=200". `soong_config_module_type` modules will work best when used to wrap defaults modules (`cc_defaults`, `java_defaults`, etc.), which can then be referenced diff --git a/android/soong_config_modules.go b/android/soong_config_modules.go index fa1e20448..8126dee18 100644 --- a/android/soong_config_modules.go +++ b/android/soong_config_modules.go @@ -73,6 +73,9 @@ type soongConfigModuleTypeImportProperties struct { // feature: { // cflags: ["-DFEATURE"], // }, +// width: { +// cflags: ["-DWIDTH=%s"], +// }, // }, // } // @@ -90,6 +93,7 @@ type soongConfigModuleTypeImportProperties struct { // config_namespace: "acme", // variables: ["board"], // bool_variables: ["feature"], +// value_variables: ["width"], // properties: ["cflags", "srcs"], // } // @@ -107,8 +111,9 @@ type soongConfigModuleTypeImportProperties struct { // // SOONG_CONFIG_acme_board := soc_a // SOONG_CONFIG_acme_feature := true +// SOONG_CONFIG_acme_width := 200 // -// Then libacme_foo would build with cflags "-DGENERIC -DSOC_A -DFEATURE". +// Then libacme_foo would build with cflags "-DGENERIC -DSOC_A -DFEATURE -DWIDTH=200". func soongConfigModuleTypeImportFactory() Module { module := &soongConfigModuleTypeImport{} @@ -148,6 +153,7 @@ type soongConfigModuleTypeModule struct { // config_namespace: "acme", // variables: ["board"], // bool_variables: ["feature"], +// value_variables: ["width"], // properties: ["cflags", "srcs"], // } // @@ -171,6 +177,9 @@ type soongConfigModuleTypeModule struct { // feature: { // cflags: ["-DFEATURE"], // }, +// width: { +// cflags: ["-DWIDTH=%s"], +// }, // }, // } // @@ -189,6 +198,7 @@ type soongConfigModuleTypeModule struct { // // SOONG_CONFIG_acme_board := soc_a // SOONG_CONFIG_acme_feature := true +// SOONG_CONFIG_acme_width := 200 // // Then libacme_foo would build with cflags "-DGENERIC -DSOC_A -DFEATURE". func soongConfigModuleTypeFactory() Module { @@ -349,7 +359,12 @@ func soongConfigModuleFactory(factory blueprint.ModuleFactory, AddLoadHook(module, func(ctx LoadHookContext) { config := ctx.Config().VendorConfig(moduleType.ConfigNamespace) - for _, ps := range soongconfig.PropertiesToApply(moduleType, conditionalProps, config) { + newProps, err := soongconfig.PropertiesToApply(moduleType, conditionalProps, config) + if err != nil { + ctx.ModuleErrorf("%s", err) + return + } + for _, ps := range newProps { ctx.AppendProperties(ps) } }) diff --git a/android/soong_config_modules_test.go b/android/soong_config_modules_test.go index 1cf060dc0..f905b1ab2 100644 --- a/android/soong_config_modules_test.go +++ b/android/soong_config_modules_test.go @@ -45,6 +45,7 @@ func TestSoongConfigModule(t *testing.T) { config_namespace: "acme", variables: ["board", "feature1", "FEATURE3"], bool_variables: ["feature2"], + value_variables: ["size"], properties: ["cflags", "srcs"], } @@ -82,6 +83,9 @@ func TestSoongConfigModule(t *testing.T) { cflags: ["-DSOC_B"], }, }, + size: { + cflags: ["-DSIZE=%s"], + }, feature1: { cflags: ["-DFEATURE1"], }, @@ -101,6 +105,7 @@ func TestSoongConfigModule(t *testing.T) { config.TestProductVariables.VendorVars = map[string]map[string]string{ "acme": map[string]string{ "board": "soc_a", + "size": "42", "feature1": "true", "feature2": "false", // FEATURE3 unset @@ -121,7 +126,7 @@ func TestSoongConfigModule(t *testing.T) { FailIfErrored(t, errs) foo := ctx.ModuleForTests("foo", "").Module().(*soongConfigTestModule) - if g, w := foo.props.Cflags, []string{"-DGENERIC", "-DSOC_A", "-DFEATURE1"}; !reflect.DeepEqual(g, w) { + if g, w := foo.props.Cflags, []string{"-DGENERIC", "-DSIZE=42", "-DSOC_A", "-DFEATURE1"}; !reflect.DeepEqual(g, w) { t.Errorf("wanted foo cflags %q, got %q", w, g) } } diff --git a/android/soongconfig/modules.go b/android/soongconfig/modules.go index 2d6063d79..142a81387 100644 --- a/android/soongconfig/modules.go +++ b/android/soongconfig/modules.go @@ -112,6 +112,10 @@ type ModuleTypeProperties struct { // the list of boolean SOONG_CONFIG variables that this module type will read Bool_variables []string + // the list of SOONG_CONFIG variables that this module type will read. The value will be + // inserted into the properties with %s substitution. + Value_variables []string + // the list of properties that this module type will extend. Properties []string } @@ -161,6 +165,18 @@ func processModuleTypeDef(v *SoongConfigDefinition, def *parser.Module) (errs [] }) } + for _, name := range props.Value_variables { + if name == "" { + return []error{fmt.Errorf("value_variables entry must not be blank")} + } + + mt.Variables = append(mt.Variables, &valueVariable{ + baseVariable: baseVariable{ + variable: name, + }, + }) + } + return nil } @@ -404,15 +420,17 @@ func typeForPropertyFromPropertyStruct(ps interface{}, property string) reflect. // PropertiesToApply returns the applicable properties from a ModuleType that should be applied // based on SoongConfig values. -func PropertiesToApply(moduleType *ModuleType, props reflect.Value, config SoongConfig) []interface{} { +func PropertiesToApply(moduleType *ModuleType, props reflect.Value, config SoongConfig) ([]interface{}, error) { var ret []interface{} props = props.Elem().FieldByName(soongConfigProperty) for i, c := range moduleType.Variables { - if ps := c.PropertiesToApply(config, props.Field(i)); ps != nil { + if ps, err := c.PropertiesToApply(config, props.Field(i)); err != nil { + return nil, err + } else if ps != nil { ret = append(ret, ps) } } - return ret + return ret, nil } type ModuleType struct { @@ -438,7 +456,7 @@ type soongConfigVariable interface { // PropertiesToApply should return one of the interface{} values set by initializeProperties to be applied // to the module. - PropertiesToApply(config SoongConfig, values reflect.Value) interface{} + PropertiesToApply(config SoongConfig, values reflect.Value) (interface{}, error) } type baseVariable struct { @@ -473,14 +491,14 @@ func (s *stringVariable) initializeProperties(v reflect.Value, typ reflect.Type) } } -func (s *stringVariable) PropertiesToApply(config SoongConfig, values reflect.Value) interface{} { +func (s *stringVariable) PropertiesToApply(config SoongConfig, values reflect.Value) (interface{}, error) { for j, v := range s.values { if config.String(s.variable) == v { - return values.Field(j).Interface() + return values.Field(j).Interface(), nil } } - return nil + return nil, nil } type boolVariable struct { @@ -495,11 +513,83 @@ func (b boolVariable) initializeProperties(v reflect.Value, typ reflect.Type) { v.Set(reflect.Zero(typ)) } -func (b boolVariable) PropertiesToApply(config SoongConfig, values reflect.Value) interface{} { +func (b boolVariable) PropertiesToApply(config SoongConfig, values reflect.Value) (interface{}, error) { if config.Bool(b.variable) { - return values.Interface() + return values.Interface(), nil } + return nil, nil +} + +type valueVariable struct { + baseVariable +} + +func (s *valueVariable) variableValuesType() reflect.Type { + return emptyInterfaceType +} + +func (s *valueVariable) initializeProperties(v reflect.Value, typ reflect.Type) { + v.Set(reflect.Zero(typ)) +} + +func (s *valueVariable) PropertiesToApply(config SoongConfig, values reflect.Value) (interface{}, error) { + if !config.IsSet(s.variable) { + return nil, nil + } + configValue := config.String(s.variable) + + propStruct := values.Elem().Elem() + for i := 0; i < propStruct.NumField(); i++ { + field := propStruct.Field(i) + kind := field.Kind() + if kind == reflect.Ptr { + if field.IsNil() { + continue + } + field = field.Elem() + } + switch kind { + case reflect.String: + err := printfIntoProperty(field, configValue) + if err != nil { + return nil, fmt.Errorf("soong_config_variables.%s.%s: %s", s.variable, propStruct.Type().Field(i).Name, err) + } + case reflect.Slice: + for j := 0; j < field.Len(); j++ { + err := printfIntoProperty(field.Index(j), configValue) + if err != nil { + return nil, fmt.Errorf("soong_config_variables.%s.%s: %s", s.variable, propStruct.Type().Field(i).Name, err) + } + } + case reflect.Bool: + // Nothing to do + default: + return nil, fmt.Errorf("soong_config_variables.%s.%s: unsupported property type %q", s.variable, propStruct.Type().Field(i).Name, kind) + } + } + + return values.Interface(), nil +} + +func printfIntoProperty(propertyValue reflect.Value, configValue string) error { + s := propertyValue.String() + + count := strings.Count(s, "%") + if count == 0 { + return nil + } + + if count > 1 { + return fmt.Errorf("value variable properties only support a single '%%'") + } + + if !strings.Contains(s, "%s") { + return fmt.Errorf("unsupported %% in value variable property") + } + + propertyValue.Set(reflect.ValueOf(fmt.Sprintf(s, configValue))) + return nil }