Gracefully handle filesystem inconsistencies in `RecursiveFilesystemTraversalFunction`. PiperOrigin-RevId: 577878638 Change-Id: I696e1ace5338a026345ece2222b620cea99ae264
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java index 8c2a068..62e2ca4 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
@@ -319,6 +319,9 @@ case SYMLINK_CYCLE_OR_INFINITE_EXPANSION: throw new ArtifactFunctionException( SourceArtifactException.create(artifact, e), Transience.PERSISTENT); + case INCONSISTENT_FILESYSTEM: + throw new ArtifactFunctionException( + SourceArtifactException.create(artifact, e), Transience.TRANSIENT); case CANNOT_CROSS_PACKAGE_BOUNDARY: throw new IllegalStateException( String.format(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD index cc6855b..a39bc53 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -2003,6 +2003,7 @@ "//src/main/java/com/google/devtools/build/lib/io:file_symlink_exception", "//src/main/java/com/google/devtools/build/lib/io:file_symlink_infinite_expansion_exception", "//src/main/java/com/google/devtools/build/lib/io:file_symlink_infinite_expansion_uniqueness_function", + "//src/main/java/com/google/devtools/build/lib/io:inconsistent_filesystem_exception", "//src/main/java/com/google/devtools/build/lib/packages", "//src/main/java/com/google/devtools/build/lib/profiler", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java index 70fa69d..6c03cb3 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java
@@ -15,7 +15,6 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; -import static com.google.common.base.Preconditions.checkState; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.annotations.VisibleForTesting; @@ -36,6 +35,7 @@ import com.google.devtools.build.lib.io.FileSymlinkException; import com.google.devtools.build.lib.io.FileSymlinkInfiniteExpansionException; import com.google.devtools.build.lib.io.FileSymlinkInfiniteExpansionUniquenessFunction; +import com.google.devtools.build.lib.io.InconsistentFilesystemException; import com.google.devtools.build.lib.packages.BuildFileNotFoundException; import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.ProfilerTask; @@ -106,6 +106,9 @@ /** A file/directory visited was part of a symlink cycle or infinite expansion. */ SYMLINK_CYCLE_OR_INFINITE_EXPANSION, + + /** The filesystem told us inconsistent information. */ + INCONSISTENT_FILESYSTEM, } private final RecursiveFilesystemTraversalException.Type type; @@ -170,7 +173,11 @@ if (!rootInfo.type.exists()) { // May be a dangling symlink or a non-existent file. Handle gracefully. if (rootInfo.type.isSymlink()) { - return resultForDanglingSymlink(traversal.root().asRootedPath(), rootInfo); + return RecursiveFilesystemTraversalValue.of( + ResolvedFileFactory.danglingSymlink( + traversal.root().asRootedPath(), + rootInfo.unresolvedSymlinkTarget, + rootInfo.metadata)); } else { return RecursiveFilesystemTraversalValue.EMPTY; } @@ -248,6 +255,9 @@ // trying to get a package lookup value may have failed due to a symlink cycle. RecursiveFilesystemTraversalException.Type exceptionType = RecursiveFilesystemTraversalException.Type.FILE_OPERATION_FAILURE; + if (e instanceof InconsistentFilesystemException) { + exceptionType = RecursiveFilesystemTraversalException.Type.INCONSISTENT_FILESYSTEM; + } if (e instanceof FileSymlinkException) { exceptionType = RecursiveFilesystemTraversalException.Type.SYMLINK_CYCLE_OR_INFINITE_EXPANSION; @@ -570,27 +580,19 @@ } /** - * Creates result for a dangling symlink. - * - * @param linkName path to the symbolic link - * @param info the {@link FileInfo} associated with the link file - */ - private static RecursiveFilesystemTraversalValue resultForDanglingSymlink( - RootedPath linkName, FileInfo info) { - checkState(info.type.isSymlink() && !info.type.exists(), "{%s} {%s}", linkName, info.type); - return RecursiveFilesystemTraversalValue.of( - ResolvedFileFactory.danglingSymlink(linkName, info.unresolvedSymlinkTarget, info.metadata)); - } - - /** * Creates results for a file or for a symlink that points to one. * * <p>A symlink may be direct (points to a file) or transitive (points at a direct or transitive * symlink). */ - private static RecursiveFilesystemTraversalValue resultForFileRoot( - RootedPath path, FileInfo info) { - checkState(info.type.isFile() && info.type.exists(), "{%s} {%s}", path, info.type); + private static RecursiveFilesystemTraversalValue resultForFileRoot(RootedPath path, FileInfo info) + throws InconsistentFilesystemException { + if (!info.type.isFile() || !info.type.exists()) { + throw new InconsistentFilesystemException( + String.format( + "We were previously told %s was an existing file but it's actually %s", path, info)); + } + if (info.type.isSymlink()) { return RecursiveFilesystemTraversalValue.of( ResolvedFileFactory.symlinkToFile(
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java index abad770d..e4c01dc 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java
@@ -70,8 +70,10 @@ import com.google.devtools.build.lib.testutil.TimestampGranularityUtils; import com.google.devtools.build.lib.util.io.OutErr; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; +import com.google.devtools.build.lib.vfs.DelegateFileSystem; import com.google.devtools.build.lib.vfs.FileStateKey; 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; import com.google.devtools.build.lib.vfs.Root; @@ -125,6 +127,22 @@ private NonHermeticArtifactFakeFunction artifactFunction; private List<Artifact.DerivedArtifact> artifacts; + private final Set<PathFragment> pathsToPretendDontExist = Sets.newConcurrentHashSet(); + + @Override + protected FileSystem createFileSystem() { + return new DelegateFileSystem(super.createFileSystem()) { + @Override + protected FileStatus statIfFound(PathFragment path, boolean followSymlinks) + throws IOException { + if (pathsToPretendDontExist.contains(path)) { + return null; + } + return super.statIfFound(path, followSymlinks); + } + }; + } + @Before public void setUp() { artifacts = new ArrayList<>(); @@ -1254,6 +1272,24 @@ assertThat(result.getDigest()).isEqualTo(expectedBytes); } + @Test + public void testGracefullyHandlesInconsistentFilesystem() throws Exception { + scratch.dir("parent"); + PathFragment childPathFragment = scratch.file("parent/child").asFragment(); + pathsToPretendDontExist.add(childPathFragment); + Artifact childArtifact = sourceArtifact("parent/child"); + SkyKey key = pkgRoot(parentOf(rootedPath(childArtifact)), DONT_CROSS); + EvaluationResult<SkyValue> result = eval(key); + assertThat(result.hasError()).isTrue(); + ErrorInfo error = result.getError(key); + assertThat(error.getException()).isInstanceOf(RecursiveFilesystemTraversalException.class); + assertThat(((RecursiveFilesystemTraversalException) error.getException()).getType()) + .isEqualTo(RecursiveFilesystemTraversalException.Type.INCONSISTENT_FILESYSTEM); + assertThat(error.getException()) + .hasMessageThat() + .contains("We were previously told [/workspace]/[parent/child] was an existing file but"); + } + private static class NonHermeticArtifactSkyKey extends AbstractSkyKey<SkyKey> { private NonHermeticArtifactSkyKey(SkyKey arg) { super(arg);