Add hook to be called after defaults have been applied
Previously, the only way for a module type to create modules (other
than defining its own mutator) was to register a LoadHook. However,
that is called before defaults are applied so any properties used in
that hook cannot take advantage of defaults, e.g. java_sdk_library
cannot use defaults to set its sdk_version property and have that
affect its child modules.
This change adds a new SetDefaultableHook() to DefaultableModule to
register a hook that is called after any defaults have been applied.
Also adds some tests to ensure that errors in the visibility property
introduced in a DefaultableHook are reported in the gather phase of
visibility processing.
A follow up change will switch java_sdk_library to use that instead
of the AddLoadHook() mechanism.
Bug: 155295806
Test: m checkapi
Change-Id: I13df3115f9e225f7324b6725eaeb16a78cc2538a
diff --git a/android/defaults.go b/android/defaults.go
index 6a908ea..81e340e 100644
--- a/android/defaults.go
+++ b/android/defaults.go
@@ -35,6 +35,9 @@
defaultsProperties defaultsProperties
defaultableProperties []interface{}
defaultableVariableProperties interface{}
+
+ // The optional hook to call after any defaults have been applied.
+ hook DefaultableHook
}
func (d *DefaultableModuleBase) defaults() *defaultsProperties {
@@ -46,6 +49,16 @@
d.defaultableVariableProperties = variableProperties
}
+func (d *DefaultableModuleBase) SetDefaultableHook(hook DefaultableHook) {
+ d.hook = hook
+}
+
+func (d *DefaultableModuleBase) callHookIfAvailable(ctx DefaultableHookContext) {
+ if d.hook != nil {
+ d.hook(ctx)
+ }
+}
+
// Interface that must be supported by any module to which defaults can be applied.
type Defaultable interface {
// Get a pointer to the struct containing the Defaults property.
@@ -57,6 +70,15 @@
// Apply defaults from the supplied Defaults to the property structures supplied to
// setProperties(...).
applyDefaults(TopDownMutatorContext, []Defaults)
+
+ // Set the hook to be called after any defaults have been applied.
+ //
+ // Should be used in preference to a AddLoadHook when the behavior of the load
+ // hook is dependent on properties supplied in the Android.bp file.
+ SetDefaultableHook(hook DefaultableHook)
+
+ // Call the hook if specified.
+ callHookIfAvailable(context DefaultableHookContext)
}
type DefaultableModule interface {
@@ -75,6 +97,15 @@
module.AddProperties(module.defaults())
}
+// A restricted subset of context methods, similar to LoadHookContext.
+type DefaultableHookContext interface {
+ EarlyModuleContext
+
+ CreateModule(ModuleFactory, ...interface{}) Module
+}
+
+type DefaultableHook func(ctx DefaultableHookContext)
+
// The Defaults_visibility property.
type DefaultsVisibilityProperties struct {
@@ -268,25 +299,29 @@
}
func defaultsMutator(ctx TopDownMutatorContext) {
- if defaultable, ok := ctx.Module().(Defaultable); ok && len(defaultable.defaults().Defaults) > 0 {
- var defaultsList []Defaults
- seen := make(map[Defaults]bool)
+ if defaultable, ok := ctx.Module().(Defaultable); ok {
+ if len(defaultable.defaults().Defaults) > 0 {
+ var defaultsList []Defaults
+ seen := make(map[Defaults]bool)
- ctx.WalkDeps(func(module, parent Module) bool {
- if ctx.OtherModuleDependencyTag(module) == DefaultsDepTag {
- if defaults, ok := module.(Defaults); ok {
- if !seen[defaults] {
- seen[defaults] = true
- defaultsList = append(defaultsList, defaults)
- return len(defaults.defaults().Defaults) > 0
+ ctx.WalkDeps(func(module, parent Module) bool {
+ if ctx.OtherModuleDependencyTag(module) == DefaultsDepTag {
+ if defaults, ok := module.(Defaults); ok {
+ if !seen[defaults] {
+ seen[defaults] = true
+ defaultsList = append(defaultsList, defaults)
+ return len(defaults.defaults().Defaults) > 0
+ }
+ } else {
+ ctx.PropertyErrorf("defaults", "module %s is not an defaults module",
+ ctx.OtherModuleName(module))
}
- } else {
- ctx.PropertyErrorf("defaults", "module %s is not an defaults module",
- ctx.OtherModuleName(module))
}
- }
- return false
- })
- defaultable.applyDefaults(ctx, defaultsList)
+ return false
+ })
+ defaultable.applyDefaults(ctx, defaultsList)
+ }
+
+ defaultable.callHookIfAvailable(ctx)
}
}
diff --git a/android/hooks.go b/android/hooks.go
index 47f69d1..f43a007 100644
--- a/android/hooks.go
+++ b/android/hooks.go
@@ -39,6 +39,12 @@
moduleFactories() map[string]blueprint.ModuleFactory
}
+// Add a hook that will be called once the module has been loaded, i.e. its
+// properties have been initialized from the Android.bp file.
+//
+// Consider using SetDefaultableHook to register a hook for any module that implements
+// DefaultableModule as the hook is called after any defaults have been applied to the
+// module which could reduce duplication and make it easier to use.
func AddLoadHook(m blueprint.Module, hook func(LoadHookContext)) {
blueprint.AddLoadHook(m, func(ctx blueprint.LoadHookContext) {
actx := &loadHookContext{
diff --git a/android/mutator.go b/android/mutator.go
index be74099..77d5f43 100644
--- a/android/mutator.go
+++ b/android/mutator.go
@@ -110,10 +110,17 @@
RegisterVisibilityRuleChecker,
// Apply properties from defaults modules to the referencing modules.
+ //
+ // Any mutators that are added before this will not see any modules created by
+ // a DefaultableHook.
RegisterDefaultsPreArchMutators,
// Create an association between prebuilt modules and their corresponding source
// modules (if any).
+ //
+ // Must be run after defaults mutators to ensure that any modules created by
+ // a DefaultableHook can be either a prebuilt or a source module with a matching
+ // prebuilt.
RegisterPrebuiltsPreArchMutators,
// Gather the visibility rules for all modules for us during visibility enforcement.
diff --git a/android/visibility_test.go b/android/visibility_test.go
index 8dd6a8f..4cf41a6 100644
--- a/android/visibility_test.go
+++ b/android/visibility_test.go
@@ -903,6 +903,69 @@
}`),
},
},
+ {
+ name: "ensure visibility properties are checked for correctness",
+ fs: map[string][]byte{
+ "top/Blueprints": []byte(`
+ mock_parent {
+ name: "parent",
+ visibility: ["//top/nested"],
+ child: {
+ name: "libchild",
+ visibility: ["top/other"],
+ },
+ }`),
+ },
+ expectedErrors: []string{
+ `module "parent": child.visibility: invalid visibility pattern "top/other"`,
+ },
+ },
+ {
+ name: "invalid visibility added to child detected during gather phase",
+ fs: map[string][]byte{
+ "top/Blueprints": []byte(`
+ mock_parent {
+ name: "parent",
+ visibility: ["//top/nested"],
+ child: {
+ name: "libchild",
+ invalid_visibility: ["top/other"],
+ },
+ }`),
+ },
+ expectedErrors: []string{
+ // That this error is reported against the child not the parent shows it was
+ // not being detected in the parent which is correct as invalid_visibility is
+ // purposely not added to the list of visibility properties to check, and was
+ // in fact detected in the child in the gather phase. Contrast this error message
+ // with the preceding one.
+ `module "libchild" \(created by module "parent"\): visibility: invalid visibility pattern "top/other"`,
+ },
+ },
+ {
+ name: "automatic visibility inheritance enabled",
+ fs: map[string][]byte{
+ "top/Blueprints": []byte(`
+ mock_parent {
+ name: "parent",
+ visibility: ["//top/nested"],
+ child: {
+ name: "libchild",
+ visibility: ["//top/other"],
+ },
+ }`),
+ "top/nested/Blueprints": []byte(`
+ mock_library {
+ name: "libnested",
+ deps: ["libchild"],
+ }`),
+ "top/other/Blueprints": []byte(`
+ mock_library {
+ name: "libother",
+ deps: ["libchild"],
+ }`),
+ },
+ },
}
func TestVisibility(t *testing.T) {
@@ -936,6 +999,7 @@
ctx := NewTestArchContext()
ctx.RegisterModuleType("mock_library", newMockLibraryModule)
+ ctx.RegisterModuleType("mock_parent", newMockParentFactory)
ctx.RegisterModuleType("mock_defaults", defaultsFactory)
// Order of the following method calls is significant.
@@ -996,3 +1060,42 @@
InitDefaultsModule(m)
return m
}
+
+type mockParentProperties struct {
+ Child struct {
+ Name *string
+
+ // Visibility to pass to the child module.
+ Visibility []string
+
+ // Purposely not validated visibility to pass to the child.
+ Invalid_visibility []string
+ }
+}
+
+type mockParent struct {
+ ModuleBase
+ DefaultableModuleBase
+ properties mockParentProperties
+}
+
+func (p *mockParent) GenerateAndroidBuildActions(ModuleContext) {
+}
+
+func newMockParentFactory() Module {
+ m := &mockParent{}
+ m.AddProperties(&m.properties)
+ InitAndroidArchModule(m, HostAndDeviceSupported, MultilibCommon)
+ InitDefaultableModule(m)
+ AddVisibilityProperty(m, "child.visibility", &m.properties.Child.Visibility)
+
+ m.SetDefaultableHook(func(ctx DefaultableHookContext) {
+ visibility := m.properties.Child.Visibility
+ visibility = append(visibility, m.properties.Child.Invalid_visibility...)
+ ctx.CreateModule(newMockLibraryModule, &struct {
+ Name *string
+ Visibility []string
+ }{m.properties.Child.Name, visibility})
+ })
+ return m
+}