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

10 files changed, 203 insertions, 35 deletions

diff --git a/android/mutator.go b/android/mutator.go
index eba0e2fd4..e9ccd7f00 100644
--- a/android/mutator.go
+++ b/android/mutator.go
@@ -78,11 +78,11 @@ var preArch = []RegisterMutatorFunc{
 	registerLoadHookMutator,
 	RegisterNamespaceMutator,
 	// Rename package module types.
-	registerPackageRenamer,
+	RegisterPackageRenamer,
 	RegisterPrebuiltsPreArchMutators,
-	registerVisibilityRuleChecker,
+	RegisterVisibilityRuleChecker,
 	RegisterDefaultsPreArchMutators,
-	registerVisibilityRuleGatherer,
+	RegisterVisibilityRuleGatherer,
 }
 
 func registerArchMutator(ctx RegisterMutatorsContext) {
@@ -99,7 +99,7 @@ var preDeps = []RegisterMutatorFunc{
 var postDeps = []RegisterMutatorFunc{
 	registerPathDepsMutator,
 	RegisterPrebuiltsPostDepsMutators,
-	registerVisibilityRuleEnforcer,
+	RegisterVisibilityRuleEnforcer,
 	registerNeverallowMutator,
 	RegisterOverridePostDepsMutators,
 }
diff --git a/android/package.go b/android/package.go
index 880d6a97b..ed604c606 100644
--- a/android/package.go
+++ b/android/package.go
@@ -98,7 +98,7 @@ func PackageFactory() Module {
 }
 
 // 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 ae286d662..8071c51b0 100644
--- a/android/package_test.go
+++ b/android/package_test.go
@@ -88,7 +88,7 @@ func testPackage(fs map[string][]byte) (*TestContext, []error) {
 
 	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 a7e718ba7..c28ec93f4 100644
--- a/android/visibility.go
+++ b/android/visibility.go
@@ -117,12 +117,15 @@ func (c compositeRule) matches(m qualifiedModuleName) bool {
 }
 
 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 @@ func moduleToVisibilityRuleMap(ctx BaseModuleContext) *sync.Map {
 
 // 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 @@ func registerVisibilityRuleChecker(ctx RegisterMutatorsContext) {
 // 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 @@ func visibilityRuleEnforcer(ctx TopDownMutatorContext) {
 
 	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 @@ func visibilityRuleEnforcer(ctx TopDownMutatorContext) {
 			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 @@ func packageDefaultVisibility(ctx BaseModuleContext, moduleId qualifiedModuleNam
 		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 fd9e98c55..1984a2189 100644
--- a/android/visibility_test.go
+++ b/android/visibility_test.go
@@ -874,11 +874,11 @@ func testVisibility(buildDir string, fs map[string][]byte) (*TestContext, []erro
 	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 315669afe..7620ec1ab 100644
--- a/sdk/cc_sdk_test.go
+++ b/sdk/cc_sdk_test.go
@@ -164,7 +164,7 @@ func TestSnapshotWithCcShared(t *testing.T) {
 		}
 	`)
 
-	result.CheckSnapshot("mysdk", "android_common",
+	result.CheckSnapshot("mysdk", "android_common", "",
 		checkAndroidBpContents(`
 // This is auto-generated. DO NOT EDIT.
 
@@ -263,7 +263,7 @@ func TestHostSnapshotWithCcShared(t *testing.T) {
 		}
 	`)
 
-	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 5b7224879..1aa918469 100644
--- a/sdk/java_sdk_test.go
+++ b/sdk/java_sdk_test.go
@@ -123,7 +123,7 @@ func TestSnapshotWithJavaHeaderLibrary(t *testing.T) {
 		}
 	`)
 
-	result.CheckSnapshot("mysdk", "android_common",
+	result.CheckSnapshot("mysdk", "android_common", "",
 		checkAndroidBpContents(`
 // This is auto-generated. DO NOT EDIT.
 
@@ -178,7 +178,7 @@ func TestHostSnapshotWithJavaHeaderLibrary(t *testing.T) {
 		}
 	`)
 
-	result.CheckSnapshot("mysdk", "linux_glibc_common",
+	result.CheckSnapshot("mysdk", "linux_glibc_common", "",
 		checkAndroidBpContents(`
 // This is auto-generated. DO NOT EDIT.
 
@@ -232,7 +232,7 @@ func TestSnapshotWithJavaImplLibrary(t *testing.T) {
 		}
 	`)
 
-	result.CheckSnapshot("mysdk", "android_common",
+	result.CheckSnapshot("mysdk", "android_common", "",
 		checkAndroidBpContents(`
 // This is auto-generated. DO NOT EDIT.
 
@@ -287,7 +287,7 @@ func TestHostSnapshotWithJavaImplLibrary(t *testing.T) {
 		}
 	`)
 
-	result.CheckSnapshot("mysdk", "linux_glibc_common",
+	result.CheckSnapshot("mysdk", "linux_glibc_common", "",
 		checkAndroidBpContents(`
 // This is auto-generated. DO NOT EDIT.
 
@@ -379,7 +379,7 @@ func TestSnapshotWithDroidstubs(t *testing.T) {
 		}
 	`)
 
-	result.CheckSnapshot("mysdk", "android_common",
+	result.CheckSnapshot("mysdk", "android_common", "",
 		checkAndroidBpContents(`
 // This is auto-generated. DO NOT EDIT.
 
@@ -428,7 +428,7 @@ func TestHostSnapshotWithDroidstubs(t *testing.T) {
 		}
 	`)
 
-	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 f4e944a80..d376e5906 100644
--- a/sdk/sdk_test.go
+++ b/sdk/sdk_test.go
@@ -79,3 +79,123 @@ func TestDepNotInRequiredSdks(t *testing.T) {
 		}
 	`)
 }
+
+// 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 604fa1690..bd929a4f8 100644
--- a/sdk/testing.go
+++ b/sdk/testing.go
@@ -33,7 +33,12 @@ func testSdkContext(bp string, fs map[string][]byte) (*android.TestContext, andr
 	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 @@ func testSdkContext(bp string, fs map[string][]byte) (*android.TestContext, andr
 		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 testSdkContext(bp string, fs map[string][]byte) (*android.TestContext, andr
 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 @@ func (r *testSdkResult) pathsRelativeToBuildDir(paths android.Paths) []string {
 // 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 @@ func (r *testSdkResult) CheckSnapshot(name string, variant string, checkers ...s
 
 	// 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 c96723308..7fad5c745 100644
--- a/sdk/update.go
+++ b/sdk/update.go
@@ -211,6 +211,13 @@ func (s *sdk) buildSnapshot(ctx android.ModuleContext) android.OutputPath {
 	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 @@ func (s *snapshotBuilder) AddPrebuiltModule(member android.SdkMember, moduleType
 
 	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