diff --git a/android/module.go b/android/module.go index 8076a99ff..bb0aada9f 100644 --- a/android/module.go +++ b/android/module.go @@ -510,8 +510,19 @@ func InitAndroidModule(m Module) { m.AddProperties( &base.nameProperties, - &base.commonProperties, - &base.variableProperties) + &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) + } + base.generalProperties = m.GetProperties() base.customizableProperties = m.GetProperties() @@ -593,7 +604,7 @@ type ModuleBase struct { nameProperties nameProperties commonProperties commonProperties - variableProperties variableProperties + variableProperties interface{} hostAndDeviceProperties hostAndDeviceProperties generalProperties []interface{} archProperties [][]interface{} diff --git a/android/mutator.go b/android/mutator.go index 510e63c0c..88ac52117 100644 --- a/android/mutator.go +++ b/android/mutator.go @@ -15,6 +15,8 @@ package android import ( + "reflect" + "github.com/google/blueprint" "github.com/google/blueprint/proptools" ) @@ -244,8 +246,23 @@ func (t *topDownMutatorContext) Rename(name string) { } func (t *topDownMutatorContext) CreateModule(factory ModuleFactory, props ...interface{}) Module { - inherited := []interface{}{&t.Module().base().commonProperties, &t.Module().base().variableProperties} + inherited := []interface{}{&t.Module().base().commonProperties} module := t.bp.CreateModule(ModuleFactoryAdaptor(factory), append(inherited, props...)...).(Module) + + if t.Module().base().variableProperties != nil && module.base().variableProperties != nil { + src := t.Module().base().variableProperties + dst := []interface{}{ + module.base().variableProperties, + // 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(src).Elem()).Interface(), + } + err := proptools.AppendMatchingProperties(dst, src, nil) + if err != nil { + panic(err) + } + } + return module } diff --git a/android/variable.go b/android/variable.go index 3f2b9e9ff..41943b0f9 100644 --- a/android/variable.go +++ b/android/variable.go @@ -122,7 +122,7 @@ type variableProperties struct { } `android:"arch_variant"` } -var zeroProductVariables variableProperties +var zeroProductVariables interface{} = variableProperties{} type productVariables struct { // Suffix to add to generated Makefiles @@ -366,8 +366,13 @@ func variableMutator(mctx BottomUpMutatorContext) { // TODO: depend on config variable, create variants, propagate variants up tree a := module.base() - variableValues := reflect.ValueOf(&a.variableProperties.Product_variables).Elem() - zeroValues := reflect.ValueOf(zeroProductVariables.Product_variables) + + if a.variableProperties == nil { + return + } + + 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) @@ -496,3 +501,106 @@ func printfIntoProperty(propertyValue reflect.Value, variableValue interface{}) return nil } + +var variablePropTypeMap OncePer + +// sliceToTypeArray takes a slice of property structs and returns a reflection created array containing the +// reflect.Types of each property struct. The result can be used as a key in a map. +func sliceToTypeArray(s []interface{}) interface{} { + // Create an array using reflection whose length is the length of the input slice + ret := reflect.New(reflect.ArrayOf(len(s), reflect.TypeOf(reflect.TypeOf(0)))).Elem() + for i, e := range s { + ret.Index(i).Set(reflect.ValueOf(reflect.TypeOf(e))) + } + return ret.Interface() +} + +// 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. +func createVariableProperties(moduleTypeProps []interface{}, productVariables interface{}) interface{} { + // Convert the moduleTypeProps to an array of reflect.Types that can be used as a key in the OncePer. + key := sliceToTypeArray(moduleTypeProps) + + // Use the variablePropTypeMap OncePer to cache the result for each set of property struct types. + typ, _ := variablePropTypeMap.Once(NewCustomOnceKey(key), func() interface{} { + // Compute the filtered property struct type. + return createVariablePropertiesType(moduleTypeProps, productVariables) + }).(reflect.Type) + + if typ == nil { + return nil + } + + // Create a new pointer to a filtered property struct. + return reflect.New(typ).Interface() +} + +// createVariablePropertiesType creates a new type that contains only the product variable properties that exist in +// a list of property structs. +func createVariablePropertiesType(moduleTypeProps []interface{}, productVariables interface{}) reflect.Type { + typ, _ := proptools.FilterPropertyStruct(reflect.TypeOf(productVariables), + func(field reflect.StructField, prefix string) (bool, reflect.StructField) { + // Filter function, returns true if the field should be in the resulting struct + if prefix == "" { + // Keep the top level Product_variables field + return true, field + } + _, rest := splitPrefix(prefix) + if rest == "" { + // Keep the 2nd level field (i.e. Product_variables.Eng) + return true, field + } + + // Strip off the first 2 levels of the prefix + _, prefix = splitPrefix(rest) + + for _, p := range moduleTypeProps { + if fieldExistsByNameRecursive(reflect.TypeOf(p).Elem(), prefix, field.Name) { + // Keep any fields that exist in one of the property structs + return true, field + } + } + + return false, field + }) + return typ +} + +func splitPrefix(prefix string) (first, rest string) { + index := strings.IndexByte(prefix, '.') + if index == -1 { + return prefix, "" + } + return prefix[:index], prefix[index+1:] +} + +func fieldExistsByNameRecursive(t reflect.Type, prefix, name string) bool { + if t.Kind() != reflect.Struct { + panic(fmt.Errorf("fieldExistsByNameRecursive can only be called on a reflect.Struct")) + } + + if prefix != "" { + split := strings.SplitN(prefix, ".", 2) + firstPrefix := split[0] + rest := "" + if len(split) > 1 { + rest = split[1] + } + f, exists := t.FieldByName(firstPrefix) + if !exists { + return false + } + ft := f.Type + if ft.Kind() == reflect.Ptr { + ft = ft.Elem() + } + if ft.Kind() != reflect.Struct { + panic(fmt.Errorf("field %q in %q is not a struct", firstPrefix, t)) + } + return fieldExistsByNameRecursive(ft, rest, name) + } else { + _, exists := t.FieldByName(name) + return exists + } +} diff --git a/android/variable_test.go b/android/variable_test.go index ce9ba5485..c1910fe03 100644 --- a/android/variable_test.go +++ b/android/variable_test.go @@ -16,7 +16,10 @@ package android import ( "reflect" + "strconv" "testing" + + "github.com/google/blueprint/proptools" ) type printfIntoPropertyTestCase struct { @@ -122,3 +125,111 @@ func TestPrintfIntoProperty(t *testing.T) { } } } + +type testProductVariableModule struct { + ModuleBase +} + +func (m *testProductVariableModule) GenerateAndroidBuildActions(ctx ModuleContext) { +} + +var testProductVariableProperties = struct { + Product_variables struct { + Eng struct { + Srcs []string + Cflags []string + } + } +}{} + +func testProductVariableModuleFactoryFactory(props interface{}) func() Module { + return func() Module { + m := &testProductVariableModule{} + clonedProps := proptools.CloneProperties(reflect.ValueOf(props)).Interface() + m.AddProperties(clonedProps) + + // Set a default variableProperties, this will be used as the input to the property struct filter + // for this test module. + m.variableProperties = testProductVariableProperties + InitAndroidModule(m) + return m + } +} + +func TestProductVariables(t *testing.T) { + ctx := NewTestContext() + // A module type that has a srcs property but not a cflags property. + ctx.RegisterModuleType("module1", ModuleFactoryAdaptor(testProductVariableModuleFactoryFactory(struct { + Srcs []string + }{}))) + // A module type that has a cflags property but not a srcs property. + ctx.RegisterModuleType("module2", ModuleFactoryAdaptor(testProductVariableModuleFactoryFactory(struct { + Cflags []string + }{}))) + // A module type that does not have any properties that match product_variables. + ctx.RegisterModuleType("module3", ModuleFactoryAdaptor(testProductVariableModuleFactoryFactory(struct { + Foo []string + }{}))) + ctx.PreDepsMutators(func(ctx RegisterMutatorsContext) { + ctx.BottomUp("variable", variableMutator).Parallel() + }) + + // Test that a module can use one product variable even if it doesn't have all the properties + // supported by that product variable. + bp := ` + module1 { + name: "foo", + product_variables: { + eng: { + srcs: ["foo.c"], + }, + }, + } + module2 { + name: "bar", + product_variables: { + eng: { + cflags: ["-DBAR"], + }, + }, + } + + module3 { + name: "baz", + } + ` + + mockFS := map[string][]byte{ + "Android.bp": []byte(bp), + } + + ctx.MockFileSystem(mockFS) + + ctx.Register() + + config := TestConfig(buildDir, nil) + config.TestProductVariables.Eng = proptools.BoolPtr(true) + + _, errs := ctx.ParseFileList(".", []string{"Android.bp"}) + FailIfErrored(t, errs) + _, errs = ctx.PrepareBuildActions(config) + FailIfErrored(t, errs) +} + +func BenchmarkSliceToTypeArray(b *testing.B) { + for _, n := range []int{1, 2, 4, 8, 100} { + var propStructs []interface{} + for i := 0; i < n; i++ { + propStructs = append(propStructs, &struct { + A *string + B string + }{}) + + } + b.Run(strconv.Itoa(n), func(b *testing.B) { + for i := 0; i < b.N; i++ { + _ = sliceToTypeArray(propStructs) + } + }) + } +}