Browse Source

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
Liz Kammer 3 years ago
parent
commit
432bd598ae

+ 53 - 4
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

+ 64 - 1
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"],
+//                 },
 //             },
 //         },
 //     }

+ 141 - 37
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)
-
-		config.TestProductVariables.VendorVars = map[string]map[string]string{
-			"acme": map[string]string{
-				"board":    "soc_a",
-				"size":     "42",
-				"feature1": "true",
-				"feature2": "false",
-				// FEATURE3 unset
+		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",
+				},
 			},
-		}
 
-		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()
-
-		_, errs := ctx.ParseBlueprintsFiles("Android.bp")
-		FailIfErrored(t, errs)
-		_, errs = ctx.PrepareBuildActions(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, basicCFlags; !reflect.DeepEqual(g, w) {
-			t.Errorf("wanted foo cflags %q, got %q", w, g)
+			{
+				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",
+				},
+			},
 		}
 
-		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)
+		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(tc.config)
+			FailIfErrored(t, errs)
+
+			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)
+			}
+
+			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
+}

+ 3 - 0
android/soongconfig/Android.bp

@@ -10,4 +10,7 @@ bootstrap_go_package {
         "config.go",
         "modules.go",
     ],
+    testSrcs: [
+        "modules_test.go",
+    ],
 }

+ 14 - 2
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

+ 151 - 38
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)
 }
 
+// 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
 	}

+ 42 - 34
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},
 		},
 		{
-			config:    Config(map[string]string{"bool_var": "y"}),
-			wantProps: []interface{}{props.Soong_config_variables.Bool_var},
+			name:      "vendor_config_false",
+			config:    Config(map[string]string{"bool_var": "n"}),
+			wantProps: []interface{}{conditionsDefault},
 		},
 		{
-			config:    Config(map[string]string{"other_bool_var": "y"}),
-			wantProps: []interface{}{props.Soong_config_variables.Other_bool_var},
+			name:      "bool_var_true",
+			config:    Config(map[string]string{"bool_var": "y"}),
+			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)
 		}
 	}
 }