Simplify visibility rules that include //visibility:public
While it is invalid to mix //visibility:public with other rules in the
visibility property in a .bp file tt was possible, by overriding
defaults, to have //visibility:public mixed in with other rules in the
effective visibility rules. That caused problems when those effective
rules were used in an sdk snapshot.
This change replaces any set of rules that include //visibility:public
with just the //visibility:public rule. That simplifies those rules,
making them cheaper to process and ensures that the effective rules are
valid in the visibility property.
Adding test support required some refactoring of the
effectiveVisibilityRules(BaseModuleContext, ...) and underlying methods
to take a Config instead of BaseModuleContext as the tests do not have
access to BaseModuleContext.
Bug: 142935992
Test: m nothing - new tests failed without change, work with it
Add dex2oat to art-module-host-exports, build it and check the
generated Android.bp file in the snapshot to ensure the
visibility property for the dex2oat prebuilt does not mix
//visibility:public with other rules.
Change-Id: I08e7f0dcb40838d426fe88fedf69eae27b77473c
diff --git a/android/visibility.go b/android/visibility.go
index a597687..3f04123 100644
--- a/android/visibility.go
+++ b/android/visibility.go
@@ -186,8 +186,8 @@
var visibilityRuleMap = NewOnceKey("visibilityRuleMap")
// The map from qualifiedModuleName to visibilityRule.
-func moduleToVisibilityRuleMap(ctx BaseModuleContext) *sync.Map {
- return ctx.Config().Once(visibilityRuleMap, func() interface{} {
+func moduleToVisibilityRuleMap(config Config) *sync.Map {
+ return config.Once(visibilityRuleMap, func() interface{} {
return &sync.Map{}
}).(*sync.Map)
}
@@ -304,7 +304,7 @@
if visibility := m.visibility(); visibility != nil {
rule := parseRules(ctx, currentPkg, m.visibility())
if rule != nil {
- moduleToVisibilityRuleMap(ctx).Store(qualifiedModuleId, rule)
+ moduleToVisibilityRuleMap(ctx.Config()).Store(qualifiedModuleId, rule)
}
}
}
@@ -312,6 +312,7 @@
func parseRules(ctx BaseModuleContext, currentPkg string, visibility []string) compositeRule {
rules := make(compositeRule, 0, len(visibility))
hasPrivateRule := false
+ hasPublicRule := false
hasNonPrivateRule := false
for _, v := range visibility {
ok, pkg, name := splitRule(v, currentPkg)
@@ -328,6 +329,7 @@
isPrivateRule = true
case "public":
r = publicRule{}
+ hasPublicRule = true
}
} else {
switch name {
@@ -355,6 +357,11 @@
return compositeRule{privateRule{}}
}
+ if hasPublicRule {
+ // Public overrides all other rules so just return it.
+ return compositeRule{publicRule{}}
+ }
+
return rules
}
@@ -415,21 +422,21 @@
return
}
- rule := effectiveVisibilityRules(ctx, depQualified)
+ rule := effectiveVisibilityRules(ctx.Config(), depQualified)
if rule != nil && !rule.matches(qualified) {
ctx.ModuleErrorf("depends on %s which is not visible to this module", depQualified)
}
})
}
-func effectiveVisibilityRules(ctx BaseModuleContext, qualified qualifiedModuleName) compositeRule {
- moduleToVisibilityRule := moduleToVisibilityRuleMap(ctx)
+func effectiveVisibilityRules(config Config, qualified qualifiedModuleName) compositeRule {
+ moduleToVisibilityRule := moduleToVisibilityRuleMap(config)
value, ok := moduleToVisibilityRule.Load(qualified)
var rule compositeRule
if ok {
rule = value.(compositeRule)
} else {
- rule = packageDefaultVisibility(ctx, qualified)
+ rule = packageDefaultVisibility(config, qualified)
}
return rule
}
@@ -441,8 +448,8 @@
return qualified
}
-func packageDefaultVisibility(ctx BaseModuleContext, moduleId qualifiedModuleName) compositeRule {
- moduleToVisibilityRule := moduleToVisibilityRuleMap(ctx)
+func packageDefaultVisibility(config Config, moduleId qualifiedModuleName) compositeRule {
+ moduleToVisibilityRule := moduleToVisibilityRuleMap(config)
packageQualifiedId := moduleId.getContainingPackageId()
for {
value, ok := moduleToVisibilityRule.Load(packageQualifiedId)
@@ -469,7 +476,7 @@
dir := ctx.OtherModuleDir(module)
qualified := qualifiedModuleName{dir, moduleName}
- rule := effectiveVisibilityRules(ctx, qualified)
+ rule := effectiveVisibilityRules(ctx.Config(), qualified)
return rule.Strings()
}
diff --git a/android/visibility_test.go b/android/visibility_test.go
index 6006072..8dd6a8f 100644
--- a/android/visibility_test.go
+++ b/android/visibility_test.go
@@ -1,15 +1,17 @@
package android
import (
+ "reflect"
"testing"
"github.com/google/blueprint"
)
var visibilityTests = []struct {
- name string
- fs map[string][]byte
- expectedErrors []string
+ name string
+ fs map[string][]byte
+ expectedErrors []string
+ effectiveVisibility map[qualifiedModuleName][]string
}{
{
name: "invalid visibility: empty list",
@@ -493,6 +495,9 @@
deps: ["libexample"],
}`),
},
+ effectiveVisibility: map[qualifiedModuleName][]string{
+ qualifiedModuleName{pkg: "top", name: "libexample"}: {"//visibility:public"},
+ },
},
{
name: "//visibility:public mixed with other from different defaults 1",
@@ -903,13 +908,27 @@
func TestVisibility(t *testing.T) {
for _, test := range visibilityTests {
t.Run(test.name, func(t *testing.T) {
- _, errs := testVisibility(buildDir, test.fs)
+ ctx, errs := testVisibility(buildDir, test.fs)
CheckErrorsAgainstExpectations(t, errs, test.expectedErrors)
+
+ if test.effectiveVisibility != nil {
+ checkEffectiveVisibility(t, ctx, test.effectiveVisibility)
+ }
})
}
}
+func checkEffectiveVisibility(t *testing.T, ctx *TestContext, effectiveVisibility map[qualifiedModuleName][]string) {
+ for moduleName, expectedRules := range effectiveVisibility {
+ rule := effectiveVisibilityRules(ctx.config, moduleName)
+ stringRules := rule.Strings()
+ if !reflect.DeepEqual(expectedRules, stringRules) {
+ t.Errorf("effective rules mismatch: expected %q, found %q", expectedRules, stringRules)
+ }
+ }
+}
+
func testVisibility(buildDir string, fs map[string][]byte) (*TestContext, []error) {
// Create a new config per test as visibility information is stored in the config.