Ensure prebuilt modules have same visibility as source modules

am: 593b3c9fb0

Change-Id: Ie04e0cc30de41d13b138f0fe2ddf16039c81ae40

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,
 }

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()
 }

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)

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()
+}

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)

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.
 

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.
 

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",
+    ],
+}
+`))
+}

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.

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