Merge changes from topic "package default_visibility" am: 4190926e61

am: b915658436

Change-Id: Ib207ca13b58e789ea7e545cb99db789cb5d83cfe

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

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 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 {
+    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,22 @@ 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
+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
+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
 

android/module.go

@@ -202,6 +202,55 @@ 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
+}
+
+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 == "" {
+		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)
+}
+
+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 +285,20 @@ 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 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
+	// 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.
 	Visibility []string
@@ -571,6 +634,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

android/mutator.go

@@ -75,6 +75,8 @@ type RegisterMutatorFunc func(RegisterMutatorsContext)
 var preArch = []RegisterMutatorFunc{
 	registerLoadHookMutator,
 	RegisterNamespaceMutator,
+	// Rename package module types.
+	registerPackageRenamer,
 	RegisterPrebuiltsPreArchMutators,
 	registerVisibilityRuleChecker,
 	RegisterDefaultsPreArchMutators,

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

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

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. //<pkg>:<name>. 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"+
 					" //<package>:<module>, //<package> or :<module>",
 				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,11 +408,15 @@ 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 {
+			rule = packageDefaultVisibility(ctx, depQualified)
+		}
+		if rule != nil && !rule.matches(qualified) {
+			ctx.ModuleErrorf("depends on %s which is not visible to this module", depQualified)
 		}
 	})
 }
@@ -393,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()
+	}
+}

android/visibility_test.go

@@ -658,6 +658,167 @@ 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`,
+		},
+	},
+	{
+		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) {
@@ -692,8 +853,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)