A bunch of unrelated cleanups:
-Have SkylarkImportLookupFunction include causes in the SkyFunctionExceptions it throws.
-Better transitive skyframe error declarations in ASTFileLookupFunction.
-Have ErrorInfo differentiate between direct and transitive transience.
-Introduce ErrorInfoManager and have ParallelEvaluator/ParallelEvaluatorContext use it.
RELNOTES: None
PiperOrigin-RevId: 159163186
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java
index 30a9c08..f6385f2 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java
@@ -85,7 +85,10 @@
try {
fileValue = (FileValue) env.getValueOrThrow(fileSkyKey, IOException.class,
FileSymlinkException.class, InconsistentFilesystemException.class);
- } catch (IOException | FileSymlinkException e) {
+ } catch (IOException e) {
+ throw new ASTLookupFunctionException(new ErrorReadingSkylarkExtensionException(e),
+ Transience.PERSISTENT);
+ } catch (FileSymlinkException e) {
throw new ASTLookupFunctionException(new ErrorReadingSkylarkExtensionException(e),
Transience.PERSISTENT);
} catch (InconsistentFilesystemException e) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ErrorReadingSkylarkExtensionException.java b/src/main/java/com/google/devtools/build/lib/skyframe/ErrorReadingSkylarkExtensionException.java
index af4f039..cf6904a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ErrorReadingSkylarkExtensionException.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ErrorReadingSkylarkExtensionException.java
@@ -13,9 +13,20 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import com.google.devtools.build.lib.packages.BuildFileNotFoundException;
+import java.io.IOException;
+
/** Indicates some sort of IO error while dealing with a Skylark extension. */
public class ErrorReadingSkylarkExtensionException extends Exception {
- public ErrorReadingSkylarkExtensionException(Exception e) {
+ public ErrorReadingSkylarkExtensionException(BuildFileNotFoundException e) {
+ super(e.getMessage(), e);
+ }
+
+ public ErrorReadingSkylarkExtensionException(IOException e) {
+ super(e.getMessage(), e);
+ }
+
+ public ErrorReadingSkylarkExtensionException(FileSymlinkException e) {
super(e.getMessage(), e);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
index b5cb5b8..15e21e3 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import com.google.common.base.Throwables;
import com.google.common.cache.Cache;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
@@ -633,6 +634,16 @@
return buildFileValue;
}
+ private static BuildFileContainsErrorsException propagateSkylarkImportFailedException(
+ PackageIdentifier packageId, SkylarkImportFailedException e)
+ throws BuildFileContainsErrorsException {
+ Throwable rootCause = Throwables.getRootCause(e);
+ throw (rootCause instanceof IOException)
+ ? new BuildFileContainsErrorsException(
+ packageId, e.getMessage(), (IOException) rootCause)
+ : new BuildFileContainsErrorsException(packageId, e.getMessage());
+ }
+
/**
* Fetch the skylark loads for this BUILD file. If any of them haven't been computed yet,
* returns null.
@@ -667,7 +678,7 @@
return null;
}
} catch (SkylarkImportFailedException e) {
- throw new BuildFileContainsErrorsException(packageId, e.getMessage());
+ throw propagateSkylarkImportFailedException(packageId, e);
}
// Look up and load the imports.
@@ -721,7 +732,7 @@
}
} catch (SkylarkImportFailedException e) {
- throw new BuildFileContainsErrorsException(packageId, e.getMessage());
+ throw propagateSkylarkImportFailedException(packageId, e);
} catch (InconsistentFilesystemException e) {
throw new NoSuchPackageException(packageId, e.getMessage(), e);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
index 7d3a196..92daf44 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
@@ -133,7 +133,7 @@
astLookupValue = (ASTFileLookupValue) env.getValueOrThrow(astLookupKey,
ErrorReadingSkylarkExtensionException.class, InconsistentFilesystemException.class);
} catch (ErrorReadingSkylarkExtensionException e) {
- throw SkylarkImportFailedException.errorReadingFile(filePath, e.getMessage());
+ throw SkylarkImportFailedException.errorReadingFile(filePath, e);
}
if (astLookupValue == null) {
return null;
@@ -441,16 +441,20 @@
super(errorMessage);
}
+ private SkylarkImportFailedException(String errorMessage, Exception cause) {
+ super(errorMessage, cause);
+ }
+
private SkylarkImportFailedException(InconsistentFilesystemException e) {
- super(e.getMessage());
+ this(e.getMessage(), e);
}
private SkylarkImportFailedException(BuildFileNotFoundException e) {
- super(e.getMessage());
+ this(e.getMessage(), e);
}
private SkylarkImportFailedException(LabelSyntaxException e) {
- super(e.getMessage());
+ this(e.getMessage(), e);
}
static SkylarkImportFailedException errors(PathFragment file) {
@@ -458,9 +462,14 @@
String.format("Extension file '%s' has errors", file));
}
- static SkylarkImportFailedException errorReadingFile(PathFragment file, String error) {
+ static SkylarkImportFailedException errorReadingFile(
+ PathFragment file, ErrorReadingSkylarkExtensionException cause) {
return new SkylarkImportFailedException(
- String.format("Encountered error while reading extension file '%s': %s", file, error));
+ String.format(
+ "Encountered error while reading extension file '%s': %s",
+ file,
+ cause.getMessage()),
+ cause);
}
static SkylarkImportFailedException noFile(String reason) {
diff --git a/src/main/java/com/google/devtools/build/skyframe/ErrorInfo.java b/src/main/java/com/google/devtools/build/skyframe/ErrorInfo.java
index 418a4cf..bd90d8a 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ErrorInfo.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ErrorInfo.java
@@ -42,6 +42,7 @@
Preconditions.checkNotNull(rootCauseException, "Cause null %s", rootCauseException),
rootCauseSkyKey,
/*cycles=*/ ImmutableList.<CycleInfo>of(),
+ skyFunctionException.isTransient(),
isTransitivelyTransient || skyFunctionException.isTransient(),
skyFunctionException.isCatastrophic());
}
@@ -53,7 +54,8 @@
/*exception=*/ null,
/*rootCauseOfException=*/ null,
ImmutableList.of(cycleInfo),
- /*isTransient=*/ false,
+ /*isDirectlyTransient=*/ false,
+ /*isTransitivelyTransient=*/ false,
/*isCatastrophic=*/ false);
}
@@ -66,7 +68,7 @@
ImmutableList.Builder<CycleInfo> cycleBuilder = ImmutableList.builder();
Exception firstException = null;
SkyKey firstChildKey = null;
- boolean isTransient = false;
+ boolean isTransitivelyTransient = false;
boolean isCatastrophic = false;
for (ErrorInfo child : childErrors) {
if (firstException == null) {
@@ -76,7 +78,7 @@
}
rootCausesBuilder.addTransitive(child.rootCauses);
cycleBuilder.addAll(CycleInfo.prepareCycles(currentValue, child.cycles));
- isTransient |= child.isTransient();
+ isTransitivelyTransient |= child.isTransitivelyTransient();
isCatastrophic |= child.isCatastrophic();
}
@@ -85,7 +87,8 @@
firstException,
firstChildKey,
cycleBuilder.build(),
- isTransient,
+ /*isDirectlyTransient=*/ false,
+ isTransitivelyTransient,
isCatastrophic);
}
@@ -96,11 +99,17 @@
private final ImmutableList<CycleInfo> cycles;
- private final boolean isTransient;
+ private final boolean isDirectlyTransient;
+ private final boolean isTransitivelyTransient;
private final boolean isCatastrophic;
- public ErrorInfo(NestedSet<SkyKey> rootCauses, @Nullable Exception exception,
- SkyKey rootCauseOfException, ImmutableList<CycleInfo> cycles, boolean isTransient,
+ public ErrorInfo(
+ NestedSet<SkyKey> rootCauses,
+ @Nullable Exception exception,
+ SkyKey rootCauseOfException,
+ ImmutableList<CycleInfo> cycles,
+ boolean isDirectlyTransient,
+ boolean isTransitivelyTransient,
boolean isCatostrophic) {
Preconditions.checkState(exception != null || !Iterables.isEmpty(cycles),
"At least one of exception and cycles must be non-null/empty, respectively");
@@ -112,7 +121,8 @@
this.exception = exception;
this.rootCauseOfException = rootCauseOfException;
this.cycles = cycles;
- this.isTransient = isTransient;
+ this.isDirectlyTransient = isDirectlyTransient;
+ this.isTransitivelyTransient = isTransitivelyTransient;
this.isCatastrophic = isCatostrophic;
}
@@ -162,11 +172,19 @@
}
/**
+ * Returns true iff the error is directly transient, i.e. if there was a transient error
+ * encountered during the computation itself.
+ */
+ public boolean isDirectlyTransient() {
+ return isDirectlyTransient;
+ }
+
+ /**
* Returns true iff the error is transitively transient, i.e. if retrying the same computation
* could lead to a different result.
*/
- public boolean isTransient() {
- return isTransient;
+ public boolean isTransitivelyTransient() {
+ return isTransitivelyTransient;
}
/**
diff --git a/src/main/java/com/google/devtools/build/skyframe/ErrorInfoManager.java b/src/main/java/com/google/devtools/build/skyframe/ErrorInfoManager.java
new file mode 100644
index 0000000..01ca34b
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/skyframe/ErrorInfoManager.java
@@ -0,0 +1,65 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.skyframe;
+
+import com.google.devtools.build.skyframe.SkyFunctionException.ReifiedSkyFunctionException;
+import java.util.Collection;
+import javax.annotation.Nullable;
+
+/** Used by {@link ParallelEvaluator} to produce and consume {@link ErrorInfo} instances. */
+public interface ErrorInfoManager {
+ ErrorInfo fromException(
+ SkyKey key,
+ ReifiedSkyFunctionException skyFunctionException,
+ boolean isTransitivelyTransient);
+
+ /**
+ * Returns the {@link ErrorInfo} to use when there isn't currently one because
+ * {@link SkyFunction#compute} didn't throw a {@link SkyFunctionException} and there was no cycle.
+ */
+ @Nullable
+ ErrorInfo getErrorInfoToUse(
+ SkyKey skyKey,
+ boolean hasValue,
+ Collection<ErrorInfo> childErrorInfos);
+
+ /**
+ * Trivial {@link ErrorInfoManager} implementation whose {@link #fromException} simply uses
+ * {@link ErrorInfo#fromException} and whose {@link #getErrorInfoToUse} makes an {@link ErrorInfo}
+ * from the given {@code childErrorInfos}.
+ */
+ static class UseChildErrorInfoIfNecessary implements ErrorInfoManager {
+ public static final UseChildErrorInfoIfNecessary INSTANCE = new UseChildErrorInfoIfNecessary();
+
+ private UseChildErrorInfoIfNecessary() {
+ }
+
+ @Override
+ public ErrorInfo fromException(
+ SkyKey key,
+ ReifiedSkyFunctionException skyFunctionException,
+ boolean isTransitivelyTransient) {
+ return ErrorInfo.fromException(skyFunctionException, isTransitivelyTransient);
+ }
+
+ @Override
+ @Nullable
+ public ErrorInfo getErrorInfoToUse(
+ SkyKey skyKey,
+ boolean hasValue,
+ Collection<ErrorInfo> childErrorInfos) {
+ return !childErrorInfos.isEmpty() ? ErrorInfo.fromChildErrors(skyKey, childErrorInfos) : null;
+ }
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
index 4ebde56..504de3a 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
@@ -176,6 +176,7 @@
eventHandler,
emittedEventState,
DEFAULT_STORED_EVENT_FILTER,
+ ErrorInfoManager.UseChildErrorInfoIfNecessary.INSTANCE,
keepGoing,
numThreads,
progressReceiver);
diff --git a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java
index 0904405..beb4a25 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java
@@ -105,6 +105,7 @@
final ExtendedEventHandler reporter,
EmittedEventState emittedEventState,
EventFilter storedEventFilter,
+ ErrorInfoManager errorInfoManager,
boolean keepGoing,
int threadCount,
DirtyTrackingProgressReceiver progressReceiver) {
@@ -117,9 +118,9 @@
reporter,
emittedEventState,
keepGoing,
- /*storeErrorsAlongsideValues=*/ true,
progressReceiver,
storedEventFilter,
+ errorInfoManager,
createEvaluateRunnable(),
threadCount);
cycleDetector = new SimpleCycleDetector();
@@ -132,14 +133,13 @@
final ExtendedEventHandler reporter,
EmittedEventState emittedEventState,
EventFilter storedEventFilter,
+ ErrorInfoManager errorInfoManager,
boolean keepGoing,
- boolean storeErrorsAlongsideValues,
DirtyTrackingProgressReceiver progressReceiver,
ForkJoinPool forkJoinPool,
CycleDetector cycleDetector) {
this.graph = graph;
this.cycleDetector = cycleDetector;
- Preconditions.checkState(storeErrorsAlongsideValues || keepGoing);
evaluatorContext =
new ParallelEvaluatorContext(
graph,
@@ -148,9 +148,9 @@
reporter,
emittedEventState,
keepGoing,
- storeErrorsAlongsideValues,
progressReceiver,
storedEventFilter,
+ errorInfoManager,
createEvaluateRunnable(),
Preconditions.checkNotNull(forkJoinPool));
}
@@ -449,15 +449,16 @@
}
ErrorInfo depError = depEntry.getErrorInfo();
if (depError != null) {
- isTransitivelyTransient |= depError.isTransient();
+ isTransitivelyTransient |= depError.isTransitivelyTransient();
}
}
- ErrorInfo errorInfo =
- ErrorInfo.fromException(reifiedBuilderException, isTransitivelyTransient);
+ ErrorInfo errorInfo = evaluatorContext.getErrorInfoManager().fromException(
+ skyKey,
+ reifiedBuilderException,
+ isTransitivelyTransient);
registerNewlyDiscoveredDepsForDoneEntry(
skyKey, state, newlyRequestedDeps, oldDeps, env);
- env.setError(
- state, errorInfo, /*isDirectlyTransient=*/ reifiedBuilderException.isTransient());
+ env.setError(state, errorInfo);
env.commit(
state,
evaluatorContext.keepGoing()
diff --git a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluatorContext.java b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluatorContext.java
index 49782ce..2f585ff 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluatorContext.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluatorContext.java
@@ -47,9 +47,10 @@
private final ExtendedEventHandler reporter;
private final NestedSetVisitor<TaggedEvents> replayingNestedSetEventVisitor;
private final boolean keepGoing;
- private final boolean storeErrorsAlongsideValues;
private final DirtyTrackingProgressReceiver progressReceiver;
private final EventFilter storedEventFilter;
+ private final ErrorInfoManager errorInfoManager;
+
/**
* The visitor managing the thread pool. Used to enqueue parents when an entry is finished, and,
* during testing, to block until an exception is thrown if a node builder requests that.
@@ -65,9 +66,9 @@
ExtendedEventHandler reporter,
EmittedEventState emittedEventState,
boolean keepGoing,
- boolean storeErrorsAlongsideValues,
final DirtyTrackingProgressReceiver progressReceiver,
EventFilter storedEventFilter,
+ ErrorInfoManager errorInfoManager,
final Function<SkyKey, Runnable> runnableMaker,
final int threadCount) {
this.graph = graph;
@@ -77,9 +78,9 @@
this.replayingNestedSetEventVisitor =
new NestedSetVisitor<>(new NestedSetEventReceiver(reporter), emittedEventState);
this.keepGoing = keepGoing;
- this.storeErrorsAlongsideValues = storeErrorsAlongsideValues;
this.progressReceiver = Preconditions.checkNotNull(progressReceiver);
this.storedEventFilter = storedEventFilter;
+ this.errorInfoManager = errorInfoManager;
visitorSupplier =
Suppliers.memoize(
new Supplier<NodeEntryVisitor>() {
@@ -98,9 +99,9 @@
ExtendedEventHandler reporter,
EmittedEventState emittedEventState,
boolean keepGoing,
- boolean storeErrorsAlongsideValues,
final DirtyTrackingProgressReceiver progressReceiver,
EventFilter storedEventFilter,
+ ErrorInfoManager errorInfoManager,
final Function<SkyKey, Runnable> runnableMaker,
final ForkJoinPool forkJoinPool) {
this.graph = graph;
@@ -110,9 +111,9 @@
this.replayingNestedSetEventVisitor =
new NestedSetVisitor<>(new NestedSetEventReceiver(reporter), emittedEventState);
this.keepGoing = keepGoing;
- this.storeErrorsAlongsideValues = storeErrorsAlongsideValues;
this.progressReceiver = Preconditions.checkNotNull(progressReceiver);
this.storedEventFilter = storedEventFilter;
+ this.errorInfoManager = errorInfoManager;
visitorSupplier =
Suppliers.memoize(
new Supplier<NodeEntryVisitor>() {
@@ -203,8 +204,8 @@
return storedEventFilter;
}
- boolean storeErrorsAlongsideValues() {
- return storeErrorsAlongsideValues;
+ ErrorInfoManager getErrorInfoManager() {
+ return errorInfoManager;
}
/** Receives the events from the NestedSet and delegates to the reporter. */
diff --git a/src/main/java/com/google/devtools/build/skyframe/SimpleCycleDetector.java b/src/main/java/com/google/devtools/build/skyframe/SimpleCycleDetector.java
index 2adbbc5..4a8acc1 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SimpleCycleDetector.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SimpleCycleDetector.java
@@ -160,8 +160,7 @@
directDeps,
Sets.difference(entry.getAllRemainingDirtyDirectDeps(), removedDeps),
evaluatorContext);
- env.setError(
- entry, ErrorInfo.fromChildErrors(key, errorDeps), /*isDirectlyTransient=*/ false);
+ env.setError(entry, ErrorInfo.fromChildErrors(key, errorDeps));
env.commit(entry, EnqueueParentBehavior.SIGNAL);
} else {
entry = evaluatorContext.getGraph().get(null, Reason.CYCLE_CHECKING, key);
@@ -220,7 +219,7 @@
CycleInfo cycleInfo = new CycleInfo(cycle);
// Add in this cycle.
allErrors.add(ErrorInfo.fromCycle(cycleInfo));
- env.setError(entry, ErrorInfo.fromChildErrors(key, allErrors), /*isTransient=*/ false);
+ env.setError(entry, ErrorInfo.fromChildErrors(key, allErrors));
env.commit(entry, EnqueueParentBehavior.SIGNAL);
continue;
} else {
diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
index 00f4fa8..427ec94 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
@@ -226,27 +226,6 @@
return postBuilder.build();
}
- /**
- * If this node has an error, that is, if errorInfo is non-null, do nothing. Otherwise, set
- * errorInfo to the union of the child errors that were recorded earlier by getValueOrException,
- * if there are any.
- *
- * <p>Child errors are remembered, if there are any and yet the parent recovered without error, so
- * that subsequent noKeepGoing evaluations can stop as soon as they encounter a node whose
- * (transitive) children had experienced an error, even if that (transitive) parent node had been
- * able to recover from it during a keepGoing build. This behavior can be suppressed by setting
- * {@link ParallelEvaluatorContext#storeErrorsAlongsideValues} to false, which will cause nodes
- * with values to have no stored error info. This may be useful if this graph will only ever be
- * used for keepGoing builds, since in that case storing errors from recovered nodes is pointless.
- */
- private void finalizeErrorInfo() {
- if (errorInfo == null
- && (evaluatorContext.storeErrorsAlongsideValues() || value == null)
- && !childErrorInfos.isEmpty()) {
- errorInfo = ErrorInfo.fromChildErrors(skyKey, childErrorInfos);
- }
- }
-
void setValue(SkyValue newValue) {
Preconditions.checkState(
errorInfo == null && bubbleErrorInfo == null,
@@ -264,12 +243,11 @@
* dependencies of this node <i>must</i> already have been registered, since this method may
* register a dependence on the error transience node, which should always be the last dep.
*/
- void setError(NodeEntry state, ErrorInfo errorInfo, boolean isDirectlyTransient)
- throws InterruptedException {
+ void setError(NodeEntry state, ErrorInfo errorInfo) throws InterruptedException {
Preconditions.checkState(value == null, "%s %s %s", skyKey, value, errorInfo);
Preconditions.checkState(this.errorInfo == null, "%s %s %s", skyKey, this.errorInfo, errorInfo);
- if (isDirectlyTransient) {
+ if (errorInfo.isDirectlyTransient()) {
NodeEntry errorTransienceNode =
Preconditions.checkNotNull(
evaluatorContext
@@ -541,7 +519,10 @@
void commit(NodeEntry primaryEntry, EnqueueParentBehavior enqueueParents)
throws InterruptedException {
// Construct the definitive error info, if there is one.
- finalizeErrorInfo();
+ if (errorInfo == null) {
+ errorInfo = evaluatorContext.getErrorInfoManager().getErrorInfoToUse(
+ skyKey, value != null, childErrorInfos);
+ }
// We have the following implications:
// errorInfo == null => value != null => enqueueParents.
diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionException.java b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionException.java
index 54ad3c7..0db7fa6 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionException.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionException.java
@@ -78,7 +78,7 @@
}
@Nullable
- final SkyKey getRootCauseSkyKey() {
+ public final SkyKey getRootCauseSkyKey() {
return rootCause;
}
@@ -124,9 +124,17 @@
private final boolean isCatastrophic;
public ReifiedSkyFunctionException(SkyFunctionException e, SkyKey key) {
- super(e.getCause(), e.transience, Preconditions.checkNotNull(e.getRootCauseSkyKey() == null
- ? key : e.getRootCauseSkyKey()));
- this.isCatastrophic = e.isCatastrophic();
+ this(e.getCause(), e.transience, key, e.getRootCauseSkyKey(), e.isCatastrophic());
+ }
+
+ protected ReifiedSkyFunctionException(
+ Exception cause,
+ Transience transience,
+ SkyKey key,
+ @Nullable SkyKey rootCauseSkyKey,
+ boolean isCatastrophic) {
+ super(cause, transience, rootCauseSkyKey == null ? key : rootCauseSkyKey);
+ this.isCatastrophic = isCatastrophic;
}
@Override
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
index f9ff08e..ea00194 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
@@ -681,6 +681,24 @@
assertThat(errorInfo.getException()).hasCauseThat().isSameAs(exn);
}
+ @Test
+ public void testPackageLoadingErrorOnIOExceptionReadingBzlFile() throws Exception {
+ scratch.file("foo/BUILD", "load('//foo:bzl.bzl', 'x')");
+ Path fooBzlFilePath = scratch.file("foo/bzl.bzl");
+ IOException exn = new IOException("nope");
+ fs.throwExceptionOnGetInputStream(fooBzlFilePath, exn);
+
+ SkyKey skyKey = PackageValue.key(PackageIdentifier.parse("@//foo"));
+ EvaluationResult<PackageValue> result = SkyframeExecutorTestUtils.evaluate(
+ getSkyframeExecutor(), skyKey, /*keepGoing=*/false, reporter);
+ assertThat(result.hasError()).isTrue();
+ ErrorInfo errorInfo = result.getError(skyKey);
+ String errorMessage = errorInfo.getException().getMessage();
+ assertThat(errorMessage).contains("nope");
+ assertThat(errorInfo.getException()).isInstanceOf(NoSuchPackageException.class);
+ assertThat(errorInfo.getException()).hasCauseThat().isSameAs(exn);
+ }
+
private static class CustomInMemoryFs extends InMemoryFileSystem {
private abstract static class FileStatusOrException {
abstract FileStatus get() throws IOException;
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 2d121cb..e42ac6e 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
@@ -811,7 +811,7 @@
EvaluationResult<SkyValue> result = eval(key);
assertThat(result.hasError()).isTrue();
ErrorInfo error = result.getError(key);
- assertThat(error.isTransient()).isFalse();
+ assertThat(error.isTransitivelyTransient()).isFalse();
assertThat(error.getException())
.hasMessageThat()
.contains("Generated directory a/b/c conflicts with package under the same path.");
diff --git a/src/test/java/com/google/devtools/build/skyframe/EagerInvalidatorTest.java b/src/test/java/com/google/devtools/build/skyframe/EagerInvalidatorTest.java
index d91e5ca..95781b6 100644
--- a/src/test/java/com/google/devtools/build/skyframe/EagerInvalidatorTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/EagerInvalidatorTest.java
@@ -140,6 +140,7 @@
reporter,
new MemoizingEvaluator.EmittedEventState(),
InMemoryMemoizingEvaluator.DEFAULT_STORED_EVENT_FILTER,
+ ErrorInfoManager.UseChildErrorInfoIfNecessary.INSTANCE,
keepGoing,
200,
new DirtyTrackingProgressReceiver(null));
diff --git a/src/test/java/com/google/devtools/build/skyframe/ErrorInfoSubject.java b/src/test/java/com/google/devtools/build/skyframe/ErrorInfoSubject.java
index 4551494..e3fd258 100644
--- a/src/test/java/com/google/devtools/build/skyframe/ErrorInfoSubject.java
+++ b/src/test/java/com/google/devtools/build/skyframe/ErrorInfoSubject.java
@@ -46,13 +46,13 @@
}
public void isTransient() {
- if (!getSubject().isTransient()) {
+ if (!getSubject().isTransitivelyTransient()) {
fail("is transient");
}
}
public void isNotTransient() {
- if (getSubject().isTransient()) {
+ if (getSubject().isTransitivelyTransient()) {
fail("is not transient");
}
}
diff --git a/src/test/java/com/google/devtools/build/skyframe/ErrorInfoTest.java b/src/test/java/com/google/devtools/build/skyframe/ErrorInfoTest.java
index 6dbeede..a003914 100644
--- a/src/test/java/com/google/devtools/build/skyframe/ErrorInfoTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/ErrorInfoTest.java
@@ -59,7 +59,9 @@
assertThat(errorInfo.getException()).isSameAs(exception);
assertThat(errorInfo.getRootCauseOfException()).isSameAs(causeOfException);
assertThat(errorInfo.getCycleInfo()).isEmpty();
- assertThat(errorInfo.isTransient()).isEqualTo(isDirectlyTransient || isTransitivelyTransient);
+ assertThat(errorInfo.isDirectlyTransient()).isEqualTo(isDirectlyTransient);
+ assertThat(errorInfo.isTransitivelyTransient()).isEqualTo(
+ isDirectlyTransient || isTransitivelyTransient);
assertThat(errorInfo.isCatastrophic()).isFalse();
}
@@ -95,7 +97,7 @@
assertThat(errorInfo.getRootCauses()).isEmpty();
assertThat(errorInfo.getException()).isNull();
assertThat(errorInfo.getRootCauseOfException()).isNull();
- assertThat(errorInfo.isTransient()).isFalse();
+ assertThat(errorInfo.isTransitivelyTransient()).isFalse();
assertThat(errorInfo.isCatastrophic()).isFalse();
}
@@ -141,7 +143,7 @@
new CycleInfo(
ImmutableList.of(currentKey, Iterables.getOnlyElement(cycle.getPathToCycle())),
cycle.getCycle()));
- assertThat(errorInfo.isTransient()).isTrue();
+ assertThat(errorInfo.isTransitivelyTransient()).isTrue();
assertThat(errorInfo.isCatastrophic()).isTrue();
}
@@ -154,6 +156,7 @@
/*rootCauseOfException=*/ null,
ImmutableList.<CycleInfo>of(),
false,
+ false,
false);
} catch (IllegalStateException e) {
// Brittle, but confirms we failed for the right reason.
@@ -171,6 +174,7 @@
/*rootCauseOfException=*/ null,
ImmutableList.<CycleInfo>of(),
false,
+ false,
false);
} catch (IllegalStateException e) {
// Brittle, but confirms we failed for the right reason.
diff --git a/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java b/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
index 8842f84..314d4ac 100644
--- a/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
@@ -1200,7 +1200,7 @@
if (errorsStoredAlongsideValues) {
// The parent should be transitively transient, since it transitively depends on a transient
// error.
- assertThat(errorInfo.isTransient()).isTrue();
+ assertThat(errorInfo.isTransitivelyTransient()).isTrue();
assertThat(errorInfo.getException()).hasMessage(NODE_TYPE.getName() + ":errorKey");
assertThat(errorInfo.getRootCauseOfException()).isEqualTo(errorKey);
} else {
diff --git a/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java b/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java
index 109b2fb..094f75a 100644
--- a/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java
@@ -99,6 +99,7 @@
new Reporter(new EventBus(), eventCollector),
new MemoizingEvaluator.EmittedEventState(),
storedEventFilter,
+ ErrorInfoManager.UseChildErrorInfoIfNecessary.INSTANCE,
keepGoing,
150,
revalidationReceiver);
