From 06a931bdb6228bdbe425696b01e27522f4b00d71 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Wed, 28 Oct 2015 17:23:31 -0700 Subject: Replace extendProperties with pathtools.AppendProperties Blueprint has a generic AppendProperties/AppendMatchingProperties now, use it, and replace all bool properties that might be modified by a mutator with *bool, which provides the correct replace-if-set semantics for append. Also remove uses of ContainsProperty except when explicitly checking if a property was set in a blueprints file. Change-Id: If523af61d6b4630e79504d7fc2840f36e98571cc --- common/arch.go | 95 ++++++++++++++++++++++----------- common/extend.go | 152 ----------------------------------------------------- common/module.go | 22 ++------ common/variable.go | 17 +++--- 4 files changed, 80 insertions(+), 206 deletions(-) delete mode 100644 common/extend.go (limited to 'common') diff --git a/common/arch.go b/common/arch.go index 4dd01c30..05945672 100644 --- a/common/arch.go +++ b/common/arch.go @@ -426,6 +426,38 @@ func InitArchModule(m AndroidModule, defaultMultilib Multilib, var dashToUnderscoreReplacer = strings.NewReplacer("-", "_") +func (a *AndroidModuleBase) appendProperties(ctx blueprint.EarlyMutatorContext, + dst, src interface{}, field, srcPrefix string) { + + src = reflect.ValueOf(src).FieldByName(field).Elem().Interface() + + filter := func(property string, + dstField, srcField reflect.StructField, + dstValue, srcValue interface{}) (bool, error) { + + srcProperty := srcPrefix + "." + property + + if !proptools.HasTag(dstField, "android", "arch_variant") { + if ctx.ContainsProperty(srcProperty) { + return false, fmt.Errorf("can't be specific to a build variant") + } else { + return false, nil + } + } + + return true, nil + } + + err := proptools.AppendProperties(dst, src, filter) + if err != nil { + if propertyErr, ok := err.(*proptools.ExtendPropertyError); ok { + ctx.PropertyErrorf(propertyErr.Property, "%s", propertyErr.Err.Error()) + } else { + panic(err) + } + } +} + // Rewrite the module's properties structs to contain arch-specific values. func (a *AndroidModuleBase) setArchProperties(ctx blueprint.EarlyMutatorContext) { arch := a.commonProperties.CompileArch @@ -435,13 +467,9 @@ func (a *AndroidModuleBase) setArchProperties(ctx blueprint.EarlyMutatorContext) return } - callback := func(srcPropertyName, dstPropertyName string) { - a.extendedProperties[dstPropertyName] = struct{}{} - } - for i := range a.generalProperties { - generalPropsValue := []reflect.Value{reflect.ValueOf(a.generalProperties[i]).Elem()} - + genProps := a.generalProperties[i] + archProps := a.archProperties[i] // Handle arch-specific properties in the form: // arch: { // arm64: { @@ -449,9 +477,10 @@ func (a *AndroidModuleBase) setArchProperties(ctx blueprint.EarlyMutatorContext) // }, // }, t := arch.ArchType + field := proptools.FieldNameForProperty(t.Name) - extendProperties(ctx, "arch_variant", "arch."+t.Name, generalPropsValue, - reflect.ValueOf(a.archProperties[i].Arch).FieldByName(field).Elem().Elem(), callback) + prefix := "arch." + t.Name + a.appendProperties(ctx, genProps, archProps.Arch, field, prefix) // Handle arch-variant-specific properties in the form: // arch: { @@ -462,8 +491,8 @@ func (a *AndroidModuleBase) setArchProperties(ctx blueprint.EarlyMutatorContext) v := dashToUnderscoreReplacer.Replace(arch.ArchVariant) if v != "" { field := proptools.FieldNameForProperty(v) - extendProperties(ctx, "arch_variant", "arch."+v, generalPropsValue, - reflect.ValueOf(a.archProperties[i].Arch).FieldByName(field).Elem().Elem(), callback) + prefix := "arch." + v + a.appendProperties(ctx, genProps, archProps.Arch, field, prefix) } // Handle cpu-variant-specific properties in the form: @@ -475,8 +504,8 @@ func (a *AndroidModuleBase) setArchProperties(ctx blueprint.EarlyMutatorContext) c := dashToUnderscoreReplacer.Replace(arch.CpuVariant) if c != "" { field := proptools.FieldNameForProperty(c) - extendProperties(ctx, "arch_variant", "arch."+c, generalPropsValue, - reflect.ValueOf(a.archProperties[i].Arch).FieldByName(field).Elem().Elem(), callback) + prefix := "arch." + c + a.appendProperties(ctx, genProps, archProps.Arch, field, prefix) } // Handle multilib-specific properties in the form: @@ -485,9 +514,9 @@ func (a *AndroidModuleBase) setArchProperties(ctx blueprint.EarlyMutatorContext) // key: value, // }, // }, - multilibField := proptools.FieldNameForProperty(t.Multilib) - extendProperties(ctx, "arch_variant", "multilib."+t.Multilib, generalPropsValue, - reflect.ValueOf(a.archProperties[i].Multilib).FieldByName(multilibField).Elem().Elem(), callback) + field = proptools.FieldNameForProperty(t.Multilib) + prefix = "multilib." + t.Multilib + a.appendProperties(ctx, genProps, archProps.Multilib, field, prefix) // Handle host-or-device-specific properties in the form: // target: { @@ -496,9 +525,9 @@ func (a *AndroidModuleBase) setArchProperties(ctx blueprint.EarlyMutatorContext) // }, // }, hodProperty := hod.Property() - hodField := proptools.FieldNameForProperty(hodProperty) - extendProperties(ctx, "arch_variant", "target."+hodProperty, generalPropsValue, - reflect.ValueOf(a.archProperties[i].Target).FieldByName(hodField).Elem().Elem(), callback) + field = proptools.FieldNameForProperty(hodProperty) + prefix = "target." + hodProperty + a.appendProperties(ctx, genProps, archProps.Target, field, prefix) // Handle host target properties in the form: // target: { @@ -527,15 +556,18 @@ func (a *AndroidModuleBase) setArchProperties(ctx blueprint.EarlyMutatorContext) if hod.Host() { for _, v := range osList { if v.goos == runtime.GOOS { - extendProperties(ctx, "arch_variant", "target."+v.goos, generalPropsValue, - reflect.ValueOf(a.archProperties[i].Target).FieldByName(v.field).Elem().Elem(), callback) + field := v.field + prefix := "target." + v.goos + a.appendProperties(ctx, genProps, archProps.Target, field, prefix) t := arch.ArchType - extendProperties(ctx, "arch_variant", "target."+v.goos+"_"+t.Name, generalPropsValue, - reflect.ValueOf(a.archProperties[i].Target).FieldByName(v.field+"_"+t.Name).Elem().Elem(), callback) + field = v.field + "_" + t.Name + prefix = "target." + v.goos + "_" + t.Name + a.appendProperties(ctx, genProps, archProps.Target, field, prefix) } } - extendProperties(ctx, "arch_variant", "target.not_windows", generalPropsValue, - reflect.ValueOf(a.archProperties[i].Target).FieldByName("Not_windows").Elem().Elem(), callback) + field := "Not_windows" + prefix := "target.not_windows" + a.appendProperties(ctx, genProps, archProps.Target, field, prefix) } // Handle 64-bit device properties in the form: @@ -553,11 +585,13 @@ func (a *AndroidModuleBase) setArchProperties(ctx blueprint.EarlyMutatorContext) // debuggerd that need to know when they are a 32-bit process running on a 64-bit device if hod.Device() { if true /* && target_is_64_bit */ { - extendProperties(ctx, "arch_variant", "target.android64", generalPropsValue, - reflect.ValueOf(a.archProperties[i].Target).FieldByName("Android64").Elem().Elem(), callback) + field := "Android64" + prefix := "target.android64" + a.appendProperties(ctx, genProps, archProps.Target, field, prefix) } else { - extendProperties(ctx, "arch_variant", "target.android32", generalPropsValue, - reflect.ValueOf(a.archProperties[i].Target).FieldByName("Android32").Elem().Elem(), callback) + field := "Android32" + prefix := "target.android32" + a.appendProperties(ctx, genProps, archProps.Target, field, prefix) } } @@ -572,8 +606,9 @@ func (a *AndroidModuleBase) setArchProperties(ctx blueprint.EarlyMutatorContext) // }, if hod.Device() { t := arch.ArchType - extendProperties(ctx, "arch_variant", "target.android_"+t.Name, generalPropsValue, - reflect.ValueOf(a.archProperties[i].Target).FieldByName("Android_"+t.Name).Elem().Elem(), callback) + field := "Android_" + t.Name + prefix := "target.android_" + t.Name + a.appendProperties(ctx, genProps, archProps.Target, field, prefix) } if ctx.Failed() { diff --git a/common/extend.go b/common/extend.go deleted file mode 100644 index f2d061bd..00000000 --- a/common/extend.go +++ /dev/null @@ -1,152 +0,0 @@ -// Copyright 2015 Google Inc. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package common - -import ( - "fmt" - "reflect" - "strings" - - "github.com/google/blueprint" - "github.com/google/blueprint/proptools" -) - -// TODO: move this to proptools -func extendProperties(ctx blueprint.EarlyMutatorContext, - requiredTag, srcPrefix string, dstValues []reflect.Value, srcValue reflect.Value, - callback func(string, string)) { - if srcPrefix != "" { - srcPrefix += "." - } - extendPropertiesRecursive(ctx, requiredTag, srcValue, dstValues, srcPrefix, "", callback) -} - -func extendPropertiesRecursive(ctx blueprint.EarlyMutatorContext, requiredTag string, - srcValue reflect.Value, dstValues []reflect.Value, srcPrefix, dstPrefix string, - callback func(string, string)) { - - typ := srcValue.Type() - for i := 0; i < srcValue.NumField(); i++ { - srcField := typ.Field(i) - if srcField.PkgPath != "" { - // The field is not exported so just skip it. - continue - } - - localPropertyName := proptools.PropertyNameForField(srcField.Name) - srcPropertyName := srcPrefix + localPropertyName - srcFieldValue := srcValue.Field(i) - - if !ctx.ContainsProperty(srcPropertyName) { - continue - } - - found := false - for _, dstValue := range dstValues { - dstField, ok := dstValue.Type().FieldByName(srcField.Name) - if !ok { - continue - } - - dstFieldValue := dstValue.FieldByIndex(dstField.Index) - - if srcFieldValue.Type() != dstFieldValue.Type() { - panic(fmt.Errorf("can't extend mismatching types for %q (%s <- %s)", - srcPropertyName, dstFieldValue.Type(), srcFieldValue.Type())) - } - - dstPropertyName := dstPrefix + localPropertyName - - if requiredTag != "" { - tag := dstField.Tag.Get("android") - tags := map[string]bool{} - for _, entry := range strings.Split(tag, ",") { - if entry != "" { - tags[entry] = true - } - } - - if !tags[requiredTag] { - ctx.PropertyErrorf(srcPropertyName, "property can't be specific to a build variant") - continue - } - } - - if callback != nil { - callback(srcPropertyName, dstPropertyName) - } - - found = true - - switch srcFieldValue.Kind() { - case reflect.Bool: - // Replace the original value. - dstFieldValue.Set(srcFieldValue) - case reflect.String: - // Append the extension string. - dstFieldValue.SetString(dstFieldValue.String() + - srcFieldValue.String()) - case reflect.Slice: - dstFieldValue.Set(reflect.AppendSlice(dstFieldValue, srcFieldValue)) - case reflect.Interface: - if dstFieldValue.IsNil() != srcFieldValue.IsNil() { - panic(fmt.Errorf("can't extend field %q: nilitude mismatch", srcPropertyName)) - } - if dstFieldValue.IsNil() { - continue - } - - dstFieldValue = dstFieldValue.Elem() - srcFieldValue = srcFieldValue.Elem() - - if dstFieldValue.Type() != srcFieldValue.Type() { - panic(fmt.Errorf("can't extend field %q: type mismatch", srcPropertyName)) - } - if srcFieldValue.Kind() != reflect.Ptr { - panic(fmt.Errorf("can't extend field %q: interface not a pointer", srcPropertyName)) - } - fallthrough - case reflect.Ptr: - if dstFieldValue.IsNil() != srcFieldValue.IsNil() { - panic(fmt.Errorf("can't extend field %q: nilitude mismatch", srcPropertyName)) - } - if dstFieldValue.IsNil() { - continue - } - - dstFieldValue = dstFieldValue.Elem() - srcFieldValue = srcFieldValue.Elem() - - if dstFieldValue.Type() != srcFieldValue.Type() { - panic(fmt.Errorf("can't extend field %q: type mismatch", srcPropertyName)) - } - if srcFieldValue.Kind() != reflect.Struct { - panic(fmt.Errorf("can't extend field %q: pointer not to a struct", srcPropertyName)) - } - fallthrough - case reflect.Struct: - // Recursively extend the struct's fields. - extendPropertiesRecursive(ctx, requiredTag, srcFieldValue, []reflect.Value{dstFieldValue}, - srcPropertyName+".", srcPropertyName+".", callback) - default: - panic(fmt.Errorf("unexpected kind for property struct field %q: %s", - srcPropertyName, srcFieldValue.Kind())) - } - } - if !found { - ctx.PropertyErrorf(srcPropertyName, "failed to find property to extend") - } - } -} diff --git a/common/module.go b/common/module.go index feaba83d..d31a9fc2 100644 --- a/common/module.go +++ b/common/module.go @@ -123,7 +123,6 @@ func InitAndroidModule(m AndroidModule, base := m.base() base.module = m - base.extendedProperties = make(map[string]struct{}) propertyStructs = append(propertyStructs, &base.commonProperties, &base.variableProperties) @@ -198,7 +197,6 @@ type AndroidModuleBase struct { hostAndDeviceProperties hostAndDeviceProperties generalProperties []interface{} archProperties []*archProperties - extendedProperties map[string]struct{} noAddressSanitizer bool installFiles []string @@ -340,9 +338,8 @@ func (a *AndroidModuleBase) GenerateBuildActions(ctx blueprint.ModuleContext) { hod: a.commonProperties.CompileHostOrDevice, config: ctx.Config().(Config), }, - installDeps: a.computeInstallDeps(ctx), - installFiles: a.installFiles, - extendedProperties: a.extendedProperties, + installDeps: a.computeInstallDeps(ctx), + installFiles: a.installFiles, } if a.commonProperties.Disabled { @@ -373,10 +370,9 @@ type androidBaseContextImpl struct { type androidModuleContext struct { blueprint.ModuleContext androidBaseContextImpl - installDeps []string - installFiles []string - checkbuildFiles []string - extendedProperties map[string]struct{} + installDeps []string + installFiles []string + checkbuildFiles []string } func (a *androidModuleContext) Build(pctx *blueprint.PackageContext, params blueprint.BuildParams) { @@ -384,14 +380,6 @@ func (a *androidModuleContext) Build(pctx *blueprint.PackageContext, params blue a.ModuleContext.Build(pctx, params) } -func (a *androidModuleContext) ContainsProperty(property string) bool { - if a.ModuleContext.ContainsProperty(property) { - return true - } - _, ok := a.extendedProperties[property] - return ok -} - func (a *androidBaseContextImpl) Arch() Arch { return a.arch } diff --git a/common/variable.go b/common/variable.go index 23a97a6d..39496810 100644 --- a/common/variable.go +++ b/common/variable.go @@ -113,7 +113,7 @@ func VariableMutator(mctx blueprint.EarlyMutatorContext) { // TODO: depend on config variable, create variants, propagate variants up tree a := module.base() - variableValues := reflect.ValueOf(a.variableProperties.Product_variables) + variableValues := reflect.ValueOf(&a.variableProperties.Product_variables).Elem() zeroValues := reflect.ValueOf(zeroProductVariables.Product_variables) for i := 0; i < variableValues.NumField(); i++ { @@ -147,16 +147,19 @@ func VariableMutator(mctx blueprint.EarlyMutatorContext) { func (a *AndroidModuleBase) setVariableProperties(ctx blueprint.EarlyMutatorContext, prefix string, productVariablePropertyValue reflect.Value, variableValue interface{}) { - generalPropertyValues := make([]reflect.Value, len(a.generalProperties)) - for i := range a.generalProperties { - generalPropertyValues[i] = reflect.ValueOf(a.generalProperties[i]).Elem() - } - if variableValue != nil { printfIntoProperties(productVariablePropertyValue, variableValue) } - extendProperties(ctx, "", prefix, generalPropertyValues, productVariablePropertyValue, nil) + err := proptools.AppendMatchingProperties(a.generalProperties, + productVariablePropertyValue.Addr().Interface(), nil) + if err != nil { + if propertyErr, ok := err.(*proptools.ExtendPropertyError); ok { + ctx.PropertyErrorf(propertyErr.Property, "%s", propertyErr.Err.Error()) + } else { + panic(err) + } + } } func printfIntoProperties(productVariablePropertyValue reflect.Value, variableValue interface{}) { -- cgit v1.2.3