Don't require --keep_going to discard graph edges. It's unnecessary.

PiperOrigin-RevId: 174202685

4 files changed, 19 insertions, 18 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
index 364f9957b3..1cd9c2b841 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
@@ -497,7 +497,7 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor {
       // Some blaze commands don't include the view options. Don't bother with them.
       return;
     }
-    if (batch && viewOptions.keepGoing && viewOptions.discardAnalysisCache) {
+    if (batch && viewOptions.discardAnalysisCache) {
       Preconditions.checkState(
           incrementalState == IncrementalState.NORMAL,
           "May only be called once if successful: %s",
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 4737a6ec23..54b0d02581 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java
@@ -199,16 +199,6 @@ public class ParallelEvaluator extends AbstractParallelEvaluator implements Eval
       }
     }
 
-    // We delay this check until we know that some kind of evaluation is necessary, since !keepGoing
-    // and !keepsEdges are incompatible only in the case of a failed evaluation -- there is no
-    // need to be overly harsh to callers who are just trying to retrieve a cached result.
-    Preconditions.checkState(
-        evaluatorContext.keepGoing()
-            || !(graph instanceof InMemoryGraphImpl)
-            || ((InMemoryGraphImpl) graph).keepsEdges(),
-        "nokeep_going evaluations are not allowed if graph edges are not kept: %s",
-        skyKeys);
-
     Profiler.instance().startTask(ProfilerTask.SKYFRAME_EVAL, skyKeySet);
     try {
       return doMutatingEvaluation(skyKeySet);
diff --git a/src/test/shell/integration/discard_graph_edges_lib.sh b/src/test/shell/integration/discard_graph_edges_lib.sh
index 84770166ea..e5bea5ba47 100644
--- a/src/test/shell/integration/discard_graph_edges_lib.sh
+++ b/src/test/shell/integration/discard_graph_edges_lib.sh
@@ -17,7 +17,7 @@
 # discard_graph_edges_lib.sh: functions needed by discard_graph_edges_test.sh
 
 STARTUP_FLAGS="--batch"
-BUILD_FLAGS="--keep_going --discard_analysis_cache"
+BUILD_FLAGS="--discard_analysis_cache"
 
 function extract_histogram_count() {
   local histofile="$1"
diff --git a/src/test/shell/integration/discard_graph_edges_test.sh b/src/test/shell/integration/discard_graph_edges_test.sh
index 3857256a0b..7cc2d8435b 100755
--- a/src/test/shell/integration/discard_graph_edges_test.sh
+++ b/src/test/shell/integration/discard_graph_edges_test.sh
@@ -46,6 +46,22 @@ function test_test() {
     || fail "Expected success"
 }
 
+function test_failed_build() {
+  mkdir -p foo || fail "Couldn't make directory"
+  cat > foo/BUILD <<'EOF' || fail "Couldn't make BUILD file"
+cc_library(name = 'foo', srcs = ['foo.cc'], deps = [':bar'])
+cc_library(name = 'bar', srcs = ['bar.cc'])
+EOF
+  touch foo/foo.cc || fail "Couldn't make foo.cc"
+  echo "#ERROR" > foo/bar.cc || fail "Couldn't make bar.cc"
+  bazel $STARTUP_FLAGS build $BUILD_FLAGS //foo:foo >& "$TEST_log" \
+      && fail "Expected failure"
+  exit_code=$?
+  [ $exit_code -eq 1 ] || fail "Wrong exit code: $exit_code"
+  expect_log "#ERROR"
+  expect_not_log "Graph edges not stored"
+}
+
 # bazel info inherits from bazel build, but it doesn't have much in common with it.
 function test_info() {
   bazel $STARTUP_FLAGS info $BUILD_FLAGS >& $TEST_log || fail "Expected success"
@@ -265,7 +281,7 @@ genrule(name='foo',
 EOF
 
   # --nocache_test_results to make log-grepping easier.
-  bazel $STARTUP_FLAGS test $BUILD_FLAGS \
+  bazel $STARTUP_FLAGS test --keep_going $BUILD_FLAGS \
       --nocache_test_results //conflict:foo //testing:mytest >& $TEST_log \
       && fail "Expected failure"
   exit_code=$?
@@ -305,11 +321,6 @@ function test_no_batch() {
     || fail "Expected success"
 }
 
-function test_no_keep_going() {
-  bazel $STARTUP_FLAGS test $BUILD_FLAGS --nokeep_going //testing:mytest >& $TEST_log \
-    || fail "Expected success"
-}
-
 function test_no_discard_analysis_cache() {
   bazel $STARTUP_FLAGS test $BUILD_FLAGS --nodiscard_analysis_cache //testing:mytest >& $TEST_log \
     || fail "Expected success"