diff --git a/android/defaults.go b/android/defaults.go index 759744693..fd707a45f 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 defaultsProperties struct { } 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 @@ type Defaultable interface { 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 @@ type DefaultableModule interface { 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 @@ type Defaults interface { // 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 @@ func (d *DefaultsModuleBase) properties() []interface{} { return d.defaultableProperties } +func (d *DefaultsModuleBase) productVariableProperties() interface{} { + return d.defaultableVariableProperties +} + func (d *DefaultsModuleBase) common() *commonProperties { return &d.commonProperties } @@ -151,9 +164,10 @@ func InitDefaultsModule(module DefaultsModule) { module.AddProperties( &hostAndDeviceProperties{}, commonProperties, - &variableProperties{}, &ApexProperties{}) + initAndroidModuleBase(module) + initProductVariableModule(module) InitArchModule(module) InitDefaultableModule(module) @@ -185,16 +199,57 @@ func (defaultable *DefaultableModuleBase) applyDefaults(ctx TopDownMutatorContex 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 ba607ef53..d096b2f92 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 (d *defaultsTestModule) GenerateAndroidBuildActions(ctx ModuleContext) { func defaultsTestModuleFactory() Module { module := &defaultsTestModule{} module.AddProperties(&module.properties) - InitDefaultableModule(module) InitAndroidModule(module) + InitDefaultableModule(module) return module } @@ -57,6 +58,49 @@ func defaultsTestDefaultsFactory() Module { 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 96c2e1e90..0a8737bee 100644 --- a/android/module.go +++ b/android/module.go @@ -537,16 +537,7 @@ func InitAndroidModule(m Module) { &base.nameProperties, &base.commonProperties) - // Allow tests to override the default product variables - if base.variableProperties == nil { - base.variableProperties = zeroProductVariables - } - - // 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 9625a879a..58e59404e 100644 --- a/android/variable.go +++ b/android/variable.go @@ -25,7 +25,7 @@ import ( func init() { PreDepsMutators(func(ctx RegisterMutatorsContext) { - ctx.BottomUp("variable", variableMutator).Parallel() + ctx.BottomUp("variable", VariableMutator).Parallel() }) } @@ -127,13 +127,14 @@ type variableProperties struct { } `android:"arch_variant"` Native_coverage struct { + Src *string `android:"arch_variant"` Srcs []string `android:"arch_variant"` Exclude_srcs []string `android:"arch_variant"` } `android:"arch_variant"` } `android:"arch_variant"` } -var zeroProductVariables interface{} = variableProperties{} +var defaultProductVariables interface{} = variableProperties{} type productVariables struct { // Suffix to add to generated Makefiles @@ -384,7 +385,7 @@ func (v *productVariables) SetDefaultConfig() { } } -func variableMutator(mctx BottomUpMutatorContext) { +func VariableMutator(mctx BottomUpMutatorContext) { var module Module var ok bool if module, ok = mctx.Module().(Module); !ok { @@ -399,11 +400,9 @@ func variableMutator(mctx BottomUpMutatorContext) { } variableValues := reflect.ValueOf(a.variableProperties).Elem().FieldByName("Product_variables") - zeroValues := reflect.ValueOf(zeroProductVariables).FieldByName("Product_variables") for i := 0; i < variableValues.NumField(); i++ { variableValue := variableValues.Field(i) - zeroValue := zeroValues.Field(i) name := variableValues.Type().Field(i).Name property := "product_variables." + proptools.PropertyNameForField(name) @@ -421,10 +420,9 @@ func variableMutator(mctx BottomUpMutatorContext) { } // Check if any properties were set for the module - if reflect.DeepEqual(variableValue.Interface(), zeroValue.Interface()) { + if variableValue.IsZero() { continue } - a.setVariableProperties(mctx, property, variableValue, val.Interface()) } } @@ -542,6 +540,20 @@ func sliceToTypeArray(s []interface{}) interface{} { 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 cde2b1ac9..9cafedd50 100644 --- a/android/variable_test.go +++ b/android/variable_test.go @@ -171,7 +171,7 @@ func TestProductVariables(t *testing.T) { 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 @@ func TestProductVariables(t *testing.T) { 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 332cc45e7..c105954e3 100644 --- a/cc/cc_test.go +++ b/cc/cc_test.go @@ -2751,3 +2751,42 @@ func TestDefaults(t *testing.T) { 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) + } +}