From 9f97fc2b5a6ea4dde7ce655240e0e76d550841e7 Mon Sep 17 00:00:00 2001
From: Googler
Date: Fri, 21 Oct 2022 19:45:58 -0700
Subject: [PATCH] Add load visibility concept documentation
Outside visibility.md:
- package_group documentation: Put the mention of legacy behavior in relation to 6.0 release.
- mention both relevant flags in visibility() function's docs.
- glossary: Update entry for "visibility" to say there's two kinds.
Within visibility.md:
- add link at discussion of package_group syntax.
- all new content under "Load visibility" heading: intro, how to declare, practices.
Fixes #11261. (Yay!)
PiperOrigin-RevId: 482944990
Change-Id: I2c2cbb4e6c76e02a28cd8ce9b641ac898a42b9e0
---
site/en/concepts/visibility.md | 223 +++++++++++++++++-
site/en/reference/glossary.md | 11 +-
.../build/docgen/templates/be/functions.vm | 18 +-
.../StarlarkBuildApiGlobals.java | 9 +-
4 files changed, 233 insertions(+), 28 deletions(-)
diff --git a/site/en/concepts/visibility.md b/site/en/concepts/visibility.md
index 23de14011a1904..8bddd2b2b3b7ae 100644
--- a/site/en/concepts/visibility.md
+++ b/site/en/concepts/visibility.md
@@ -57,11 +57,13 @@ are just syntactic placeholders that do not correspond to any actual target.
* `"//some_pkg:my_package_group"`: Grants access to all of the packages that
are part of the given [`package_group`](/reference/be/functions#package_group).
- * Package groups use a different syntax for specifying packages. Within a
- package group, the forms `"//foo/bar:__pkg__"` and
- `"//foo/bar:__subpackages__"` are respectively replaced by `"//foo/bar"`
- and `"//foo/bar/..."`. Likewise, `"//visibility:public"` and
- `"//visibility:private"` are just `"public"` and `"private"`.
+ * Package groups use a
+ [different syntax](/reference/be/functions#package_group.packages) for
+ specifying packages. Within a package group, the forms
+ `"//foo/bar:__pkg__"` and `"//foo/bar:__subpackages__"` are respectively
+ replaced by `"//foo/bar"` and `"//foo/bar/..."`. Likewise,
+ `"//visibility:public"` and `"//visibility:private"` are just `"public"`
+ and `"private"`.
For example, if `//some/package:mytarget` has its `visibility` set to
`[":__subpackages__", "//tests:__pkg__"]`, then it could be used by any target
@@ -248,11 +250,208 @@ private so long as it lives in the same package as the definition of the
**Load visibility** controls whether a `.bzl` file may be loaded from other
`BUILD` or `.bzl` files.
-`BUILD` and `.bzl` files, as processed by Bazel during loading, are not
-considered to be targets and therefore are not subject to visibility. It is
-possible to load a `.bzl` file from anywhere in the workspace.
+In the same way that target visibility protects source code that is encapsulated
+by targets, load visibility protects build logic that is encapsulated by `.bzl`
+files. For instance, a `BUILD` file author might wish to factor some repetitive
+target definitions into a macro in a `.bzl` file. Without the protection of load
+visibility, they might find their macro reused by other collaborators in the
+same workspace, so that modifying the macro breaks other teams' builds.
-However, users may choose to run the Buildifier linter.
-The [bzl-visibility](https://github.com/bazelbuild/buildtools/blob/master/WARNINGS.md#bzl-visibility)
-check provides a warning if users `load` from beneath a subdirectory named
-`internal` or `private`.
+Note that a `.bzl` file may or may not have a corresponding source file target.
+If it does, there is no guarantee that the load visibility and the target
+visibility coincide. That is, the same `BUILD` file might be able to load the
+`.bzl` file but not list it in the `srcs` of a [`filegroup`](/reference/be/general#filegroup),
+or vice versa. This can sometimes cause problems for rules that wish to consume
+`.bzl` files as source code, such as for documentation generation or testing.
+
+For prototyping, you may disable load visibility enforcement by setting
+`--check_bzl_visibility=false`. As with `--check_visibility=false`, this should
+not be done for submitted code.
+
+Load visibility is available as of Bazel 6.0.
+
+### Declaring load visibility {:#declaring-load-visibility}
+
+To set the load visibility of a `.bzl` file, call the
+[`visibility()`](/rules/lib/globals#visibility) function from within the file.
+The argument to `visibility()` is a list of package specifications, just like
+the [`packages`](/reference/be/functions#package_group.packages) attribute of
+`package_group`. However, `visibility()` does not accept negative package
+specifications.
+
+The call to `visibility()` must only occur once per file, at the top level (not
+inside a function), and ideally immediately following the `load()` statements.
+
+Unlike target visibility, the default load visibility is always public. Files
+that do not call `visibility()` are always loadable from anywhere in the
+workspace. It is a good idea to add `visibility("private")` to the top of any
+new `.bzl` file that is not specifically intended for use outside the package.
+
+### Example {:#load-visibility-example}
+
+```python
+# //mylib/internal_defs.bzl
+
+# Available to subpackages and to mylib's tests.
+visibility(["//mylib/...", "//tests/mylib/..."])
+
+def helper(...):
+ ...
+```
+
+```python
+# //mylib/rules.bzl
+
+load(":internal_defs.bzl", "helper")
+# Set visibility explicitly, even though public is the default.
+# Note the [] can be omitted when there's only one entry.
+visibility("public")
+
+myrule = rule(
+ ...
+)
+```
+
+```python
+# //someclient/BUILD
+
+load("//mylib:rules.bzl", "myrule") # ok
+load("//mylib:internal_defs.bzl", "helper") # error
+
+...
+```
+
+### Load visibility practices {:#load-visibility-practices}
+
+This section describes tips for managing load visibility declarations.
+
+#### Factoring visibilities {:#factoring-visibilities}
+
+When multiple `.bzl` files should have the same visibility, it can be helpful to
+factor their package specifications into a common list. For example:
+
+```python
+# //mylib/internal_defs.bzl
+
+visibility("private")
+
+clients = [
+ "//foo",
+ "//bar/baz/...",
+ ...
+]
+```
+
+```python
+# //mylib/feature_A.bzl
+
+load(":internal_defs.bzl", "clients")
+visibility(clients)
+
+...
+```
+
+```python
+# //mylib/feature_B.bzl
+
+load(":internal_defs.bzl", "clients")
+visibility(clients)
+
+...
+```
+
+This helps prevent accidental skew between the various `.bzl` files'
+visibilities. It also is more readable when the `clients` list is large.
+
+#### Composing visibilities {:#composing-visibilities}
+
+Sometimes a `.bzl` file might need to be visible to an allowlist that is
+composed of multiple smaller allowlists. This is analogous to how a
+`package_group` can incorporate other `package_group`s via its
+[`includes`](/reference/be/functions#package_group.includes) attribute.
+
+Suppose you are deprecating a widely used macro. You want it to be visible only
+to existing users and to the packages owned by your own team. You might write:
+
+```python
+# //mylib/macros.bzl
+
+load(":internal_defs.bzl", "our_packages")
+load("//some_big_client:defs.bzl", "their_remaining_uses)
+
+# List concatenation. Duplicates are fine.
+visibility(our_packages + their_remaining_uses)
+```
+
+#### Deduplicating with package groups {:#deduplicating-with-package-groups}
+
+Unlike target visibility, you cannot define a load visibility in terms of a
+`package_group`. If you want to reuse the same allowlist for both target
+visibility and load visibility, it's best to move the list of package
+specifications into a .bzl file, where both kinds of declarations may refer to
+it. Building off the example in [Factoring visibilities](#factoring-visibilities)
+above, you might write:
+
+```python
+# //mylib/BUILD
+
+load(":internal_defs", "clients")
+
+package_group(
+ name = "my_pkg_grp",
+ packages = clients,
+)
+```
+
+This only works if the list does not contain any negative package
+specifications.
+
+#### Protecting individual symbols {:#protecting-individual-symbols}
+
+Any Starlark symbol whose name begins with an underscore cannot be loaded from
+another file. This makes it easy to create private symbols, but does not allow
+you to share these symbols with a limited set of trusted files. On the other
+hand, load visibility gives you control over what other packages may see your
+`.bzl file`, but does not allow you to prevent any non-underscored symbol from
+being loaded.
+
+Luckily, you can combine these two features to get fine-grained control.
+
+```python
+# //mylib/internal_defs.bzl
+
+# Can't be public, because internal_helper shouldn't be exposed to the world.
+visibility("private")
+
+# Can't be underscore-prefixed, because this is
+# needed by other .bzl files in mylib.
+def internal_helper(...):
+ ...
+
+def public_util(...):
+ ...
+```
+
+```python
+# //mylib/defs.bzl
+
+load(":internal_defs", "internal_helper", _public_util="public_util")
+visibility("public")
+
+# internal_helper, as a loaded symbol, is available for use in this file but
+# can't be imported by clients who load this file.
+...
+
+# Re-export public_util from this file by assigning it to a global variable.
+# We needed to import it under a different name ("_public_util") in order for
+# this assignment to be legal.
+public_util = _public_util
+```
+
+#### bzl-visibility Buildifier lint {:#bzl-visibility-buildifier-lint}
+
+There is a [Buildifier lint](https://github.com/bazelbuild/buildtools/blob/master/WARNINGS.md#bzl-visibility)
+that provides a warning if users load a file from a directory named `internal`
+or `private`, when the user's file is not itself underneath the parent of that
+directory. This lint predates the load visibility feature and is unnecessary in
+workspaces where `.bzl` files declare visibilities.
diff --git a/site/en/reference/glossary.md b/site/en/reference/glossary.md
index fb4d3d638a2b11..9532e140067982 100644
--- a/site/en/reference/glossary.md
+++ b/site/en/reference/glossary.md
@@ -606,12 +606,13 @@ instead register the tree artifact as its input or output.
### Visibility {:#visibility}
-Defines whether a [target](#target) can be depended upon by other targets. By
-default, target visibility is private. That is, the target can only be depended
-upon by other targets in the same [package](#package). Can be made visible to
-specific packages or be completely public.
+One of two mechanisms for preventing unwanted dependencies in the build system:
+*target visibility* for controlling whether a [target](#target) can be depended
+upon by other targets; and *load visibility* for controlling whether a `BUILD`
+or `.bzl` file may load a given `.bzl` file. Without context, usually
+"visibility" refers to target visibility.
-**See also:** [Visibility documentation](/reference/be/common-definitions#common-attributes)
+**See also:** [Visibility documentation](/concepts/visibility)
### Workspace {:#workspace}
diff --git a/src/main/java/com/google/devtools/build/docgen/templates/be/functions.vm b/src/main/java/com/google/devtools/build/docgen/templates/be/functions.vm
index c8c64fd2aea3aa..0c976a2c16b1fd 100644
--- a/src/main/java/com/google/devtools/build/docgen/templates/be/functions.vm
+++ b/src/main/java/com/google/devtools/build/docgen/templates/be/functions.vm
@@ -205,17 +205,19 @@ not themselves have any visibility protection.
empty list, which is also the same as setting it to a list containing
only private
.
- Note: The specification //...
has a legacy behavior
- of being the same as public
. This behavior is disabled when
+
Note: Prior to Bazel 6.0, the specification //...
+ had a legacy behavior of being the same as public
. This
+ behavior is fixed when
--incompatible_fix_package_group_reporoot_syntax
is
- enabled.
+ enabled, which is the default after Bazel 6.0.
- Note: As a legacy behavior, when this attribute is serialized as
+
Note: Prior to Bazel 6.0, when this attribute is serialized as
part of bazel query --output=proto
(or
- --output=xml
), the leading slashes are omitted if
- --incompatible_package_group_includes_double_slash
is not
- enabled. For instance, //pkg/foo/...
will output as
- \"pkg/foo/...\"
.
+ --output=xml
), the leading slashes are omitted. For
+ instance, //pkg/foo/...
will output as
+ \"pkg/foo/...\"
. This behavior is fixed when
+ --incompatible_package_group_includes_double_slash
is
+ enabled, which is the default after Bazel 6.0.
diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkBuildApiGlobals.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkBuildApiGlobals.java
index 8ff9ae6ad83655..1418dc16866b00 100644
--- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkBuildApiGlobals.java
+++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkBuildApiGlobals.java
@@ -67,9 +67,12 @@ public interface StarlarkBuildApiGlobals {
+ " specification. (An empty list has the same effect as private
.)"
+ " If value
is a single string, it is treated as if it were the"
+ " singleton list [value]
."
- + "Note that the specification \"//...\"
is always interpreted"
- + " as \"all packages in the current repository\", regardless of the value of"
- + " the --incompatible_fix_package_group_reporoot_syntax
flag.")
+ + "
Note that the flags"
+ + " --incompatible_package_group_has_public_syntax
and"
+ + " --incompatible_fix_package_group_reporoot_syntax
have no"
+ + " effect on this argument. The \"public\"
and \"private\""
+ + "
values are always available, and \"//...\"
is always"
+ + " interpreted as \"all packages in the current repository\".")
},
// Ordinarily we'd use enableOnlyWithFlag here to gate access on
// --experimental_bzl_visibility. However, the StarlarkSemantics isn't available at the point