tequilaOS/platform_build_soong
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Add protected_properties support in defaults modules

Browse source 
Previously, there was no way to prevent a module from overriding a
value provided by a defaults. That made it difficult to ensure
consistency across modules, e.g. for modules that use
framework-module-defaults.

This change adds the protected_properties property to defaults modules
which allows a default module to list those properties that should not
be changed by a module applying those defaults.

Properties can either be listed explicitly by name, or it can just be
a single "*" in which case all properties that are set on the defaults
will be protected.

Bug: 230841626
Test: m nothing
Change-Id: Ibb0e482c2856a572437e7818466f41c5493baf33
This commit is contained in:
Paul Duffin 2022-05-04 11:39:52 +00:00
parent a0bab18a47
commit 799962789a
2 changed files with 463 additions and 31 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

321
android/defaults.go
View file

@ -15,6 +15,8 @@
package android
import (
"bytes"
"fmt"
"reflect"
"github.com/google/blueprint"
@ -67,9 +69,11 @@ type Defaultable interface {
// Set the property structures into which defaults will be added.
setProperties(props []interface{}, variableProperties interface{})
// Apply defaults from the supplied Defaults to the property structures supplied to
// Apply defaults from the supplied DefaultsModule to the property structures supplied to
// setProperties(...).
applyDefaults(TopDownMutatorContext, []Defaults)
applyDefaults(TopDownMutatorContext, []DefaultsModule)
applySingleDefaultsWithTracker(EarlyModuleContext, DefaultsModule, defaultsTrackerFunc)
// Set the hook to be called after any defaults have been applied.
//
@ -115,9 +119,23 @@ type DefaultsVisibilityProperties struct {
Defaults_visibility []string
}
// AdditionalDefaultsProperties contains properties of defaults modules which
// can have other defaults applied.
type AdditionalDefaultsProperties struct {
// The list of properties set by the default whose values must not be changed by any module that
// applies these defaults. It is an error if a property is not supported by the defaults module or
// has not been set to a non-zero value. If this contains "*" then that must be the only entry in
// which case all properties that are set on this defaults will be protected (except the
// protected_properties and visibility.
Protected_properties []string
}
type DefaultsModuleBase struct {
DefaultableModuleBase
defaultsProperties AdditionalDefaultsProperties
// Included to support setting bazel_module.label for multiple Soong modules to the same Bazel
// target. This is primarily useful for modules that were architecture specific and instead are
// handled in Bazel as a select().
@ -151,6 +169,18 @@ type Defaults interface {
// DefaultsModuleBase will type-assert to the Defaults interface.
isDefaults() bool
// additionalDefaultableProperties returns additional properties provided by the defaults which
// can themselves have defaults applied.
additionalDefaultableProperties() []interface{}
// protectedProperties returns the names of the properties whose values cannot be changed by a
// module that applies these defaults.
protectedProperties() []string
// setProtectedProperties sets the names of the properties whose values cannot be changed by a
// module that applies these defaults.
setProtectedProperties(protectedProperties []string)
// Get the structures containing the properties for which defaults can be provided.
properties() []interface{}
@ -167,6 +197,18 @@ type DefaultsModule interface {
Bazelable
}
func (d *DefaultsModuleBase) additionalDefaultableProperties() []interface{} {
return []interface{}{&d.defaultsProperties}
}
func (d *DefaultsModuleBase) protectedProperties() []string {
return d.defaultsProperties.Protected_properties
}
func (d *DefaultsModuleBase) setProtectedProperties(protectedProperties []string) {
d.defaultsProperties.Protected_properties = protectedProperties
}
func (d *DefaultsModuleBase) properties() []interface{} {
return d.defaultableProperties
}
@ -190,6 +232,10 @@ func InitDefaultsModule(module DefaultsModule) {
&ApexProperties{},
&distProperties{})
// Additional properties of defaults modules that can themselves have
// defaults applied.
module.AddProperties(module.additionalDefaultableProperties()...)
// Bazel module must be initialized _before_ Defaults to be included in cc_defaults module.
InitBazelModule(module)
initAndroidModuleBase(module)
@ -218,6 +264,57 @@ func InitDefaultsModule(module DefaultsModule) {
// The applicable licenses property for defaults is 'licenses'.
setPrimaryLicensesProperty(module, "licenses", &commonProperties.Licenses)
AddLoadHook(module, func(ctx LoadHookContext) {
protectedProperties := module.protectedProperties()
if len(protectedProperties) == 0 {
return
}
propertiesAvailable := map[string]struct{}{}
propertiesSet := map[string]struct{}{}
// A defaults tracker which will keep track of which properties have been set on this module.
collector := func(defaults DefaultsModule, property string, dstValue interface{}, srcValue interface{}) bool {
value := reflect.ValueOf(dstValue)
propertiesAvailable[property] = struct{}{}
if !value.IsZero() {
propertiesSet[property] = struct{}{}
}
// Skip all the properties so that there are no changes to the defaults.
return false
}
// Try and apply this module's defaults to itself, so that the properties can be collected but
// skip all the properties so it doesn't actually do anything.
module.applySingleDefaultsWithTracker(ctx, module, collector)
if InList("*", protectedProperties) {
if len(protectedProperties) != 1 {
ctx.PropertyErrorf("protected_properties", `if specified then "*" must be the only property listed`)
return
}
// Do not automatically protect the protected_properties property.
delete(propertiesSet, "protected_properties")
// Or the visibility property.
delete(propertiesSet, "visibility")
// Replace the "*" with the names of all the properties that have been set.
protectedProperties = SortedStringKeys(propertiesSet)
module.setProtectedProperties(protectedProperties)
} else {
for _, property := range protectedProperties {
if _, ok := propertiesAvailable[property]; !ok {
ctx.PropertyErrorf(property, "property is not supported by this module type %q",
ctx.ModuleType())
} else if _, ok := propertiesSet[property]; !ok {
ctx.PropertyErrorf(property, "is not set; protected properties must be explicitly set")
}
}
}
})
}
var _ Defaults = (*DefaultsModuleBase)(nil)
@ -269,35 +366,204 @@ func applyNamespacedVariableDefaults(defaultDep Defaults, ctx TopDownMutatorCont
b.setNamespacedVariableProps(dst)
}
// defaultValueInfo contains information about each default value that applies to a protected
// property.
type defaultValueInfo struct {
// The DefaultsModule providing the value, which may be defined on that module or applied as a
// default from other modules.
module Module
// The default value, as returned by getComparableValue
defaultValue reflect.Value
}
// protectedPropertyInfo contains information about each property that has to be protected when
// applying defaults.
type protectedPropertyInfo struct {
// True if the property was set on the module to which defaults are applied, this is an error.
propertySet bool
// The original value of the property on the module, as returned by getComparableValue.
originalValue reflect.Value
// A list of defaults for the property that are being applied.
defaultValues []defaultValueInfo
}
// getComparableValue takes a reflect.Value that may be a pointer to another value and returns a
// reflect.Value to the underlying data or the original if was not a pointer or was nil. The
// returned values can then be compared for equality.
func getComparableValue(value reflect.Value) reflect.Value {
if value.IsZero() {
return value
}
for value.Kind() == reflect.Ptr {
value = value.Elem()
}
return value
}
func (defaultable *DefaultableModuleBase) applyDefaults(ctx TopDownMutatorContext,
defaultsList []Defaults) {
defaultsList []DefaultsModule) {
// Collate information on all the properties protected by each of the default modules applied
// to this module.
allProtectedProperties := map[string]*protectedPropertyInfo{}
for _, defaults := range defaultsList {
for _, property := range defaults.protectedProperties() {
info := allProtectedProperties[property]
if info == nil {
info = &protectedPropertyInfo{}
allProtectedProperties[property] = info
}
}
}
// If there are any protected properties then collate information about attempts to change them.
var protectedPropertyInfoCollector defaultsTrackerFunc
if len(allProtectedProperties) > 0 {
protectedPropertyInfoCollector = func(defaults DefaultsModule, property string,
dstValue interface{}, srcValue interface{}) bool {
// If the property is not protected then return immediately.
info := allProtectedProperties[property]
if info == nil {
return true
}
currentValue := reflect.ValueOf(dstValue)
if info.defaultValues == nil {
info.propertySet = !currentValue.IsZero()
info.originalValue = getComparableValue(currentValue)
}
defaultValue := reflect.ValueOf(srcValue)
if !defaultValue.IsZero() {
info.defaultValues = append(info.defaultValues,
defaultValueInfo{defaults, getComparableValue(defaultValue)})
}
return true
}
}
for _, defaults := range defaultsList {
if ctx.Config().runningAsBp2Build {
applyNamespacedVariableDefaults(defaults, ctx)
}
for _, prop := range defaultable.defaultableProperties {
if prop == defaultable.defaultableVariableProperties {
defaultable.applyDefaultVariableProperties(ctx, defaults, prop)
} else {
defaultable.applyDefaultProperties(ctx, defaults, prop)
defaultable.applySingleDefaultsWithTracker(ctx, defaults, protectedPropertyInfoCollector)
}
// Check the status of any protected properties.
for property, info := range allProtectedProperties {
if len(info.defaultValues) == 0 {
// No defaults were applied to the protected properties. Possibly because this module type
// does not support any of them.
continue
}
// Check to make sure that there are no conflicts between the defaults.
conflictingDefaults := false
previousDefaultValue := reflect.ValueOf(false)
for _, defaultInfo := range info.defaultValues {
defaultValue := defaultInfo.defaultValue
if previousDefaultValue.IsZero() {
previousDefaultValue = defaultValue
} else if !reflect.DeepEqual(previousDefaultValue.Interface(), defaultValue.Interface()) {
conflictingDefaults = true
break
}
}
if conflictingDefaults {
var buf bytes.Buffer
for _, defaultInfo := range info.defaultValues {
buf.WriteString(fmt.Sprintf("\n defaults module %q provides value %#v",
ctx.OtherModuleName(defaultInfo.module), defaultInfo.defaultValue))
}
result := buf.String()
ctx.ModuleErrorf("has conflicting default values for protected property %q:%s", property, result)
continue
}
// Now check to see whether there the current module tried to override/append to the defaults.
if info.propertySet {
originalValue := info.originalValue
// Just compare against the first defaults.
defaultValue := info.defaultValues[0].defaultValue
defaults := info.defaultValues[0].module
if originalValue.Kind() == reflect.Slice {
ctx.ModuleErrorf("attempts to append %q to protected property %q's value of %q defined in module %q",
originalValue,
property,
defaultValue,
ctx.OtherModuleName(defaults))
} else {
same := reflect.DeepEqual(originalValue.Interface(), defaultValue.Interface())
message := ""
if same {
message = fmt.Sprintf(" with a matching value (%#v) so this property can simply be removed.", originalValue)
} else {
message = fmt.Sprintf(" with a different value (override %#v with %#v) so removing the property may necessitate other changes.", defaultValue, originalValue)
}
ctx.ModuleErrorf("attempts to override protected property %q defined in module %q%s",
property,
ctx.OtherModuleName(defaults), message)
}
}
}
}
func (defaultable *DefaultableModuleBase) applySingleDefaultsWithTracker(ctx EarlyModuleContext, defaults DefaultsModule, tracker defaultsTrackerFunc) {
for _, prop := range defaultable.defaultableProperties {
var err error
if prop == defaultable.defaultableVariableProperties {
err = defaultable.applyDefaultVariableProperties(defaults, prop, tracker)
} else {
err = defaultable.applyDefaultProperties(defaults, prop, tracker)
}
if err != nil {
if propertyErr, ok := err.(*proptools.ExtendPropertyError); ok {
ctx.PropertyErrorf(propertyErr.Property, "%s", propertyErr.Err.Error())
} else {
panic(err)
}
}
}
}
// defaultsTrackerFunc is the type of a function that can be used to track how defaults are applied.
type defaultsTrackerFunc func(defaults DefaultsModule, property string,
dstValue interface{}, srcValue interface{}) bool
// filterForTracker wraps a defaultsTrackerFunc in a proptools.ExtendPropertyFilterFunc
func filterForTracker(defaults DefaultsModule, tracker defaultsTrackerFunc) proptools.ExtendPropertyFilterFunc {
if tracker == nil {
return nil
}
return func(property string,
dstField, srcField reflect.StructField,
dstValue, srcValue interface{}) (bool, error) {
apply := tracker(defaults, property, dstValue, srcValue)
return apply, nil
}
}
// 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{}) {
func (defaultable *DefaultableModuleBase) applyDefaultVariableProperties(defaults DefaultsModule,
defaultableProp interface{}, tracker defaultsTrackerFunc) error {
if defaultableProp == nil {
return
return nil
}
defaultsProp := defaults.productVariableProperties()
if defaultsProp == nil {
return
return nil
}
dst := []interface{}{
@ -307,31 +573,26 @@ func (defaultable *DefaultableModuleBase) applyDefaultVariableProperties(ctx Top
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)
}
}
filter := filterForTracker(defaults, tracker)
return proptools.PrependMatchingProperties(dst, defaultsProp, filter)
}
func (defaultable *DefaultableModuleBase) applyDefaultProperties(ctx TopDownMutatorContext,
defaults Defaults, defaultableProp interface{}) {
func (defaultable *DefaultableModuleBase) applyDefaultProperties(defaults DefaultsModule,
defaultableProp interface{}, checker defaultsTrackerFunc) error {
filter := filterForTracker(defaults, checker)
for _, def := range defaults.properties() {
if proptools.TypeEqual(defaultableProp, def) {
err := proptools.PrependProperties(defaultableProp, def, nil)
err := proptools.PrependProperties(defaultableProp, def, filter)
if err != nil {
if propertyErr, ok := err.(*proptools.ExtendPropertyError); ok {
ctx.PropertyErrorf(propertyErr.Property, "%s", propertyErr.Err.Error())
} else {
panic(err)
}
return err
}
}
}
return nil
}
func RegisterDefaultsPreArchMutators(ctx RegisterMutatorsContext) {
@ -348,12 +609,12 @@ func defaultsDepsMutator(ctx BottomUpMutatorContext) {
func defaultsMutator(ctx TopDownMutatorContext) {
if defaultable, ok := ctx.Module().(Defaultable); ok {
if len(defaultable.defaults().Defaults) > 0 {
var defaultsList []Defaults
var defaultsList []DefaultsModule
seen := make(map[Defaults]bool)
ctx.WalkDeps(func(module, parent Module) bool {
if ctx.OtherModuleDependencyTag(module) == DefaultsDepTag {
if defaults, ok := module.(Defaults); ok {
if defaults, ok := module.(DefaultsModule); ok {
if !seen[defaults] {
seen[defaults] = true
defaultsList = append(defaultsList, defaults)

173
android/defaults_test.go
View file

@ -19,7 +19,14 @@ import (
)
type defaultsTestProperties struct {
Foo []string
Foo []string
Bar []string
Nested struct {
Fizz *bool
}
Other struct {
Buzz *string
}
}
type defaultsTestModule struct {
@ -130,3 +137,167 @@ func TestDefaultsAllowMissingDependencies(t *testing.T) {
// TODO: missing transitive defaults is currently not handled
_ = missingTransitiveDefaults
}
func TestProtectedProperties_ProtectedPropertyNotSet(t *testing.T) {
bp := `
defaults {
name: "transitive",
protected_properties: ["foo"],
}
`
GroupFixturePreparers(
prepareForDefaultsTest,
FixtureWithRootAndroidBp(bp),
).ExtendWithErrorHandler(FixtureExpectsAtLeastOneErrorMatchingPattern(
"module \"transitive\": foo: is not set; protected properties must be explicitly set")).
RunTest(t)
}
func TestProtectedProperties_ProtectedPropertyNotLeaf(t *testing.T) {
bp := `
defaults {
name: "transitive",
protected_properties: ["nested"],
nested: {
fizz: true,
},
}
`
GroupFixturePreparers(
prepareForDefaultsTest,
FixtureWithRootAndroidBp(bp),
).ExtendWithErrorHandler(FixtureExpectsAtLeastOneErrorMatchingPattern(
`\Qmodule "transitive": nested: property is not supported by this module type "defaults"\E`)).
RunTest(t)
}
// TestProtectedProperties_ApplyDefaults makes sure that the protected_properties property has
// defaults applied.
func TestProtectedProperties_HasDefaultsApplied(t *testing.T) {
bp := `
defaults {
name: "transitive",
protected_properties: ["foo"],
foo: ["transitive"],
}
defaults {
name: "defaults",
defaults: ["transitive"],
protected_properties: ["bar"],
bar: ["defaults"],
}
`
result := GroupFixturePreparers(
prepareForDefaultsTest,
FixtureWithRootAndroidBp(bp),
).RunTest(t)
defaults := result.Module("defaults", "").(DefaultsModule)
AssertDeepEquals(t, "defaults protected properties", []string{"foo", "bar"}, defaults.protectedProperties())
}
// TestProtectedProperties_ProtectAllProperties makes sure that protected_properties: ["*"] protects
// all properties.
func TestProtectedProperties_ProtectAllProperties(t *testing.T) {
bp := `
defaults {
name: "transitive",
protected_properties: ["other.buzz"],
other: {
buzz: "transitive",
},
}
defaults {
name: "defaults",
defaults: ["transitive"],
visibility: ["//visibility:private"],
protected_properties: ["*"],
foo: ["other"],
bar: ["defaults"],
nested: {
fizz: true,
}
}
`
result := GroupFixturePreparers(
prepareForDefaultsTest,
FixtureWithRootAndroidBp(bp),
).RunTest(t)
defaults := result.Module("defaults", "").(DefaultsModule)
AssertDeepEquals(t, "defaults protected properties", []string{"other.buzz", "bar", "foo", "nested.fizz"},
defaults.protectedProperties())
}
func TestProtectedProperties_DetectedOverride(t *testing.T) {
bp := `
defaults {
name: "defaults",
protected_properties: ["foo", "nested.fizz"],
foo: ["defaults"],
nested: {
fizz: true,
},
}
test {
name: "foo",
defaults: ["defaults"],
foo: ["module"],
nested: {
fizz: false,
},
}
`
GroupFixturePreparers(
prepareForDefaultsTest,
FixtureWithRootAndroidBp(bp),
).ExtendWithErrorHandler(FixtureExpectsAllErrorsToMatchAPattern(
[]string{
`\Qmodule "foo": attempts to append ["module"] to protected property "foo"'s value of ["defaults"] defined in module "defaults"\E`,
`\Qmodule "foo": attempts to override protected property "nested.fizz" defined in module "defaults" with a different value (override true with false) so removing the property may necessitate other changes.\E`,
})).RunTest(t)
}
func TestProtectedProperties_DefaultsConflict(t *testing.T) {
bp := `
defaults {
name: "defaults1",
protected_properties: ["other.buzz"],
other: {
buzz: "value",
},
}
defaults {
name: "defaults2",
protected_properties: ["other.buzz"],
other: {
buzz: "another",
},
}
test {
name: "foo",
defaults: ["defaults1", "defaults2"],
}
`
GroupFixturePreparers(
prepareForDefaultsTest,
FixtureWithRootAndroidBp(bp),
).ExtendWithErrorHandler(FixtureExpectsAtLeastOneErrorMatchingPattern(
`\Qmodule "foo": has conflicting default values for protected property "other.buzz":
defaults module "defaults1" provides value "value"
defaults module "defaults2" provides value "another"\E`,
)).RunTest(t)
}