Ensure prebuilt modules have same visibility as source modules
Exports visibility and package mutator registration functions so they
can be used in sdk testing. Updates sdk test to support visibility and
package modules.
Adds EffectiveVisibility(...)[]string function to make the effective
visibility rules available to sdk snapshot creation.
Extracts compositeRule.Strings() []string from compositeRule.String()
method so that it can be used by above func.
Adds visibility property to sdk snapshot and prebuilt modules along
with a test to ensure it works properly.
Adds dir parameter to CheckSnapshot so that it can check the snapshot
generated for a non-root package. That is required in order to ensure
that visibility of :__subpackages__ on a source module in package
<pkg> is resolved to an effective visibility of
//<pkg>:__subpackages__ on its corresponding prebuilt.
Test: m conscrypt-module-sdk
Bug: 143678475
Change-Id: Icaacac5b9c04726d28e6fec93e49715ac45df7f4
diff --git a/android/mutator.go b/android/mutator.go
index eba0e2f..e9ccd7f 100644
--- a/android/mutator.go
+++ b/android/mutator.go
@@ -78,11 +78,11 @@
registerLoadHookMutator,
RegisterNamespaceMutator,
// Rename package module types.
- registerPackageRenamer,
+ RegisterPackageRenamer,
RegisterPrebuiltsPreArchMutators,
- registerVisibilityRuleChecker,
+ RegisterVisibilityRuleChecker,
RegisterDefaultsPreArchMutators,
- registerVisibilityRuleGatherer,
+ RegisterVisibilityRuleGatherer,
}
func registerArchMutator(ctx RegisterMutatorsContext) {
@@ -99,7 +99,7 @@
var postDeps = []RegisterMutatorFunc{
registerPathDepsMutator,
RegisterPrebuiltsPostDepsMutators,
- registerVisibilityRuleEnforcer,
+ RegisterVisibilityRuleEnforcer,
registerNeverallowMutator,
RegisterOverridePostDepsMutators,
}
diff --git a/android/package.go b/android/package.go
index 880d6a9..ed604c6 100644
--- a/android/package.go
+++ b/android/package.go
@@ -98,7 +98,7 @@
}
// Registers the function that renames the packages.
-func registerPackageRenamer(ctx RegisterMutatorsContext) {
+func RegisterPackageRenamer(ctx RegisterMutatorsContext) {
ctx.BottomUp("packageRenamer", packageRenamer).Parallel()
ctx.BottomUp("packageErrorReporter", packageErrorReporter).Parallel()
}
diff --git a/android/package_test.go b/android/package_test.go
index ae286d6..8071c51 100644
--- a/android/package_test.go
+++ b/android/package_test.go
@@ -88,7 +88,7 @@
ctx := NewTestArchContext()
ctx.RegisterModuleType("package", PackageFactory)
- ctx.PreArchMutators(registerPackageRenamer)
+ ctx.PreArchMutators(RegisterPackageRenamer)
ctx.Register()
ctx.MockFileSystem(fs)
diff --git a/android/visibility.go b/android/visibility.go
index a7e718b..c28ec93 100644
--- a/android/visibility.go
+++ b/android/visibility.go
@@ -117,12 +117,15 @@
}
func (c compositeRule) String() string {
+ return "[" + strings.Join(c.Strings(), ", ") + "]"
+}
+
+func (c compositeRule) Strings() []string {
s := make([]string, 0, len(c))
for _, r := range c {
s = append(s, r.String())
}
-
- return "[" + strings.Join(s, ", ") + "]"
+ return s
}
// A packageRule is a visibility rule that matches modules in a specific package (i.e. directory).
@@ -189,7 +192,7 @@
// The rule checker needs to be registered before defaults expansion to correctly check that
// //visibility:xxx isn't combined with other packages in the same list in any one module.
-func registerVisibilityRuleChecker(ctx RegisterMutatorsContext) {
+func RegisterVisibilityRuleChecker(ctx RegisterMutatorsContext) {
ctx.BottomUp("visibilityRuleChecker", visibilityRuleChecker).Parallel()
}
@@ -199,12 +202,12 @@
// having to process multiple variants for each module. This goes after defaults expansion to gather
// the complete visibility lists from flat lists and after the package info is gathered to ensure
// that default_visibility is available.
-func registerVisibilityRuleGatherer(ctx RegisterMutatorsContext) {
+func RegisterVisibilityRuleGatherer(ctx RegisterMutatorsContext) {
ctx.BottomUp("visibilityRuleGatherer", visibilityRuleGatherer).Parallel()
}
// This must be registered after the deps have been resolved.
-func registerVisibilityRuleEnforcer(ctx RegisterMutatorsContext) {
+func RegisterVisibilityRuleEnforcer(ctx RegisterMutatorsContext) {
ctx.TopDown("visibilityRuleEnforcer", visibilityRuleEnforcer).Parallel()
}
@@ -384,8 +387,6 @@
qualified := createQualifiedModuleName(ctx)
- moduleToVisibilityRule := moduleToVisibilityRuleMap(ctx)
-
// Visit all the dependencies making sure that this module has access to them all.
ctx.VisitDirectDeps(func(dep Module) {
depName := ctx.OtherModuleName(dep)
@@ -397,19 +398,25 @@
return
}
- value, ok := moduleToVisibilityRule.Load(depQualified)
- var rule compositeRule
- if ok {
- rule = value.(compositeRule)
- } else {
- rule = packageDefaultVisibility(ctx, depQualified)
- }
+ rule := effectiveVisibilityRules(ctx, 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)
+ value, ok := moduleToVisibilityRule.Load(qualified)
+ var rule compositeRule
+ if ok {
+ rule = value.(compositeRule)
+ } else {
+ rule = packageDefaultVisibility(ctx, qualified)
+ }
+ return rule
+}
+
func createQualifiedModuleName(ctx BaseModuleContext) qualifiedModuleName {
moduleName := ctx.ModuleName()
dir := ctx.ModuleDir()
@@ -433,3 +440,19 @@
packageQualifiedId = packageQualifiedId.getContainingPackageId()
}
}
+
+// Get the effective visibility rules, i.e. the actual rules that affect the visibility of the
+// property irrespective of where they are defined.
+//
+// Includes visibility rules specified by package default_visibility and/or on defaults.
+// Short hand forms, e.g. //:__subpackages__ are replaced with their full form, e.g.
+// //package/containing/rule:__subpackages__.
+func EffectiveVisibilityRules(ctx BaseModuleContext, module Module) []string {
+ moduleName := ctx.OtherModuleName(module)
+ dir := ctx.OtherModuleDir(module)
+ qualified := qualifiedModuleName{dir, moduleName}
+
+ rule := effectiveVisibilityRules(ctx, qualified)
+
+ return rule.Strings()
+}
diff --git a/android/visibility_test.go b/android/visibility_test.go
index fd9e98c..1984a21 100644
--- a/android/visibility_test.go
+++ b/android/visibility_test.go
@@ -874,11 +874,11 @@
ctx.RegisterModuleType("package", PackageFactory)
ctx.RegisterModuleType("mock_library", newMockLibraryModule)
ctx.RegisterModuleType("mock_defaults", defaultsFactory)
- ctx.PreArchMutators(registerPackageRenamer)
- ctx.PreArchMutators(registerVisibilityRuleChecker)
+ ctx.PreArchMutators(RegisterPackageRenamer)
+ ctx.PreArchMutators(RegisterVisibilityRuleChecker)
ctx.PreArchMutators(RegisterDefaultsPreArchMutators)
- ctx.PreArchMutators(registerVisibilityRuleGatherer)
- ctx.PostDepsMutators(registerVisibilityRuleEnforcer)
+ ctx.PreArchMutators(RegisterVisibilityRuleGatherer)
+ ctx.PostDepsMutators(RegisterVisibilityRuleEnforcer)
ctx.Register()
ctx.MockFileSystem(fs)
diff --git a/sdk/cc_sdk_test.go b/sdk/cc_sdk_test.go
index 315669a..7620ec1 100644
--- a/sdk/cc_sdk_test.go
+++ b/sdk/cc_sdk_test.go
@@ -164,7 +164,7 @@
}
`)
- result.CheckSnapshot("mysdk", "android_common",
+ result.CheckSnapshot("mysdk", "android_common", "",
checkAndroidBpContents(`
// This is auto-generated. DO NOT EDIT.
@@ -263,7 +263,7 @@
}
`)
- result.CheckSnapshot("mysdk", "linux_glibc_common",
+ result.CheckSnapshot("mysdk", "linux_glibc_common", "",
checkAndroidBpContents(`
// This is auto-generated. DO NOT EDIT.
diff --git a/sdk/java_sdk_test.go b/sdk/java_sdk_test.go
index 5b72248..1aa9184 100644
--- a/sdk/java_sdk_test.go
+++ b/sdk/java_sdk_test.go
@@ -123,7 +123,7 @@
}
`)
- result.CheckSnapshot("mysdk", "android_common",
+ result.CheckSnapshot("mysdk", "android_common", "",
checkAndroidBpContents(`
// This is auto-generated. DO NOT EDIT.
@@ -178,7 +178,7 @@
}
`)
- result.CheckSnapshot("mysdk", "linux_glibc_common",
+ result.CheckSnapshot("mysdk", "linux_glibc_common", "",
checkAndroidBpContents(`
// This is auto-generated. DO NOT EDIT.
@@ -232,7 +232,7 @@
}
`)
- result.CheckSnapshot("mysdk", "android_common",
+ result.CheckSnapshot("mysdk", "android_common", "",
checkAndroidBpContents(`
// This is auto-generated. DO NOT EDIT.
@@ -287,7 +287,7 @@
}
`)
- result.CheckSnapshot("mysdk", "linux_glibc_common",
+ result.CheckSnapshot("mysdk", "linux_glibc_common", "",
checkAndroidBpContents(`
// This is auto-generated. DO NOT EDIT.
@@ -379,7 +379,7 @@
}
`)
- result.CheckSnapshot("mysdk", "android_common",
+ result.CheckSnapshot("mysdk", "android_common", "",
checkAndroidBpContents(`
// This is auto-generated. DO NOT EDIT.
@@ -428,7 +428,7 @@
}
`)
- result.CheckSnapshot("mysdk", "linux_glibc_common",
+ result.CheckSnapshot("mysdk", "linux_glibc_common", "",
checkAndroidBpContents(`
// This is auto-generated. DO NOT EDIT.
diff --git a/sdk/sdk_test.go b/sdk/sdk_test.go
index f4e944a..d376e59 100644
--- a/sdk/sdk_test.go
+++ b/sdk/sdk_test.go
@@ -79,3 +79,123 @@
}
`)
}
+
+// Ensure that prebuilt modules have the same effective visibility as the source
+// modules.
+func TestSnapshotVisibility(t *testing.T) {
+ packageBp := `
+ package {
+ default_visibility: ["//other/foo"],
+ }
+
+ sdk {
+ name: "mysdk",
+ visibility: [
+ "//other/foo",
+ // This short form will be replaced with //package:__subpackages__ in the
+ // generated sdk_snapshot.
+ ":__subpackages__",
+ ],
+ java_header_libs: [
+ "myjavalib",
+ "mypublicjavalib",
+ "mydefaultedjavalib",
+ ],
+ }
+
+ java_library {
+ name: "myjavalib",
+ // Uses package default visibility
+ srcs: ["Test.java"],
+ system_modules: "none",
+ sdk_version: "none",
+ }
+
+ java_library {
+ name: "mypublicjavalib",
+ visibility: ["//visibility:public"],
+ srcs: ["Test.java"],
+ system_modules: "none",
+ sdk_version: "none",
+ }
+
+ java_defaults {
+ name: "myjavadefaults",
+ visibility: ["//other/bar"],
+ }
+
+ java_library {
+ name: "mydefaultedjavalib",
+ defaults: ["myjavadefaults"],
+ srcs: ["Test.java"],
+ system_modules: "none",
+ sdk_version: "none",
+ }
+ `
+
+ result := testSdkWithFs(t, ``,
+ map[string][]byte{
+ "package/Test.java": nil,
+ "package/Android.bp": []byte(packageBp),
+ })
+
+ result.CheckSnapshot("mysdk", "android_common", "package",
+ checkAndroidBpContents(`
+// This is auto-generated. DO NOT EDIT.
+
+java_import {
+ name: "mysdk_myjavalib@current",
+ sdk_member_name: "myjavalib",
+ visibility: ["//other/foo:__pkg__"],
+ jars: ["java/myjavalib.jar"],
+}
+
+java_import {
+ name: "myjavalib",
+ prefer: false,
+ visibility: ["//other/foo:__pkg__"],
+ jars: ["java/myjavalib.jar"],
+}
+
+java_import {
+ name: "mysdk_mypublicjavalib@current",
+ sdk_member_name: "mypublicjavalib",
+ visibility: ["//visibility:public"],
+ jars: ["java/mypublicjavalib.jar"],
+}
+
+java_import {
+ name: "mypublicjavalib",
+ prefer: false,
+ visibility: ["//visibility:public"],
+ jars: ["java/mypublicjavalib.jar"],
+}
+
+java_import {
+ name: "mysdk_mydefaultedjavalib@current",
+ sdk_member_name: "mydefaultedjavalib",
+ visibility: ["//other/bar:__pkg__"],
+ jars: ["java/mydefaultedjavalib.jar"],
+}
+
+java_import {
+ name: "mydefaultedjavalib",
+ prefer: false,
+ visibility: ["//other/bar:__pkg__"],
+ jars: ["java/mydefaultedjavalib.jar"],
+}
+
+sdk_snapshot {
+ name: "mysdk@current",
+ visibility: [
+ "//other/foo:__pkg__",
+ "//package:__subpackages__",
+ ],
+ java_header_libs: [
+ "mysdk_myjavalib@current",
+ "mysdk_mypublicjavalib@current",
+ "mysdk_mydefaultedjavalib@current",
+ ],
+}
+`))
+}
diff --git a/sdk/testing.go b/sdk/testing.go
index 604fa16..bd929a4 100644
--- a/sdk/testing.go
+++ b/sdk/testing.go
@@ -33,7 +33,12 @@
ctx := android.NewTestArchContext()
// from android package
+ ctx.PreArchMutators(android.RegisterPackageRenamer)
+ ctx.PreArchMutators(android.RegisterVisibilityRuleChecker)
ctx.PreArchMutators(android.RegisterDefaultsPreArchMutators)
+ ctx.PreArchMutators(android.RegisterVisibilityRuleGatherer)
+ ctx.PostDepsMutators(android.RegisterVisibilityRuleEnforcer)
+
ctx.PreArchMutators(func(ctx android.RegisterMutatorsContext) {
ctx.BottomUp("prebuilts", android.PrebuiltMutator).Parallel()
})
@@ -41,9 +46,11 @@
ctx.TopDown("prebuilt_select", android.PrebuiltSelectModuleMutator).Parallel()
ctx.BottomUp("prebuilt_postdeps", android.PrebuiltPostDepsMutator).Parallel()
})
+ ctx.RegisterModuleType("package", android.PackageFactory)
// from java package
ctx.RegisterModuleType("android_app_certificate", java.AndroidAppCertificateFactory)
+ ctx.RegisterModuleType("java_defaults", java.DefaultsFactory)
ctx.RegisterModuleType("java_library", java.LibraryFactory)
ctx.RegisterModuleType("java_import", java.ImportFactory)
ctx.RegisterModuleType("droidstubs", java.DroidstubsFactory)
@@ -115,7 +122,7 @@
func testSdkWithFs(t *testing.T, bp string, fs map[string][]byte) *testSdkResult {
t.Helper()
ctx, config := testSdkContext(bp, fs)
- _, errs := ctx.ParseFileList(".", []string{"Android.bp"})
+ _, errs := ctx.ParseBlueprintsFiles(".")
android.FailIfErrored(t, errs)
_, errs = ctx.PrepareBuildActions(config)
android.FailIfErrored(t, errs)
@@ -268,7 +275,7 @@
// Takes a list of functions which check different facets of the snapshot build rules.
// Allows each test to customize what is checked without duplicating lots of code
// or proliferating check methods of different flavors.
-func (r *testSdkResult) CheckSnapshot(name string, variant string, checkers ...snapshotBuildInfoChecker) {
+func (r *testSdkResult) CheckSnapshot(name string, variant string, dir string, checkers ...snapshotBuildInfoChecker) {
r.t.Helper()
sdk := r.Module(name, variant).(*sdk)
@@ -282,8 +289,11 @@
// Make sure that the generated zip file is in the correct place.
actual := snapshotBuildInfo.outputZip
+ if dir != "" {
+ dir = filepath.Clean(dir) + "/"
+ }
r.AssertStringEquals("Snapshot zip file in wrong place",
- fmt.Sprintf(".intermediates/%s/%s/%s-current.zip", name, variant, name), actual)
+ fmt.Sprintf(".intermediates/%s%s/%s/%s-current.zip", dir, name, variant, name), actual)
// Populate a mock filesystem with the files that would have been copied by
// the rules.
diff --git a/sdk/update.go b/sdk/update.go
index c967233..7fad5c7 100644
--- a/sdk/update.go
+++ b/sdk/update.go
@@ -211,6 +211,13 @@
snapshotName := ctx.ModuleName() + string(android.SdkVersionSeparator) + builder.version
snapshotModule := bpFile.newModule("sdk_snapshot")
snapshotModule.AddProperty("name", snapshotName)
+
+ // Make sure that the snapshot has the same visibility as the sdk.
+ visibility := android.EffectiveVisibilityRules(ctx, s)
+ if len(visibility) != 0 {
+ snapshotModule.AddProperty("visibility", visibility)
+ }
+
addHostDeviceSupportedProperties(&s.ModuleBase, snapshotModule)
for _, memberListProperty := range sdkMemberListProperties {
names := memberListProperty.getter(&s.properties)
@@ -376,6 +383,14 @@
m := s.bpFile.newModule(moduleType)
m.AddProperty("name", name)
+
+ // Extract visibility information from a member variant. All variants have the same
+ // visibility so it doesn't matter which one is used.
+ visibility := android.EffectiveVisibilityRules(s.ctx, member.Variants()[0])
+ if len(visibility) != 0 {
+ m.AddProperty("visibility", visibility)
+ }
+
addHostDeviceSupportedProperties(&s.sdk.ModuleBase, m)
s.prebuiltModules[name] = m