diff options
3 files changed, 27 insertions, 1 deletions
diff --git a/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java b/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java index 0fafa861a3..7b38324110 100644 --- a/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java +++ b/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java @@ -152,6 +152,9 @@ public interface NodeEntry extends ThinNodeEntry { * * <p>If the parameter is {@code null}, then no reverse dependency is added, but we still check * if the node needs to be scheduled. + * + * <p>If {@code reverseDep} is a rebuilding dirty entry that was already a reverse dep of this + * entry, then {@link #checkIfDoneForDirtyReverseDep} must be called instead. */ @ThreadSafe DependencyState addReverseDepAndCheckIfDone(@Nullable SkyKey reverseDep); 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 926a4c6ff1..b88ef42b61 100644 --- a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java +++ b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java @@ -1059,7 +1059,12 @@ public final class ParallelEvaluator implements Evaluator { Preconditions.checkState( newDirectDeps.contains(childErrorKey), "%s %s %s", state, childErrorKey, newDirectDeps); state.addTemporaryDirectDeps(GroupedListHelper.create(ImmutableList.of(childErrorKey))); - DependencyState childErrorState = childErrorEntry.addReverseDepAndCheckIfDone(skyKey); + DependencyState childErrorState; + if (oldDeps.contains(childErrorKey)) { + childErrorState = childErrorEntry.checkIfDoneForDirtyReverseDep(skyKey); + } else { + childErrorState = childErrorEntry.addReverseDepAndCheckIfDone(skyKey); + } Preconditions.checkState( childErrorState == DependencyState.DONE, "skyKey: %s, state: %s childErrorKey: %s", 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 c4f0047f3b..08b0f26c6b 100644 --- a/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java +++ b/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java @@ -596,6 +596,24 @@ public class MemoizingEvaluatorTest { } @Test + public void noKeepGoingErrorAfterKeepGoingError() throws Exception { + SkyKey topKey = GraphTester.skyKey("top"); + SkyKey errorKey = GraphTester.skyKey("error"); + tester.getOrCreate(errorKey).setHasError(true); + tester.getOrCreate(topKey).addDependency(errorKey).setComputedValue(CONCATENATE); + EvaluationResult<StringValue> result = tester.eval(/*keepGoing=*/ true, topKey); + assertThatEvaluationResult(result) + .hasErrorEntryForKeyThat(topKey) + .rootCauseOfExceptionIs(errorKey); + tester.getOrCreate(topKey, /*markAsModified=*/ true); + tester.invalidate(); + result = tester.eval(/*keepGoing=*/ false, topKey); + assertThatEvaluationResult(result) + .hasErrorEntryForKeyThat(topKey) + .rootCauseOfExceptionIs(errorKey); + } + + @Test public void transientErrorValueInvalidation() throws Exception { // Verify that invalidating errors causes all transient error values to be rerun. tester.getOrCreate("error-value").setHasTransientError(true).setProgress( |