From 432bd598ae5964f045e92aa29bc1cbef3dea04dd Mon Sep 17 00:00:00 2001 From: Liz Kammer Date: Wed, 16 Dec 2020 12:42:02 -0800 Subject: [PATCH] Add conditions_default for soong config variables. Each variable can specify a conditions_default for properties to be used when the variable is not set, not set to a true value (for bools), or is set to a value that is not present in the module (for strings). Test: m nothing Test: go test soong tests Change-Id: I76ec026da2369b407f0f530f77760f530e7958fc --- README.md | 57 +++++++- android/soong_config_modules.go | 65 ++++++++- android/soong_config_modules_test.go | 172 +++++++++++++++++++----- android/soongconfig/Android.bp | 3 + android/soongconfig/config.go | 16 ++- android/soongconfig/modules.go | 193 +++++++++++++++++++++------ android/soongconfig/modules_test.go | 78 ++++++----- 7 files changed, 468 insertions(+), 116 deletions(-) diff --git a/README.md b/README.md index f1857f872..b7e93f431 100644 --- a/README.md +++ b/README.md @@ -430,14 +430,24 @@ soong_config_module_type { soong_config_string_variable { name: "board", - values: ["soc_a", "soc_b"], + values: ["soc_a", "soc_b", "soc_c"], } ``` This example describes a new `acme_cc_defaults` module type that extends the `cc_defaults` module type, with three additional conditionals based on variables `board`, `feature` and `width`, which can affect properties `cflags` -and `srcs`. +and `srcs`. Additionally, each conditional will contain a `conditions_default` +property can affect `cflags` and `srcs` in the following conditions: + +* bool variable (e.g. `feature`): the variable is unspecified or not set to a true value +* value variable (e.g. `width`): the variable is unspecified +* string variable (e.g. `board`): the variable is unspecified or the variable is set to a string unused in the +given module. For example, with `board`, if the `board` +conditional contains the properties `soc_a` and `conditions_default`, when +board=soc_b, the `cflags` and `srcs` values under `conditions_default` will be +used. To specify that no properties should be amended for `soc_b`, you can set +`soc_b: {},`. The values of the variables can be set from a product's `BoardConfig.mk` file: ``` @@ -445,6 +455,7 @@ SOONG_CONFIG_NAMESPACES += acme SOONG_CONFIG_acme += \ board \ feature \ + width \ SOONG_CONFIG_acme_board := soc_a SOONG_CONFIG_acme_feature := true @@ -473,12 +484,21 @@ acme_cc_defaults { soc_b: { cflags: ["-DSOC_B"], }, + conditions_default: { + cflags: ["-DSOC_DEFAULT"], + }, }, feature: { cflags: ["-DFEATURE"], + conditions_default: { + cflags: ["-DFEATURE_DEFAULT"], + }, }, width: { cflags: ["-DWIDTH=%s"], + conditions_default: { + cflags: ["-DWIDTH=DEFAULT"], + }, }, }, } @@ -490,8 +510,37 @@ cc_library { } ``` -With the `BoardConfig.mk` snippet above, libacme_foo would build with -cflags "-DGENERIC -DSOC_A -DFEATURE -DWIDTH=200". +With the `BoardConfig.mk` snippet above, `libacme_foo` would build with +`cflags: "-DGENERIC -DSOC_A -DFEATURE -DWIDTH=200"`. + +Alternatively, with `DefaultBoardConfig.mk`: + +``` +SOONG_CONFIG_NAMESPACES += acme +SOONG_CONFIG_acme += \ + board \ + feature \ + width \ + +SOONG_CONFIG_acme_feature := false +``` + +then `libacme_foo` would build with `cflags: "-DGENERIC -DSOC_DEFAULT -DFEATURE_DEFAULT -DSIZE=DEFAULT"`. + +Alternatively, with `DefaultBoardConfig.mk`: + +``` +SOONG_CONFIG_NAMESPACES += acme +SOONG_CONFIG_acme += \ + board \ + feature \ + width \ + +SOONG_CONFIG_acme_board := soc_c +``` + +then `libacme_foo` would build with `cflags: "-DGENERIC -DSOC_DEFAULT +-DFEATURE_DEFAULT -DSIZE=DEFAULT"`. `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 619cf8615..289e91088 100644 --- a/android/soong_config_modules.go +++ b/android/soong_config_modules.go @@ -51,6 +51,16 @@ type soongConfigModuleTypeImportProperties struct { // variables from another Android.bp file. The imported module type will exist for all // modules after the import in the Android.bp file. // +// Each soong_config_variable supports an additional value `conditions_default`. The properties +// specified in `conditions_default` will only be used under the following conditions: +// bool variable: the variable is unspecified or not set to a true value +// value variable: the variable is unspecified +// string variable: the variable is unspecified or the variable is set to a string unused in the +// given module. For example, string variable `test` takes values: "a" and "b", +// if the module contains a property `a` and `conditions_default`, when test=b, +// the properties under `conditions_default` will be used. To specify that no +// properties should be amended for `b`, you can set `b: {},`. +// // For example, an Android.bp file could have: // // soong_config_module_type_import { @@ -69,12 +79,21 @@ type soongConfigModuleTypeImportProperties struct { // soc_b: { // cflags: ["-DSOC_B"], // }, +// conditions_default: { +// cflags: ["-DSOC_DEFAULT"], +// }, // }, // feature: { // cflags: ["-DFEATURE"], +// conditions_default: { +// cflags: ["-DFEATURE_DEFAULT"], +// }, // }, // width: { // cflags: ["-DWIDTH=%s"], +// conditions_default: { +// cflags: ["-DWIDTH=DEFAULT"], +// }, // }, // }, // } @@ -99,7 +118,7 @@ type soongConfigModuleTypeImportProperties struct { // // soong_config_string_variable { // name: "board", -// values: ["soc_a", "soc_b"], +// values: ["soc_a", "soc_b", "soc_c"], // } // // If an acme BoardConfig.mk file contained: @@ -114,6 +133,31 @@ type soongConfigModuleTypeImportProperties struct { // SOONG_CONFIG_acme_width := 200 // // Then libacme_foo would build with cflags "-DGENERIC -DSOC_A -DFEATURE -DWIDTH=200". +// +// Alternatively, if acme BoardConfig.mk file contained: +// +// SOONG_CONFIG_NAMESPACES += acme +// SOONG_CONFIG_acme += \ +// board \ +// feature \ +// +// SOONG_CONFIG_acme_feature := false +// +// Then libacme_foo would build with cflags: +// "-DGENERIC -DSOC_DEFAULT -DFEATURE_DEFAULT -DSIZE=DEFAULT". +// +// Similarly, if acme BoardConfig.mk file contained: +// +// SOONG_CONFIG_NAMESPACES += acme +// SOONG_CONFIG_acme += \ +// board \ +// feature \ +// +// SOONG_CONFIG_acme_board := soc_c +// +// Then libacme_foo would build with cflags: +// "-DGENERIC -DSOC_DEFAULT -DFEATURE_DEFAULT -DSIZE=DEFAULT". + func soongConfigModuleTypeImportFactory() Module { module := &soongConfigModuleTypeImport{} @@ -148,6 +192,16 @@ type soongConfigModuleTypeModule struct { // in an Android.bp file, and can be imported into other Android.bp files using // soong_config_module_type_import. // +// Each soong_config_variable supports an additional value `conditions_default`. The properties +// specified in `conditions_default` will only be used under the following conditions: +// bool variable: the variable is unspecified or not set to a true value +// value variable: the variable is unspecified +// string variable: the variable is unspecified or the variable is set to a string unused in the +// given module. For example, string variable `test` takes values: "a" and "b", +// if the module contains a property `a` and `conditions_default`, when test=b, +// the properties under `conditions_default` will be used. To specify that no +// properties should be amended for `b`, you can set `b: {},`. +// // For example, an Android.bp file could have: // // soong_config_module_type { @@ -176,12 +230,21 @@ type soongConfigModuleTypeModule struct { // soc_b: { // cflags: ["-DSOC_B"], // }, +// conditions_default: { +// cflags: ["-DSOC_DEFAULT"], +// }, // }, // feature: { // cflags: ["-DFEATURE"], +// conditions_default: { +// cflags: ["-DFEATURE_DEFAULT"], +// }, // }, // width: { // cflags: ["-DWIDTH=%s"], +// conditions_default: { +// cflags: ["-DWIDTH=DEFAULT"], +// }, // }, // }, // } diff --git a/android/soong_config_modules_test.go b/android/soong_config_modules_test.go index b1810b305..45463fd31 100644 --- a/android/soong_config_modules_test.go +++ b/android/soong_config_modules_test.go @@ -60,15 +60,20 @@ func TestSoongConfigModule(t *testing.T) { name: "acme_test", module_type: "test", config_namespace: "acme", - variables: ["board", "feature1", "FEATURE3"], - bool_variables: ["feature2"], - value_variables: ["size"], + variables: ["board", "feature1", "FEATURE3", "unused_string_var"], + bool_variables: ["feature2", "unused_feature"], + value_variables: ["size", "unused_size"], properties: ["cflags", "srcs", "defaults"], } soong_config_string_variable { name: "board", - values: ["soc_a", "soc_b"], + values: ["soc_a", "soc_b", "soc_c", "soc_d"], + } + + soong_config_string_variable { + name: "unused_string_var", + values: ["a", "b"], } soong_config_bool_variable { @@ -105,15 +110,28 @@ func TestSoongConfigModule(t *testing.T) { soc_b: { cflags: ["-DSOC_B"], }, + soc_c: {}, + conditions_default: { + cflags: ["-DSOC_CONDITIONS_DEFAULT"], + }, }, size: { cflags: ["-DSIZE=%s"], + conditions_default: { + cflags: ["-DSIZE=CONDITIONS_DEFAULT"], + }, }, feature1: { + conditions_default: { + cflags: ["-DF1_CONDITIONS_DEFAULT"], + }, cflags: ["-DFEATURE1"], }, feature2: { cflags: ["-DFEATURE2"], + conditions_default: { + cflags: ["-DF2_CONDITIONS_DEFAULT"], + }, }, FEATURE3: { cflags: ["-DFEATURE3"], @@ -145,6 +163,7 @@ func TestSoongConfigModule(t *testing.T) { cflags: ["-DSOC_B"], defaults: ["foo_defaults_b"], }, + soc_c: {}, }, size: { cflags: ["-DSIZE=%s"], @@ -163,43 +182,120 @@ func TestSoongConfigModule(t *testing.T) { ` run := func(t *testing.T, bp string, fs map[string][]byte) { - config := TestConfig(buildDir, nil, bp, fs) + testCases := []struct { + name string + config Config + fooExpectedFlags []string + fooDefaultsExpectedFlags []string + }{ + { + name: "withValues", + config: testConfigWithVendorVars(buildDir, bp, fs, map[string]map[string]string{ + "acme": map[string]string{ + "board": "soc_a", + "size": "42", + "feature1": "true", + "feature2": "false", + // FEATURE3 unset + "unused_feature": "true", // unused + "unused_size": "1", // unused + "unused_string_var": "a", // unused + }, + }), + fooExpectedFlags: []string{ + "DEFAULT", + "-DGENERIC", + "-DF2_CONDITIONS_DEFAULT", + "-DSIZE=42", + "-DSOC_A", + "-DFEATURE1", + }, + fooDefaultsExpectedFlags: []string{ + "DEFAULT_A", + "DEFAULT", + "-DGENERIC", + "-DSIZE=42", + "-DSOC_A", + "-DFEATURE1", + }, + }, + { + name: "empty_prop_for_string_var", + config: testConfigWithVendorVars(buildDir, bp, fs, map[string]map[string]string{ + "acme": map[string]string{"board": "soc_c"}}), + fooExpectedFlags: []string{ + "DEFAULT", + "-DGENERIC", + "-DF2_CONDITIONS_DEFAULT", + "-DSIZE=CONDITIONS_DEFAULT", + "-DF1_CONDITIONS_DEFAULT", + }, + fooDefaultsExpectedFlags: []string{ + "DEFAULT", + "-DGENERIC", + }, + }, + { + name: "unused_string_var", + config: testConfigWithVendorVars(buildDir, bp, fs, map[string]map[string]string{ + "acme": map[string]string{"board": "soc_d"}}), + fooExpectedFlags: []string{ + "DEFAULT", + "-DGENERIC", + "-DF2_CONDITIONS_DEFAULT", + "-DSIZE=CONDITIONS_DEFAULT", + "-DSOC_CONDITIONS_DEFAULT", // foo does not contain a prop "soc_d", so we use the default + "-DF1_CONDITIONS_DEFAULT", + }, + fooDefaultsExpectedFlags: []string{ + "DEFAULT", + "-DGENERIC", + }, + }, - config.TestProductVariables.VendorVars = map[string]map[string]string{ - "acme": map[string]string{ - "board": "soc_a", - "size": "42", - "feature1": "true", - "feature2": "false", - // FEATURE3 unset + { + name: "conditions_default", + config: testConfigWithVendorVars(buildDir, bp, fs, map[string]map[string]string{}), + fooExpectedFlags: []string{ + "DEFAULT", + "-DGENERIC", + "-DF2_CONDITIONS_DEFAULT", + "-DSIZE=CONDITIONS_DEFAULT", + "-DSOC_CONDITIONS_DEFAULT", + "-DF1_CONDITIONS_DEFAULT", + }, + fooDefaultsExpectedFlags: []string{ + "DEFAULT", + "-DGENERIC", + }, }, } - ctx := NewTestContext(config) - ctx.RegisterModuleType("soong_config_module_type_import", soongConfigModuleTypeImportFactory) - ctx.RegisterModuleType("soong_config_module_type", soongConfigModuleTypeFactory) - ctx.RegisterModuleType("soong_config_string_variable", soongConfigStringVariableDummyFactory) - ctx.RegisterModuleType("soong_config_bool_variable", soongConfigBoolVariableDummyFactory) - ctx.RegisterModuleType("test_defaults", soongConfigTestDefaultsModuleFactory) - ctx.RegisterModuleType("test", soongConfigTestModuleFactory) - ctx.PreArchMutators(RegisterDefaultsPreArchMutators) - ctx.Register() + for _, tc := range testCases { + ctx := NewTestContext(tc.config) + ctx.RegisterModuleType("soong_config_module_type_import", soongConfigModuleTypeImportFactory) + ctx.RegisterModuleType("soong_config_module_type", soongConfigModuleTypeFactory) + ctx.RegisterModuleType("soong_config_string_variable", soongConfigStringVariableDummyFactory) + ctx.RegisterModuleType("soong_config_bool_variable", soongConfigBoolVariableDummyFactory) + ctx.RegisterModuleType("test_defaults", soongConfigTestDefaultsModuleFactory) + ctx.RegisterModuleType("test", soongConfigTestModuleFactory) + ctx.PreArchMutators(RegisterDefaultsPreArchMutators) + ctx.Register() - _, errs := ctx.ParseBlueprintsFiles("Android.bp") - FailIfErrored(t, errs) - _, errs = ctx.PrepareBuildActions(config) - FailIfErrored(t, errs) + _, errs := ctx.ParseBlueprintsFiles("Android.bp") + FailIfErrored(t, errs) + _, errs = ctx.PrepareBuildActions(tc.config) + FailIfErrored(t, errs) - basicCFlags := []string{"DEFAULT", "-DGENERIC", "-DSIZE=42", "-DSOC_A", "-DFEATURE1"} + foo := ctx.ModuleForTests("foo", "").Module().(*soongConfigTestModule) + if g, w := foo.props.Cflags, tc.fooExpectedFlags; !reflect.DeepEqual(g, w) { + t.Errorf("%s: wanted foo cflags %q, got %q", tc.name, w, g) + } - foo := ctx.ModuleForTests("foo", "").Module().(*soongConfigTestModule) - if g, w := foo.props.Cflags, basicCFlags; !reflect.DeepEqual(g, w) { - t.Errorf("wanted foo cflags %q, got %q", w, g) - } - - fooDefaults := ctx.ModuleForTests("foo_with_defaults", "").Module().(*soongConfigTestModule) - if g, w := fooDefaults.props.Cflags, append([]string{"DEFAULT_A"}, basicCFlags...); !reflect.DeepEqual(g, w) { - t.Errorf("wanted foo_with_defaults cflags %q, got %q", w, g) + fooDefaults := ctx.ModuleForTests("foo_with_defaults", "").Module().(*soongConfigTestModule) + if g, w := fooDefaults.props.Cflags, tc.fooDefaultsExpectedFlags; !reflect.DeepEqual(g, w) { + t.Errorf("%s: wanted foo_with_defaults cflags %q, got %q", tc.name, w, g) + } } } @@ -214,3 +310,11 @@ func TestSoongConfigModule(t *testing.T) { }) }) } + +func testConfigWithVendorVars(buildDir, bp string, fs map[string][]byte, vendorVars map[string]map[string]string) Config { + config := TestConfig(buildDir, nil, bp, fs) + + config.TestProductVariables.VendorVars = vendorVars + + return config +} diff --git a/android/soongconfig/Android.bp b/android/soongconfig/Android.bp index df912e62e..6bb68eb2d 100644 --- a/android/soongconfig/Android.bp +++ b/android/soongconfig/Android.bp @@ -10,4 +10,7 @@ bootstrap_go_package { "config.go", "modules.go", ], + testSrcs: [ + "modules_test.go", + ], } diff --git a/android/soongconfig/config.go b/android/soongconfig/config.go index 39a776c6d..c72da2f84 100644 --- a/android/soongconfig/config.go +++ b/android/soongconfig/config.go @@ -14,7 +14,10 @@ package soongconfig -import "strings" +import ( + "fmt" + "strings" +) type SoongConfig interface { // Bool interprets the variable named `name` as a boolean, returning true if, after @@ -31,7 +34,16 @@ type SoongConfig interface { } func Config(vars map[string]string) SoongConfig { - return soongConfig(vars) + configVars := make(map[string]string) + if len(vars) > 0 { + for k, v := range vars { + configVars[k] = v + } + if _, exists := configVars[conditionsDefault]; exists { + panic(fmt.Sprintf("%q is a reserved soong config variable name", conditionsDefault)) + } + } + return soongConfig(configVars) } type soongConfig map[string]string diff --git a/android/soongconfig/modules.go b/android/soongconfig/modules.go index 9f3f804ef..c62e76dd6 100644 --- a/android/soongconfig/modules.go +++ b/android/soongconfig/modules.go @@ -26,6 +26,8 @@ import ( "github.com/google/blueprint/proptools" ) +const conditionsDefault = "conditions_default" + var soongConfigProperty = proptools.FieldNameForProperty("soong_config_variables") // loadSoongConfigModuleTypeDefinition loads module types from an Android.bp file. It caches the @@ -145,32 +147,10 @@ func processModuleTypeDef(v *SoongConfigDefinition, def *parser.Module) (errs [] return errs } - mt := &ModuleType{ - affectableProperties: props.Properties, - ConfigNamespace: props.Config_namespace, - BaseModuleType: props.Module_type, - variableNames: props.Variables, - } - 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, newBoolVariable(name)) - } - - 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, - }, - }) + if mt, errs := newModuleType(props); len(errs) > 0 { + return errs + } else { + v.ModuleTypes[props.Name] = mt } return nil @@ -196,6 +176,12 @@ func processStringVariableDef(v *SoongConfigDefinition, def *parser.Module) (err return []error{fmt.Errorf("values property must be set")} } + for _, name := range stringProps.Values { + if err := checkVariableName(name); err != nil { + return []error{fmt.Errorf("soong_config_string_variable: values property error %s", err)} + } + } + v.variables[base.variable] = &stringVariable{ baseVariable: base, values: CanonicalizeToProperties(stringProps.Values), @@ -417,8 +403,7 @@ func typeForPropertyFromPropertyStruct(ps interface{}, property string) reflect. // PropertiesToApply returns the applicable properties from a ModuleType that should be applied // based on SoongConfig values. // Expects that props contains a struct field with name soong_config_variables. The fields within -// soong_config_variables are expected to be in the same order as moduleType.Variables. In general, -// props should be generated via CreateProperties. +// soong_config_variables are expected to be in the same order as moduleType.Variables. func PropertiesToApply(moduleType *ModuleType, props reflect.Value, config SoongConfig) ([]interface{}, error) { var ret []interface{} props = props.Elem().FieldByName(soongConfigProperty) @@ -441,6 +426,46 @@ type ModuleType struct { variableNames []string } +func newModuleType(props *ModuleTypeProperties) (*ModuleType, []error) { + mt := &ModuleType{ + affectableProperties: props.Properties, + ConfigNamespace: props.Config_namespace, + BaseModuleType: props.Module_type, + variableNames: props.Variables, + } + + for _, name := range props.Bool_variables { + if err := checkVariableName(name); err != nil { + return nil, []error{fmt.Errorf("bool_variables %s", err)} + } + + mt.Variables = append(mt.Variables, newBoolVariable(name)) + } + + for _, name := range props.Value_variables { + if err := checkVariableName(name); err != nil { + return nil, []error{fmt.Errorf("value_variables %s", err)} + } + + mt.Variables = append(mt.Variables, &valueVariable{ + baseVariable: baseVariable{ + variable: name, + }, + }) + } + + return mt, nil +} + +func checkVariableName(name string) error { + if name == "" { + return fmt.Errorf("name must not be blank") + } else if name == conditionsDefault { + return fmt.Errorf("%q is reserved", conditionsDefault) + } + return nil +} + type soongConfigVariable interface { // variableProperty returns the name of the variable. variableProperty() string @@ -474,7 +499,10 @@ type stringVariable struct { func (s *stringVariable) variableValuesType() reflect.Type { var fields []reflect.StructField - for _, v := range s.values { + var values []string + values = append(values, s.values...) + values = append(values, conditionsDefault) + for _, v := range values { fields = append(fields, reflect.StructField{ Name: proptools.FieldNameForProperty(v), Type: emptyInterfaceType, @@ -484,26 +512,36 @@ func (s *stringVariable) variableValuesType() reflect.Type { return reflect.StructOf(fields) } +// initializeProperties initializes properties to zero value of typ for supported values and a final +// conditions default field. func (s *stringVariable) initializeProperties(v reflect.Value, typ reflect.Type) { for i := range s.values { v.Field(i).Set(reflect.Zero(typ)) } + v.Field(len(s.values)).Set(reflect.Zero(typ)) // conditions default is the final value } +// Extracts an interface from values containing the properties to apply based on config. +// If config does not match a value with a non-nil property set, the default value will be returned. 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(), nil + f := values.Field(j) + if config.String(s.variable) == v && !f.Elem().IsNil() { + return f.Interface(), nil } } - - return nil, nil + // if we have reached this point, we have checked all valid values of string and either: + // * the value was not set + // * the value was set but that value was not specified in the Android.bp file + return values.Field(len(s.values)).Interface(), nil } +// Struct to allow conditions set based on a boolean variable type boolVariable struct { baseVariable } +// newBoolVariable constructs a boolVariable with the given name func newBoolVariable(name string) *boolVariable { return &boolVariable{ baseVariable{ @@ -516,18 +554,82 @@ func (b boolVariable) variableValuesType() reflect.Type { return emptyInterfaceType } +// initializeProperties initializes a property to zero value of typ with an additional conditions +// default field. func (b boolVariable) initializeProperties(v reflect.Value, typ reflect.Type) { - v.Set(reflect.Zero(typ)) + initializePropertiesWithDefault(v, typ) } -func (b boolVariable) PropertiesToApply(config SoongConfig, values reflect.Value) (interface{}, error) { - if config.Bool(b.variable) { - return values.Interface(), nil +// initializePropertiesWithDefault, initialize with zero value, v to contain a field for each field +// in typ, with an additional field for defaults of type typ. This should be used to initialize +// boolVariable, valueVariable, or any future implementations of soongConfigVariable which support +// one variable and a default. +func initializePropertiesWithDefault(v reflect.Value, typ reflect.Type) { + sTyp := typ.Elem() + var fields []reflect.StructField + for i := 0; i < sTyp.NumField(); i++ { + fields = append(fields, sTyp.Field(i)) } + // create conditions_default field + nestedFieldName := proptools.FieldNameForProperty(conditionsDefault) + fields = append(fields, reflect.StructField{ + Name: nestedFieldName, + Type: typ, + }) + + newTyp := reflect.PtrTo(reflect.StructOf(fields)) + v.Set(reflect.Zero(newTyp)) +} + +// conditionsDefaultField extracts the conditions_default field from v. This is always the final +// field if initialized with initializePropertiesWithDefault. +func conditionsDefaultField(v reflect.Value) reflect.Value { + return v.Field(v.NumField() - 1) +} + +// removeDefault removes the conditions_default field from values while retaining values from all +// other fields. This allows +func removeDefault(values reflect.Value) reflect.Value { + v := values.Elem().Elem() + s := conditionsDefaultField(v) + // if conditions_default field was not set, there will be no issues extending properties. + if !s.IsValid() { + return v + } + + // If conditions_default field was set, it has the correct type for our property. Create a new + // reflect.Value of the conditions_default type and copy all fields (except for + // conditions_default) based on values to the result. + res := reflect.New(s.Type().Elem()) + for i := 0; i < res.Type().Elem().NumField(); i++ { + val := v.Field(i) + res.Elem().Field(i).Set(val) + } + + return res +} + +// PropertiesToApply returns an interface{} value based on initializeProperties to be applied to +// the module. If the value was not set, conditions_default interface will be returned; otherwise, +// the interface in values, without conditions_default will be returned. +func (b boolVariable) PropertiesToApply(config SoongConfig, values reflect.Value) (interface{}, error) { + // If this variable was not referenced in the module, there are no properties to apply. + if values.Elem().IsZero() { + return nil, nil + } + if config.Bool(b.variable) { + values = removeDefault(values) + return values.Interface(), nil + } + v := values.Elem().Elem() + if f := conditionsDefaultField(v); f.IsValid() { + return f.Interface(), nil + } return nil, nil } +// Struct to allow conditions set based on a value variable, supporting string substitution. type valueVariable struct { baseVariable } @@ -536,17 +638,28 @@ func (s *valueVariable) variableValuesType() reflect.Type { return emptyInterfaceType } +// initializeProperties initializes a property to zero value of typ with an additional conditions +// default field. func (s *valueVariable) initializeProperties(v reflect.Value, typ reflect.Type) { - v.Set(reflect.Zero(typ)) + initializePropertiesWithDefault(v, typ) } +// PropertiesToApply returns an interface{} value based on initializeProperties to be applied to +// the module. If the variable was not set, conditions_default interface will be returned; +// otherwise, the interface in values, without conditions_default will be returned with all +// appropriate string substitutions based on variable being set. func (s *valueVariable) PropertiesToApply(config SoongConfig, values reflect.Value) (interface{}, error) { - if !config.IsSet(s.variable) || !values.IsValid() { + // If this variable was not referenced in the module, there are no properties to apply. + if !values.IsValid() || values.Elem().IsZero() { return nil, nil } + if !config.IsSet(s.variable) { + return conditionsDefaultField(values.Elem().Elem()).Interface(), nil + } configValue := config.String(s.variable) - propStruct := values.Elem().Elem() + values = removeDefault(values) + propStruct := values.Elem() if !propStruct.IsValid() { return nil, nil } diff --git a/android/soongconfig/modules_test.go b/android/soongconfig/modules_test.go index fb0e1899d..b824c7898 100644 --- a/android/soongconfig/modules_test.go +++ b/android/soongconfig/modules_test.go @@ -254,67 +254,75 @@ type properties struct { A *string B bool } -type soongConfigVariables struct { - Bool_var properties - Other_bool_var properties + +type boolVarProps struct { + A *string + B bool + Conditions_default *properties } -type soongConfigProps struct { - Soong_config_variables soongConfigVariables +type soongConfigVars struct { + Bool_var interface{} } func Test_PropertiesToApply(t *testing.T) { - - mt := &ModuleType{ - BaseModuleType: "foo", - ConfigNamespace: "bar", - Variables: []soongConfigVariable{ - newBoolVariable("bool_var"), - newBoolVariable("other_bool_var"), - }, - affectableProperties: []string{ - "a", - "b", - }, + mt, _ := newModuleType(&ModuleTypeProperties{ + Module_type: "foo", + Config_namespace: "bar", + Bool_variables: []string{"bool_var"}, + Properties: []string{"a", "b"}, + }) + boolVarPositive := &properties{ + A: proptools.StringPtr("A"), + B: true, } - props := soongConfigProps{ - Soong_config_variables: soongConfigVariables{ - Bool_var: properties{ - A: proptools.StringPtr("a"), - B: true, - }, - Other_bool_var: properties{ - A: proptools.StringPtr("other"), - B: false, + conditionsDefault := &properties{ + A: proptools.StringPtr("default"), + B: false, + } + actualProps := &struct { + Soong_config_variables soongConfigVars + }{ + Soong_config_variables: soongConfigVars{ + Bool_var: &boolVarProps{ + A: boolVarPositive.A, + B: boolVarPositive.B, + Conditions_default: conditionsDefault, }, }, } + props := reflect.ValueOf(actualProps) testCases := []struct { + name string config SoongConfig wantProps []interface{} }{ { - config: Config(map[string]string{}), + name: "no_vendor_config", + config: Config(map[string]string{}), + wantProps: []interface{}{conditionsDefault}, }, { + name: "vendor_config_false", + config: Config(map[string]string{"bool_var": "n"}), + wantProps: []interface{}{conditionsDefault}, + }, + { + name: "bool_var_true", config: Config(map[string]string{"bool_var": "y"}), - wantProps: []interface{}{props.Soong_config_variables.Bool_var}, - }, - { - config: Config(map[string]string{"other_bool_var": "y"}), - wantProps: []interface{}{props.Soong_config_variables.Other_bool_var}, + wantProps: []interface{}{boolVarPositive}, }, } for _, tc := range testCases { - gotProps, err := PropertiesToApply(mt, reflect.ValueOf(&props), tc.config) + gotProps, err := PropertiesToApply(mt, props, tc.config) if err != nil { - t.Errorf("Unexpected error in PropertiesToApply: %s", err) + t.Errorf("%s: Unexpected error in PropertiesToApply: %s", tc.name, err) } if !reflect.DeepEqual(gotProps, tc.wantProps) { - t.Errorf("Expected %s, got %s", tc.wantProps, gotProps) + t.Errorf("%s: Expected %s, got %s", tc.name, tc.wantProps, gotProps) } } }