Add //visibility:override to allow control over inheritance

Visibility rules can be 'inherited' in one of two ways. Either from
defaults or from a module that called ctx.CreateModule(...).
Previously, in both cases the inheriting module could only append
additional visibility rules to the end of the inherited rules. That
made it impossible to restrict the visibility by removing or ignore
inherited rules.

The //visibility:override rectifies that by allowing the inheriting
module to ignore all the rules that they would have inherited. It can
only go at the beginning of a list of rules specified in a module but
after defaults are applied it can end up in the middle of a list of
rules. In that case it behaves as if all the rules up to and including
the //visibility:override rule were discarded.

It can be used with //visibility:private to override
//visibility:public and vice versa.

Bug: 155787200
Test: m nothing
Merged-In: I8a9c9c5a1bdceaee387c08864ae2b34629e0d46f
Change-Id: I8a9c9c5a1bdceaee387c08864ae2b34629e0d46f
(cherry picked from commit 51084ff6cf)
This commit is contained in:
Paul Duffin 2020-05-05 19:19:22 +01:00
parent def8a89a22
commit 31c43e7fb3
4 changed files with 193 additions and 2 deletions

View File

@ -289,6 +289,9 @@ Each rule in the property must be in one of the following forms:
* `["//visibility:public"]`: Anyone can use this module. * `["//visibility:public"]`: Anyone can use this module.
* `["//visibility:private"]`: Only rules in the module's package (not its * `["//visibility:private"]`: Only rules in the module's package (not its
subpackages) can use this module. subpackages) can use this module.
* `["//visibility:override"]`: Discards any rules inherited from defaults or a
creating module. Can only be used at the beginning of a list of visibility
rules.
* `["//some/package:__pkg__", "//other/package:__pkg__"]`: Only modules in * `["//some/package:__pkg__", "//other/package:__pkg__"]`: Only modules in
`some/package` and `other/package` (defined in `some/package/*.bp` and `some/package` and `other/package` (defined in `some/package/*.bp` and
`other/package/*.bp`) have access to this module. Note that sub-packages do not `other/package/*.bp`) have access to this module. Note that sub-packages do not

View File

