Skip to content

Commit

Permalink
Gracefully handle filesystem inconsistencies in `RecursiveFilesystemT…
Browse files Browse the repository at this point in the history
…raversalFunction`.

PiperOrigin-RevId: 577878638
Change-Id: I696e1ace5338a026345ece2222b620cea99ae264
  • Loading branch information
haxorz authored and copybara-github committed Oct 30, 2023
1 parent a69fdb8 commit 0d4739e
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/skyframe/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
*
* <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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -125,6 +127,22 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe
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<>();
Expand Down Expand Up @@ -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<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);
Expand Down

0 comments on commit 0d4739e

Please sign in to comment.