diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java index 5f78a98fc5d1d4..3a111ed844784d 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java @@ -630,13 +630,17 @@ protected FileStatus statNullable(PathFragment path, boolean followSymlinks) { private FileStatus statInternal(PathFragment path, FollowMode followMode, StatSources statSources) throws IOException { // Canonicalize the path. - if (followMode == FollowMode.FOLLOW_ALL) { - path = resolveSymbolicLinks(path).asFragment(); - } else if (followMode == FollowMode.FOLLOW_PARENT) { - PathFragment parent = path.getParentDirectory(); - if (parent != null) { - path = resolveSymbolicLinks(parent).asFragment().getChild(path.getBaseName()); + try { + if (followMode == FollowMode.FOLLOW_ALL) { + path = resolveSymbolicLinks(path).asFragment(); + } else if (followMode == FollowMode.FOLLOW_PARENT) { + PathFragment parent = path.getParentDirectory(); + if (parent != null) { + path = resolveSymbolicLinks(parent).asFragment().getChild(path.getBaseName()); + } } + } catch (FileNotFoundException e) { + return null; } // Since the path has been canonicalized, the operations below never need to follow symlinks. diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java index 6809ca303aa19c..3486b65a73e3af 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java @@ -1488,11 +1488,12 @@ private static void reportOutputTreeArtifactErrors( Action action, Artifact output, Reporter reporter, IOException e) { String errorMessage; if (e instanceof FileNotFoundException) { - errorMessage = String.format("TreeArtifact %s was not created", output.prettyPrint()); + errorMessage = String.format("output tree artifact %s was not created", output.prettyPrint()); } else { errorMessage = String.format( - "Error while validating output TreeArtifact %s : %s", output, e.getMessage()); + "error while validating output tree artifact %s: %s", + output.prettyPrint(), e.getMessage()); } reporter.handle(Event.error(action.getOwner().getLocation(), errorMessage)); 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 58f0add3e730f3..a0c0a83bd0eac0 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 @@ -558,9 +558,7 @@ private void visit( if (statFollow == null) { throw new IOException( - String.format( - "Child %s of tree artifact %s is a dangling symbolic link", - parentRelativePath, parentDir)); + String.format("child %s is a dangling symbolic link", parentRelativePath)); } if (statFollow.isFile() && !statFollow.isSpecialFile()) { @@ -574,9 +572,7 @@ private void visit( if (type == Dirent.Type.UNKNOWN) { throw new IOException( - String.format( - "Child %s of tree artifact %s has an unsupported type", - parentRelativePath, parentDir)); + String.format("child %s has an unsupported type", parentRelativePath)); } visitor.visit(parentRelativePath, type, traversedSymlink); diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java index f2bbc4c99b05e3..4e6a3bf8d6ba08 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java @@ -451,11 +451,43 @@ public void statAndExists_followSymlinks( @Test public void statAndExists_notFound() throws Exception { RemoteActionFileSystem actionFs = (RemoteActionFileSystem) createActionFileSystem(); - Artifact artifact = ActionsTestUtil.createArtifact(outputRoot, "out"); - PathFragment path = artifact.getPath().asFragment(); + PathFragment path = getOutputPath("does_not_exist"); + + assertThat(actionFs.exists(path)).isFalse(); + + assertThat(actionFs.statIfFound(path, /* followSymlinks= */ true)).isNull(); + + assertThrows( + FileNotFoundException.class, () -> actionFs.stat(path, /* followSymlinks= */ true)); + } + + @Test + public void statAndExists_isNotDirectory() throws Exception { + RemoteActionFileSystem actionFs = (RemoteActionFileSystem) createActionFileSystem(); + PathFragment nonDirPath = getOutputPath("non_dir"); + PathFragment path = nonDirPath.getChild("file"); + + writeLocalFile(actionFs, nonDirPath, "content"); assertThat(actionFs.exists(path)).isFalse(); + assertThat(actionFs.statIfFound(path, /* followSymlinks= */ true)).isNull(); + + assertThrows( + FileNotFoundException.class, () -> actionFs.stat(path, /* followSymlinks= */ true)); + } + + @Test + public void statAndExists_danglingSymlink_notFound() throws Exception { + RemoteActionFileSystem actionFs = (RemoteActionFileSystem) createActionFileSystem(); + PathFragment path = getOutputPath("sym"); + + actionFs.getPath(path).createSymbolicLink(PathFragment.create("/does_not_exist")); + + assertThat(actionFs.exists(path)).isFalse(); + + assertThat(actionFs.statIfFound(path, /* followSymlinks= */ true)).isNull(); + assertThrows( FileNotFoundException.class, () -> actionFs.stat(path, /* followSymlinks= */ true)); } diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStoreTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStoreTest.java index afcf8281bb36b8..d594263d912043 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStoreTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStoreTest.java @@ -259,6 +259,20 @@ public void createsTreeArtifactValueFromFilesystem() throws Exception { assertThat(chmodCalls).isEmpty(); } + @Test + public void withDanglingSymlinkInTreeArtifactFailsWithException() throws Exception { + SpecialArtifact treeArtifact = + ActionsTestUtil.createTreeArtifactWithGeneratingAction(outputRoot, "foo/bar"); + TreeFileArtifact child = TreeFileArtifact.createTreeOutput(treeArtifact, "child"); + treeArtifact.getPath().createDirectoryAndParents(); + child.getPath().createSymbolicLink(PathFragment.create("/does_not_exist")); + + ActionOutputMetadataStore store = createStore(/* outputs= */ ImmutableSet.of(treeArtifact)); + + IOException e = assertThrows(IOException.class, () -> store.getOutputMetadata(treeArtifact)); + assertThat(e).hasMessageThat().contains("dangling symbolic link"); + } + @Test public void resettingOutputs() throws Exception { PathFragment path = PathFragment.create("foo/bar"); 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 46b0304b56218f..12c72258c3898c 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 @@ -505,9 +505,7 @@ void run(ActionExecutionContext actionExecutionContext) throws IOException { ImmutableList errors = ImmutableList.copyOf(eventCollector); assertThat(errors).hasSize(2); - assertThat(errors.get(0).getMessage()) - .contains( - "Child links/link of tree artifact " + out.getPath() + " is a dangling symbolic link"); + assertThat(errors.get(0).getMessage()).contains("child links/link is a dangling symbolic link"); assertThat(errors.get(1).getMessage()).contains("not all outputs were created or valid"); } @@ -555,9 +553,7 @@ void run(ActionExecutionContext actionExecutionContext) throws IOException { ImmutableList errors = ImmutableList.copyOf(eventCollector); assertThat(errors).hasSize(2); - assertThat(errors.get(0).getMessage()) - .contains( - "Child links/link of tree artifact " + out.getPath() + " is a dangling symbolic link"); + assertThat(errors.get(0).getMessage()).contains("child links/link is a dangling symbolic link"); assertThat(errors.get(1).getMessage()).contains("not all outputs were created or valid"); } @@ -607,9 +603,7 @@ void run(ActionExecutionContext actionExecutionContext) throws IOException { ImmutableList errors = ImmutableList.copyOf(eventCollector); assertThat(errors).hasSize(2); - assertThat(errors.get(0).getMessage()) - .contains( - "Child links/link of tree artifact " + out.getPath() + " is a dangling symbolic link"); + assertThat(errors.get(0).getMessage()).contains("child links/link is a dangling symbolic link"); assertThat(errors.get(1).getMessage()).contains("not all outputs were created or valid"); } 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 a65a0c4203b31f..bf3f25c5392ad4 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 @@ -419,9 +419,7 @@ public void visitTree_throwsOnDanglingSymlink() throws Exception { assertThrows( IOException.class, () -> TreeArtifactValue.visitTree(treeDir, (child, type, traversedSymlink) -> {})); - assertThat(e) - .hasMessageThat() - .contains("Child symlink of tree artifact /tree is a dangling symbolic link"); + assertThat(e).hasMessageThat().contains("child symlink is a dangling symbolic link"); } @Test @@ -456,9 +454,7 @@ public Collection readdir(PathFragment path, boolean followSymlinks) assertThrows( IOException.class, () -> TreeArtifactValue.visitTree(treeDir, (child, type, traversedSymlink) -> {})); - assertThat(e) - .hasMessageThat() - .contains("Child unknown of tree artifact /tree has an unsupported type"); + assertThat(e).hasMessageThat().contains("child unknown has an unsupported type"); } @Test @@ -523,9 +519,7 @@ public long getSize() { assertThrows( IOException.class, () -> TreeArtifactValue.visitTree(treeDir, (child, type, traversedSymlink) -> {})); - assertThat(e) - .hasMessageThat() - .contains("Child sym of tree artifact /tree has an unsupported type"); + assertThat(e).hasMessageThat().contains("child sym has an unsupported type"); } @Test