Replace UnixGlob's package glob prefetching functionality with an

optional method in FileSystem.  Custom FileSystem implementations can
use this to provide their own implementation of glob prefetching.

--
MOS_MIGRATED_REVID=140736304

1 files changed, 45 insertions, 228 deletions

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 3c960df63f..58321cc327 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
@@ -24,6 +24,7 @@ import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
@@ -35,7 +36,6 @@ import com.google.devtools.build.lib.pkgcache.PackageCacheOptions;
 import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
 import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils;
 import com.google.devtools.build.lib.testutil.ManualClock;
-import com.google.devtools.build.lib.testutil.TestUtils;
 import com.google.devtools.build.lib.util.BlazeClock;
 import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
 import com.google.devtools.build.lib.vfs.Dirent;
@@ -56,10 +56,8 @@ import com.google.devtools.common.options.Options;
 import java.io.IOException;
 import java.util.Collection;
 import java.util.Map;
+import java.util.Set;
 import java.util.UUID;
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicBoolean;
 import javax.annotation.Nullable;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -760,255 +758,74 @@ public class PackageFunctionTest extends BuildViewTestCase {
     }
   }
 
-  @Test
-  public void testGlobsHappenInParallel() throws Exception {
-    scratch.file(
-        "foo/BUILD",
-        "load('//foo:my_library.bzl', 'my_library')",
-        "[sh_library(name = x + '-matched') for x in glob(['bar/*'], exclude_directories = 0)]",
-        "cc_library(name = 'cc', srcs = glob(['cc/*']))",
-        "my_library(name = 'my', srcs = glob(['sh/*']))");
-    scratch.file(
-        "foo/my_library.bzl",
-        "def my_library(name = None, srcs = []):",
-        "  native.sh_library(name = name, srcs = srcs, deps = native.glob(['inner/*']))");
-    scratch.file("foo/bar/1");
-    Path barPath = scratch.file("foo/bar/2").getParentDirectory();
-    Path ccPath = scratch.file("foo/cc/src.file").getParentDirectory();
-    Path shPath = scratch.dir("foo/sh");
-    Path innerPath = scratch.dir("foo/inner");
-    PackageCacheOptions packageCacheOptions = Options.getDefaults(PackageCacheOptions.class);
-    packageCacheOptions.defaultVisibility = ConstantRuleVisibility.PUBLIC;
-    packageCacheOptions.showLoadingProgress = true;
-    packageCacheOptions.globbingThreads = 7;
-    packageCacheOptions.maxDirectoriesToEagerlyVisitInGlobbing = 10;
-    getSkyframeExecutor()
-        .preparePackageLoading(
-            new PathPackageLocator(outputBase, ImmutableList.of(rootDirectory)),
-            packageCacheOptions,
-            "",
-            UUID.randomUUID(),
-            ImmutableMap.<String, String>of(),
-            new TimestampGranularityMonitor(BlazeClock.instance()));
+  private static class CustomInMemoryFs extends InMemoryFileSystem {
+    private abstract static class FileStatusOrException {
+      abstract FileStatus get() throws IOException;
 
-    SkyKey skyKey = PackageValue.key(PackageIdentifier.parse("@//foo"));
-    final CountDownLatch allDirsRequested = new CountDownLatch(4);
-    Listener synchronizeListener =
-        new Listener() {
-          @Override
-          public Object accept(Path path, FileOp op, Order order) throws IOException {
-            if (op == FileOp.READDIR && order == Order.BEFORE) {
-              allDirsRequested.countDown();
-              try {
-                assertThat(
-                        allDirsRequested.await(
-                            TestUtils.WAIT_TIMEOUT_MILLISECONDS, TimeUnit.MILLISECONDS))
-                    .isTrue();
-              } catch (InterruptedException e) {
-                throw new IllegalStateException(e);
-              }
-            }
-            return NO_RESULT_MARKER;
-          }
-        };
-    fs.setCustomOverride(barPath, synchronizeListener);
-    fs.setCustomOverride(ccPath, synchronizeListener);
-    fs.setCustomOverride(shPath, synchronizeListener);
-    fs.setCustomOverride(innerPath, synchronizeListener);
-    PackageValue value = validPackage(skyKey);
-    assertFalse(value.getPackage().containsErrors());
-    assertThat(value.getPackage().getTarget("bar/1-matched").getName()).isEqualTo("bar/1-matched");
-    assertThat(value.getPackage().getTarget("cc/src.file")).isNotNull();
-    assertThat(
-            (Iterable<?>)
-                value
-                    .getPackage()
-                    .getTarget("my")
-                    .getAssociatedRule()
-                    .getAttributeContainer()
-                    .getAttr("srcs"))
-        .isEmpty();
-  }
+      private static class ExceptionImpl extends FileStatusOrException {
+        private final IOException exn;
 
-  @Test
-  public void testGlobsDontHappenInParallel() throws Exception {
-    scratch.file(
-        "foo/BUILD",
-        "load('//foo:my_library.bzl', 'my_library')",
-        "[sh_library(name = x + '-matched') for x in glob(['bar/*'], exclude_directories = 0)]",
-        "cc_library(name = 'cc', srcs = glob(['cc/*']))",
-        "my_library(name = 'my', srcs = glob(['sh/*']))");
-    scratch.file(
-        "foo/my_library.bzl",
-        "def my_library(name = None, srcs = []):",
-        "  native.sh_library(name = name, srcs = srcs, deps = native.glob(['inner/*']))");
-    scratch.file("foo/bar/1");
-    Path barPath = scratch.file("foo/bar/2").getParentDirectory();
-    Path ccPath = scratch.file("foo/cc/src.file").getParentDirectory();
-    Path shPath = scratch.dir("foo/sh");
-    Path innerPath = scratch.dir("foo/inner");
-    PackageCacheOptions packageCacheOptions = Options.getDefaults(PackageCacheOptions.class);
-    packageCacheOptions.defaultVisibility = ConstantRuleVisibility.PUBLIC;
-    packageCacheOptions.showLoadingProgress = true;
-    packageCacheOptions.globbingThreads = 7;
-    packageCacheOptions.maxDirectoriesToEagerlyVisitInGlobbing = -1;
-    getSkyframeExecutor()
-        .preparePackageLoading(
-            new PathPackageLocator(outputBase, ImmutableList.of(rootDirectory)),
-            packageCacheOptions,
-            "",
-            UUID.randomUUID(),
-            ImmutableMap.<String, String>of(),
-            new TimestampGranularityMonitor(BlazeClock.instance()));
+        private ExceptionImpl(IOException exn) {
+          this.exn = exn;
+        }
 
-    SkyKey skyKey = PackageValue.key(PackageIdentifier.parse("@//foo"));
-    final AtomicBoolean atLeastOneUnfinishedRequest = new AtomicBoolean(false);
-    final CountDownLatch allDirsRequested = new CountDownLatch(4);
-    Listener synchronizeListener =
-        new Listener() {
-          @Override
-          public Object accept(Path path, FileOp op, Order order) throws IOException {
-            if (op == FileOp.READDIR && order == Order.BEFORE) {
-              allDirsRequested.countDown();
-              try {
-                if (!allDirsRequested.await(1, TimeUnit.SECONDS)) {
-                  atLeastOneUnfinishedRequest.set(true);
-                }
-              } catch (InterruptedException e) {
-                throw new IllegalStateException(e);
-              }
-            }
-            return NO_RESULT_MARKER;
-          }
-        };
-    fs.setCustomOverride(barPath, synchronizeListener);
-    fs.setCustomOverride(ccPath, synchronizeListener);
-    fs.setCustomOverride(shPath, synchronizeListener);
-    fs.setCustomOverride(innerPath, synchronizeListener);
-    PackageValue value = validPackage(skyKey);
-    assertFalse(value.getPackage().containsErrors());
-    assertThat(value.getPackage().getTarget("bar/1-matched").getName()).isEqualTo("bar/1-matched");
-    assertThat(value.getPackage().getTarget("cc/src.file")).isNotNull();
-    assertThat(
-            (Iterable<?>)
-                value
-                    .getPackage()
-                    .getTarget("my")
-                    .getAssociatedRule()
-                    .getAttributeContainer()
-                    .getAttr("srcs"))
-        .isEmpty();
-    assertThat(atLeastOneUnfinishedRequest.get()).isTrue();
-  }
+        @Override
+        FileStatus get() throws IOException {
+          throw exn;
+        }
+      }
 
-  private static class CustomInMemoryFs extends InMemoryFileSystem {
-    private final Map<Path, Listener> customOverrides = Maps.newHashMap();
+      private static class FileStatusImpl extends FileStatusOrException {
 
-    public CustomInMemoryFs(ManualClock manualClock) {
-      super(manualClock);
+        @Nullable
+        private final FileStatus fileStatus;
+
+        private  FileStatusImpl(@Nullable FileStatus fileStatus) {
+          this.fileStatus = fileStatus;
+        }
+
+        @Override
+        @Nullable
+        FileStatus get() {
+          return fileStatus;
+        }
+      }
     }
 
-    public void stubStat(final Path targetPath, @Nullable final FileStatus stubbedResult) {
-      setCustomOverride(
-          targetPath,
-          new Listener() {
-            @Override
-            public Object accept(Path path, FileOp op, Order order) {
-              if (targetPath.equals(path) && op == FileOp.STAT && order == Order.BEFORE) {
-                return stubbedResult;
-              } else {
-                return NO_RESULT_MARKER;
-              }
-            }
-          });
+    private Map<Path, FileStatusOrException> stubbedStats = Maps.newHashMap();
+    private Set<Path> makeUnreadableAfterReaddir = Sets.newHashSet();
+
+    public CustomInMemoryFs(ManualClock manualClock) {
+      super(manualClock);
     }
 
-    public void stubStatError(final Path targetPath, final IOException stubbedResult) {
-      setCustomOverride(
-          targetPath,
-          new Listener() {
-            @Override
-            public Object accept(Path path, FileOp op, Order order) throws IOException {
-              if (targetPath.equals(path) && op == FileOp.STAT && order == Order.BEFORE) {
-                throw stubbedResult;
-              } else {
-                return NO_RESULT_MARKER;
-              }
-            }
-          });
+    public void stubStat(Path path, @Nullable FileStatus stubbedResult) {
+      stubbedStats.put(path, new FileStatusOrException.FileStatusImpl(stubbedResult));
     }
 
-    void setCustomOverride(Path path, Listener listener) {
-      customOverrides.put(path, listener);
+    public void stubStatError(Path path, IOException stubbedResult) {
+      stubbedStats.put(path, new FileStatusOrException.ExceptionImpl(stubbedResult));
     }
 
     @Override
     public FileStatus stat(Path path, boolean followSymlinks) throws IOException {
-      Listener listener = customOverrides.get(path);
-      if (listener != null) {
-        Object status = listener.accept(path, FileOp.STAT, Order.BEFORE);
-        if (status != NO_RESULT_MARKER) {
-          return (FileStatus) status;
-        }
-      }
-      FileStatus fileStatus = super.stat(path, followSymlinks);
-      if (listener != null) {
-        Object status = listener.accept(path, FileOp.STAT, Order.AFTER);
-        if (status != NO_RESULT_MARKER) {
-          return (FileStatus) status;
-        }
+      if (stubbedStats.containsKey(path)) {
+        return stubbedStats.get(path).get();
       }
-      return fileStatus;
+      return super.stat(path, followSymlinks);
     }
 
-    public void scheduleMakeUnreadableAfterReaddir(final Path targetPath) {
-      setCustomOverride(
-          targetPath,
-          new Listener() {
-            @Override
-            public Object accept(Path path, FileOp op, Order order) throws IOException {
-              if (targetPath.equals(path) && op == FileOp.READDIR && order == Order.AFTER) {
-                targetPath.setReadable(false);
-              }
-              return NO_RESULT_MARKER;
-            }
-          });
+    public void scheduleMakeUnreadableAfterReaddir(Path path) {
+      makeUnreadableAfterReaddir.add(path);
     }
 
-    @SuppressWarnings("unchecked")
     @Override
     public Collection<Dirent> readdir(Path path, boolean followSymlinks) throws IOException {
-      Listener listener = customOverrides.get(path);
-      if (listener != null) {
-        Object status = listener.accept(path, FileOp.READDIR, Order.BEFORE);
-        if (status != NO_RESULT_MARKER) {
-          return (Collection<Dirent>) status;
-        }
-      }
       Collection<Dirent> result = super.readdir(path, followSymlinks);
-      if (listener != null) {
-        Object status = listener.accept(path, FileOp.READDIR, Order.AFTER);
-        if (status != NO_RESULT_MARKER) {
-          return (Collection<Dirent>) status;
-        }
+      if (makeUnreadableAfterReaddir.contains(path)) {
+        path.setReadable(false);
       }
       return result;
     }
   }
-
-  private static final Object NO_RESULT_MARKER = new Object();
-
-  private enum Order {
-    BEFORE,
-    AFTER
-  }
-
-  private enum FileOp {
-    STAT,
-    READDIR
-  }
-
-  private interface Listener {
-    Object accept(Path path, FileOp op, Order order) throws IOException;
-  }
 }