From 0d4739e88c4a142c5f7634d5dacbb0383bec683a Mon Sep 17 00:00:00 2001 From: Googler Date: Mon, 30 Oct 2023 10:00:55 -0700 Subject: [PATCH] Gracefully handle filesystem inconsistencies in `RecursiveFilesystemTraversalFunction`. PiperOrigin-RevId: 577878638 Change-Id: I696e1ace5338a026345ece2222b620cea99ae264 --- .../build/lib/skyframe/ArtifactFunction.java | 3 ++ .../google/devtools/build/lib/skyframe/BUILD | 1 + .../RecursiveFilesystemTraversalFunction.java | 38 ++++++++++--------- ...ursiveFilesystemTraversalFunctionTest.java | 36 ++++++++++++++++++ 4 files changed, 60 insertions(+), 18 deletions(-) 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 8c2a0689d1e26b..62e2ca45247406 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 @@ private SkyValue createSourceValue(Artifact artifact, Environment env) 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 cc6855ba3a996f..a39bc532f4c9d7 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 @@ java_library( "//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 70fa69d79d6095..6c03cb3d64087e 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 @@ public enum Type { /** 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 @@ public RecursiveFilesystemTraversalValue compute(SkyKey skyKey, Environment env) 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 @@ public RecursiveFilesystemTraversalValue compute(SkyKey skyKey, Environment env) // 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; @@ -569,28 +579,20 @@ private static PkgLookupResult checkIfPackage( } } - /** - * 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. * *

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 abad770d633d17..e4c01dcc4a0b69 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 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe private NonHermeticArtifactFakeFunction artifactFunction; private List artifacts; + private final Set 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 @@ public void testWithDigestByteStringDigest() throws Exception { 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 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 { private NonHermeticArtifactSkyKey(SkyKey arg) { super(arg);