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);
