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

4 files changed, 80 insertions, 206 deletions

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{}) {