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 bazelbuild#11261.
Work toward bazelbuild#16323.
Work toward bazelbuild#16355.
Work toward bazelbuild#16391.

RELNOTES: Added three `package_group`-related flags: `--incompatible_package_group_includes_double_slash` (bazelbuild#16391), `--incompatible_package_group_has_public_syntax` (bazelbuild#16355), and `--incompatible_fix_package_group_reporoot_syntax` (bazelbuild#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

Author: brandjon

site/en/concepts/visibility.md

@@ -45,7 +45,8 @@ target.
     *   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

src/main/java/com/google/devtools/build/docgen/templates/be/functions.vm

@@ -178,13 +178,18 @@ not themselves have any visibility protection.</p>
 
         <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 @@ not themselves have any visibility protection.</p>
         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>

src/main/java/com/google/devtools/build/lib/packages/Package.java

@@ -1652,14 +1652,26 @@ Label createLabel(String targetName) throws LabelSyntaxException {
       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);

src/main/java/com/google/devtools/build/lib/packages/PackageGroup.java

@@ -47,6 +47,8 @@ public PackageGroup(
       Package pkg,
       Collection<String> packageSpecifications,
       Collection<Label> includes,
+      boolean allowPublicPrivate,
+      boolean repoRootMeansCurrentRepo,
       EventHandler eventHandler,
       Location location) {
     this.label = label;
@@ -61,7 +63,11 @@ public PackageGroup(
       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(