diff --git a/Android.bp b/Android.bp index fe3a39097..1b68adb1f 100644 --- a/Android.bp +++ b/Android.bp @@ -58,6 +58,7 @@ bootstrap_go_package { "android/notices.go", "android/onceper.go", "android/override_module.go", + "android/package.go", "android/package_ctx.go", "android/path_properties.go", "android/paths.go", @@ -88,6 +89,7 @@ bootstrap_go_package { "android/namespace_test.go", "android/neverallow_test.go", "android/onceper_test.go", + "android/package_test.go", "android/path_properties_test.go", "android/paths_test.go", "android/prebuilt_test.go", diff --git a/README.md b/README.md index 8fdce4b25..51bd9e49c 100644 --- a/README.md +++ b/README.md @@ -133,6 +133,25 @@ directory) there are two packages, `my/app`, and the subpackage `my/app/tests`. This is based on the Bazel package concept. +The `package` module type allows information to be specified about a package. Only a single +`package` module can be specified per package and in the case where there are multiple `.bp` files +in the same package directory it is highly recommended that the `package` module (if required) is +specified in the `Android.bp` file. + +Unlike most module type `package` does not have a `name` property. Instead the name is set to the +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. + +``` +package { + default_visibility: [":__subpackages"] +} +``` + ### Name resolution Soong provides the ability for modules in different directories to specify @@ -191,13 +210,13 @@ this module. For example, `//project:rule`, `//project/library:lib` or `//independent:evil`) * `["//project"]`: This is shorthand for `["//project:__pkg__"]` * `[":__subpackages__"]`: This is shorthand for `["//project:__subpackages__"]` -where `//project` is the module's package. e.g. using `[":__subpackages__"]` in +where `//project` is the module's package, e.g. using `[":__subpackages__"]` in `packages/apps/Settings/Android.bp` is equivalent to `//packages/apps/Settings:__subpackages__`. * `["//visibility:legacy_public"]`: The default visibility, behaves as `//visibility:public` for now. It is an error if it is used in a module. -The visibility rules of `//visibility:public` and `//visibility:private` can not +The visibility rules of `//visibility:public` and `//visibility:private` cannot be combined with any other visibility specifications, except `//visibility:public` is allowed to override visibility specifications imported through the `defaults` property. @@ -207,13 +226,18 @@ in `vendor/`, e.g. a module in `libcore` cannot declare that it is visible to say `vendor/google`, instead it must make itself visible to all packages within `vendor/` using `//vendor:__subpackages__`. -If a module does not specify the `visibility` property the module is -`//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 which point all modules that do not currently specify -a `visibility` property will be updated to have -`visibility = [//visibility:legacy_public]` added. It will then be the owner's -responsibility to replace that with a more appropriate visibility. +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`. + +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 +which point all packages that do not currently specify a `default_visibility` +property will be updated to have +`default_visibility = [//visibility:legacy_public]` added. It will then be the +owner's responsibility to replace that with a more appropriate visibility. ### Formatter diff --git a/android/module.go b/android/module.go index 66822f080..b9127262e 100644 --- a/android/module.go +++ b/android/module.go @@ -202,6 +202,42 @@ type Module interface { BuildParamsForTests() []BuildParams RuleParamsForTests() map[blueprint.Rule]blueprint.RuleParams VariablesForTests() map[string]string + + // Get the qualified module id for this module. + qualifiedModuleId(ctx BaseModuleContext) qualifiedModuleName + + // Get information about the properties that can contain visibility rules. + visibilityProperties() []visibilityProperty +} + +// Qualified id for a module +type qualifiedModuleName struct { + // The package (i.e. directory) in which the module is defined, without trailing / + pkg string + + // The name of the module, empty string if package. + name string +} + +func (q qualifiedModuleName) String() string { + if q.name == "" { + return "//" + q.pkg + } + 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)) + } + return newPackageId(pkg) +} + +func newPackageId(pkg string) qualifiedModuleName { + // A qualified id for a package module has no name. + return qualifiedModuleName{pkg: pkg, name: ""} } type nameProperties struct { @@ -236,6 +272,13 @@ type commonProperties struct { // //packages/apps/Settings:__subpackages__. // ["//visibility:legacy_public"]: The default visibility, behaves as //visibility:public // for now. It is an error if it is used in a module. + // + // 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`. + // // See https://android.googlesource.com/platform/build/soong/+/master/README.md#visibility for // more details. Visibility []string @@ -571,6 +614,18 @@ func (m *ModuleBase) base() *ModuleBase { return m } +func (m *ModuleBase) qualifiedModuleId(ctx BaseModuleContext) qualifiedModuleName { + return qualifiedModuleName{pkg: ctx.ModuleDir(), name: ctx.ModuleName()} +} + +func (m *ModuleBase) visibilityProperties() []visibilityProperty { + return []visibilityProperty{ + newVisibilityProperty("visibility", func() []string { + return m.base().commonProperties.Visibility + }), + } +} + func (m *ModuleBase) SetTarget(target Target, multiTargets []Target, primary bool) { m.commonProperties.CompileTarget = target m.commonProperties.CompileMultiTargets = multiTargets diff --git a/android/mutator.go b/android/mutator.go index 081c2b248..070b420ff 100644 --- a/android/mutator.go +++ b/android/mutator.go @@ -75,6 +75,8 @@ type RegisterMutatorFunc func(RegisterMutatorsContext) var preArch = []RegisterMutatorFunc{ registerLoadHookMutator, RegisterNamespaceMutator, + // Rename package module types. + registerPackageRenamer, RegisterPrebuiltsPreArchMutators, registerVisibilityRuleChecker, RegisterDefaultsPreArchMutators, diff --git a/android/package.go b/android/package.go new file mode 100644 index 000000000..03f6a1e3d --- /dev/null +++ b/android/package.go @@ -0,0 +1,194 @@ +// Copyright 2019 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package android + +import ( + "fmt" + "sync/atomic" + + "github.com/google/blueprint" +) + +func init() { + RegisterModuleType("package", PackageFactory) +} + +// The information maintained about each package. +type packageInfo struct { + // The module from which this information was populated. If `duplicated` = true then this is the + // module that has been renamed and must be used to report errors. + module *packageModule + + // If true this indicates that there are two package statements in the same package which is not + // allowed and will cause the build to fail. This flag is set by packageRenamer and checked in + // packageErrorReporter + duplicated bool +} + +type packageProperties struct { + Name string `blueprint:"mutated"` + + // Specifies the default visibility for all modules defined in this package. + Default_visibility []string +} + +type packageModule struct { + ModuleBase + + properties packageProperties + packageInfo *packageInfo +} + +func (p *packageModule) GenerateAndroidBuildActions(ModuleContext) { + // Nothing to do. +} + +func (p *packageModule) GenerateBuildActions(ctx blueprint.ModuleContext) { + // Nothing to do. +} + +func (p *packageModule) qualifiedModuleId(ctx BaseModuleContext) qualifiedModuleName { + // Override to create a package id. + return newPackageId(ctx.ModuleDir()) +} + +// Override to ensure that the default_visibility rules are checked by the visibility module during +// its checking phase. +func (p *packageModule) visibilityProperties() []visibilityProperty { + return []visibilityProperty{ + newVisibilityProperty("default_visibility", func() []string { + return p.properties.Default_visibility + }), + } +} + +func (p *packageModule) Name() string { + return p.properties.Name +} + +func (p *packageModule) setName(name string) { + p.properties.Name = name +} + +// Counter to ensure package modules are created with a unique name within whatever namespace they +// belong. +var packageCount uint32 = 0 + +func PackageFactory() Module { + module := &packageModule{} + + // Get a unique if for the package. Has to be done atomically as the creation of the modules are + // done in parallel. + id := atomic.AddUint32(&packageCount, 1) + name := fmt.Sprintf("soong_package_%d", id) + + module.properties.Name = name + + module.AddProperties(&module.properties) + return module +} + +// Registers the function that renames the packages. +func registerPackageRenamer(ctx RegisterMutatorsContext) { + ctx.BottomUp("packageRenamer", packageRenamer).Parallel() + ctx.BottomUp("packageErrorReporter", packageErrorReporter).Parallel() +} + +// Renames the package to match the package directory. +// +// This also creates a PackageInfo object for each package and uses that to detect and remember +// duplicates for later error reporting. +func packageRenamer(ctx BottomUpMutatorContext) { + m, ok := ctx.Module().(*packageModule) + if !ok { + return + } + + packageName := "//" + ctx.ModuleDir() + + pi := newPackageInfo(ctx, packageName, m) + if pi.module != m { + // Remember that the package was duplicated but do not rename as that will cause an error to + // be logged with the generated name. Similarly, reporting the error here will use the generated + // name as renames are only processed after this phase. + pi.duplicated = true + } else { + // This is the first package module in this package so rename it to match the package name. + m.setName(packageName) + ctx.Rename(packageName) + + // Store a package info reference in the module. + m.packageInfo = pi + } +} + +// Logs any deferred errors. +func packageErrorReporter(ctx BottomUpMutatorContext) { + m, ok := ctx.Module().(*packageModule) + if !ok { + return + } + + packageDir := ctx.ModuleDir() + packageName := "//" + packageDir + + // Get the PackageInfo for the package. Should have been populated in the packageRenamer phase. + pi := findPackageInfo(ctx, packageName) + if pi == nil { + ctx.ModuleErrorf("internal error, expected package info to be present for package '%s'", + packageName) + return + } + + if pi.module != m { + // The package module has been duplicated but this is not the module that has been renamed so + // ignore it. An error will be logged for the renamed module which will ensure that the error + // message uses the correct name. + return + } + + // Check to see whether there are duplicate package modules in the package. + if pi.duplicated { + ctx.ModuleErrorf("package {...} specified multiple times") + return + } +} + +type defaultPackageInfoKey string + +func newPackageInfo( + ctx BaseModuleContext, packageName string, module *packageModule) *packageInfo { + key := NewCustomOnceKey(defaultPackageInfoKey(packageName)) + + return ctx.Config().Once(key, func() interface{} { + return &packageInfo{module: module} + }).(*packageInfo) +} + +// Get the PackageInfo for the package name (starts with //, no trailing /), is nil if no package +// module type was specified. +func findPackageInfo(ctx BaseModuleContext, packageName string) *packageInfo { + key := NewCustomOnceKey(defaultPackageInfoKey(packageName)) + + pi := ctx.Config().Once(key, func() interface{} { + return nil + }) + + if pi == nil { + return nil + } else { + return pi.(*packageInfo) + } +} diff --git a/android/package_test.go b/android/package_test.go new file mode 100644 index 000000000..f1f47acb4 --- /dev/null +++ b/android/package_test.go @@ -0,0 +1,111 @@ +package android + +import ( + "io/ioutil" + "os" + "testing" +) + +var packageTests = []struct { + name string + fs map[string][]byte + expectedErrors []string +}{ + // Package default_visibility handling is tested in visibility_test.go + { + name: "package must not accept visibility and name properties", + fs: map[string][]byte{ + "top/Blueprints": []byte(` + package { + name: "package", + visibility: ["//visibility:private"], + }`), + }, + expectedErrors: []string{ + `top/Blueprints:3:10: mutated field name cannot be set in a Blueprint file`, + `top/Blueprints:4:16: unrecognized property "visibility"`, + }, + }, + { + name: "multiple packages in separate directories", + fs: map[string][]byte{ + "top/Blueprints": []byte(` + package { + }`), + "other/Blueprints": []byte(` + package { + }`), + "other/nested/Blueprints": []byte(` + package { + }`), + }, + }, + { + name: "package must not be specified more than once per package", + fs: map[string][]byte{ + "top/Blueprints": []byte(` + package { + default_visibility: ["//visibility:private"], + } + + package { + }`), + }, + expectedErrors: []string{ + `module "//top": package {...} specified multiple times`, + }, + }, +} + +func TestPackage(t *testing.T) { + buildDir, err := ioutil.TempDir("", "soong_package_test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(buildDir) + + for _, test := range packageTests { + t.Run(test.name, func(t *testing.T) { + _, errs := testPackage(buildDir, test.fs) + + expectedErrors := test.expectedErrors + if expectedErrors == nil { + FailIfErrored(t, errs) + } else { + for _, expectedError := range expectedErrors { + FailIfNoMatchingErrors(t, expectedError, errs) + } + if len(errs) > len(expectedErrors) { + t.Errorf("additional errors found, expected %d, found %d", len(expectedErrors), len(errs)) + for i, expectedError := range expectedErrors { + t.Errorf("expectedErrors[%d] = %s", i, expectedError) + } + for i, err := range errs { + t.Errorf("errs[%d] = %s", i, err) + } + } + } + }) + } +} + +func testPackage(buildDir string, fs map[string][]byte) (*TestContext, []error) { + + // Create a new config per test as visibility information is stored in the config. + config := TestArchConfig(buildDir, nil) + + ctx := NewTestArchContext() + ctx.RegisterModuleType("package", ModuleFactoryAdaptor(PackageFactory)) + ctx.PreArchMutators(registerPackageRenamer) + ctx.Register() + + ctx.MockFileSystem(fs) + + _, errs := ctx.ParseBlueprintsFiles(".") + if len(errs) > 0 { + return ctx, errs + } + + _, errs = ctx.PrepareBuildActions(config) + return ctx, errs +} diff --git a/android/visibility.go b/android/visibility.go index c7ef1dae6..2e01ff627 100644 --- a/android/visibility.go +++ b/android/visibility.go @@ -23,14 +23,21 @@ import ( // Enforces visibility rules between modules. // -// Two stage process: -// * First stage works bottom up to extract visibility information from the modules, parse it, +// Multi stage process: +// * First stage works bottom up, before defaults expansion, to check the syntax of the visibility +// rules that have been specified. +// +// * Second stage works bottom up to extract the package info for each package and store them in a +// map by package name. See package.go for functionality for this. +// +// * Third stage works bottom up to extract visibility information from the modules, parse it, // create visibilityRule structures and store them in a map keyed by the module's // qualifiedModuleName instance, i.e. //:. The map is stored in the context rather // than a global variable for testing. Each test has its own Config so they do not share a map -// and so can be run in parallel. +// and so can be run in parallel. If a module has no visibility specified then it uses the +// default package visibility if specified. // -// * Second stage works top down and iterates over all the deps for each module. If the dep is in +// * Fourth stage works top down and iterates over all the deps for each module. If the dep is in // the same package then it is automatically visible. Otherwise, for each dep it first extracts // its visibilityRule from the config map. If one could not be found then it assumes that it is // publicly visible. Otherwise, it calls the visibility rule to check that the module can see @@ -48,19 +55,6 @@ const ( var visibilityRuleRegexp = regexp.MustCompile(visibilityRulePattern) -// Qualified id for a module -type qualifiedModuleName struct { - // The package (i.e. directory) in which the module is defined, without trailing / - pkg string - - // The name of the module. - name string -} - -func (q qualifiedModuleName) String() string { - return fmt.Sprintf("//%s:%s", q.pkg, q.name) -} - // A visibility rule is associated with a module and determines which other modules it is visible // to, i.e. which other modules can depend on the rule's module. type visibilityRule interface { @@ -71,6 +65,32 @@ type visibilityRule interface { String() string } +// Describes the properties provided by a module that contain visibility rules. +type visibilityPropertyImpl struct { + name string + stringsGetter func() []string +} + +type visibilityProperty interface { + getName() string + getStrings() []string +} + +func newVisibilityProperty(name string, stringsGetter func() []string) visibilityProperty { + return visibilityPropertyImpl{ + name: name, + stringsGetter: stringsGetter, + } +} + +func (p visibilityPropertyImpl) getName() string { + return p.name +} + +func (p visibilityPropertyImpl) getStrings() []string { + return p.stringsGetter() +} + // A compositeRule is a visibility rule composed from a list of atomic visibility rules. // // The list corresponds to the list of strings in the visibility property after defaults expansion. @@ -96,9 +116,9 @@ func (c compositeRule) matches(m qualifiedModuleName) bool { return false } -func (r compositeRule) String() string { - s := make([]string, 0, len(r)) - for _, r := range r { +func (c compositeRule) String() string { + s := make([]string, 0, len(c)) + for _, r := range c { s = append(s, r.String()) } @@ -173,9 +193,12 @@ func registerVisibilityRuleChecker(ctx RegisterMutatorsContext) { ctx.BottomUp("visibilityRuleChecker", visibilityRuleChecker).Parallel() } +// Registers the function that gathers the visibility rules for each module. +// // Visibility is not dependent on arch so this must be registered before the arch phase to avoid // having to process multiple variants for each module. This goes after defaults expansion to gather -// the complete visibility lists from flat lists. +// 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) { ctx.BottomUp("visibilityRuleGatherer", visibilityRuleGatherer).Parallel() } @@ -193,33 +216,36 @@ func visibilityRuleChecker(ctx BottomUpMutatorContext) { for _, props := range d.properties() { if cp, ok := props.(*commonProperties); ok { if visibility := cp.Visibility; visibility != nil { - checkRules(ctx, qualified.pkg, visibility) + checkRules(ctx, qualified.pkg, "visibility", visibility) } } } } else if m, ok := ctx.Module().(Module); ok { - if visibility := m.base().commonProperties.Visibility; visibility != nil { - checkRules(ctx, qualified.pkg, visibility) + visibilityProperties := m.visibilityProperties() + for _, p := range visibilityProperties { + if visibility := p.getStrings(); visibility != nil { + checkRules(ctx, qualified.pkg, p.getName(), visibility) + } } } } -func checkRules(ctx BottomUpMutatorContext, currentPkg string, visibility []string) { +func checkRules(ctx BaseModuleContext, currentPkg, property string, visibility []string) { ruleCount := len(visibility) if ruleCount == 0 { // This prohibits an empty list as its meaning is unclear, e.g. it could mean no visibility and // it could mean public visibility. Requiring at least one rule makes the owner's intent // clearer. - ctx.PropertyErrorf("visibility", "must contain at least one visibility rule") + ctx.PropertyErrorf(property, "must contain at least one visibility rule") return } for _, v := range visibility { - ok, pkg, name := splitRule(ctx, v, currentPkg) + ok, pkg, name := splitRule(v, currentPkg) if !ok { // Visibility rule is invalid so ignore it. Keep going rather than aborting straight away to // ensure all the rules on this module are checked. - ctx.PropertyErrorf("visibility", + ctx.PropertyErrorf(property, "invalid visibility pattern %q must match"+ " //:, // or :", v) @@ -230,14 +256,14 @@ func checkRules(ctx BottomUpMutatorContext, currentPkg string, visibility []stri switch name { case "private", "public": case "legacy_public": - ctx.PropertyErrorf("visibility", "//visibility:legacy_public must not be used") + ctx.PropertyErrorf(property, "//visibility:legacy_public must not be used") continue default: - ctx.PropertyErrorf("visibility", "unrecognized visibility rule %q", v) + ctx.PropertyErrorf(property, "unrecognized visibility rule %q", v) continue } if ruleCount != 1 { - ctx.PropertyErrorf("visibility", "cannot mix %q with any other visibility rules", v) + ctx.PropertyErrorf(property, "cannot mix %q with any other visibility rules", v) continue } } @@ -246,7 +272,7 @@ func checkRules(ctx BottomUpMutatorContext, currentPkg string, visibility []stri // restrictions on the rules. if !isAncestor("vendor", currentPkg) { if !isAllowedFromOutsideVendor(pkg, name) { - ctx.PropertyErrorf("visibility", + ctx.PropertyErrorf(property, "%q is not allowed. Packages outside //vendor cannot make themselves visible to specific"+ " targets within //vendor, they can only use //vendor:__subpackages__.", v) continue @@ -265,23 +291,27 @@ func visibilityRuleGatherer(ctx BottomUpMutatorContext) { return } - qualified := createQualifiedModuleName(ctx) + qualifiedModuleId := m.qualifiedModuleId(ctx) + currentPkg := qualifiedModuleId.pkg - visibility := m.base().commonProperties.Visibility - if visibility != nil { - rule := parseRules(ctx, qualified.pkg, visibility) - if rule != nil { - moduleToVisibilityRuleMap(ctx).Store(qualified, rule) + // Parse all the properties into rules and store them. + visibilityProperties := m.visibilityProperties() + for _, p := range visibilityProperties { + if visibility := p.getStrings(); visibility != nil { + rule := parseRules(ctx, currentPkg, visibility) + if rule != nil { + moduleToVisibilityRuleMap(ctx).Store(qualifiedModuleId, rule) + } } } } -func parseRules(ctx BottomUpMutatorContext, currentPkg string, visibility []string) compositeRule { +func parseRules(ctx BaseModuleContext, currentPkg string, visibility []string) compositeRule { rules := make(compositeRule, 0, len(visibility)) hasPrivateRule := false hasNonPrivateRule := false for _, v := range visibility { - ok, pkg, name := splitRule(ctx, v, currentPkg) + ok, pkg, name := splitRule(v, currentPkg) if !ok { continue } @@ -336,7 +366,7 @@ func isAllowedFromOutsideVendor(pkg string, name string) bool { return !isAncestor("vendor", pkg) } -func splitRule(ctx BaseModuleContext, ruleExpression string, currentPkg string) (bool, string, string) { +func splitRule(ruleExpression string, currentPkg string) (bool, string, string) { // Make sure that the rule is of the correct format. matches := visibilityRuleRegexp.FindStringSubmatch(ruleExpression) if ruleExpression == "" || matches == nil { @@ -378,12 +408,20 @@ func visibilityRuleEnforcer(ctx TopDownMutatorContext) { return } - rule, ok := moduleToVisibilityRule.Load(depQualified) + value, ok := moduleToVisibilityRule.Load(depQualified) + var rule compositeRule if ok { - if !rule.(compositeRule).matches(qualified) { - ctx.ModuleErrorf("depends on %s which is not visible to this module", depQualified) + rule = value.(compositeRule) + } else { + packageQualifiedId := depQualified.getContainingPackageId() + value, ok = moduleToVisibilityRule.Load(packageQualifiedId) + if ok { + rule = value.(compositeRule) } } + if rule != nil && !rule.matches(qualified) { + ctx.ModuleErrorf("depends on %s which is not visible to this module", depQualified) + } }) } diff --git a/android/visibility_test.go b/android/visibility_test.go index 1a514955d..9a3e6aadb 100644 --- a/android/visibility_test.go +++ b/android/visibility_test.go @@ -658,6 +658,109 @@ var visibilityTests = []struct { ` visible to this module`, }, }, + // Package default_visibility tests + { + name: "package default_visibility property is checked", + fs: map[string][]byte{ + "top/Blueprints": []byte(` + package { + default_visibility: ["//visibility:invalid"], + }`), + }, + expectedErrors: []string{`default_visibility: unrecognized visibility rule "//visibility:invalid"`}, + }, + { + // This test relies on the default visibility being legacy_public. + name: "package default_visibility property used when no visibility specified", + fs: map[string][]byte{ + "top/Blueprints": []byte(` + package { + default_visibility: ["//visibility:private"], + } + + mock_library { + name: "libexample", + }`), + "outsider/Blueprints": []byte(` + mock_library { + name: "liboutsider", + deps: ["libexample"], + }`), + }, + expectedErrors: []string{ + `module "liboutsider" variant "android_common": depends on //top:libexample which is not` + + ` visible to this module`, + }, + }, + { + name: "package default_visibility public does not override visibility private", + fs: map[string][]byte{ + "top/Blueprints": []byte(` + package { + default_visibility: ["//visibility:public"], + } + + mock_library { + name: "libexample", + visibility: ["//visibility:private"], + }`), + "outsider/Blueprints": []byte(` + mock_library { + name: "liboutsider", + deps: ["libexample"], + }`), + }, + expectedErrors: []string{ + `module "liboutsider" variant "android_common": depends on //top:libexample which is not` + + ` visible to this module`, + }, + }, + { + name: "package default_visibility private does not override visibility public", + fs: map[string][]byte{ + "top/Blueprints": []byte(` + package { + default_visibility: ["//visibility:private"], + } + + mock_library { + name: "libexample", + visibility: ["//visibility:public"], + }`), + "outsider/Blueprints": []byte(` + mock_library { + name: "liboutsider", + deps: ["libexample"], + }`), + }, + }, + { + name: "package default_visibility :__subpackages__", + fs: map[string][]byte{ + "top/Blueprints": []byte(` + package { + default_visibility: [":__subpackages__"], + } + + mock_library { + name: "libexample", + }`), + "top/nested/Blueprints": []byte(` + mock_library { + name: "libnested", + deps: ["libexample"], + }`), + "outsider/Blueprints": []byte(` + mock_library { + name: "liboutsider", + deps: ["libexample"], + }`), + }, + expectedErrors: []string{ + `module "liboutsider" variant "android_common": depends on //top:libexample which is not` + + ` visible to this module`, + }, + }, } func TestVisibility(t *testing.T) { @@ -692,8 +795,10 @@ func testVisibility(buildDir string, fs map[string][]byte) (*TestContext, []erro config := TestArchConfig(buildDir, nil) ctx := NewTestArchContext() + ctx.RegisterModuleType("package", ModuleFactoryAdaptor(PackageFactory)) ctx.RegisterModuleType("mock_library", ModuleFactoryAdaptor(newMockLibraryModule)) ctx.RegisterModuleType("mock_defaults", ModuleFactoryAdaptor(defaultsFactory)) + ctx.PreArchMutators(registerPackageRenamer) ctx.PreArchMutators(registerVisibilityRuleChecker) ctx.PreArchMutators(RegisterDefaultsPreArchMutators) ctx.PreArchMutators(registerVisibilityRuleGatherer)