Implement getDirectoryEntries and readdir for RemoteActionFileSystem.

So spawns can read content of directires within action exuection.

Part of bazelbuild#16556.

PiperOrigin-RevId: 486918859
Change-Id: Ida86e4c927093d26f7f96d2f0c2aa0d1d74cc8a4

Author: coeuvre

src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java

@@ -22,6 +22,7 @@
 import static com.google.common.collect.Streams.stream;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.actions.ActionInputMap;
 import com.google.devtools.build.lib.actions.Artifact;
@@ -50,6 +51,8 @@
 import java.io.OutputStream;
 import java.nio.channels.ReadableByteChannel;
 import java.util.Collection;
+import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
 import javax.annotation.Nullable;
 
@@ -604,6 +607,73 @@ public boolean createDirectory(PathFragment path) throws IOException {
     return created;
   }
 
+  @Override
+  protected ImmutableList<String> getDirectoryEntries(PathFragment path) throws IOException {
+    HashSet<String> entries = new HashSet<>();
+
+    boolean ignoredNotFoundInRemote = false;
+    if (isOutput(path)) {
+      try {
+        delegateFs.getPath(path).getDirectoryEntries().stream()
+            .map(Path::getBaseName)
+            .forEach(entries::add);
+        ignoredNotFoundInRemote = true;
+      } catch (FileNotFoundException ignored) {
+        // Intentionally ignored
+      }
+    }
+
+    try {
+      remoteOutputTree.getPath(path).getDirectoryEntries().stream()
+          .map(Path::getBaseName)
+          .forEach(entries::add);
+    } catch (FileNotFoundException e) {
+      if (!ignoredNotFoundInRemote) {
+        throw e;
+      }
+    }
+
+    // sort entries to get a deterministic order.
+    return ImmutableList.sortedCopyOf(entries);
+  }
+
+  @Override
+  protected Collection<Dirent> readdir(PathFragment path, boolean followSymlinks)
+      throws IOException {
+    HashMap<String, Dirent> entries = new HashMap<>();
+
+    boolean ignoredNotFoundInRemote = false;
+    if (isOutput(path)) {
+      try {
+        for (var entry :
+            delegateFs
+                .getPath(path)
+                .readdir(followSymlinks ? Symlinks.FOLLOW : Symlinks.NOFOLLOW)) {
+          entries.put(entry.getName(), entry);
+        }
+        ignoredNotFoundInRemote = true;
+      } catch (FileNotFoundException ignored) {
+        // Intentionally ignored
+      }
+    }
+
+    try {
+      for (var entry :
+          remoteOutputTree
+              .getPath(path)
+              .readdir(followSymlinks ? Symlinks.FOLLOW : Symlinks.NOFOLLOW)) {
+        entries.put(entry.getName(), entry);
+      }
+    } catch (FileNotFoundException e) {
+      if (!ignoredNotFoundInRemote) {
+        throw e;
+      }
+    }
+
+    // sort entries to get a deterministic order.
+    return ImmutableList.sortedCopyOf(entries.values());
+  }
+
   /*
    * -------------------- TODO(buchgr): Not yet implemented --------------------
    *
@@ -613,23 +683,12 @@ public boolean createDirectory(PathFragment path) throws IOException {
    * sure to fully implement this file system.
    */
 
-  @Override
-  protected Collection<String> getDirectoryEntries(PathFragment path) throws IOException {
-    return super.getDirectoryEntries(path);
-  }
-
   @Override
   protected void createFSDependentHardLink(PathFragment linkPath, PathFragment originalPath)
       throws IOException {
     super.createFSDependentHardLink(linkPath, originalPath);
   }
 
