package_group: Add "public"/"private" constants, fix "//..." meaning
This CL does three things:
1. It adds the special package specifications "public" and "private, guarded behind the flag `--incompatible_package_group_has_public_syntax`.
2. It fixes the behavior of package specification "//..." to mean "all packages in the current repo" rather than "all packages in any repo", guarded behind the flag `--incompatible_fix_package_group_reporoot_syntax`.
3. It fixes the behavior of `bazel query --output=proto` (and `--output=xml`) so that the `packages` attribute is serialized without dropping the leading `//` from package names -- it is now `"//foo/bar/..."` instead of `"foo/bar/..."`. This behavioral change is guarded behind `--incompatible_package_group_includes_double_slash`.
The .bzl `visibility()` builtin always acts as if the first two flags are enabled.
Work toward #11261.
Work toward #16323.
Work toward #16355.
Work toward #16391.
RELNOTES: Added three `package_group`-related flags: `--incompatible_package_group_includes_double_slash` (#16391), `--incompatible_package_group_has_public_syntax` (#16355), and `--incompatible_fix_package_group_reporoot_syntax` (#16323). With these flags, `package_group` can now easily specify "all packages", "no packages", and "all packages in the current repo".
In PackageSpecification:
- fromString(), fromStringPositive(): rewrote docstring, added bool args for flags, renamed or removed vars for simplicity, added handling of public/private and flag behavior
- AllPackagesBeneath(): fixed asStringWithoutRepository() for case of //..., which wasn't likely to arise before.
- normalize order of equals(), hashCode(), etc.
- AllPackages: change hash, and change str representation to "public"
- Add NoPackages, symmetric to AllPackages
- NegativePackageSpecification: don't reuse the delegate's hash as our own hash verbatim
In PackageGroupTest:
- Move `testEverythingSpecificationWorks` to above `testNegativeEverything`, and made it use "public" syntax in place of "//...".
- Add test for "private" syntax.
- Add test for flag guarding "public"/"private".
- Add tests for old and new "//..." behavior.
- Add tests that you can't negate "public" or "private".
- Add test for illegal combination of flags.
- Add assertions for stringification of "public"/"private".
Other changes:
- Update XmlOutputFormatterTest and ProtoOutputFormatterTest with test cases for the flag. In the latter case, parameterize a helper to allow setting per-test-case flags.
- Add test cases to BzlLoadFunctionTest for bzl visibility accepting the list forms `["public"]`/`["private"]`, and for its behavior being unaffected by the new flags.
PiperOrigin-RevId: 479347358
Change-Id: I124ca5406bff15615adaa1256e2d7bef69b9d9a5
diff --git a/site/en/concepts/visibility.md b/site/en/concepts/visibility.md
index 89efcfb..596e838 100644
--- a/site/en/concepts/visibility.md
+++ b/site/en/concepts/visibility.md
@@ -45,7 +45,8 @@
* 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/..."`.
+ 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
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 164ece7..c8c64fd 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
@@ -178,13 +178,18 @@
<li>As above, but with a trailing <code>/...</code>. For example, <code>
//foo/...</code> specifies the set of <code>//foo</code> and all its
- subpackages. As a special case, <code>//...</code> specifies all
- packages in any repository (in other words, public).
+ subpackages. <code>//...</code> specifies all packages in the current
+ repository.
+
+ <li>The strings <code>public</code> or <code>private</code>, which
+ respectively specify every package or no package. (This form requires
+ the flag <code>--incompatible_package_group_has_public_syntax</code> to
+ be set.)
</ol>
- <p>In addition, a package specification may also be prefixed with
- <code>-</code> to indicate that it is negated.</p>
+ <p>In addition, the first two kinds of package specifications may also
+ be prefixed with <code>-</code> to indicate that they are negated.</p>
<p>The package group contains any package that matches at least one of
its positive specifications and none of its negative specifications
@@ -193,11 +198,24 @@
subpackages of <code>//foo/tests</code>. (<code>//foo</code> itself is
included while </code>//foo/tests</code> itself is not.)</p>
- <p>Aside from <code>//...</code>, there is no way to directly specify
+ <p>Aside from public visibility, there is no way to directly specify
packages outside the current repository.</p>
<p>If this attribute is missing, it is the same as setting it to an
- empty list (in other words, private).
+ empty list, which is also the same as setting it to a list containing
+ only <code>private</code>.
+
+ <p><i>Note:</i> The specification <code>//...</code> has a legacy behavior
+ of being the same as <code>public</code>. This behavior is disabled when
+ <code>--incompatible_fix_package_group_reporoot_syntax</code> is
+ enabled.</p>
+
+ <p><i>Note:</i> As a legacy behavior, when this attribute is serialized as
+ part of <code>bazel query --output=proto</code> (or
+ <code>--output=xml</code>), the leading slashes are omitted if
+ <code>--incompatible_package_group_includes_double_slash</code> is not
+ enabled. For instance, <code>//pkg/foo/...</code> will output as
+ <code>\"pkg/foo/...\"</code>.</p>
</td>
</tr>
<tr>
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index f9dec2a..a73a3d3 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -1652,14 +1652,26 @@
return Label.create(pkg.getPackageIdentifier(), targetName);
}
- /**
- * Adds a package group to the package.
- */
- void addPackageGroup(String name, Collection<String> packages, Collection<Label> includes,
- EventHandler eventHandler, Location location)
+ /** Adds a package group to the package. */
+ void addPackageGroup(
+ String name,
+ Collection<String> packages,
+ Collection<Label> includes,
+ boolean allowPublicPrivate,
+ boolean repoRootMeansCurrentRepo,
+ EventHandler eventHandler,
+ Location location)
throws NameConflictException, LabelSyntaxException {
PackageGroup group =
- new PackageGroup(createLabel(name), pkg, packages, includes, eventHandler, location);
+ new PackageGroup(
+ createLabel(name),
+ pkg,
+ packages,
+ includes,
+ allowPublicPrivate,
+ repoRootMeansCurrentRepo,
+ eventHandler,
+ location);
Target existing = targets.get(group.getName());
if (existing != null) {
throw nameConflict(group, existing);
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageGroup.java b/src/main/java/com/google/devtools/build/lib/packages/PackageGroup.java
index ebad8d7..f9f9a79 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageGroup.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageGroup.java
@@ -47,6 +47,8 @@
Package pkg,
Collection<String> packageSpecifications,
Collection<Label> includes,
+ boolean allowPublicPrivate,
+ boolean repoRootMeansCurrentRepo,
EventHandler eventHandler,
Location location) {
this.label = label;
@@ -61,7 +63,11 @@
PackageSpecification specification = null;
try {
specification =
- PackageSpecification.fromString(label.getRepository(), packageSpecification);
+ PackageSpecification.fromString(
+ label.getRepository(),
+ packageSpecification,
+ allowPublicPrivate,
+ repoRootMeansCurrentRepo);
} catch (PackageSpecification.InvalidPackageSpecificationException e) {
errorsFound = true;
eventHandler.handle(
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageSpecification.java b/src/main/java/com/google/devtools/build/lib/packages/PackageSpecification.java
index 8c5433d..51696ae 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageSpecification.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageSpecification.java
@@ -47,11 +47,15 @@
* packages" specification.
*/
public abstract class PackageSpecification {
- private static final String PACKAGE_LABEL = "__pkg__";
- private static final String SUBTREE_LABEL = "__subpackages__";
+ private static final String PUBLIC_VISIBILITY = "public";
+ private static final String PRIVATE_VISIBILITY = "private";
private static final String ALL_BENEATH_SUFFIX = "/...";
private static final String NEGATIVE_PREFIX = "-";
+ // Used for interpreting `visibility` labels.
+ private static final String PACKAGE_LABEL = "__pkg__";
+ private static final String SUBTREE_LABEL = "__subpackages__";
+
/** Returns {@code true} if the package spec includes the provided {@code packageName}. */
protected abstract boolean containsPackage(PackageIdentifier packageName);
@@ -82,9 +86,13 @@
* additional context. But, for instance, if interpreted with respect to a {@code package_group}'s
* {@code packages} attribute, the strings always have the same repository as the package group.
*/
- // TODO(brandjon): This method's main benefit is that it's round-trippable. We could eliminate
- // it in favor of asString() if we provided a public variant of fromString() that tolerates
- // repositories.
+ // TODO(brandjon): This method's main benefit is that it's round-trippable. But we can already
+ // achieve the same thing with asString(), if the caller parses out the repo to pass to
+ // fromString() as a separate arg. It'd be nice to eliminate this method in favor of asString()
+ // and make a version of fromString() that can parse repo names in the label. We'd just have to
+ // mimic the Label parsing of repo (we can't reuse Label parsing directly since it won't like the
+ // `/...` syntax). Repo remapping shouldn't come up, since the names we get from stringification
+ // ought to already be canonical/absolute.
protected abstract String asStringWithoutRepository();
@Override
@@ -93,71 +101,124 @@
}
/**
- * Parses the provided {@link String} into a {@link PackageSpecification}.
+ * Parses the string {@code spec} into a {@link PackageSpecification}, within the context of the
+ * given repository name.
*
- * <p>The {@link String} must have one of the following forms:
+ * <p>{@code spec} may have the following forms:
*
- * <ul>
- * <li>The full name of a single package, without repository qualification, prefixed with "//"
- * (e.g. "//foo/bar"). This results in a {@link PackageSpecification} that contains exactly
- * the named package.
- * <li>The full name of a single package, without repository qualification, prefixed with "//",
- * and suffixed with "/..." (e.g. "//foo/bar/...") This results in a {@link
- * PackageSpecification} that contains all transitive subpackages of that package, inclusive.
- * <li>Exactly "//...". This results in a {@link PackageSpecification} that contains all packages.
- * </ul>
+ * <ol>
+ * <li>The full name of a single package, without repository qualification, prefixed with "//"
+ * (e.g. "//foo/bar"). The resulting specification contains exactly that package.
+ * <li>The same, but suffixed with "/..." for a non-root package ("//foo/bar/...") or "..." for
+ * the root package ("//..."). The resulting specification contains that package and all its
+ * subpackages.
+ * <li>The string constants "public" or "private". The resulting specification contains either
+ * all packages or no packages, respectively.
+ * </ol>
*
- * <p>If and only if the {@link String} is one of the first two forms, the returned {@link
- * PackageSpecification} is specific to the provided {@link RepositoryName}. Note that it is not
- * possible to construct a repository-specific {@link PackageSpecification} for all transitive
- * subpackages of the root package (i.e. a repository-specific "//...").
+ * In the first two cases, the repository of the given package name is taken to be {@code
+ * repositoryName}. In the third case the repository name is ignored.
*
- * <p>Throws {@link InvalidPackageSpecificationException} if the {@link String} cannot be parsed.
+ * <p>In the first two cases, {@code spec} may also be prefixed by a "-". The resulting
+ * specification contains the same set of packages but is marked as being negated. (Negation logic
+ * is applied at the level of {@link PackageGroupContents}.)
+ *
+ * <p>Setting {@code allowPublicPrivate} to false disallows the string constants "public" and
+ * "private". Note that if {@link #asString} is called with {@code includeDoubleSlash} set to
+ * false, the stringification of "public" and "//public" is ambiguous (likewise for private),
+ * hence why it might be appropriate to prohibit these forms.
+ *
+ * <p>Setting {@code repoRootMeansCurrentRepo} to false restores the following legacy behavior: In
+ * the specific case where {@code spec} is "//..." (or its negation), the package specification
+ * contains <i>all</i> packages (possibly marked as negated) rather than just those packages in
+ * {@code repositoryName}. In other words, "//..." behaves the same as "public". However, "//"
+ * still represents just the root package of {@code repositoryName}.
+ *
+ * <p>To protect against requiring users to update to a disallowed syntax, it is illegal to
+ * specify {@code repoRootMeansCurrentRepo} without also specifying {@code allowPublicPrivate}.
+ *
+ * @throws InvalidPackageSpecificationException if the string does not fit one of these forms
*/
- public static PackageSpecification fromString(RepositoryName repositoryName, String spec)
+ // TODO(#16365): Remove allowPublicPrivate.
+ // TODO(#16324): Remove legacy behavior and repoRootMeansCurrentRepo param.
+ public static PackageSpecification fromString(
+ RepositoryName repositoryName,
+ String spec,
+ boolean allowPublicPrivate,
+ boolean repoRootMeansCurrentRepo)
throws InvalidPackageSpecificationException {
- String result = spec;
- boolean negative = false;
- if (result.startsWith(NEGATIVE_PREFIX)) {
- negative = true;
- result = result.substring(NEGATIVE_PREFIX.length());
+ if (repoRootMeansCurrentRepo && !allowPublicPrivate) {
+ throw new InvalidPackageSpecificationException(
+ "Cannot use new \"//...\" meaning without allowing new \"public\" syntax. Try enabling"
+ + " --incompatible_package_group_has_public_syntax or disabling"
+ + " --incompatible_fix_package_group_reporoot_syntax.");
}
- PackageSpecification packageSpecification = fromStringPositive(repositoryName, result);
+ if (!allowPublicPrivate
+ && (spec.equals(PUBLIC_VISIBILITY) || spec.equals(PRIVATE_VISIBILITY))) {
+ throw new InvalidPackageSpecificationException(
+ String.format(
+ "Use of \"%s\" package specification requires enabling"
+ + " --incompatible_package_group_has_public_syntax",
+ spec));
+ }
+ boolean negative = false;
+ if (spec.startsWith(NEGATIVE_PREFIX)) {
+ negative = true;
+ spec = spec.substring(NEGATIVE_PREFIX.length());
+ if (spec.equals(PUBLIC_VISIBILITY) || spec.equals(PRIVATE_VISIBILITY)) {
+ throw new InvalidPackageSpecificationException(
+ String.format("Cannot negate \"%s\" package specification", spec));
+ }
+ }
+ PackageSpecification packageSpecification =
+ fromStringPositive(repositoryName, spec, repoRootMeansCurrentRepo);
return negative ? new NegativePackageSpecification(packageSpecification) : packageSpecification;
}
- private static PackageSpecification fromStringPositive(RepositoryName repositoryName, String spec)
+ private static PackageSpecification fromStringPositive(
+ RepositoryName repositoryName, String spec, boolean repoRootMeansCurrentRepo)
throws InvalidPackageSpecificationException {
- String result = spec;
- boolean allBeneath = false;
- if (result.endsWith(ALL_BENEATH_SUFFIX)) {
- allBeneath = true;
- result = result.substring(0, result.length() - ALL_BENEATH_SUFFIX.length());
- if (result.equals("/")) {
- // spec was "//...".
- return AllPackages.EVERYTHING;
- }
+ if (spec.equals(PUBLIC_VISIBILITY)) {
+ return AllPackages.INSTANCE;
+ } else if (spec.equals(PRIVATE_VISIBILITY)) {
+ return NoPackages.INSTANCE;
}
-
if (!spec.startsWith("//")) {
throw new InvalidPackageSpecificationException(
- String.format("invalid package name '%s': must start with '//'", spec));
+ String.format(
+ "invalid package name '%s': must start with '//' or be 'public' or 'private'", spec));
}
- PackageIdentifier packageId;
+ String pkgPath;
+ boolean allBeneath = false;
+ if (spec.endsWith(ALL_BENEATH_SUFFIX)) {
+ allBeneath = true;
+ pkgPath = spec.substring(0, spec.length() - ALL_BENEATH_SUFFIX.length());
+ if (pkgPath.equals("/")) {
+ // spec was "//...".
+ if (repoRootMeansCurrentRepo) {
+ pkgPath = "//";
+ } else {
+ // Legacy behavior: //... is "public".
+ return AllPackages.INSTANCE;
+ }
+ }
+ } else {
+ pkgPath = spec;
+ }
+
+ PackageIdentifier unqualifiedPkgId;
try {
- packageId = PackageIdentifier.parse(result);
+ unqualifiedPkgId = PackageIdentifier.parse(pkgPath);
} catch (LabelSyntaxException e) {
throw new InvalidPackageSpecificationException(
String.format("invalid package name '%s': %s", spec, e.getMessage()));
}
- Verify.verify(packageId.getRepository().isMain());
+ Verify.verify(unqualifiedPkgId.getRepository().isMain());
- PackageIdentifier packageIdForSpecifiedRepository =
- PackageIdentifier.create(repositoryName, packageId.getPackageFragment());
- return allBeneath
- ? new AllPackagesBeneath(packageIdForSpecifiedRepository)
- : new SinglePackage(packageIdForSpecifiedRepository);
+ PackageIdentifier pkgId =
+ PackageIdentifier.create(repositoryName, unqualifiedPkgId.getPackageFragment());
+ return allBeneath ? new AllPackagesBeneath(pkgId) : new SinglePackage(pkgId);
}
/**
@@ -165,13 +226,21 @@
*
* <p>This rejects negative package patterns, and translates the exception type into {@code
* EvalException}.
+ *
+ * <p>Note that .bzl visibility package specifications always behave as if {@code
+ * --incompatible_package_group_has_public_syntax} and {@code
+ * --incompatible_fix_package_group_reporoot_syntax} are enabled.
*/
- // TODO(b/22193153): Support negatives too.
public static PackageSpecification fromStringForBzlVisibility(
RepositoryName repositoryName, String spec) throws EvalException {
PackageSpecification result;
try {
- result = fromString(repositoryName, spec);
+ result =
+ fromString(
+ repositoryName,
+ spec,
+ /*allowPublicPrivate=*/ true,
+ /*repoRootMeansCurrentRepo=*/ true);
} catch (InvalidPackageSpecificationException e) {
throw new EvalException(e.getMessage());
}
@@ -193,8 +262,8 @@
*
* <p>If the label's name is neither "__pkg__" nor "__subpackages__", this returns {@code null}.
*
- * <p>Note that there is no {@link Label} associated with the {@link RepositoryName}-agnostic "all
- * packages" specification (corresponding to {@code #fromString(null, "//...")}).
+ * <p>Note that there is no {@link Label} associated with the {@link RepositoryName}-agnostic
+ * "public" specification ("//..." under legacy semantics).
*/
@Nullable
static PackageSpecification fromLabel(Label label) {
@@ -208,7 +277,11 @@
}
public static PackageSpecification everything() {
- return AllPackages.EVERYTHING;
+ return AllPackages.INSTANCE;
+ }
+
+ public static PackageSpecification nothing() {
+ return NoPackages.INSTANCE;
}
private static final class SinglePackage extends PackageSpecification {
@@ -291,7 +364,12 @@
@Override
protected String asStringWithoutRepository() {
- return "//" + prefix.getPackageFragment().getPathString() + ALL_BENEATH_SUFFIX;
+ if (prefix.getPackageFragment().equals(PathFragment.EMPTY_FRAGMENT)) {
+ // Special case: Emit "//..." rather than suffixing "/...", which would yield "///...".
+ return "//...";
+ } else {
+ return "//" + prefix.getPackageFragment().getPathString() + ALL_BENEATH_SUFFIX;
+ }
}
@Override
@@ -336,11 +414,6 @@
}
@Override
- public int hashCode() {
- return delegate.hashCode();
- }
-
- @Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
@@ -348,12 +421,17 @@
return obj instanceof NegativePackageSpecification
&& delegate.equals(((NegativePackageSpecification) obj).delegate);
}
+
+ @Override
+ public int hashCode() {
+ return NegativePackageSpecification.class.hashCode() ^ delegate.hashCode();
+ }
}
@VisibleForSerialization
static final class AllPackages extends PackageSpecification {
@SerializationConstant @VisibleForSerialization
- static final PackageSpecification EVERYTHING = new AllPackages();
+ static final PackageSpecification INSTANCE = new AllPackages();
@Override
protected boolean containsPackage(PackageIdentifier packageName) {
@@ -362,14 +440,15 @@
@Override
protected String asString(boolean includeDoubleSlash) {
- // Note that "//..." is the desired result, not "...", even under the legacy behavior of
- // includeDoubleSlash=false.
- return "//...";
+ // Under legacy formatting rules, use legacy syntax. This avoids ambiguity between "public"
+ // and "//public", and ensures that AllPackages is round-trippable when the value of
+ // includeDoubleSlash matches allowPublicPrivate.
+ return includeDoubleSlash ? "public" : "//...";
}
@Override
protected String asStringWithoutRepository() {
- return "//...";
+ return "public";
}
@Override
@@ -379,7 +458,38 @@
@Override
public int hashCode() {
- return "//...".hashCode();
+ return AllPackages.class.hashCode();
+ }
+ }
+
+ @VisibleForSerialization
+ static final class NoPackages extends PackageSpecification {
+ @SerializationConstant @VisibleForSerialization
+ static final PackageSpecification INSTANCE = new NoPackages();
+
+ @Override
+ protected boolean containsPackage(PackageIdentifier packageName) {
+ return false;
+ }
+
+ @Override
+ protected String asString(boolean includeDoubleSlash) {
+ return "private";
+ }
+
+ @Override
+ protected String asStringWithoutRepository() {
+ return "private";
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ return o instanceof NoPackages;
+ }
+
+ @Override
+ public int hashCode() {
+ return NoPackages.class.hashCode();
}
}
@@ -437,7 +547,11 @@
for (PackageSpecification spec : packageSpecifications) {
if (spec instanceof SinglePackage) {
singlePositives.put(((SinglePackage) spec).singlePackageName, spec);
- } else if (spec instanceof AllPackages || spec instanceof AllPackagesBeneath) {
+ } else if (spec instanceof AllPackages
+ || spec instanceof AllPackagesBeneath
+ // We can't drop NoPackages because it still needs to be serialized, e.g. in bazel query
+ // output.
+ || spec instanceof NoPackages) {
otherPositives.add(spec);
} else if (spec instanceof NegativePackageSpecification) {
negatives.add(spec);
@@ -483,6 +597,10 @@
*
* <p>Note that strings for specs that cross repositories can't be reparsed using {@link
* PackageSpecification#fromString}.
+ *
+ * <p>The special public constant will serialize as {@code "public"} if {@code
+ * includeDoubleSlash} is true, and {@code "//..."} otherwise. The private constant will always
+ * serialize as {@code "private"},
*/
public Stream<String> streamPackageStrings(boolean includeDoubleSlash) {
return streamSpecs().map(spec -> spec.asString(includeDoubleSlash));
@@ -492,6 +610,9 @@
* Maps {@link PackageSpecification#asStringWithoutRepository} to the component package specs.
*
* <p>Note that this is ambiguous w.r.t. specs that reference other repositories.
+ *
+ * <p>The special public and private constants will serialize as {@code "public"} and {@code
+ * "private"} respectively.
*/
public Stream<String> streamPackageStringsWithoutRepository() {
return streamSpecs().map(PackageSpecification::asStringWithoutRepository);
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java
index f45c882..703b9de 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java
@@ -542,7 +542,18 @@
Location loc = thread.getCallerLocation();
try {
- context.pkgBuilder.addPackageGroup(name, packages, includes, context.eventHandler, loc);
+ context.pkgBuilder.addPackageGroup(
+ name,
+ packages,
+ includes,
+ /*allowPublicPrivate=*/ thread
+ .getSemantics()
+ .getBool(BuildLanguageOptions.INCOMPATIBLE_PACKAGE_GROUP_HAS_PUBLIC_SYNTAX),
+ /*repoRootMeansCurrentRepo=*/ thread
+ .getSemantics()
+ .getBool(BuildLanguageOptions.INCOMPATIBLE_FIX_PACKAGE_GROUP_REPOROOT_SYNTAX),
+ context.eventHandler,
+ loc);
return Starlark.NONE;
} catch (LabelSyntaxException e) {
throw Starlark.errorf("package group has invalid name: %s: %s", name, e.getMessage());
diff --git a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java
index a3518ad..4605061 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java
@@ -443,6 +443,31 @@
public boolean incompatibleDisallowStructProviderSyntax;
@Option(
+ name = "incompatible_package_group_has_public_syntax",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
+ effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
+ metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
+ help =
+ "In package_group's `packages` attribute, allows writing \"public\" or \"private\" to"
+ + " refer to all packages or no packages respectively.")
+ public boolean incompatiblePackageGroupHasPublicSyntax;
+
+ @Option(
+ name = "incompatible_fix_package_group_reporoot_syntax",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
+ effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
+ metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
+ help =
+ "In package_group's `packages` attribute, changes the meaning of the value \"//...\" to"
+ + " refer to all packages in the current repository instead of all packages in any"
+ + " repository. You can use the special value \"public\" in place of \"//...\" to"
+ + " obtain the old behavior. This flag requires"
+ + " that --incompatible_package_group_has_public_syntax also be enabled.")
+ public boolean incompatibleFixPackageGroupReporootSyntax;
+
+ @Option(
name = "incompatible_visibility_private_attributes_at_definition",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
@@ -675,6 +700,12 @@
.setBool(
INCOMPATIBLE_DISALLOW_STRUCT_PROVIDER_SYNTAX,
incompatibleDisallowStructProviderSyntax)
+ .setBool(
+ INCOMPATIBLE_PACKAGE_GROUP_HAS_PUBLIC_SYNTAX,
+ incompatiblePackageGroupHasPublicSyntax)
+ .setBool(
+ INCOMPATIBLE_FIX_PACKAGE_GROUP_REPOROOT_SYNTAX,
+ incompatibleFixPackageGroupReporootSyntax)
.setBool(INCOMPATIBLE_JAVA_COMMON_PARAMETERS, incompatibleJavaCommonParameters)
.setBool(INCOMPATIBLE_NEW_ACTIONS_API, incompatibleNewActionsApi)
.setBool(INCOMPATIBLE_NO_ATTR_LICENSE, incompatibleNoAttrLicense)
@@ -761,6 +792,10 @@
public static final String INCOMPATIBLE_DISALLOW_EMPTY_GLOB = "-incompatible_disallow_empty_glob";
public static final String INCOMPATIBLE_DISALLOW_STRUCT_PROVIDER_SYNTAX =
"-incompatible_disallow_struct_provider_syntax";
+ public static final String INCOMPATIBLE_PACKAGE_GROUP_HAS_PUBLIC_SYNTAX =
+ "-incompatible_package_group_has_public_syntax";
+ public static final String INCOMPATIBLE_FIX_PACKAGE_GROUP_REPOROOT_SYNTAX =
+ "-incompatible_fix_package_group_reporoot_syntax";
public static final String INCOMPATIBLE_DO_NOT_SPLIT_LINKING_CMDLINE =
"+incompatible_do_not_split_linking_cmdline";
public static final String INCOMPATIBLE_JAVA_COMMON_PARAMETERS =
diff --git a/src/main/java/com/google/devtools/build/lib/query2/common/CommonQueryOptions.java b/src/main/java/com/google/devtools/build/lib/query2/common/CommonQueryOptions.java
index 5bd8261..eb44883 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/common/CommonQueryOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/common/CommonQueryOptions.java
@@ -137,6 +137,17 @@
+ "query: no-op (aspects are always followed).")
public boolean useAspects;
+ @Option(
+ name = "incompatible_package_group_includes_double_slash",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.QUERY,
+ effectTags = {OptionEffectTag.TERMINAL_OUTPUT},
+ metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
+ help =
+ "If enabled, when outputting package_group's `packages` attribute, the leading `//`"
+ + " will not be omitted.")
+ public boolean incompatiblePackageGroupIncludesDoubleSlash;
+
/** Return the current options as a set of QueryEnvironment settings. */
public Set<Setting> toSettings() {
Set<Setting> settings = EnumSet.noneOf(Setting.class);
diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/output/ProtoOutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/query/output/ProtoOutputFormatter.java
index d3f85d8..3c1eb27 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/query/output/ProtoOutputFormatter.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/query/output/ProtoOutputFormatter.java
@@ -101,6 +101,7 @@
private AspectResolver aspectResolver;
private DependencyFilter dependencyFilter;
+ private boolean packageGroupIncludesDoubleSlash;
private boolean relativeLocations;
private boolean displaySourceFileLocation;
private boolean includeDefaultValues = true;
@@ -126,6 +127,7 @@
super.setOptions(options, aspectResolver, hashFunction);
this.aspectResolver = aspectResolver;
this.dependencyFilter = FormatUtils.getDependencyFilter(options);
+ this.packageGroupIncludesDoubleSlash = options.incompatiblePackageGroupIncludesDoubleSlash;
this.relativeLocations = options.relativeLocations;
this.displaySourceFileLocation = options.displaySourceFileLocation;
this.includeDefaultValues = options.protoIncludeDefaultValues;
@@ -344,9 +346,8 @@
PackageGroup packageGroup = (PackageGroup) target;
Build.PackageGroup.Builder packageGroupPb =
Build.PackageGroup.newBuilder().setName(packageGroup.getLabel().toString());
- // TODO(b/77598306): Migrate to format with leading double slash
for (String containedPackage :
- packageGroup.getContainedPackages(/*includeDoubleSlash=*/ false)) {
+ packageGroup.getContainedPackages(packageGroupIncludesDoubleSlash)) {
packageGroupPb.addContainedPackage(containedPackage);
}
for (Label include : packageGroup.getIncludes()) {
diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/output/XmlOutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/query/output/XmlOutputFormatter.java
index bcac1c6..1fff552 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/query/output/XmlOutputFormatter.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/query/output/XmlOutputFormatter.java
@@ -61,6 +61,7 @@
private AspectResolver aspectResolver;
private DependencyFilter dependencyFilter;
+ private boolean packageGroupIncludesDoubleSlash;
private boolean relativeLocations;
private boolean displaySourceFileLocation;
private QueryOptions queryOptions;
@@ -83,6 +84,7 @@
super.setOptions(options, aspectResolver, hashFunction);
this.aspectResolver = aspectResolver;
this.dependencyFilter = FormatUtils.getDependencyFilter(options);
+ this.packageGroupIncludesDoubleSlash = options.incompatiblePackageGroupIncludesDoubleSlash;
this.relativeLocations = options.relativeLocations;
this.displaySourceFileLocation = options.displaySourceFileLocation;
@@ -208,8 +210,7 @@
createValueElement(
doc,
Type.STRING_LIST,
- // TODO(b/77598306): Migrate to format with leading double slash
- packageGroup.getContainedPackages(/*includeDoubleSlash=*/ false));
+ packageGroup.getContainedPackages(packageGroupIncludesDoubleSlash));
packages.setAttribute("name", "packages");
elem.appendChild(packages);
} else if (target instanceof OutputFile) {
diff --git a/src/test/java/com/google/devtools/build/lib/packages/PackageGroupStaticInitializationTest.java b/src/test/java/com/google/devtools/build/lib/packages/PackageGroupStaticInitializationTest.java
index ed83559..07c9a77 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/PackageGroupStaticInitializationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/PackageGroupStaticInitializationTest.java
@@ -43,7 +43,12 @@
try {
RepositoryName defaultRepoName =
Label.parseAbsoluteUnchecked("//context").getRepository();
- groupQueue.put(PackageSpecification.fromString(defaultRepoName, "//fruits/..."));
+ groupQueue.put(
+ PackageSpecification.fromString(
+ defaultRepoName,
+ "//fruits/...",
+ /*allowPublicPrivate=*/ true,
+ /*repoRootMeansCurrentRepo=*/ true));
} catch (Exception e) {
// Can't throw from Runnable, but this will cause the test to timeout
// when the consumer can't take the object.
diff --git a/src/test/java/com/google/devtools/build/lib/packages/PackageGroupTest.java b/src/test/java/com/google/devtools/build/lib/packages/PackageGroupTest.java
index b6baeaa..f91a849 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/PackageGroupTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/PackageGroupTest.java
@@ -218,7 +218,133 @@
}
@Test
- public void testNegative_everything() throws Exception {
+ public void testEverythingSpecificationWorks() throws Exception {
+ setBuildLanguageOptions("--incompatible_package_group_has_public_syntax=true");
+
+ scratch.file(
+ "fruits/BUILD", //
+ "package_group(",
+ " name = 'mango',",
+ " packages = ['public'],",
+ ")");
+ PackageGroup grp = getPackageGroup("fruits", "mango");
+
+ // Assert that we're using the right package spec.
+ assertThat(grp.getContainedPackages(/*includeDoubleSlash=*/ true)).containsExactly("public");
+ // Assert that this package spec contains packages from both inside and outside the main repo.
+ assertThat(grp.contains(pkgId("pkg"))).isTrue();
+ assertThat(grp.contains(pkgId("somerepo", "pkg"))).isTrue();
+ }
+
+ @Test
+ public void testNothingSpecificationWorks() throws Exception {
+ setBuildLanguageOptions("--incompatible_package_group_has_public_syntax=true");
+
+ scratch.file(
+ "fruits/BUILD", //
+ "package_group(",
+ " name = 'mango',",
+ " packages = ['private'],",
+ ")");
+ PackageGroup grp = getPackageGroup("fruits", "mango");
+
+ // Assert that we're using the right package spec.
+ assertThat(grp.getContainedPackages(/*includeDoubleSlash=*/ true)).containsExactly("private");
+ assertThat(grp.contains(pkgId("anything"))).isFalse();
+ }
+
+ @Test
+ public void testPublicPrivateAreNotAccessibleWithoutFlag() throws Exception {
+ setBuildLanguageOptions("--incompatible_package_group_has_public_syntax=false");
+
+ scratch.file(
+ "foo/BUILD", //
+ "package_group(",
+ " name = 'grp1',",
+ " packages = ['public'],",
+ ")");
+ scratch.file(
+ "bar/BUILD", //
+ "package_group(",
+ " name = 'grp2',",
+ " packages = ['private'],",
+ ")");
+
+ reporter.removeHandler(failFastHandler);
+ getPackageGroup("foo", "grp1");
+ assertContainsEvent(
+ "Use of \"public\" package specification requires enabling"
+ + " --incompatible_package_group_has_public_syntax");
+ getPackageGroup("bar", "grp2");
+ assertContainsEvent(
+ "Use of \"private\" package specification requires enabling"
+ + " --incompatible_package_group_has_public_syntax");
+ }
+
+ @Test
+ public void testRepoRootSubpackagesIsPublic_withoutFlag() throws Exception {
+ setBuildLanguageOptions("--incompatible_fix_package_group_reporoot_syntax=false");
+
+ scratch.file(
+ "fruits/BUILD", //
+ "package_group(",
+ " name = 'mango',",
+ " packages = ['//...'],",
+ ")");
+ PackageGroup grp = getPackageGroup("fruits", "mango");
+
+ // Use includeDoubleSlash=true to make package spec stringification distinguish AllPackages from
+ // AllPackagesBeneath with empty package path.
+ assertThat(grp.getContainedPackages(/*includeDoubleSlash=*/ true))
+ // Assert that "//..." gave us AllPackages.
+ .containsExactly("public");
+ assertThat(grp.contains(pkgId("pkg"))).isTrue();
+ assertThat(grp.contains(pkgId("somerepo", "pkg"))).isTrue();
+ }
+
+ @Test
+ public void testRepoRootSubpackagesIsNotPublic_withFlag() throws Exception {
+ setBuildLanguageOptions(
+ "--incompatible_package_group_has_public_syntax=true",
+ "--incompatible_fix_package_group_reporoot_syntax=true");
+
+ scratch.file(
+ "fruits/BUILD", //
+ "package_group(",
+ " name = 'mango',",
+ " packages = ['//...'],",
+ ")");
+ PackageGroup grp = getPackageGroup("fruits", "mango");
+
+ // Use includeDoubleSlash=true to make package spec stringification distinguish AllPackages from
+ // AllPackagesBeneath with empty package path.
+ assertThat(grp.getContainedPackages(/*includeDoubleSlash=*/ true))
+ // Assert that "//..." gave us AllPackagesBeneath.
+ .containsExactly("//...");
+ assertThat(grp.contains(pkgId("pkg"))).isTrue();
+ assertThat(grp.contains(pkgId("somerepo", "pkg"))).isFalse();
+ }
+
+ @Test
+ public void testCannotUseNewRepoRootSyntaxWithoutPublicSyntax() throws Exception {
+ setBuildLanguageOptions(
+ "--incompatible_package_group_has_public_syntax=false",
+ "--incompatible_fix_package_group_reporoot_syntax=true");
+
+ scratch.file(
+ "fruits/BUILD", //
+ "package_group(",
+ " name = 'mango',",
+ " packages = ['//something'],",
+ ")");
+
+ reporter.removeHandler(failFastHandler);
+ getPackageGroup("fruits", "mango");
+ assertContainsEvent("Cannot use new \"//...\" meaning without allowing new \"public\" syntax.");
+ }
+
+ @Test
+ public void testNegative_repoRootSubpackages() throws Exception {
scratch.file(
"test/BUILD",
"package_group(",
@@ -236,20 +362,35 @@
}
@Test
- public void testEverythingSpecificationWorks() throws Exception {
- scratch.file(
- "fruits/BUILD", //
- "package_group(",
- " name = 'mango',",
- " packages = ['//...'],",
- ")");
- PackageGroup grp = getPackageGroup("fruits", "mango");
+ public void testNegative_public() throws Exception {
+ setBuildLanguageOptions("--incompatible_package_group_has_public_syntax=true");
- // Assert that we're actually using the "everything" package specification, and assert that this
- // means we include packages from the main repo but also from other repos.
- assertThat(grp.getContainedPackages(/*includeDoubleSlash=*/ false)).containsExactly("//...");
- assertThat(grp.contains(pkgId("pkg"))).isTrue();
- assertThat(grp.contains(pkgId("somerepo", "pkg"))).isTrue();
+ scratch.file(
+ "fruits/BUILD",
+ "package_group(",
+ " name = 'apple',",
+ " packages = ['-public'],",
+ ")");
+
+ reporter.removeHandler(failFastHandler);
+ getPackageGroup("fruits", "apple");
+ assertContainsEvent("Cannot negate \"public\" package specification");
+ }
+
+ @Test
+ public void testNegative_private() throws Exception {
+ setBuildLanguageOptions("--incompatible_package_group_has_public_syntax=true");
+
+ scratch.file(
+ "fruits/BUILD",
+ "package_group(",
+ " name = 'apple',",
+ " packages = ['-private'],",
+ ")");
+
+ reporter.removeHandler(failFastHandler);
+ getPackageGroup("fruits", "apple");
+ assertContainsEvent("Cannot negate \"private\" package specification");
}
@Test
@@ -275,16 +416,18 @@
PackageGroupContents contents =
PackageGroupContents.create(
ImmutableList.of(
- PackageSpecification.fromString(main, "//a"),
- PackageSpecification.fromString(main, "//a/b/..."),
- PackageSpecification.fromString(main, "-//c"),
- PackageSpecification.fromString(main, "-//c/d/..."),
- PackageSpecification.fromString(main, "//..."),
- PackageSpecification.fromString(main, "-//..."),
- PackageSpecification.fromString(main, "//"),
- PackageSpecification.fromString(main, "-//"),
- PackageSpecification.fromString(other, "//z"),
- PackageSpecification.fromString(other, "//...")));
+ pkgSpec(main, "//a"),
+ pkgSpec(main, "//a/b/..."),
+ pkgSpec(main, "-//c"),
+ pkgSpec(main, "-//c/d/..."),
+ pkgSpec(main, "//..."),
+ pkgSpec(main, "-//..."),
+ pkgSpec(main, "//"),
+ pkgSpec(main, "-//"),
+ pkgSpec(other, "//z"),
+ pkgSpec(other, "//..."),
+ pkgSpec(main, "public"),
+ pkgSpec(main, "private")));
assertThat(
contents.streamPackageStrings(/*includeDoubleSlash=*/ false).collect(toImmutableList()))
.containsExactly(
@@ -297,8 +440,9 @@
"",
"-",
"@other//z",
- // TODO(#16323): When parsing is fixed, change this "//..." to "@other//...".
- "//...");
+ "@other//...",
+ "//...", // legacy syntax for public
+ "private");
assertThat(
contents.streamPackageStrings(/*includeDoubleSlash=*/ true).collect(toImmutableList()))
.containsExactly(
@@ -311,8 +455,9 @@
"//",
"-//",
"@other//z",
- // TODO(#16323): When parsing is fixed, change this "//..." to "@other//...".
- "//...");
+ "@other//...",
+ "public",
+ "private");
assertThat(contents.streamPackageStringsWithoutRepository().collect(toImmutableList()))
.containsExactly(
"//a",
@@ -324,7 +469,15 @@
"//",
"-//",
"//z",
- "//...");
+ "//...",
+ "public",
+ "private");
+ }
+
+ /** Convenience method for obtaining a PackageSpecification. */
+ private PackageSpecification pkgSpec(RepositoryName repository, String spec) throws Exception {
+ return PackageSpecification.fromString(
+ repository, spec, /*allowPublicPrivate=*/ true, /*repoRootMeansCurrentRepo=*/ true);
}
/** Convenience method for obtaining a PackageIdentifier. */
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/BzlLoadFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/BzlLoadFunctionTest.java
index f0ac79e..c6c712a 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/BzlLoadFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/BzlLoadFunctionTest.java
@@ -601,6 +601,54 @@
}
@Test
+ public void testBzlVisibility_publicListElement() throws Exception {
+ setBuildLanguageOptions(
+ "--experimental_bzl_visibility=true", "--experimental_bzl_visibility_allowlist=everyone");
+
+ scratch.file("a/BUILD");
+ scratch.file(
+ "a/foo.bzl", //
+ "load(\"//b:bar.bzl\", \"x\")");
+ scratch.file("b/BUILD");
+ scratch.file(
+ "b/bar.bzl", //
+ // Tests "public" as a list item, and alongside other list items.
+ "visibility([\"public\", \"//c\"])",
+ "x = 1");
+
+ checkSuccessfulLookup("//a:foo.bzl");
+ assertNoEvents();
+ }
+
+ @Test
+ public void testBzlVisibility_privateListElement() throws Exception {
+ setBuildLanguageOptions(
+ "--experimental_bzl_visibility=true", "--experimental_bzl_visibility_allowlist=everyone");
+
+ scratch.file("a1/BUILD");
+ scratch.file(
+ "a1/foo.bzl", //
+ "load(\"//b:bar.bzl\", \"x\")");
+ scratch.file("a2/BUILD");
+ scratch.file(
+ "a2/foo.bzl", //
+ "load(\"//b:bar.bzl\", \"x\")");
+ scratch.file("b/BUILD");
+ scratch.file(
+ "b/bar.bzl", //
+ // Tests "private" as a list item, and alongside other list items.
+ "visibility([\"private\", \"//a1\"])",
+ "x = 1");
+
+ checkSuccessfulLookup("//a1:foo.bzl");
+ assertNoEvents();
+ reporter.removeHandler(failFastHandler);
+ checkFailingLookup(
+ "//a2:foo.bzl", "module //a2:foo.bzl contains .bzl load-visibility violations");
+ assertContainsEvent("Starlark file //b:bar.bzl is not visible for loading from package //a2.");
+ }
+
+ @Test
public void testBzlVisibility_failureInDependency() throws Exception {
setBuildLanguageOptions(
"--experimental_bzl_visibility=true", "--experimental_bzl_visibility_allowlist=everyone");
@@ -743,6 +791,65 @@
"Starlark file @repo//lib:bar.bzl is not visible for loading from package //pkg.");
}
+ // TODO(#16365): This test case can be deleted once --incompatible_package_group_has_public_syntax
+ // is deleted (not just flipped).
+ @Test
+ public void testBzlVisibility_canUsePublicPrivate_regardlessOfFlag() throws Exception {
+ setBuildLanguageOptions(
+ "--experimental_bzl_visibility=true",
+ "--experimental_bzl_visibility_allowlist=everyone",
+ // Test that we can use "public" and "private" visibility for .bzl files even when the
+ // incompatible flag is disabled.
+ "--incompatible_package_group_has_public_syntax=false");
+
+ scratch.file("a/BUILD");
+ scratch.file(
+ "a/foo1.bzl", //
+ "visibility(\"public\")");
+ scratch.file(
+ "a/foo2.bzl", //
+ "visibility(\"private\")");
+
+ checkSuccessfulLookup("//a:foo1.bzl");
+ checkSuccessfulLookup("//a:foo2.bzl");
+ assertNoEvents();
+ }
+
+ // TODO(#16324): Once --incompatible_fix_package_group_reporoot_syntax is deleted (not just
+ // flipped), this test case will be redundant with tests for //... in PackageGroupTest. At that
+ // point we'll just delete this test case.
+ @Test
+ public void testBzlVisibility_repoRootSubpackagesIsNotPublic_regardlessOfFlag() throws Exception {
+ setBuildLanguageOptions(
+ "--experimental_bzl_visibility=true",
+ "--experimental_bzl_visibility_allowlist=everyone",
+ // Test that we get the fixed behavior even when the incompatible flag is disabled.
+ "--incompatible_fix_package_group_reporoot_syntax=false");
+
+ scratch.overwriteFile(
+ "WORKSPACE", //
+ "local_repository(",
+ " name = 'repo',",
+ " path = 'repo'",
+ ")");
+ scratch.file("repo/WORKSPACE");
+ scratch.file("repo/a/BUILD");
+ scratch.file(
+ "repo/a/foo.bzl", //
+ "load(\"@//b:bar.bzl\", \"x\")");
+ scratch.file("b/BUILD");
+ scratch.file(
+ "b/bar.bzl", //
+ "visibility([\"//...\"])",
+ "x = 1");
+
+ reporter.removeHandler(failFastHandler);
+ checkFailingLookup(
+ "@repo//a:foo.bzl", "module @repo//a:foo.bzl contains .bzl load-visibility violations");
+ assertContainsEvent(
+ "Starlark file //b:bar.bzl is not visible for loading from package @repo//a.");
+ }
+
@Test
public void testBzlVisibility_disallowsSubpackagesWithoutWildcard() throws Exception {
setBuildLanguageOptions(
