Propagate IOExceptions through RecursiveFilesystemTraversalFunction.
Both PackageLookupFunction and DirectoryListingFunction can throw IOExceptions (wrapped in BuildFileNotFoundException, in the case of the former) that should be processed and not silently propagated up.
PiperOrigin-RevId: 361228178
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 f742794..8cfda43 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
@@ -32,6 +32,7 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.packages.BuildFileNotFoundException;
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.ProfilerTask;
import com.google.devtools.build.lib.profiler.SilentCloseable;
@@ -64,9 +65,6 @@
/** A {@link SkyFunction} to build {@link RecursiveFilesystemTraversalValue}s. */
public final class RecursiveFilesystemTraversalFunction implements SkyFunction {
-
- private static final class MissingDepException extends Exception {}
-
private static final byte[] MISSING_FINGERPRINT =
new BigInteger(1, "NonexistentFileStateValue".getBytes(UTF_8)).toByteArray();
@@ -98,6 +96,9 @@
/** A generated directory's root-relative path conflicts with a package's path. */
GENERATED_PATH_CONFLICT,
+
+ /** A file/directory visited was part of a symlink cycle or infinite expansion. */
+ SYMLINK_CYCLE_OR_INFINITE_EXPANSION,
}
private final Type type;
@@ -138,6 +139,7 @@
}
}
+ @Nullable
@Override
public SkyValue compute(SkyKey skyKey, Environment env)
throws RecursiveFilesystemTraversalFunctionException, InterruptedException {
@@ -147,6 +149,9 @@
.profile(ProfilerTask.FILESYSTEM_TRAVERSAL, traversal.getRoot().toString())) {
// Stat the traversal root.
FileInfo rootInfo = lookUpFileInfo(env, traversal);
+ if (rootInfo == null) {
+ return null;
+ }
if (!rootInfo.type.exists()) {
// May be a dangling symlink or a non-existent file. Handle gracefully.
@@ -179,6 +184,9 @@
// Otherwise the root is a directory or a symlink to one.
PkgLookupResult pkgLookupResult = checkIfPackage(env, traversal, rootInfo);
+ if (pkgLookupResult == null) {
+ return null;
+ }
traversal = pkgLookupResult.traversal;
if (pkgLookupResult.isConflicting()) {
@@ -211,17 +219,28 @@
// We are free to traverse this directory.
Collection<SkyKey> dependentKeys = createRecursiveTraversalKeys(env, traversal);
- return resultForDirectory(
- traversal,
- rootInfo,
- traverseChildren(env, dependentKeys, /*inline=*/ traversal.isRootGenerated));
- } catch (IOException e) {
- String message = "Error while traversing fileset: " + e.getMessage();
+ if (dependentKeys == null) {
+ return null;
+ }
+ Collection<RecursiveFilesystemTraversalValue> subdirTraversals =
+ traverseChildren(env, dependentKeys, /*inline=*/ traversal.isRootGenerated);
+ if (subdirTraversals == null) {
+ return null;
+ }
+ return resultForDirectory(traversal, rootInfo, subdirTraversals);
+ } catch (IOException | BuildFileNotFoundException e) {
+ String message =
+ String.format(
+ "Error while traversing directory %s: %s",
+ traversal.root.getRelativePart(), e.getMessage());
throw new RecursiveFilesystemTraversalFunctionException(
new RecursiveFilesystemTraversalException(
- message, RecursiveFilesystemTraversalException.Type.FILE_OPERATION_FAILURE));
- } catch (MissingDepException e) {
- return null;
+ message,
+ // Trying to stat the starting point of this root may have failed with a symlink cycle
+ // or trying to get a package lookup value may have failed due to a symlink cycle.
+ e instanceof FileSymlinkException || e.getCause() instanceof FileSymlinkException
+ ? RecursiveFilesystemTraversalException.Type.SYMLINK_CYCLE_OR_INFINITE_EXPANSION
+ : RecursiveFilesystemTraversalException.Type.FILE_OPERATION_FAILURE));
}
}
@@ -272,8 +291,9 @@
}
}
+ @Nullable
private static FileInfo lookUpFileInfo(Environment env, TraversalRequest traversal)
- throws MissingDepException, IOException, InterruptedException {
+ throws IOException, InterruptedException {
if (traversal.isRootGenerated) {
HasDigest fsVal = null;
if (traversal.root.getOutputArtifact() != null) {
@@ -281,7 +301,7 @@
SkyKey artifactKey = Artifact.key(artifact);
SkyValue value = env.getValue(artifactKey);
if (env.valuesMissing()) {
- throw new MissingDepException();
+ return null;
}
if (value instanceof FileArtifactValue || value instanceof TreeArtifactValue) {
@@ -341,7 +361,7 @@
FileValue.key(traversal.root.asRootedPath()), IOException.class);
if (env.valuesMissing()) {
- throw new MissingDepException();
+ return null;
}
if (fileValue.unboundedAncestorSymlinkExpansionChain() != null) {
SkyKey uniquenessKey =
@@ -349,7 +369,7 @@
fileValue.unboundedAncestorSymlinkExpansionChain());
env.getValue(uniquenessKey);
if (env.valuesMissing()) {
- throw new MissingDepException();
+ return null;
}
throw new FileSymlinkInfiniteExpansionException(
@@ -477,13 +497,22 @@
*/
private static PkgLookupResult checkIfPackage(
Environment env, TraversalRequest traversal, FileInfo rootInfo)
- throws MissingDepException, IOException, InterruptedException {
+ throws IOException, InterruptedException, BuildFileNotFoundException {
Preconditions.checkArgument(rootInfo.type.exists() && !rootInfo.type.isFile(),
"{%s} {%s}", traversal, rootInfo);
+ // PackageLookupFunction/dependencies can only throw IOException, BuildFileNotFoundException,
+ // and RepositoryFetchException, and RepositoryFetchException is not in play here. Note that
+ // run-of-the-mill circular symlinks will *not* throw here, and will trigger later errors during
+ // the recursive traversal.
PackageLookupValue pkgLookup =
(PackageLookupValue)
- getDependentSkyValue(env,
- PackageLookupValue.key(traversal.root.asRootedPath().getRootRelativePath()));
+ env.getValueOrThrow(
+ PackageLookupValue.key(traversal.root.asRootedPath().getRootRelativePath()),
+ BuildFileNotFoundException.class,
+ IOException.class);
+ if (env.valuesMissing()) {
+ return null;
+ }
if (pkgLookup.packageExists()) {
if (traversal.isRootGenerated) {
@@ -514,9 +543,9 @@
*
* <p>The returned keys are of type {@link SkyFunctions#RECURSIVE_FILESYSTEM_TRAVERSAL}.
*/
+ @Nullable
private static Collection<SkyKey> createRecursiveTraversalKeys(
- Environment env, TraversalRequest traversal)
- throws MissingDepException, InterruptedException, IOException {
+ Environment env, TraversalRequest traversal) throws InterruptedException, IOException {
// Use the traversal's path, even if it's a symlink. The contents of the directory, as listed
// in the result, must be relative to it.
Iterable<Dirent> dirents;
@@ -529,8 +558,14 @@
Collections.sort(direntsCollection);
dirents = direntsCollection;
} else {
- dirents = ((DirectoryListingValue) getDependentSkyValue(env,
- DirectoryListingValue.key(traversal.root.asRootedPath()))).getDirents();
+ DirectoryListingValue dirListingValue =
+ (DirectoryListingValue)
+ env.getValueOrThrow(
+ DirectoryListingValue.key(traversal.root.asRootedPath()), IOException.class);
+ if (dirListingValue == null) {
+ return null;
+ }
+ dirents = dirListingValue.getDirents();
}
List<SkyKey> result = new ArrayList<>();
@@ -618,24 +653,15 @@
return new HasDigest.ByteStringDigest(result);
}
- private static SkyValue getDependentSkyValue(Environment env, SkyKey key)
- throws MissingDepException, InterruptedException {
- SkyValue value = env.getValue(key);
- if (env.valuesMissing()) {
- throw new MissingDepException();
- }
- return value;
- }
-
/**
* Requests Skyframe to compute the dependent values and returns them.
*
* <p>The keys must all be {@link SkyFunctions#RECURSIVE_FILESYSTEM_TRAVERSAL} keys.
*/
+ @Nullable
private Collection<RecursiveFilesystemTraversalValue> traverseChildren(
Environment env, Iterable<SkyKey> keys, boolean inline)
- throws MissingDepException, InterruptedException,
- RecursiveFilesystemTraversalFunctionException {
+ throws InterruptedException, RecursiveFilesystemTraversalFunctionException {
Map<SkyKey, SkyValue> values;
if (inline) {
// Don't create Skyframe nodes for a recursive traversal over the output tree.
@@ -648,7 +674,7 @@
values = env.getValues(keys);
}
if (env.valuesMissing()) {
- throw new MissingDepException();
+ return null;
}
return Collections2.transform(values.values(), RecursiveFilesystemTraversalValue.class::cast);
