Fix product variables in defaults modules
Product variables structs are generated at runtime to contain only
the properties that apply to the current module. Defaults modules
always contained all product variable properties. Defaults modules
apply their properties to the target module using
proptools.PrependProperties, which prepends structs that have
matching types. Filtered property structs had a different type
and were dropped.
Even after adding filtering to the defaults product variable
properties, defaults modules may contain more property structs
than the target module they are applied to, so the product
variables struct for the defaults module could contain more
fields than the product variables struct for the target module.
Use proptools.PrependMatchingProperties when applying defaults
of product variables instead, which will apply matching properties
across types.
Test: defaults_test.go
Test: variable_test.go
Change-Id: I281bdefef92053457a3b7b65383493a4e7d999df
diff --git a/android/defaults.go b/android/defaults.go
index 7597446..fd707a4 100644
--- a/android/defaults.go
+++ b/android/defaults.go
@@ -15,6 +15,8 @@
package android
import (
+ "reflect"
+
"github.com/google/blueprint"
"github.com/google/blueprint/proptools"
)
@@ -30,16 +32,18 @@
}
type DefaultableModuleBase struct {
- defaultsProperties defaultsProperties
- defaultableProperties []interface{}
+ defaultsProperties defaultsProperties
+ defaultableProperties []interface{}
+ defaultableVariableProperties interface{}
}
func (d *DefaultableModuleBase) defaults() *defaultsProperties {
return &d.defaultsProperties
}
-func (d *DefaultableModuleBase) setProperties(props []interface{}) {
+func (d *DefaultableModuleBase) setProperties(props []interface{}, variableProperties interface{}) {
d.defaultableProperties = props
+ d.defaultableVariableProperties = variableProperties
}
// Interface that must be supported by any module to which defaults can be applied.
@@ -48,7 +52,7 @@
defaults() *defaultsProperties
// Set the property structures into which defaults will be added.
- setProperties([]interface{})
+ setProperties(props []interface{}, variableProperties interface{})
// Apply defaults from the supplied Defaults to the property structures supplied to
// setProperties(...).
@@ -63,7 +67,10 @@
var _ Defaultable = (*DefaultableModuleBase)(nil)
func InitDefaultableModule(module DefaultableModule) {
- module.setProperties(module.(Module).GetProperties())
+ if module.(Module).base().module == nil {
+ panic("InitAndroidModule must be called before InitDefaultableModule")
+ }
+ module.setProperties(module.(Module).GetProperties(), module.(Module).base().variableProperties)
module.AddProperties(module.defaults())
}
@@ -114,6 +121,8 @@
// Get the structures containing the properties for which defaults can be provided.
properties() []interface{}
+ productVariableProperties() interface{}
+
// Return the defaults common properties.
common() *commonProperties
@@ -134,6 +143,10 @@
return d.defaultableProperties
}
+func (d *DefaultsModuleBase) productVariableProperties() interface{} {
+ return d.defaultableVariableProperties
+}
+
func (d *DefaultsModuleBase) common() *commonProperties {
return &d.commonProperties
}
@@ -151,9 +164,10 @@
module.AddProperties(
&hostAndDeviceProperties{},
commonProperties,
- &variableProperties{},
&ApexProperties{})
+ initAndroidModuleBase(module)
+ initProductVariableModule(module)
InitArchModule(module)
InitDefaultableModule(module)
@@ -185,16 +199,57 @@
for _, defaults := range defaultsList {
for _, prop := range defaultable.defaultableProperties {
- for _, def := range defaults.properties() {
- if proptools.TypeEqual(prop, def) {
- err := proptools.PrependProperties(prop, def, nil)
- if err != nil {
- if propertyErr, ok := err.(*proptools.ExtendPropertyError); ok {
- ctx.PropertyErrorf(propertyErr.Property, "%s", propertyErr.Err.Error())
- } else {
- panic(err)
- }
- }
+ if prop == defaultable.defaultableVariableProperties {
+ defaultable.applyDefaultVariableProperties(ctx, defaults, prop)
+ } else {
+ defaultable.applyDefaultProperties(ctx, defaults, prop)
+ }
+ }
+ }
+}
+
+// Product variable properties need special handling, the type of the filtered product variable
+// property struct may not be identical between the defaults module and the defaultable module.
+// Use PrependMatchingProperties to apply whichever properties match.
+func (defaultable *DefaultableModuleBase) applyDefaultVariableProperties(ctx TopDownMutatorContext,
+ defaults Defaults, defaultableProp interface{}) {
+ if defaultableProp == nil {
+ return
+ }
+
+ defaultsProp := defaults.productVariableProperties()
+ if defaultsProp == nil {
+ return
+ }
+
+ dst := []interface{}{
+ defaultableProp,
+ // Put an empty copy of the src properties into dst so that properties in src that are not in dst
+ // don't cause a "failed to find property to extend" error.
+ proptools.CloneEmptyProperties(reflect.ValueOf(defaultsProp)).Interface(),
+ }
+
+ err := proptools.PrependMatchingProperties(dst, defaultsProp, nil)
+ if err != nil {
+ if propertyErr, ok := err.(*proptools.ExtendPropertyError); ok {
+ ctx.PropertyErrorf(propertyErr.Property, "%s", propertyErr.Err.Error())
+ } else {
+ panic(err)
+ }
+ }
+}
+
+func (defaultable *DefaultableModuleBase) applyDefaultProperties(ctx TopDownMutatorContext,
+ defaults Defaults, defaultableProp interface{}) {
+
+ for _, def := range defaults.properties() {
+ if proptools.TypeEqual(defaultableProp, def) {
+ err := proptools.PrependProperties(defaultableProp, def, nil)
+ if err != nil {
+ if propertyErr, ok := err.(*proptools.ExtendPropertyError); ok {
+ ctx.PropertyErrorf(propertyErr.Property, "%s", propertyErr.Err.Error())
+ } else {
+ panic(err)
}
}
}
diff --git a/android/defaults_test.go b/android/defaults_test.go
index ba607ef..d096b2f 100644
--- a/android/defaults_test.go
+++ b/android/defaults_test.go
@@ -15,6 +15,7 @@
package android
import (
+ "reflect"
"testing"
"github.com/google/blueprint/proptools"
@@ -40,8 +41,8 @@
func defaultsTestModuleFactory() Module {
module := &defaultsTestModule{}
module.AddProperties(&module.properties)
- InitDefaultableModule(module)
InitAndroidModule(module)
+ InitDefaultableModule(module)
return module
}
@@ -57,6 +58,49 @@
return defaults
}
+func TestDefaults(t *testing.T) {
+ bp := `
+ defaults {
+ name: "transitive",
+ foo: ["transitive"],
+ }
+
+ defaults {
+ name: "defaults",
+ defaults: ["transitive"],
+ foo: ["defaults"],
+ }
+
+ test {
+ name: "foo",
+ defaults: ["defaults"],
+ foo: ["module"],
+ }
+ `
+
+ config := TestConfig(buildDir, nil, bp, nil)
+
+ ctx := NewTestContext()
+
+ ctx.RegisterModuleType("test", defaultsTestModuleFactory)
+ ctx.RegisterModuleType("defaults", defaultsTestDefaultsFactory)
+
+ ctx.PreArchMutators(RegisterDefaultsPreArchMutators)
+
+ ctx.Register(config)
+
+ _, errs := ctx.ParseFileList(".", []string{"Android.bp"})
+ FailIfErrored(t, errs)
+ _, errs = ctx.PrepareBuildActions(config)
+ FailIfErrored(t, errs)
+
+ foo := ctx.ModuleForTests("foo", "").Module().(*defaultsTestModule)
+
+ if g, w := foo.properties.Foo, []string{"transitive", "defaults", "module"}; !reflect.DeepEqual(g, w) {
+ t.Errorf("expected foo %q, got %q", w, g)
+ }
+}
+
func TestDefaultsAllowMissingDependencies(t *testing.T) {
bp := `
defaults {
diff --git a/android/module.go b/android/module.go
index 15f3b56..0a8737b 100644
--- a/android/module.go
+++ b/android/module.go
@@ -537,16 +537,7 @@
&base.nameProperties,
&base.commonProperties)
- // Allow tests to override the default product variables
- if base.variableProperties == nil {
- base.variableProperties = defaultProductVariables
- }
-
- // Filter the product variables properties to the ones that exist on this module
- base.variableProperties = createVariableProperties(m.GetProperties(), base.variableProperties)
- if base.variableProperties != nil {
- m.AddProperties(base.variableProperties)
- }
+ initProductVariableModule(m)
base.generalProperties = m.GetProperties()
base.customizableProperties = m.GetProperties()
diff --git a/android/variable.go b/android/variable.go
index 165db1c..de6621b 100644
--- a/android/variable.go
+++ b/android/variable.go
@@ -25,7 +25,7 @@
func init() {
PreDepsMutators(func(ctx RegisterMutatorsContext) {
- ctx.BottomUp("variable", variableMutator).Parallel()
+ ctx.BottomUp("variable", VariableMutator).Parallel()
})
}
@@ -384,7 +384,7 @@
}
}
-func variableMutator(mctx BottomUpMutatorContext) {
+func VariableMutator(mctx BottomUpMutatorContext) {
var module Module
var ok bool
if module, ok = mctx.Module().(Module); !ok {
@@ -539,6 +539,20 @@
return ret.Interface()
}
+func initProductVariableModule(m Module) {
+ base := m.base()
+
+ // Allow tests to override the default product variables
+ if base.variableProperties == nil {
+ base.variableProperties = defaultProductVariables
+ }
+ // Filter the product variables properties to the ones that exist on this module
+ base.variableProperties = createVariableProperties(m.GetProperties(), base.variableProperties)
+ if base.variableProperties != nil {
+ m.AddProperties(base.variableProperties)
+ }
+}
+
// createVariableProperties takes the list of property structs for a module and returns a property struct that
// contains the product variable properties that exist in the property structs, or nil if there are none. It
// caches the result.
diff --git a/android/variable_test.go b/android/variable_test.go
index cde2b1a..9cafedd 100644
--- a/android/variable_test.go
+++ b/android/variable_test.go
@@ -171,7 +171,7 @@
Foo []string
}{}))
ctx.PreDepsMutators(func(ctx RegisterMutatorsContext) {
- ctx.BottomUp("variable", variableMutator).Parallel()
+ ctx.BottomUp("variable", VariableMutator).Parallel()
})
// Test that a module can use one product variable even if it doesn't have all the properties
@@ -209,6 +209,115 @@
FailIfErrored(t, errs)
}
+var testProductVariableDefaultsProperties = struct {
+ Product_variables struct {
+ Eng struct {
+ Foo []string
+ Bar []string
+ }
+ }
+}{}
+
+type productVariablesDefaultsTestProperties struct {
+ Foo []string
+}
+
+type productVariablesDefaultsTestProperties2 struct {
+ Foo []string
+ Bar []string
+}
+
+type productVariablesDefaultsTestModule struct {
+ ModuleBase
+ DefaultableModuleBase
+ properties productVariablesDefaultsTestProperties
+}
+
+func (d *productVariablesDefaultsTestModule) GenerateAndroidBuildActions(ctx ModuleContext) {
+ ctx.Build(pctx, BuildParams{
+ Rule: Touch,
+ Output: PathForModuleOut(ctx, "out"),
+ })
+}
+
+func productVariablesDefaultsTestModuleFactory() Module {
+ module := &productVariablesDefaultsTestModule{}
+ module.AddProperties(&module.properties)
+ module.variableProperties = testProductVariableDefaultsProperties
+ InitAndroidModule(module)
+ InitDefaultableModule(module)
+ return module
+}
+
+type productVariablesDefaultsTestDefaults struct {
+ ModuleBase
+ DefaultsModuleBase
+}
+
+func productVariablesDefaultsTestDefaultsFactory() Module {
+ defaults := &productVariablesDefaultsTestDefaults{}
+ defaults.AddProperties(&productVariablesDefaultsTestProperties{})
+ defaults.AddProperties(&productVariablesDefaultsTestProperties2{})
+ defaults.variableProperties = testProductVariableDefaultsProperties
+ InitDefaultsModule(defaults)
+ return defaults
+}
+
+// Test a defaults module that supports more product variable properties than the target module.
+func TestProductVariablesDefaults(t *testing.T) {
+ bp := `
+ defaults {
+ name: "defaults",
+ product_variables: {
+ eng: {
+ foo: ["product_variable_defaults"],
+ bar: ["product_variable_defaults"],
+ },
+ },
+ foo: ["defaults"],
+ bar: ["defaults"],
+ }
+
+ test {
+ name: "foo",
+ defaults: ["defaults"],
+ foo: ["module"],
+ product_variables: {
+ eng: {
+ foo: ["product_variable_module"],
+ },
+ },
+ }
+ `
+
+ config := TestConfig(buildDir, nil, bp, nil)
+ config.TestProductVariables.Eng = boolPtr(true)
+
+ ctx := NewTestContext()
+
+ ctx.RegisterModuleType("test", productVariablesDefaultsTestModuleFactory)
+ ctx.RegisterModuleType("defaults", productVariablesDefaultsTestDefaultsFactory)
+
+ ctx.PreArchMutators(RegisterDefaultsPreArchMutators)
+ ctx.PreDepsMutators(func(ctx RegisterMutatorsContext) {
+ ctx.BottomUp("variable", VariableMutator).Parallel()
+ })
+
+ ctx.Register(config)
+
+ _, errs := ctx.ParseFileList(".", []string{"Android.bp"})
+ FailIfErrored(t, errs)
+ _, errs = ctx.PrepareBuildActions(config)
+ FailIfErrored(t, errs)
+
+ foo := ctx.ModuleForTests("foo", "").Module().(*productVariablesDefaultsTestModule)
+
+ want := []string{"defaults", "module", "product_variable_defaults", "product_variable_module"}
+ if g, w := foo.properties.Foo, want; !reflect.DeepEqual(g, w) {
+ t.Errorf("expected foo %q, got %q", w, g)
+ }
+}
+
func BenchmarkSliceToTypeArray(b *testing.B) {
for _, n := range []int{1, 2, 4, 8, 100} {
var propStructs []interface{}
diff --git a/cc/cc_test.go b/cc/cc_test.go
index 332cc45..c105954 100644
--- a/cc/cc_test.go
+++ b/cc/cc_test.go
@@ -2751,3 +2751,42 @@
t.Errorf("libboth ar rule wanted %q, got %q", w, g)
}
}
+
+func TestProductVariableDefaults(t *testing.T) {
+ bp := `
+ cc_defaults {
+ name: "libfoo_defaults",
+ srcs: ["foo.c"],
+ cppflags: ["-DFOO"],
+ product_variables: {
+ debuggable: {
+ cppflags: ["-DBAR"],
+ },
+ },
+ }
+
+ cc_library {
+ name: "libfoo",
+ defaults: ["libfoo_defaults"],
+ }
+ `
+
+ config := TestConfig(buildDir, android.Android, nil, bp, nil)
+ config.TestProductVariables.Debuggable = BoolPtr(true)
+
+ ctx := CreateTestContext()
+ ctx.PreDepsMutators(func(ctx android.RegisterMutatorsContext) {
+ ctx.BottomUp("variable", android.VariableMutator).Parallel()
+ })
+ ctx.Register(config)
+
+ _, errs := ctx.ParseFileList(".", []string{"Android.bp"})
+ android.FailIfErrored(t, errs)
+ _, errs = ctx.PrepareBuildActions(config)
+ android.FailIfErrored(t, errs)
+
+ libfoo := ctx.ModuleForTests("libfoo", "android_arm64_armv8-a_static").Module().(*Module)
+ if !android.InList("-DBAR", libfoo.flags.Local.CppFlags) {
+ t.Errorf("expected -DBAR in cppflags, got %q", libfoo.flags.Local.CppFlags)
+ }
+}