Refactor visibility to support visibility on defaults modules
Existing modules, either general one or package ones have a single
visibility property, called visibility in general, and
default_visibility on package, that controls access to that module, or
in the case of package sets the default visibility of all modules in
that package. The property is checked and gathered during the similarly
named phases of visibility processing.
The defaults module will be different as it will have two properties.
The first, visibility, will not affect the visibility of the module, it
only affects the visibility of modules that 'extend' the defaults. So,
it will need checking but not parsing. The second property,
defaults_visibility, will affect the visibility of the module and so
will need both checking and parsing.
The current implementation does not handle those cases because:
1) It does not differentiate between the property that affects the
module and those that do not. It checks and gathers all of them with
the last property gathered overriding the rules for the previous
properties.
2) It relies on overriding methods in MethodBase in order to change the
default behavior for the package module. That works because
packageModule embeds ModuleBase but will not work for
DefaultsModuleBase as it does not embed ModuleBase and instead is
embedded alongside it so attempting to override a method in
MethodBase leads to ambiguity.
This change addresses the issues as follows:
1) It adds a new visibility() []string method to get access to the
primary visibility rules, i.e. the ones that affect the module.
2) It adds two fields, 'visibilityPropertyInfo []visibilityProperty'
to provide information about all the properties that need checking,
and 'primaryVisibilityProperty visibilityProperty' to specify the
property that affects the module.
The PackageFactory() and InitAndroidModule(Module) functions are
modified to initialize the fields. The override of the
visibilityProperties() method for packageModule is removed and the
default implementations of visibilityProperties() and visibility()
on ModuleBase return information from the two new fields.
The InitDefaultsModule is updated to also initialize the two new
fields. It uses nil for primaryVisibilityProperty for now but that
will be changed to return defaults_visibility. It also uses the
commonProperties structure created for the defaults directly instead
of having to search for it through properties().
Changed the visibilityProperty to take a pointer to the property that
can be used to retrieve the value rather than a lambda function.
Bug: 130796911
Test: m nothing
Change-Id: Icadd470a5f692a48ec61de02bf3dfde3e2eea2ef
diff --git a/android/defaults.go b/android/defaults.go
index 9db0c36..2cfd77a 100644
--- a/android/defaults.go
+++ b/android/defaults.go
@@ -70,6 +70,9 @@
type DefaultsModuleBase struct {
DefaultableModuleBase
+
+ // Container for defaults of the common properties
+ commonProperties commonProperties
}
// The common pattern for defaults modules is to register separate instances of
@@ -101,6 +104,9 @@
// Get the structures containing the properties for which defaults can be provided.
properties() []interface{}
+
+ // Return the defaults common properties.
+ common() *commonProperties
}
func (d *DefaultsModuleBase) isDefaults() bool {
@@ -116,22 +122,41 @@
return d.defaultableProperties
}
+func (d *DefaultsModuleBase) common() *commonProperties {
+ return &d.commonProperties
+}
+
func (d *DefaultsModuleBase) GenerateAndroidBuildActions(ctx ModuleContext) {
}
func InitDefaultsModule(module DefaultsModule) {
+ commonProperties := module.common()
+
module.AddProperties(
&hostAndDeviceProperties{},
- &commonProperties{},
+ commonProperties,
&variableProperties{})
InitArchModule(module)
InitDefaultableModule(module)
// Add properties that will not have defaults applied to them.
- module.AddProperties(&module.base().nameProperties)
+ base := module.base()
+ module.AddProperties(&base.nameProperties)
- module.base().module = module
+ // There is currently no way to control the visibility of a defaults module, i.e. there is no
+ // primary visibility property.
+ base.primaryVisibilityProperty = nil
+
+ // Unlike non-defaults modules the visibility property is not stored in m.base().commonProperties.
+ // Instead it is stored in a separate instance of commonProperties created above so use that.
+ // The visibility property needs to be checked (but not parsed) by the visibility module during
+ // its checking phase and parsing phase.
+ base.visibilityPropertyInfo = []visibilityProperty{
+ newVisibilityProperty("visibility", &commonProperties.Visibility),
+ }
+
+ base.module = module
}
var _ Defaults = (*DefaultsModuleBase)(nil)
diff --git a/android/module.go b/android/module.go
index adb9454..5bb1177 100644
--- a/android/module.go
+++ b/android/module.go
@@ -211,6 +211,9 @@
// Get information about the properties that can contain visibility rules.
visibilityProperties() []visibilityProperty
+
+ // Get the visibility rules that control the visibility of this module.
+ visibility() []string
}
// Qualified id for a module
@@ -503,6 +506,12 @@
&base.variableProperties)
base.generalProperties = m.GetProperties()
base.customizableProperties = m.GetProperties()
+
+ // The default_visibility property needs to be checked and parsed by the visibility module during
+ // its checking and parsing phases.
+ base.primaryVisibilityProperty =
+ newVisibilityProperty("visibility", &base.commonProperties.Visibility)
+ base.visibilityPropertyInfo = []visibilityProperty{base.primaryVisibilityProperty}
}
func InitAndroidArchModule(m Module, hod HostOrDeviceSupported, defaultMultilib Multilib) {
@@ -582,6 +591,13 @@
archProperties [][]interface{}
customizableProperties []interface{}
+ // Information about all the properties on the module that contains visibility rules that need
+ // checking.
+ visibilityPropertyInfo []visibilityProperty
+
+ // The primary visibility property, may be nil, that controls access to the module.
+ primaryVisibilityProperty visibilityProperty
+
noAddressSanitizer bool
installFiles Paths
checkbuildFiles Paths
@@ -668,10 +684,15 @@
}
func (m *ModuleBase) visibilityProperties() []visibilityProperty {
- return []visibilityProperty{
- newVisibilityProperty("visibility", func() []string {
- return m.base().commonProperties.Visibility
- }),
+ return m.visibilityPropertyInfo
+}
+
+func (m *ModuleBase) visibility() []string {
+ // The soong_namespace module does not initialize the primaryVisibilityProperty.
+ if m.primaryVisibilityProperty != nil {
+ return m.primaryVisibilityProperty.getStrings()
+ } else {
+ return nil
}
}
diff --git a/android/package.go b/android/package.go
index 03f6a1e..880d6a9 100644
--- a/android/package.go
+++ b/android/package.go
@@ -64,16 +64,6 @@
return newPackageId(ctx.ModuleDir())
}
-// Override to ensure that the default_visibility rules are checked by the visibility module during
-// its checking phase.
-func (p *packageModule) visibilityProperties() []visibilityProperty {
- return []visibilityProperty{
- newVisibilityProperty("default_visibility", func() []string {
- return p.properties.Default_visibility
- }),
- }
-}
-
func (p *packageModule) Name() string {
return p.properties.Name
}
@@ -97,6 +87,13 @@
module.properties.Name = name
module.AddProperties(&module.properties)
+
+ // The default_visibility property needs to be checked and parsed by the visibility module during
+ // its checking and parsing phases.
+ module.primaryVisibilityProperty =
+ newVisibilityProperty("default_visibility", &module.properties.Default_visibility)
+ module.visibilityPropertyInfo = []visibilityProperty{module.primaryVisibilityProperty}
+
return module
}
diff --git a/android/visibility.go b/android/visibility.go
index 94af343..a7e718b 100644
--- a/android/visibility.go
+++ b/android/visibility.go
@@ -67,8 +67,8 @@
// Describes the properties provided by a module that contain visibility rules.
type visibilityPropertyImpl struct {
- name string
- stringsGetter func() []string
+ name string
+ stringsProperty *[]string
}
type visibilityProperty interface {
@@ -76,10 +76,10 @@
getStrings() []string
}
-func newVisibilityProperty(name string, stringsGetter func() []string) visibilityProperty {
+func newVisibilityProperty(name string, stringsProperty *[]string) visibilityProperty {
return visibilityPropertyImpl{
- name: name,
- stringsGetter: stringsGetter,
+ name: name,
+ stringsProperty: stringsProperty,
}
}
@@ -88,7 +88,7 @@
}
func (p visibilityPropertyImpl) getStrings() []string {
- return p.stringsGetter()
+ return *p.stringsProperty
}
// A compositeRule is a visibility rule composed from a list of atomic visibility rules.
@@ -211,16 +211,7 @@
// Checks the per-module visibility rule lists before defaults expansion.
func visibilityRuleChecker(ctx BottomUpMutatorContext) {
qualified := createQualifiedModuleName(ctx)
- if d, ok := ctx.Module().(Defaults); ok {
- // Defaults modules don't store the payload properties in m.base().
- for _, props := range d.properties() {
- if cp, ok := props.(*commonProperties); ok {
- if visibility := cp.Visibility; visibility != nil {
- checkRules(ctx, qualified.pkg, "visibility", visibility)
- }
- }
- }
- } else if m, ok := ctx.Module().(Module); ok {
+ if m, ok := ctx.Module().(Module); ok {
visibilityProperties := m.visibilityProperties()
for _, p := range visibilityProperties {
if visibility := p.getStrings(); visibility != nil {
@@ -294,14 +285,12 @@
qualifiedModuleId := m.qualifiedModuleId(ctx)
currentPkg := qualifiedModuleId.pkg
- // Parse all the properties into rules and store them.
- visibilityProperties := m.visibilityProperties()
- for _, p := range visibilityProperties {
- if visibility := p.getStrings(); visibility != nil {
- rule := parseRules(ctx, currentPkg, visibility)
- if rule != nil {
- moduleToVisibilityRuleMap(ctx).Store(qualifiedModuleId, rule)
- }
+ // Parse the visibility rules that control access to the module and store them by id
+ // for use when enforcing the rules.
+ if visibility := m.visibility(); visibility != nil {
+ rule := parseRules(ctx, currentPkg, m.visibility())
+ if rule != nil {
+ moduleToVisibilityRuleMap(ctx).Store(qualifiedModuleId, rule)
}
}
}