-  @Override
-  protected Collection<Dirent> readdir(PathFragment path, boolean followSymlinks)
-      throws IOException {
-    return super.readdir(path, followSymlinks);
-  }
-
   @Override
   protected void createHardLink(PathFragment linkPath, PathFragment originalPath)
       throws IOException {

src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTestBase.java

@@ -13,14 +13,18 @@
 // limitations under the License.
 package com.google.devtools.build.lib.remote;
 
+import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertThrows;
 
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.ActionInputMap;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.vfs.Dirent;
 import com.google.devtools.build.lib.vfs.FileSystem;
+import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.vfs.Symlinks;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import org.junit.Test;
@@ -209,4 +213,119 @@ public void createDirectory_createLocallyAndRemotely() throws Exception {
     assertThat(getRemoteFileSystem(actionFs).getPath(path).isDirectory()).isTrue();
     assertThat(getLocalFileSystem(actionFs).getPath(path).isDirectory()).isTrue();
   }
+
+  interface DirectoryEntriesProvider {
+    ImmutableList<String> getDirectoryEntries(Path path) throws IOException;
+  }
+
+  private void readdirNonEmptyLocalDirectoryReadFromLocal(
+      DirectoryEntriesProvider directoryEntriesProvider) throws IOException {
+    FileSystem actionFs = createActionFileSystem();
+    PathFragment dir = getOutputPath("parent/dir");
+    actionFs.getPath(dir).createDirectoryAndParents();
+    writeLocalFile(actionFs, dir.getChild("file1"), "content1");
+    writeLocalFile(actionFs, dir.getChild("file2"), "content2");
+
+    ImmutableList<String> entries =
+        directoryEntriesProvider.getDirectoryEntries(actionFs.getPath(dir));
+
+    assertThat(entries).containsExactly("file1", "file2");
+  }
+
+  private void readdirNonEmptyInMemoryDirectoryReadFromMemory(
+      DirectoryEntriesProvider directoryEntriesProvider) throws IOException {
+    FileSystem actionFs = createActionFileSystem();
+    PathFragment dir = getOutputPath("parent/dir");
+    actionFs.getPath(dir).createDirectoryAndParents();
+    injectRemoteFile(actionFs, dir.getChild("file1"), "content1");
+    injectRemoteFile(actionFs, dir.getChild("file2"), "content2");
+
+    ImmutableList<String> entries =
+        directoryEntriesProvider.getDirectoryEntries(actionFs.getPath(dir));
+
+    assertThat(entries).containsExactly("file1", "file2");
+  }
+
+  private void readdirNonEmptyLocalAndInMemoryDirectoryCombineThem(
+      DirectoryEntriesProvider directoryEntriesProvider) throws IOException {
+    FileSystem actionFs = createActionFileSystem();
+    PathFragment dir = getOutputPath("parent/dir");
+    actionFs.getPath(dir).createDirectoryAndParents();
+    writeLocalFile(actionFs, dir.getChild("file1"), "content1");
+    writeLocalFile(actionFs, dir.getChild("file2"), "content2");
+    injectRemoteFile(actionFs, dir.getChild("file2"), "content2inmemory");
+    injectRemoteFile(actionFs, dir.getChild("file3"), "content3");
+
+    ImmutableList<String> entries =
+        directoryEntriesProvider.getDirectoryEntries(actionFs.getPath(dir));
+
+    assertThat(entries).containsExactly("file1", "file2", "file3");
+  }
+
+  private void readdirNothingThereThrowsFileNotFound(
+      DirectoryEntriesProvider directoryEntriesProvider) throws IOException {
+    FileSystem actionFs = createActionFileSystem();
+    PathFragment dir = getOutputPath("parent/dir");
+
+    assertThrows(
+        FileNotFoundException.class,
+        () -> directoryEntriesProvider.getDirectoryEntries(actionFs.getPath(dir)));
+  }
+
+  @Test
+  public void readdir_nonEmptyLocalDirectory_readFromLocal() throws IOException {
+    readdirNonEmptyLocalDirectoryReadFromLocal(
+        path ->
+            path.readdir(Symlinks.FOLLOW).stream().map(Dirent::getName).collect(toImmutableList()));
+  }
+
+  @Test
+  public void readdir_nonEmptyInMemoryDirectory_readFromMemory() throws IOException {
+    readdirNonEmptyInMemoryDirectoryReadFromMemory(
+        path ->
+            path.readdir(Symlinks.FOLLOW).stream().map(Dirent::getName).collect(toImmutableList()));
+  }
+
+  @Test
+  public void readdir_nonEmptyLocalAndInMemoryDirectory_combineThem() throws IOException {
+    readdirNonEmptyLocalAndInMemoryDirectoryCombineThem(
+        path ->
+            path.readdir(Symlinks.FOLLOW).stream().map(Dirent::getName).collect(toImmutableList()));
+  }
+
+  @Test
+  public void readdir_nothingThere_throwsFileNotFound() throws IOException {
+    readdirNothingThereThrowsFileNotFound(
+        path ->
+            path.readdir(Symlinks.FOLLOW).stream().map(Dirent::getName).collect(toImmutableList()));
+  }
+
+  @Test
+  public void getDirectoryEntries_nonEmptyLocalDirectory_readFromLocal() throws IOException {
+    readdirNonEmptyLocalDirectoryReadFromLocal(
+        path ->
+            path.getDirectoryEntries().stream().map(Path::getBaseName).collect(toImmutableList()));
+  }
+
+  @Test
+  public void getDirectoryEntries_nonEmptyInMemoryDirectory_readFromMemory() throws IOException {
+    readdirNonEmptyInMemoryDirectoryReadFromMemory(
+        path ->
+            path.getDirectoryEntries().stream().map(Path::getBaseName).collect(toImmutableList()));
+  }
+
+  @Test
+  public void getDirectoryEntries_nonEmptyLocalAndInMemoryDirectory_combineThem()
+      throws IOException {
+    readdirNonEmptyLocalAndInMemoryDirectoryCombineThem(
+        path ->
+            path.getDirectoryEntries().stream().map(Path::getBaseName).collect(toImmutableList()));
+  }
+
+  @Test
+  public void getDirectoryEntries_nothingThere_throwsFileNotFound() throws IOException {
+    readdirNothingThereThrowsFileNotFound(
+        path ->
+            path.getDirectoryEntries().stream().map(Path::getBaseName).collect(toImmutableList()));
+  }
 }