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