Update the glob documentation to reflect a semantic change made a very long time ago where glob(['**'], exclude_directories = 0) doesn't match the package's directory. Also add tests for this behavior.

Also update Skyframe globbing to have these semantics. Any discrepancy has always been problematic, but now that we have Skyframe-hybrid globbing it's a lot more dangerous and consequential.

Alternatives considered: do this the other way around (keep the stale documentation as-is and instead update legacy globbing). This would potentially require changing existing usages from stuff like 'data = glob(["**"], exclude_directories = 0)' to 'data = [x for x in glob(["**"], exclude_directories = 0) where x != '']'. I think this is too messy, so long as there is a valid use-case for globs matching directories in the first place.

--
MOS_MIGRATED_REVID=115511504

src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java

diff --git a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
index d8d799a..d9c175b 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
@@ -590,6 +590,21 @@
     assertEvaluates(
         pkg,
         ImmutableList.of(
+            "BUILD",
+            "a.cc",
+            "foo",
+            "foo/bar.cc",
+            "foo/foo.cc",
+            "foo/wiz",
+            "foo/wiz/bam.cc",
+            "foo/wiz/bum.cc",
+            "foo/wiz/quid",
+            "foo/wiz/quid/gav.cc"),
+        "**");
+
+    assertEvaluates(
+        pkg,
+        ImmutableList.of(
             "a.cc",
             "foo/bar.cc",
             "foo/foo.cc",

src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java

diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java
index 0246f86..8fc2eb3 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java
@@ -458,7 +458,6 @@
   public void testDoubleStar() throws Exception {
     assertGlobMatches(
         "**",
-        "",
         "BUILD",
         "a1",
         "a1/b1",
@@ -487,7 +486,6 @@
   public void testDoubleDoubleStar() throws Exception {
     assertGlobMatches(
         "**/**",
-        "",
         "BUILD",
         "a1",
         "a1/b1",

src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java

diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
index 8cd8003..c985256 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
@@ -570,6 +570,46 @@
     assertThat(newValue.getPackage()).isNotSameAs(value.getPackage());
   }
 
+  // Regression test for Skyframe globbing incorrectly matching the package's directory path on
+  // 'glob(['**'], exclude_directories = 0)'. We test for this directly by triggering
+  // hybrid globbing (gives coverage for both legacy globbing and skyframe globbing).
+  @Test
+  public void testRecursiveGlobNeverMatchesPackageDirectory() throws Exception {
+    scratch.file("foo/BUILD",
+        "[sh_library(name = x + '-matched') for x in glob(['**'], exclude_directories = 0)]");
+    scratch.file("foo/bar");
+
+    getSkyframeExecutor().preparePackageLoading(
+        new PathPackageLocator(outputBase, ImmutableList.of(rootDirectory)),
+        ConstantRuleVisibility.PUBLIC, true,
+        7, "", UUID.randomUUID());
+
+    SkyKey skyKey = PackageValue.key(PackageIdentifier.parse("foo"));
+    PackageValue value = validPackage(skyKey);
+    assertFalse(value.getPackage().containsErrors());
+    assertThat(value.getPackage().getTarget("bar-matched").getName()).isEqualTo("bar-matched");
+    try {
+      value.getPackage().getTarget("-matched");
+      fail();
+    } catch (NoSuchTargetException expected) {
+    }
+
+    scratch.overwriteFile("foo/BUILD",
+        "[sh_library(name = x + '-matched') for x in glob(['**'], exclude_directories = 0)]",
+        "#some-irrelevant-comment");
+    getSkyframeExecutor().invalidateFilesUnderPathForTesting(reporter,
+        ModifiedFileSet.builder().modify(new PathFragment("foo/BUILD")).build(), rootDirectory);
+
+    value = validPackage(skyKey);
+    assertFalse(value.getPackage().containsErrors());
+    assertThat(value.getPackage().getTarget("bar-matched").getName()).isEqualTo("bar-matched");
+    try {
+      value.getPackage().getTarget("-matched");
+      fail();
+    } catch (NoSuchTargetException expected) {
+    }
+  }
+
   private static class CustomInMemoryFs extends InMemoryFileSystem {
     private abstract static class FileStatusOrException {
       abstract FileStatus get() throws IOException;