diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java
index 240b43d1e64911..35a60742ef7aa6 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java
@@ -289,26 +289,19 @@ private TreeArtifactValue constructTreeArtifactValueFromFilesystem(SpecialArtifa
TreeArtifactValue.visitTree(
treeDir,
- (parentRelativePath, type) -> {
- if (chmod && type != Dirent.Type.SYMLINK) {
+ (parentRelativePath, type, traversedSymlink) -> {
+ checkState(type == Dirent.Type.FILE || type == Dirent.Type.DIRECTORY);
+ // Set the output permissions when the execution mode requires it, unless at least one
+ // symlink was traversed on the way to this entry, as it might have led outside of the
+ // root directory.
+ if (chmod && !traversedSymlink) {
setPathPermissions(treeDir.getRelative(parentRelativePath));
}
if (type == Dirent.Type.DIRECTORY) {
return; // The final TreeArtifactValue does not contain child directories.
}
TreeFileArtifact child = TreeFileArtifact.createTreeOutput(parent, parentRelativePath);
- FileArtifactValue metadata;
- try {
- metadata = constructFileArtifactValueFromFilesystem(child);
- } catch (FileNotFoundException e) {
- String errorMessage =
- String.format(
- "Failed to resolve relative path %s inside TreeArtifact %s. "
- + "The associated file is either missing or is an invalid symlink.",
- parentRelativePath, treeDir);
- throw new IOException(errorMessage, e);
- }
-
+ FileArtifactValue metadata = constructFileArtifactValueFromFilesystem(child);
// visitTree() uses multiple threads and putChild() is not thread-safe
synchronized (tree) {
if (metadata.isRemote()) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java b/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
index 83c4af1aafe618..8b50478673e17f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
@@ -464,7 +464,7 @@ private boolean treeArtifactIsDirty(Artifact artifact, TreeArtifactValue value)
try {
TreeArtifactValue.visitTree(
path,
- (child, type) -> {
+ (child, type, traversedSymlink) -> {
if (type != Dirent.Type.DIRECTORY) {
currentChildren.add(child);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
index 55a124f98f5501..021827b9b3759f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
@@ -43,6 +43,7 @@
import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.vfs.Dirent;
+import com.google.devtools.build.lib.vfs.FileStatus;
import com.google.devtools.build.lib.vfs.IORuntimeException;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -486,20 +487,34 @@ public interface TreeArtifactVisitor {
* Called for every directory entry encountered during tree traversal, in a nondeterministic
* order.
*
- * Symlinks are not followed during traversal and are simply reported as {@link
- * Dirent.Type#SYMLINK} regardless of whether they point to a file, directory, or are dangling.
+ *
Regular files and directories are reported as {@link Dirent.Type.FILE} or {@link
+ * Dirent.Type.DIRECTORY}, respectively. Directories are traversed recursively.
*
- *
{@code type} is guaranteed never to be {@link Dirent.Type#UNKNOWN}, since if this type is
- * encountered, {@link IOException} is immediately thrown without invoking the visitor.
+ *
Symlinks that resolve to an existing file or directory are followed and reported as the
+ * regular files or directories they point to, recursively for directories. Symlinks that fail
+ * to resolve to an existing path cause an {@link IOException} to be immediately thrown without
+ * invoking the visitor. Thus, the visitor is never called with a {@link Dirent.Type.SYMLINK}
+ * type.
*
- *
If the implementation throws {@link IOException}, traversal is immediately halted and the
+ *
Special files or files whose type could not be determined, regardless of whether they are
+ * encountered directly or indirectly through symlinks, cause an {@link IOException} to be
+ * immediately thrown without invoking the visitor. Thus, the visitor is never called with a
+ * {@link Dirent.Type.UNKNOWN} type.
+ *
+ *
The {@code parentRelativePath} argument is always set to the apparent path relative to the
+ * tree directory root, without resolving any intervening symlinks. The {@code traversedSymlink}
+ * argument is true if at least one symlink was traversed on the way to the entry being
+ * reported.
+ *
+ *
If the visitor throws {@link IOException}, traversal is immediately halted and the
* exception is propagated.
*
*
This method can be called from multiple threads in parallel during a single call of {@link
* TreeArtifactVisitor#visitTree(Path, TreeArtifactVisitor)}.
*/
@ThreadSafe
- void visit(PathFragment parentRelativePath, Dirent.Type type) throws IOException;
+ void visit(PathFragment parentRelativePath, Dirent.Type type, boolean traversedSymlink)
+ throws IOException;
}
/** An {@link AbstractQueueVisitor} that visits every file in the tree artifact. */
@@ -518,7 +533,12 @@ static class Visitor extends AbstractQueueVisitor {
}
void run() throws IOException, InterruptedException {
- execute(() -> visit(PathFragment.EMPTY_FRAGMENT, Dirent.Type.DIRECTORY));
+ execute(
+ () ->
+ visit(
+ PathFragment.EMPTY_FRAGMENT,
+ Dirent.Type.DIRECTORY,
+ /* traversedSymlink= */ false));
try {
awaitQuiescence(true);
} catch (IORuntimeException e) {
@@ -526,25 +546,49 @@ void run() throws IOException, InterruptedException {
}
}
- private void visit(PathFragment parentRelativePath, Dirent.Type type) {
+ private void visit(
+ PathFragment parentRelativePath, Dirent.Type type, boolean traversedSymlink) {
try {
Path path = parentDir.getRelative(parentRelativePath);
- if (type == Dirent.Type.UNKNOWN) {
- throw new IOException("Could not determine type of file for " + path.getPathString());
- }
-
if (type == Dirent.Type.SYMLINK) {
checkSymlink(parentRelativePath.getParentDirectory(), path);
+
+ traversedSymlink = true;
+
+ FileStatus statFollow = path.statIfFound(Symlinks.FOLLOW);
+
+ if (statFollow == null) {
+ throw new IOException(
+ String.format(
+ "Child %s of tree artifact %s is a dangling symbolic link",
+ parentRelativePath, parentDir));
+ }
+
+ if (statFollow.isFile() && !statFollow.isSpecialFile()) {
+ type = Dirent.Type.FILE;
+ } else if (statFollow.isDirectory()) {
+ type = Dirent.Type.DIRECTORY;
+ } else {
+ type = Dirent.Type.UNKNOWN;
+ }
+ }
+
+ if (type == Dirent.Type.UNKNOWN) {
+ throw new IOException(
+ String.format(
+ "Child %s of tree artifact %s has an unsupported type",
+ parentRelativePath, parentDir));
}
- visitor.visit(parentRelativePath, type);
+ visitor.visit(parentRelativePath, type, traversedSymlink);
if (type == Dirent.Type.DIRECTORY) {
for (Dirent dirent : path.readdir(Symlinks.NOFOLLOW)) {
PathFragment childPath = parentRelativePath.getChild(dirent.getName());
Dirent.Type childType = dirent.getType();
- execute(() -> visit(childPath, childType));
+ boolean finalTraversedSymlink = traversedSymlink;
+ execute(() -> visit(childPath, childType, finalTraversedSymlink));
}
}
} catch (IOException e) {
diff --git a/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java b/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java
index b75eae3e97eee3..0ec5e14ad05270 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java
@@ -1647,7 +1647,7 @@ public TreeArtifactValue getTreeArtifactValue(SpecialArtifact output)
if (treeDir.exists()) {
TreeArtifactValue.visitTree(
treeDir,
- (parentRelativePath, type) -> {
+ (parentRelativePath, type, traversedSymlink) -> {
if (type == Dirent.Type.DIRECTORY) {
return;
}
diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java
index 02b183de0abbab..69a057038cf144 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java
@@ -957,7 +957,7 @@ protected static TreeArtifactValue createTreeArtifactValue(Artifact tree) throws
TreeArtifactValue.Builder builder = TreeArtifactValue.newBuilder((SpecialArtifact) tree);
TreeArtifactValue.visitTree(
tree.getPath(),
- (parentRelativePath, type) -> {
+ (parentRelativePath, type, traversedSymlink) -> {
if (type.equals(Dirent.Type.DIRECTORY)) {
return;
}
diff --git a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java
index d92e30f69c906d..8b4b692ef956e1 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java
@@ -676,4 +676,34 @@ public void downloadTopLevel_genruleSymlinkToOutput() throws Exception {
assertSymlink("foo-link", PathFragment.create("foo"));
assertValidOutputFile("foo-link", "hello\n");
}
+
+ @Test
+ public void remoteAction_inputTreeWithSymlinks() throws Exception {
+ setDownloadToplevel();
+ write(
+ "tree.bzl",
+ "def _impl(ctx):",
+ " d = ctx.actions.declare_directory(ctx.label.name)",
+ " ctx.actions.run_shell(",
+ " outputs = [d],",
+ " command = 'mkdir $1/dir && touch $1/file $1/dir/file && ln -s file $1/filesym && ln"
+ + " -s dir $1/dirsym',",
+ " arguments = [d.path],",
+ " )",
+ " return DefaultInfo(files = depset([d]))",
+ "tree = rule(_impl)");
+ write(
+ "BUILD",
+ "load(':tree.bzl', 'tree')",
+ "tree(name = 'tree')",
+ "genrule(name = 'gen', srcs = [':tree'], outs = ['out'], cmd = 'touch $@')");
+
+ // Populate cache
+ buildTarget("//:gen");
+
+ // Delete output, replay from cache
+ getOutputPath("tree").deleteTree();
+ getOutputPath("out").delete();
+ buildTarget("//:gen");
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
index e97ad2c7ab21d8..54fe41af6b54db 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
@@ -404,6 +404,61 @@ void run(ActionExecutionContext context) throws IOException {
checkFilePermissions(out.getPath().getChild("three").getChild("four"));
}
+ @Test
+ public void doesNotSetPermissionsAfterTraversingSymlink() throws Exception {
+ SpecialArtifact out = createTreeArtifact("output");
+
+ Path fileTarget = scratch.file("file");
+ writeFile(fileTarget, "file");
+
+ Path dirTarget = scratch.dir("dir");
+ Path dirFileTarget = dirTarget.getChild("file");
+ writeFile(dirFileTarget, "dir/file");
+
+ Action action =
+ new SimpleTestAction(out) {
+ @Override
+ void run(ActionExecutionContext context) throws IOException {
+ out.getPath().getChild("file_link").createSymbolicLink(fileTarget.asFragment());
+ out.getPath().getChild("dir_link").createSymbolicLink(dirTarget.asFragment());
+ }
+ };
+
+ registerAction(action);
+ buildArtifact(out);
+
+ assertThat(fileTarget.isWritable()).isTrue();
+ assertThat(dirTarget.isWritable()).isTrue();
+ assertThat(dirFileTarget.isWritable()).isTrue();
+ }
+
+ @Test
+ public void symlinkLoopRejected() throws Exception {
+ // Failure expected
+ EventCollector eventCollector = new EventCollector(EventKind.ERROR);
+ reporter.removeHandler(failFastHandler);
+ reporter.addHandler(eventCollector);
+
+ SpecialArtifact out = createTreeArtifact("output");
+
+ Action action =
+ new SimpleTestAction(out) {
+ @Override
+ void run(ActionExecutionContext context) throws IOException {
+ writeFile(out.getPath().getRelative("dir/file"), "contents");
+ out.getPath().getRelative("dir/sym").createSymbolicLink(PathFragment.create("../dir"));
+ }
+ };
+
+ registerAction(action);
+ assertThrows(BuildFailedException.class, () -> buildArtifact(out));
+
+ ImmutableList errors = ImmutableList.copyOf(eventCollector);
+ assertThat(errors).hasSize(2);
+ assertThat(errors.get(0).getMessage()).contains("Too many levels of symbolic links");
+ assertThat(errors.get(1).getMessage()).contains("not all outputs were created or valid");
+ }
+
@Test
public void validRelativeSymlinkAccepted() throws Exception {
SpecialArtifact out = createTreeArtifact("output");
@@ -448,7 +503,9 @@ void run(ActionExecutionContext actionExecutionContext) throws IOException {
List errors = ImmutableList.copyOf(eventCollector);
assertThat(errors).hasSize(2);
- assertThat(errors.get(0).getMessage()).contains("Failed to resolve relative path links/link");
+ assertThat(errors.get(0).getMessage())
+ .contains(
+ "Child links/link of tree artifact " + out.getPath() + " is a dangling symbolic link");
assertThat(errors.get(1).getMessage()).contains("not all outputs were created or valid");
}
@@ -477,7 +534,9 @@ void run(ActionExecutionContext actionExecutionContext) throws IOException {
List errors = ImmutableList.copyOf(eventCollector);
assertThat(errors).hasSize(2);
- assertThat(errors.get(0).getMessage()).contains("Failed to resolve relative path links/link");
+ assertThat(errors.get(0).getMessage())
+ .contains(
+ "Child links/link of tree artifact " + out.getPath() + " is a dangling symbolic link");
assertThat(errors.get(1).getMessage()).contains("not all outputs were created or valid");
}
@@ -852,17 +911,18 @@ public void emptyInputAndOutputTreeArtifactInActionTemplate() throws Exception {
assertThat(artifact2.getPath().getDirectoryEntries()).isEmpty();
}
- // This happens in the wild. See https://github.com/bazelbuild/bazel/issues/11813.
@Test
- public void treeArtifactContainsSymlinkToDirectory() throws Exception {
+ public void treeArtifactWithSymlinkToFile() throws Exception {
SpecialArtifact treeArtifact = createTreeArtifact("tree");
registerAction(
- new SimpleTestAction(/*output=*/ treeArtifact) {
+ new SimpleTestAction(/* output= */ treeArtifact) {
@Override
void run(ActionExecutionContext context) throws IOException {
- PathFragment subdir = PathFragment.create("subdir");
- touchFile(treeArtifact.getPath().getRelative(subdir).getRelative("file"));
- treeArtifact.getPath().getRelative("link").createSymbolicLink(subdir);
+ touchFile(treeArtifact.getPath().getRelative("subdir/file"));
+ treeArtifact
+ .getPath()
+ .getRelative("link")
+ .createSymbolicLink(PathFragment.create("subdir/file"));
}
});
@@ -874,6 +934,29 @@ void run(ActionExecutionContext context) throws IOException {
TreeFileArtifact.createTreeOutput(treeArtifact, "link"));
}
+ @Test
+ public void treeArtifactWithSymlinkToDirectory() throws Exception {
+ SpecialArtifact treeArtifact = createTreeArtifact("tree");
+ registerAction(
+ new SimpleTestAction(/* output= */ treeArtifact) {
+ @Override
+ void run(ActionExecutionContext context) throws IOException {
+ touchFile(treeArtifact.getPath().getRelative("subdir/file"));
+ treeArtifact
+ .getPath()
+ .getRelative("link")
+ .createSymbolicLink(PathFragment.create("subdir"));
+ }
+ });
+
+ TreeArtifactValue tree = buildArtifact(treeArtifact);
+
+ assertThat(tree.getChildren())
+ .containsExactly(
+ TreeFileArtifact.createTreeOutput(treeArtifact, "subdir/file"),
+ TreeFileArtifact.createTreeOutput(treeArtifact, "link/file"));
+ }
+
private abstract static class SimpleTestAction extends TestAction {
private final Button button;
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactValueTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactValueTest.java
index 07f9b3c5b69ab7..fa0b971dab3daf 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactValueTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactValueTest.java
@@ -19,6 +19,7 @@
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.spy;
+import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Maps;
import com.google.devtools.build.lib.actions.Artifact.ArchivedTreeArtifact;
@@ -32,9 +33,9 @@
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.skyframe.TreeArtifactValue.ArchivedRepresentation;
import com.google.devtools.build.lib.testutil.Scratch;
-import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.DigestHashFunction;
import com.google.devtools.build.lib.vfs.Dirent;
+import com.google.devtools.build.lib.vfs.FileStatus;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -43,9 +44,11 @@
import com.google.testing.junit.testparameterinjector.TestParameterInjector;
import java.io.IOException;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import javax.annotation.Nullable;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -58,6 +61,21 @@ public final class TreeArtifactValueTest {
ArtifactRoot.asDerivedRoot(
scratch.resolve("root"), RootType.Output, PathFragment.create("bin"));
+ @AutoValue
+ abstract static class VisitTreeArgs {
+ abstract PathFragment getParentRelativePath();
+
+ abstract Dirent.Type getType();
+
+ abstract boolean getTraversedSymlink();
+
+ static VisitTreeArgs of(
+ PathFragment parentRelativePath, Dirent.Type type, boolean traversedSymlink) {
+ return new AutoValue_TreeArtifactValueTest_VisitTreeArgs(
+ parentRelativePath, type, traversedSymlink);
+ }
+ }
+
@Test
public void createsCorrectValue() {
SpecialArtifact parent = createTreeArtifact("bin/tree");
@@ -368,46 +386,146 @@ public void visitTree_visitsEachChild() throws Exception {
scratch.file("tree/a/file2");
scratch.file("tree/a/b/file3");
scratch.resolve("tree/file_link").createSymbolicLink(PathFragment.create("file1"));
- scratch.resolve("tree/a/dir_link").createSymbolicLink(PathFragment.create("c"));
- scratch.resolve("tree/a/b/dangling_link").createSymbolicLink(PathFragment.create("?"));
- List> children = new ArrayList<>();
+ scratch.resolve("tree/a/dir_link").createSymbolicLink(PathFragment.create("b"));
+ List children = new ArrayList<>();
TreeArtifactValue.visitTree(
treeDir,
- (child, type) -> {
+ (child, type, traversedSymlink) -> {
synchronized (children) {
- children.add(Pair.of(child, type));
+ children.add(VisitTreeArgs.of(child, type, traversedSymlink));
}
});
assertThat(children)
.containsExactly(
- Pair.of(PathFragment.create(""), Dirent.Type.DIRECTORY),
- Pair.of(PathFragment.create("a"), Dirent.Type.DIRECTORY),
- Pair.of(PathFragment.create("a/b"), Dirent.Type.DIRECTORY),
- Pair.of(PathFragment.create("file1"), Dirent.Type.FILE),
- Pair.of(PathFragment.create("a/file2"), Dirent.Type.FILE),
- Pair.of(PathFragment.create("a/b/file3"), Dirent.Type.FILE),
- Pair.of(PathFragment.create("file_link"), Dirent.Type.SYMLINK),
- Pair.of(PathFragment.create("a/dir_link"), Dirent.Type.SYMLINK),
- Pair.of(PathFragment.create("a/b/dangling_link"), Dirent.Type.SYMLINK));
+ VisitTreeArgs.of(PathFragment.create(""), Dirent.Type.DIRECTORY, false),
+ VisitTreeArgs.of(PathFragment.create("a"), Dirent.Type.DIRECTORY, false),
+ VisitTreeArgs.of(PathFragment.create("a/b"), Dirent.Type.DIRECTORY, false),
+ VisitTreeArgs.of(PathFragment.create("file1"), Dirent.Type.FILE, false),
+ VisitTreeArgs.of(PathFragment.create("a/file2"), Dirent.Type.FILE, false),
+ VisitTreeArgs.of(PathFragment.create("a/b/file3"), Dirent.Type.FILE, false),
+ VisitTreeArgs.of(PathFragment.create("file_link"), Dirent.Type.FILE, true),
+ VisitTreeArgs.of(PathFragment.create("a/dir_link"), Dirent.Type.DIRECTORY, true),
+ VisitTreeArgs.of(PathFragment.create("a/dir_link/file3"), Dirent.Type.FILE, true));
}
@Test
- public void visitTree_throwsOnUnknownDirentType() {
+ public void visitTree_throwsOnDanglingSymlink() throws Exception {
+ Path treeDir = scratch.dir("tree");
+ scratch.resolve("tree/symlink").createSymbolicLink(PathFragment.create("/does_not_exist"));
+
+ Exception e =
+ assertThrows(
+ IOException.class,
+ () -> TreeArtifactValue.visitTree(treeDir, (child, type, traversedSymlink) -> {}));
+ assertThat(e)
+ .hasMessageThat()
+ .contains("Child symlink of tree artifact /tree is a dangling symbolic link");
+ }
+
+ @Test
+ public void visitTree_throwsOnSymlinkLoop() throws Exception {
+ Path treeDir = scratch.dir("tree");
+ scratch.resolve("tree/symlink").createSymbolicLink(scratch.resolve(treeDir.asFragment()));
+
+ Exception e =
+ assertThrows(
+ IOException.class,
+ () -> TreeArtifactValue.visitTree(treeDir, (child, type, traversedSymlink) -> {}));
+ assertThat(e).hasMessageThat().contains("tree/symlink");
+ assertThat(e).hasMessageThat().contains("Too many levels of symbolic links");
+ }
+
+ @Test
+ public void visitTree_throwsOnUnknownDirentType() throws Exception {
+ FileSystem fs =
+ new InMemoryFileSystem(DigestHashFunction.SHA256) {
+ @Override
+ public Collection readdir(PathFragment path, boolean followSymlinks)
+ throws IOException {
+ if (path.equals(PathFragment.create("/tree"))) {
+ return ImmutableList.of(new Dirent("unknown", Dirent.Type.UNKNOWN));
+ }
+ return super.readdir(path, followSymlinks);
+ }
+ };
+ Path treeDir = fs.getPath("/tree");
+
+ Exception e =
+ assertThrows(
+ IOException.class,
+ () -> TreeArtifactValue.visitTree(treeDir, (child, type, traversedSymlink) -> {}));
+ assertThat(e)
+ .hasMessageThat()
+ .contains("Child unknown of tree artifact /tree has an unsupported type");
+ }
+
+ @Test
+ public void visitTree_throwsOnSymlinkToSpecialFile() throws Exception {
FileSystem fs =
new InMemoryFileSystem(DigestHashFunction.SHA256) {
@Override
- public ImmutableList readdir(PathFragment path, boolean followSymlinks) {
- return ImmutableList.of(new Dirent("?", Dirent.Type.UNKNOWN));
+ @Nullable
+ public FileStatus statIfFound(PathFragment path, boolean followSymlinks)
+ throws IOException {
+ if (path.equals(PathFragment.create("/tree/sym"))) {
+ return new FileStatus() {
+ @Override
+ public boolean isFile() {
+ return true;
+ }
+
+ @Override
+ public boolean isDirectory() {
+ return false;
+ }
+
+ @Override
+ public boolean isSymbolicLink() {
+ return false;
+ }
+
+ @Override
+ public boolean isSpecialFile() {
+ return true;
+ }
+
+ @Override
+ public long getLastChangeTime() {
+ return 0;
+ }
+
+ @Override
+ public long getLastModifiedTime() {
+ return 0;
+ }
+
+ @Override
+ public long getNodeId() {
+ return 0;
+ }
+
+ @Override
+ public long getSize() {
+ return 0;
+ }
+ };
+ }
+ return super.statIfFound(path, followSymlinks);
}
};
Path treeDir = fs.getPath("/tree");
+ treeDir.createDirectory();
+ treeDir.getChild("sym").createSymbolicLink(PathFragment.create("/special"));
Exception e =
assertThrows(
- IOException.class, () -> TreeArtifactValue.visitTree(treeDir, (child, type) -> {}));
- assertThat(e).hasMessageThat().contains("Could not determine type of file for /tree/?");
+ IOException.class,
+ () -> TreeArtifactValue.visitTree(treeDir, (child, type, traversedSymlink) -> {}));
+ assertThat(e)
+ .hasMessageThat()
+ .contains("Child sym of tree artifact /tree has an unsupported type");
}
@Test
@@ -421,7 +539,7 @@ public void visitTree_propagatesIoExceptionFromVisitor() throws Exception {
() ->
TreeArtifactValue.visitTree(
treeDir,
- (child, type) -> {
+ (child, type, traversedSymlink) -> {
throw e;
}));
assertThat(thrown).isSameInstanceAs(e);
@@ -433,42 +551,47 @@ public void visitTree_pemitsUpLevelSymlinkInsideTree() throws Exception {
scratch.file("tree/file");
scratch.dir("tree/a");
scratch.resolve("tree/a/up_link").createSymbolicLink(PathFragment.create("../file"));
- List> children = new ArrayList<>();
+ List children = new ArrayList<>();
TreeArtifactValue.visitTree(
treeDir,
- (child, type) -> {
+ (child, type, traversedSymlink) -> {
synchronized (children) {
- children.add(Pair.of(child, type));
+ children.add(VisitTreeArgs.of(child, type, traversedSymlink));
}
});
assertThat(children)
.containsExactly(
- Pair.of(PathFragment.create(""), Dirent.Type.DIRECTORY),
- Pair.of(PathFragment.create("file"), Dirent.Type.FILE),
- Pair.of(PathFragment.create("a"), Dirent.Type.DIRECTORY),
- Pair.of(PathFragment.create("a/up_link"), Dirent.Type.SYMLINK));
+ VisitTreeArgs.of(PathFragment.create(""), Dirent.Type.DIRECTORY, false),
+ VisitTreeArgs.of(PathFragment.create("file"), Dirent.Type.FILE, false),
+ VisitTreeArgs.of(PathFragment.create("a"), Dirent.Type.DIRECTORY, false),
+ VisitTreeArgs.of(PathFragment.create("a/up_link"), Dirent.Type.FILE, true));
}
@Test
public void visitTree_permitsAbsoluteSymlink() throws Exception {
Path treeDir = scratch.dir("tree");
- scratch.resolve("tree/absolute_link").createSymbolicLink(PathFragment.create("/tmp"));
- List> children = new ArrayList<>();
+ Path targetFile = scratch.file("target_file");
+ Path targetDir = scratch.dir("target_dir");
+ scratch.resolve("tree/absolute_file_link").createSymbolicLink(targetFile.asFragment());
+ scratch.resolve("tree/absolute_dir_link").createSymbolicLink(targetDir.asFragment());
+ List children = new ArrayList<>();
TreeArtifactValue.visitTree(
treeDir,
- (child, type) -> {
+ (child, type, traversedSymlink) -> {
synchronized (children) {
- children.add(Pair.of(child, type));
+ children.add(VisitTreeArgs.of(child, type, traversedSymlink));
}
});
assertThat(children)
.containsExactly(
- Pair.of(PathFragment.create(""), Dirent.Type.DIRECTORY),
- Pair.of(PathFragment.create("absolute_link"), Dirent.Type.SYMLINK));
+ VisitTreeArgs.of(PathFragment.create(""), Dirent.Type.DIRECTORY, false),
+ VisitTreeArgs.of(PathFragment.create("absolute_file_link"), Dirent.Type.FILE, true),
+ VisitTreeArgs.of(
+ PathFragment.create("absolute_dir_link"), Dirent.Type.DIRECTORY, true));
}
@Test
@@ -479,7 +602,8 @@ public void visitTree_throwsOnSymlinkPointingOutsideTree() throws Exception {
Exception e =
assertThrows(
- IOException.class, () -> TreeArtifactValue.visitTree(treeDir, (child, type) -> {}));
+ IOException.class,
+ () -> TreeArtifactValue.visitTree(treeDir, (child, type, traversedSymlink) -> {}));
assertThat(e).hasMessageThat().contains("/tree/link pointing to ../outside");
}
@@ -491,7 +615,8 @@ public void visitTree_throwsOnSymlinkTraversingOutsideThenBackInsideTree() throw
Exception e =
assertThrows(
- IOException.class, () -> TreeArtifactValue.visitTree(treeDir, (child, type) -> {}));
+ IOException.class,
+ () -> TreeArtifactValue.visitTree(treeDir, (child, type, traversedSymlink) -> {}));
assertThat(e).hasMessageThat().contains("/tree/link pointing to ../tree/file");
}