@ -319,6 +319,8 @@ type commonProperties struct {
// ["//visibility:public"]: Anyone can use this module. // ["//visibility:public"]: Anyone can use this module.
// ["//visibility:private"]: Only rules in the module's package (not its subpackages) can use // ["//visibility:private"]: Only rules in the module's package (not its subpackages) can use
// this module. // this module.
// ["//visibility:override"]: Discards any rules inherited from defaults or a creating module.
// Can only be used at the beginning of a list of visibility rules.
// ["//some/package:__pkg__", "//other/package:__pkg__"]: Only modules in some/package and // ["//some/package:__pkg__", "//other/package:__pkg__"]: Only modules in some/package and
// other/package (defined in some/package/*.bp and other/package/*.bp) have access to // other/package (defined in some/package/*.bp and other/package/*.bp) have access to
// this module. Note that sub-packages do not have access to the rule; for example, // this module. Note that sub-packages do not have access to the rule; for example,

View File

@ -245,7 +245,7 @@ func checkRules(ctx BaseModuleContext, currentPkg, property string, visibility [
return return
} }
for _, v := range visibility { for i, v := range visibility {
ok, pkg, name := splitRule(ctx, v, currentPkg, property) ok, pkg, name := splitRule(ctx, v, currentPkg, property)
if !ok { if !ok {
continue continue
@ -257,11 +257,18 @@ func checkRules(ctx BaseModuleContext, currentPkg, property string, visibility [
case "legacy_public": case "legacy_public":
ctx.PropertyErrorf(property, "//visibility:legacy_public must not be used") ctx.PropertyErrorf(property, "//visibility:legacy_public must not be used")
continue continue
case "override":
// This keyword does not create a rule so pretend it does not exist.
ruleCount -= 1
default: default:
ctx.PropertyErrorf(property, "unrecognized visibility rule %q", v) ctx.PropertyErrorf(property, "unrecognized visibility rule %q", v)
continue continue
} }
if ruleCount != 1 { if name == "override" {
if i != 0 {
ctx.PropertyErrorf(property, `"%v" may only be used at the start of the visibility rules`, v)
}
} else if ruleCount != 1 {
ctx.PropertyErrorf(property, "cannot mix %q with any other visibility rules", v) ctx.PropertyErrorf(property, "cannot mix %q with any other visibility rules", v)
continue continue
} }
@ -327,6 +334,14 @@ func parseRules(ctx BaseModuleContext, currentPkg, property string, visibility [
case "public": case "public":
r = publicRule{} r = publicRule{}
hasPublicRule = true hasPublicRule = true
case "override":
// Discard all preceding rules and any state based on them.
rules = nil
hasPrivateRule = false
hasPublicRule = false
hasNonPrivateRule = false
// This does not actually create a rule so continue onto the next rule.
continue
} }
} else { } else {
switch name { switch name {

View File

@ -635,6 +635,177 @@ var visibilityTests = []struct {
` with any other visibility rules`, ` with any other visibility rules`,
}, },
}, },
{
name: "//visibility:private override //visibility:public",
fs: map[string][]byte{
"top/Blueprints": []byte(`
mock_defaults {
name: "libexample_defaults",
visibility: ["//visibility:public"],
}
mock_library {
name: "libexample",
visibility: ["//visibility:private"],
defaults: ["libexample_defaults"],
}`),
},
expectedErrors: []string{
`module "libexample": visibility: cannot mix "//visibility:private" with any other visibility rules`,
},
},
{
name: "//visibility:public override //visibility:private",
fs: map[string][]byte{
"top/Blueprints": []byte(`
mock_defaults {
name: "libexample_defaults",
visibility: ["//visibility:private"],
}
mock_library {
name: "libexample",
visibility: ["//visibility:public"],
defaults: ["libexample_defaults"],
}`),
},
expectedErrors: []string{
`module "libexample": visibility: cannot mix "//visibility:private" with any other visibility rules`,
},
},
{
name: "//visibility:override must be first in the list",
fs: map[string][]byte{
"top/Blueprints": []byte(`
mock_library {
name: "libexample",
visibility: ["//other", "//visibility:override", "//namespace"],
}`),
},
expectedErrors: []string{
`module "libexample": visibility: "//visibility:override" may only be used at the start of the visibility rules`,
},
},
{
name: "//visibility:override discards //visibility:private",
fs: map[string][]byte{
"top/Blueprints": []byte(`
mock_defaults {
name: "libexample_defaults",
visibility: ["//visibility:private"],
}
mock_library {
name: "libexample",
// Make this visibility to //other but not //visibility:private
visibility: ["//visibility:override", "//other"],
defaults: ["libexample_defaults"],
}`),
"other/Blueprints": []byte(`
mock_library {
name: "libother",
deps: ["libexample"],
}`),
},
},
{
name: "//visibility:override discards //visibility:public",
fs: map[string][]byte{
"top/Blueprints": []byte(`
mock_defaults {
name: "libexample_defaults",
visibility: ["//visibility:public"],
}
mock_library {
name: "libexample",
// Make this visibility to //other but not //visibility:public
visibility: ["//visibility:override", "//other"],
defaults: ["libexample_defaults"],
}`),
"other/Blueprints": []byte(`
mock_library {
name: "libother",
deps: ["libexample"],
}`),
"namespace/Blueprints": []byte(`
mock_library {
name: "libnamespace",
deps: ["libexample"],
}`),
},
expectedErrors: []string{
`module "libnamespace" variant "android_common": depends on //top:libexample which is not visible to this module`,
},
},
{
name: "//visibility:override discards defaults supplied rules",
fs: map[string][]byte{
"top/Blueprints": []byte(`
mock_defaults {
name: "libexample_defaults",
visibility: ["//namespace"],
}
mock_library {
name: "libexample",
// Make this visibility to //other but not //namespace
visibility: ["//visibility:override", "//other"],
defaults: ["libexample_defaults"],
}`),
"other/Blueprints": []byte(`
mock_library {
name: "libother",
deps: ["libexample"],
}`),
"namespace/Blueprints": []byte(`
mock_library {
name: "libnamespace",
deps: ["libexample"],
}`),
},
expectedErrors: []string{
`module "libnamespace" variant "android_common": depends on //top:libexample which is not visible to this module`,
},
},
{
name: "//visibility:override can override //visibility:public with //visibility:private",
fs: map[string][]byte{
"top/Blueprints": []byte(`
mock_defaults {
name: "libexample_defaults",
visibility: ["//visibility:public"],
}
mock_library {
name: "libexample",
visibility: ["//visibility:override", "//visibility:private"],
defaults: ["libexample_defaults"],
}`),
"namespace/Blueprints": []byte(`
mock_library {
name: "libnamespace",
deps: ["libexample"],
}`),
},
expectedErrors: []string{
`module "libnamespace" variant "android_common": depends on //top:libexample which is not visible to this module`,
},
},
{
name: "//visibility:override can override //visibility:private with //visibility:public",
fs: map[string][]byte{
"top/Blueprints": []byte(`
mock_defaults {
name: "libexample_defaults",
visibility: ["//visibility:private"],
}
mock_library {
name: "libexample",
visibility: ["//visibility:override", "//visibility:public"],
defaults: ["libexample_defaults"],
}`),
"namespace/Blueprints": []byte(`
mock_library {
name: "libnamespace",
deps: ["libexample"],
}`),
},
},
{ {
name: "//visibility:private mixed with itself", name: "//visibility:private mixed with itself",
fs: map[string][]byte{ fs: map[string][]byte{