Merge changes I38fb22b2,I281bdefe,Ieaaa590c am: df51b061cd

Change-Id: I26a4f9512649c9a505faeffb693bd238e7b1d7a9

android/defaults.go

@@ -15,6 +15,8 @@
 package android
 
 import (
+	"reflect"
+
 	"github.com/google/blueprint"
 	"github.com/google/blueprint/proptools"
 )
@@ -32,14 +34,16 @@ type defaultsProperties struct {
 type DefaultableModuleBase struct {
 	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,9 +199,37 @@ func (defaultable *DefaultableModuleBase) applyDefaults(ctx TopDownMutatorContex
 
 	for _, defaults := range defaultsList {
 		for _, prop := range defaultable.defaultableProperties {
-			for _, def := range defaults.properties() {
+			if prop == defaultable.defaultableVariableProperties {
-				if proptools.TypeEqual(prop, def) {
+				defaultable.applyDefaultVariableProperties(ctx, defaults, prop)
-					err := proptools.PrependProperties(prop, def, nil)
+			} 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())
@@ -195,6 +237,19 @@ func (defaultable *DefaultableModuleBase) applyDefaults(ctx TopDownMutatorContex
 			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)
 				}
 			}
 		}

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 {

android/module.go

@@ -537,16 +537,7 @@ func InitAndroidModule(m Module) {
 		&base.nameProperties,
 		&base.commonProperties)
 
-	// Allow tests to override the default product variables
+	initProductVariableModule(m)
-	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()

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.

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

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