From e484f47a639b1afa01e4342e4b2d894f94571d1b Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Thu, 20 Jun 2019 16:38:08 +0100 Subject: [PATCH] Inherit default_visibility from parent package Enhances the visibility mechanism to use the default_visibility property of the closest ancestor package that has the property specified. Bug: 133290645 Test: m droid Change-Id: I7248e9034a73894ac8d514f913316438c4d7c079 --- README.md | 12 +++++--- android/module.go | 24 ++++++++++++++-- android/visibility.go | 23 +++++++++++---- android/visibility_test.go | 58 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 106 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 51bd9e49c..ebbe8bc50 100644 --- a/README.md +++ b/README.md @@ -142,9 +142,9 @@ Unlike most module type `package` does not have a `name` property. Instead the n name of the package, e.g. if the package is in `top/intermediate/package` then the package name is `//top/intermediate/package`. -E.g. The following will set the default visibility for all the modules defined in the package -(irrespective of whether they are in the same `.bp` file as the `package` module) to be visible to -all the subpackages by default. +E.g. The following will set the default visibility for all the modules defined in the package and +any subpackages that do not set their own default visibility (irrespective of whether they are in +the same `.bp` file as the `package` module) to be visible to all the subpackages by default. ``` package { @@ -230,7 +230,11 @@ If a module does not specify the `visibility` property then it uses the `default_visibility` property of the `package` module in the module's package. If the `default_visibility` property is not set for the module's package then -the module uses `//visibility:legacy_public`. +it will use the `default_visibility` of its closest ancestor package for which +a `default_visibility` property is specified. + +If no `default_visibility` property can be found then the module uses the +global default of `//visibility:legacy_public`. Once the build has been completely switched over to soong it is possible that a global refactoring will be done to change this to `//visibility:private` at diff --git a/android/module.go b/android/module.go index b9127262e..43b8763d2 100644 --- a/android/module.go +++ b/android/module.go @@ -226,11 +226,24 @@ func (q qualifiedModuleName) String() string { return "//" + q.pkg + ":" + q.name } +func (q qualifiedModuleName) isRootPackage() bool { + return q.pkg == "" && q.name == "" +} + // Get the id for the package containing this module. func (q qualifiedModuleName) getContainingPackageId() qualifiedModuleName { pkg := q.pkg if q.name == "" { - panic(fmt.Errorf("Cannot get containing package id of package module %s", pkg)) + if pkg == "" { + panic(fmt.Errorf("Cannot get containing package id of root package")) + } + + index := strings.LastIndex(pkg, "/") + if index == -1 { + pkg = "" + } else { + pkg = pkg[:index] + } } return newPackageId(pkg) } @@ -276,8 +289,15 @@ type commonProperties struct { // If a module does not specify the `visibility` property then it uses the // `default_visibility` property of the `package` module in the module's package. // + // If a module does not specify the `visibility` property then it uses the + // `default_visibility` property of the `package` module in the module's package. + // // If the `default_visibility` property is not set for the module's package then - // the module uses `//visibility:legacy_public`. + // it will use the `default_visibility` of its closest ancestor package for which + // a `default_visibility` property is specified. + // + // If no `default_visibility` property can be found then the module uses the + // global default of `//visibility:legacy_public`. // // See https://android.googlesource.com/platform/build/soong/+/master/README.md#visibility for // more details. diff --git a/android/visibility.go b/android/visibility.go index 2e01ff627..94af3433b 100644 --- a/android/visibility.go +++ b/android/visibility.go @@ -413,11 +413,7 @@ func visibilityRuleEnforcer(ctx TopDownMutatorContext) { if ok { rule = value.(compositeRule) } else { - packageQualifiedId := depQualified.getContainingPackageId() - value, ok = moduleToVisibilityRule.Load(packageQualifiedId) - if ok { - rule = value.(compositeRule) - } + rule = packageDefaultVisibility(ctx, depQualified) } if rule != nil && !rule.matches(qualified) { ctx.ModuleErrorf("depends on %s which is not visible to this module", depQualified) @@ -431,3 +427,20 @@ func createQualifiedModuleName(ctx BaseModuleContext) qualifiedModuleName { qualified := qualifiedModuleName{dir, moduleName} return qualified } + +func packageDefaultVisibility(ctx BaseModuleContext, moduleId qualifiedModuleName) compositeRule { + moduleToVisibilityRule := moduleToVisibilityRuleMap(ctx) + packageQualifiedId := moduleId.getContainingPackageId() + for { + value, ok := moduleToVisibilityRule.Load(packageQualifiedId) + if ok { + return value.(compositeRule) + } + + if packageQualifiedId.isRootPackage() { + return nil + } + + packageQualifiedId = packageQualifiedId.getContainingPackageId() + } +} diff --git a/android/visibility_test.go b/android/visibility_test.go index 9a3e6aadb..af6acf447 100644 --- a/android/visibility_test.go +++ b/android/visibility_test.go @@ -761,6 +761,64 @@ var visibilityTests = []struct { ` visible to this module`, }, }, + { + name: "package default_visibility inherited to subpackages", + fs: map[string][]byte{ + "top/Blueprints": []byte(` + package { + default_visibility: ["//outsider"], + } + + mock_library { + name: "libexample", + visibility: [":__subpackages__"], + }`), + "top/nested/Blueprints": []byte(` + mock_library { + name: "libnested", + deps: ["libexample"], + }`), + "outsider/Blueprints": []byte(` + mock_library { + name: "liboutsider", + deps: ["libexample", "libnested"], + }`), + }, + expectedErrors: []string{ + `module "liboutsider" variant "android_common": depends on //top:libexample which is not` + + ` visible to this module`, + }, + }, + { + name: "package default_visibility inherited to subpackages", + fs: map[string][]byte{ + "top/Blueprints": []byte(` + package { + default_visibility: ["//visibility:private"], + }`), + "top/nested/Blueprints": []byte(` + package { + default_visibility: ["//outsider"], + } + + mock_library { + name: "libnested", + }`), + "top/other/Blueprints": []byte(` + mock_library { + name: "libother", + }`), + "outsider/Blueprints": []byte(` + mock_library { + name: "liboutsider", + deps: ["libother", "libnested"], + }`), + }, + expectedErrors: []string{ + `module "liboutsider" variant "android_common": depends on //top/other:libother which is` + + ` not visible to this module`, + }, + }, } func TestVisibility(t *testing.T) {