This is an implementation detail of {@link RemoteActionFileSystem}, factored out for testing. + * Because {@link RemoteActionFileSystem} implements a union filesystem and must account for the + * possibility of symlinks straddling the underlying filesystems, the performance of large + * filesystem scans can be greatly improved with a custom {@link FileSystem#resolveSymbolicLinks} + * implementation that leverages the trie to avoid repeated work. + * + *
On case-insensitive filesystems, accessing the same path through different case variations + * will produce distinct trie entries. This could be fixed, but it's a performance rather than a + * correctness concern, and shouldn't matter most of the time. + * + *
Thread-safe: concurrent calls to {@link #resolveSymbolicLinks} are supported. As with {@link
+ * FileSystem#resolveSymbolicLinks}, the result is undefined if the filesystem is mutated
+ * concurrently.
+ */
+final class PathCanonicalizer {
+
+ interface Resolver {
+ /**
+ * Returns the result of {@link FileSystem#readSymbolicLink} if the path is a symlink, otherwise
+ * null. All but the last path segment must be canonical.
+ *
+ * @throws IOException if the file type or symlink target path could not be determined
+ */
+ @Nullable
+ PathFragment resolveOneLink(PathFragment path) throws IOException;
+ }
+
+ /** The trie node for a path segment. */
+ private static final class Node {
+ /**
+ * Maps the next segment to the corresponding node. Null iff the path ending with the current
+ * segment is a symlink.
+ */
+ @Nullable private final ConcurrentHashMap See {@link FileSystem#resolveSymbolicLinks} for the full specification.
+ *
+ * @param path the path to canonicalize.
+ * @throws FileSymlinkLoopException if too many symlinks had to be followed.
+ * @throws IOException if an I/O error occurs
+ * @return the canonical path.
+ */
+ PathFragment resolveSymbolicLinks(PathFragment path) throws IOException {
+ return resolveSymbolicLinks(path, FileSystem.MAX_SYMLINKS);
+ }
+
+ /** Removes cached information for a path prefix. */
+ void clearPrefix(PathFragment pathPrefix) {
+ Node node = getRootNode(pathPrefix);
+ Iterator It acts as a union filesystem over three different sources:
*
@@ -91,17 +91,21 @@
* to an input. Most operations call resolveSymbolicLinks upfront (which is able to canonicalize
* paths taking every source into account) and only then delegate to the underlying sources.
*
- * The implementation assumes that an action never modifies its input files, but may otherwise
- * modify any path in the output tree, including modifications that undo previous ones (e.g.,
- * writing to a path and then moving it elsewhere). No effort is made to detect irreconcilable
- * differences between sources (e.g., distinct files of the same name in two different sources).
+ * The implementation assumes that an action never modifies its input paths, but may otherwise
+ * modify any path in the output tree. Concurrent operations are supported as long as they don't
+ * affect filesystem structure (i.e., create, move or delete paths). Otherwise, they might fail or
+ * produce inconsistent results. No effort is made to detect irreconcilable differences between
+ * sources, such as the same path existing in multiple underlying sources with different type or
+ * contents.
*/
-public class RemoteActionFileSystem extends AbstractFileSystemWithCustomStat {
+public class RemoteActionFileSystem extends AbstractFileSystemWithCustomStat
+ implements PathCanonicalizer.Resolver {
private final PathFragment execRoot;
private final PathFragment outputBase;
private final InputMetadataProvider fileCache;
private final ActionInputMap inputArtifactData;
private final TreeArtifactDirectoryCache inputTreeArtifactDirectoryCache;
+ private final PathCanonicalizer pathCanonicalizer;
private final ImmutableMap (This method does not need to be synchronized; but the result may be stale in the case of
* concurrent modification.)
@@ -443,7 +447,8 @@ protected Path resolveSymbolicLinks(PathFragment path) throws IOException {
return parentNode == null
? getPath(path) // (root)
: getPath(
- appendSegment(resolveSymbolicLinks(parentNode).asFragment(), path.getBaseName(), 32));
+ appendSegment(
+ resolveSymbolicLinks(parentNode).asFragment(), path.getBaseName(), MAX_SYMLINKS));
}
/**
diff --git a/src/test/java/com/google/devtools/build/lib/remote/BUILD b/src/test/java/com/google/devtools/build/lib/remote/BUILD
index cfa1be5178d7d4..4440a2c67e6a37 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/remote/BUILD
@@ -133,6 +133,7 @@ java_library(
"//src/test/java/com/google/devtools/build/lib/testutil:TestUtils",
"//third_party:auth",
"//third_party:caffeine",
+ "//third_party:checker_framework_annotations",
"//third_party:error_prone_annotations",
"//third_party:gson",
"//third_party:guava",
diff --git a/src/test/java/com/google/devtools/build/lib/remote/PathCanonicalizerTest.java b/src/test/java/com/google/devtools/build/lib/remote/PathCanonicalizerTest.java
new file mode 100644
index 00000000000000..3cdc92a1d979a7
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/remote/PathCanonicalizerTest.java
@@ -0,0 +1,282 @@
+// Copyright 2024 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.remote;
+
+import static com.google.common.truth.Truth.assertThat;
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assume.assumeTrue;
+
+import com.google.devtools.build.lib.util.OS;
+import com.google.devtools.build.lib.vfs.DigestHashFunction;
+import com.google.devtools.build.lib.vfs.FileSymlinkLoopException;
+import com.google.devtools.build.lib.vfs.FileSystem;
+import com.google.devtools.build.lib.vfs.FileSystem.NotASymlinkException;
+import com.google.devtools.build.lib.vfs.FileSystemUtils;
+import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import org.checkerframework.checker.nullness.qual.Nullable;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for {@link PathCanonicalizer}. */
+@RunWith(JUnit4.class)
+public final class PathCanonicalizerTest {
+
+ // Test outline:
+ // 1. Set up the filesystem state by calling createSymlink, createNonSymlink or deleteTree.
+ // 2. Call assertSuccess or assertFailure to check for successful resolution or failure.
+
+ // On Windows, absolute paths start with a drive letter, e.g. C:/, instead of / as in Unix.
+ // To avoid test duplication, when the tests run on Windows, Unix-style absolute paths passed to
+ // the above methods will have a C: automatically prepended to them.
+
+ private final FileSystem fs = new InMemoryFileSystem(DigestHashFunction.SHA256);
+
+ private final PathCanonicalizer canonicalizer = new PathCanonicalizer(this::resolve);
+
+ private @Nullable PathFragment resolve(PathFragment pathFragment) throws IOException {
+ Path path = fs.getPath(pathFragment);
+ try {
+ return path.readSymbolicLink();
+ } catch (NotASymlinkException e) {
+ return null;
+ }
+ }
+
+ @Test
+ public void testRoot() throws Exception {
+ assertSuccess("/", "/");
+ }
+
+ @Test
+ public void testAlreadyCanonical() throws Exception {
+ createNonSymlink("/a/b");
+ assertSuccess("/a/b", "/a/b");
+ }
+
+ @Test
+ public void testAbsoluteSymlinkToFile() throws Exception {
+ createSymlink("/a/b", "/c/d");
+ createNonSymlink("/c/d");
+ assertSuccess("/a/b", "/c/d");
+ }
+
+ @Test
+ public void testAbsoluteSymlinkToDirectory() throws Exception {
+ createSymlink("/a/b", "/d/e");
+ createNonSymlink("/d/e/c");
+ assertSuccess("/a/b/c", "/d/e/c");
+ }
+
+ @Test
+ public void testAbsoluteSymlinkToDifferentDrive() throws Exception {
+ assumeTrue(OS.getCurrent() == OS.WINDOWS);
+
+ createSymlink("C:/a/b", "D:/e/f");
+ createNonSymlink("D:/e/f");
+ assertSuccess("C:/a/b/c/d", "D:/e/f/c/d");
+ }
+
+ @Test
+ public void testRelativeSymlinkToFileInSameDirectory() throws Exception {
+ createSymlink("/a/b", "c");
+ createNonSymlink("/a/c");
+ assertSuccess("/a/b", "/a/c");
+ }
+
+ @Test
+ public void testRelativeSymlinkToFileInDirectoryBelow() throws Exception {
+ createSymlink("/a/b", "c/d");
+ createNonSymlink("/a/c/d");
+ assertSuccess("/a/b", "/a/c/d");
+ }
+
+ @Test
+ public void testRelativeSymlinkToFileInDirectoryAbove() throws Exception {
+ createSymlink("/a/b/c", "../d/e");
+ createNonSymlink("/a/d/e");
+ assertSuccess("/a/b/c", "/a/d/e");
+ }
+
+ @Test
+ public void testRelativeSymlinkToRoot() throws Exception {
+ createSymlink("/a/b/c", "../../d");
+ createNonSymlink("/d");
+ assertSuccess("/a/b/c", "/d");
+ }
+
+ @Test
+ public void testRelativeSymlinkWithTooManyUplevelReferences() throws Exception {
+ createSymlink("/a/b", "../../d");
+ createNonSymlink("/d/c");
+ assertSuccess("/a/b/c", "/d/c");
+ }
+
+ @Test
+ public void testMultipleSymlinks() throws Exception {
+ createSymlink("/a", "/b");
+ createSymlink("/b/c", "/d");
+ createSymlink("/d/e", "/f");
+ createNonSymlink("/f");
+ assertSuccess("/a/c/e", "/f");
+ }
+
+ @Test
+ public void testReplayCanonical() throws Exception {
+ createNonSymlink("/a/b/c");
+ assertSuccess("/a/b/c", "/a/b/c");
+ assertSuccess("/a/b/c", "/a/b/c");
+ }
+
+ @Test
+ public void testReplaySymlink() throws Exception {
+ createSymlink("/a/b", "/d");
+ createNonSymlink("/d/c");
+ assertSuccess("/a/b/c", "/d/c");
+ assertSuccess("/a/b/c", "/d/c");
+ }
+
+ @Test
+ public void testDistinguishPathsWithCommonPrefix() throws Exception {
+ createSymlink("/a/b", "/d");
+ createNonSymlink("/d/c");
+ createNonSymlink("/a/e");
+ assertSuccess("/a/b/c", "/d/c");
+ assertSuccess("/a/e", "/a/e");
+ }
+
+ @Test
+ public void testDistinguishPathsWithDifferentDriveLetter() throws Exception {
+ assumeTrue(OS.getCurrent() == OS.WINDOWS);
+ createSymlink("C:/a/b", "D:/d");
+ createNonSymlink("D:/d/c");
+ createNonSymlink("C:/a/b/c");
+ assertSuccess("C:/a/b/c", "D:/d/c");
+ assertSuccess("D:/a/b/c", "D:/a/b/c");
+ }
+
+ @Test
+ public void testClearAndReplaceWithSymlink() throws Exception {
+ createNonSymlink("/a/b/c");
+ assertSuccess("/a/b/c", "/a/b/c");
+ deleteTree("/a/b");
+ createSymlink("/a/b", "/d");
+ createNonSymlink("/d/c");
+ assertSuccess("/a/b/c", "/d/c");
+ }
+
+ @Test
+ public void testClearAndReplaceWithNonSymlink() throws Exception {
+ createSymlink("/a/b", "/d");
+ createNonSymlink("/d/c");
+ assertSuccess("/a/b/c", "/d/c");
+ deleteTree("/a/b");
+ createNonSymlink("/a/b/c");
+ assertSuccess("/a/b/c", "/a/b/c");
+ }
+
+ @Test
+ public void testClearSymlinkAndDoNotReplace() throws Exception {
+ createSymlink("/a/b", "/d");
+ createNonSymlink("/d/c");
+ assertSuccess("/a/b/c", "/d/c");
+ deleteTree("/a/b");
+ assertFailure(FileNotFoundException.class, "/a/b/c");
+ }
+
+ @Test
+ public void testClearNonSymlinkAndDoNotReplace() throws Exception {
+ createNonSymlink("/a/b/c");
+ assertSuccess("/a/b/c", "/a/b/c");
+ deleteTree("/a/b");
+ assertFailure(FileNotFoundException.class, "/a/b/c");
+ }
+
+ @Test
+ public void testSymlinkSelfLoop() throws Exception {
+ createSymlink("/a/b", "/a/b");
+ assertFailure(FileSymlinkLoopException.class, "/a/b");
+ }
+
+ @Test
+ public void testSymlinkMutualLoop() throws Exception {
+ createSymlink("/a/b", "/c/d");
+ createSymlink("/c/d", "/a/b");
+ assertFailure(FileSymlinkLoopException.class, "/a/b");
+ }
+
+ @Test
+ public void testSymlinkChainTooLong() throws Exception {
+ for (int i = 0; i < FileSystem.MAX_SYMLINKS + 1; i++) {
+ createSymlink(String.format("/%s", i), String.format("/%s", i + 1));
+ }
+ assertFailure(FileSymlinkLoopException.class, "/0");
+ }
+
+ @Test
+ public void testFileNotFound() throws Exception {
+ assertFailure(FileNotFoundException.class, "/a/b");
+ createNonSymlink("/a/b");
+ assertSuccess("/a/b", "/a/b");
+ }
+
+ @Test
+ public void testEmpty() throws Exception {
+ assertFailure(IllegalArgumentException.class, "");
+ }
+
+ @Test
+ public void testNonAbsolute() throws Exception {
+ assertFailure(IllegalArgumentException.class, "a/b");
+ }
+
+ private void createSymlink(String linkPathStr, String targetPathStr) throws Exception {
+ Path linkPath = fs.getPath(pathFragment(linkPathStr));
+ linkPath.getParentDirectory().createDirectoryAndParents();
+ linkPath.createSymbolicLink(pathFragment(targetPathStr));
+ }
+
+ private void createNonSymlink(String pathStr) throws Exception {
+ Path path = fs.getPath(pathFragment(pathStr));
+ path.getParentDirectory().createDirectoryAndParents();
+ FileSystemUtils.writeContent(path, UTF_8, "");
+ }
+
+ private void deleteTree(String pathStr) throws Exception {
+ canonicalizer.clearPrefix(pathFragment(pathStr));
+ fs.getPath(pathFragment(pathStr)).deleteTree();
+ }
+
+ private void assertSuccess(String input, String output) throws Exception {
+ assertThat(canonicalizer.resolveSymbolicLinks(pathFragment(input)))
+ .isEqualTo(pathFragment(output));
+ }
+
+ private