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