Skip to content

Commit

Permalink
Implement getDirectoryEntries and readdir for RemoteActionFileSystem. (…
Browse files Browse the repository at this point in the history 
…#16738)

So spawns can read content of directires within action exuection.

Part of #16556.

PiperOrigin-RevId: 486918859
Change-Id: Ida86e4c927093d26f7f96d2f0c2aa0d1d74cc8a4

Co-authored-by: Googler <[email protected]>
  • Loading branch information
@ShreeM01 @coeuvre
ShreeM01 and coeuvre authored Nov 10, 2022
1 parent 0f95c8a commit 9c94ae4
Show file tree
Hide file tree
Showing 2 changed files with 189 additions and 11 deletions.
81 changes: 70 additions & 11 deletions src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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 --------------------
*
Expand All @@ -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 {
Expand Down
119 changes: 119 additions & 0 deletions src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTestBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()));
}
}

0 comments on commit 9c94ae4

Please sign in to comment.