Fix a hanging issue with skymeld & --combined_report=lcov.

With Skymeld, the combined report is done right before the end of the build. This works fine except for a buggy case:
- `--nokeep_going`
- when there's an error that the Coverage action transitively depends on
- The error would try to interrupt the build, and since we evaluate coverage actions uninterruptibly (consistent with the noskymeld behavior), we're stuck in an infinite loop [1].

This happened because we did not fail fast like we should have. This CL fixes the issue by checking, before generating the combined report, whether we should fail fast.

Fixes #21154

[1] https:/bazelbuild/bazel/blob/026f493a5a403fa5d770cae0b3a3baf8dcf33488/src/main/java/com/google/devtools/build/lib/concurrent/Uninterruptibles.java#L33-L39

PiperOrigin-RevId: 605572100
Change-Id: I8d57dacca58358771799161352819ec65bef6ac2

src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java

@@ -583,7 +583,7 @@ public SkyframeAnalysisResult analyzeAndExecuteTargets(
  BuildFailedException,
  TestExecException {
  Stopwatch analysisWorkTimer = Stopwatch.createStarted();
- EvaluationResult<SkyValue> evaluationResult;
+ EvaluationResult<SkyValue> mainEvaluationResult;
 
  var newKeys =
  ImmutableSet.<ActionLookupKey>builderWithExpectedSize(
@@ -660,7 +660,7 @@ public SkyframeAnalysisResult analyzeAndExecuteTargets(
  Profiler.instance().profile("skyframeExecutor.evaluateBuildDriverKeys")) {
  // Will be disabled later by the AnalysisOperationWatcher upon conclusion of analysis.
  enableAnalysis(true);
- evaluationResult =
+ mainEvaluationResult =
  skyframeExecutor.evaluateBuildDriverKeys(
  eventHandler,
  buildDriverCTKeys,
@@ -686,8 +686,10 @@ public SkyframeAnalysisResult analyzeAndExecuteTargets(
  }
 
  // The exclusive tests whose analysis succeeded i.e. those that can be run.
- ImmutableSet<ConfiguredTarget> exclusiveTestsToRun = getExclusiveTests(evaluationResult);
- boolean continueWithExclusiveTests = !evaluationResult.hasError() || keepGoing;
+ ImmutableSet<ConfiguredTarget> exclusiveTestsToRun =
+ getExclusiveTests(mainEvaluationResult);
+ boolean continueWithExclusiveTests = !mainEvaluationResult.hasError() || keepGoing;
+ boolean hasExclusiveTestsError = false;
 
  if (continueWithExclusiveTests && !exclusiveTestsToRun.isEmpty()) {
  skyframeExecutor.getIsBuildingExclusiveArtifacts().set(true);
@@ -704,6 +706,7 @@ public SkyframeAnalysisResult analyzeAndExecuteTargets(
  keepGoing,
  executors.executionParallelism());
  if (testRunResult.hasError()) {
+ hasExclusiveTestsError = true;
  detailedExitCodes.add(
  SkyframeErrorProcessor.processErrors(
  testRunResult,
@@ -718,29 +721,33 @@ public SkyframeAnalysisResult analyzeAndExecuteTargets(
  }
  }
  }
+
  // Coverage report generation should only be requested after all tests have executed.
  // We could generate baseline coverage artifacts earlier; it is only the timing of the
  // combined report that matters.
- ImmutableSet<Artifact> coverageArtifacts =
- coverageReportActionsWrapperSupplier.getCoverageArtifacts(
- buildResultListener.getAnalyzedTargets(), buildResultListener.getAnalyzedTests());
- eventBus.post(CoverageArtifactsKnownEvent.create(coverageArtifacts));
- additionalArtifactsResult =
- skyframeExecutor.evaluateSkyKeys(
- eventHandler, Artifact.keys(coverageArtifacts), keepGoing);
- eventBus.post(new CoverageActionFinishedEvent());
- if (additionalArtifactsResult.hasError()) {
- detailedExitCodes.add(
- SkyframeErrorProcessor.processErrors(
- additionalArtifactsResult,
- skyframeExecutor.getCyclesReporter(),
- eventHandler,
- keepGoing,
- skyframeExecutor.tracksStateForIncrementality(),
- eventBus,
- bugReporter,
- /* includeExecutionPhase= */ true)
- .executionDetailedExitCode());
+ // When --nokeep_going and there's an earlier error, we should skip this and fail fast.
+ if ((!mainEvaluationResult.hasError() && !hasExclusiveTestsError) || keepGoing) {
+ ImmutableSet<Artifact> coverageArtifacts =
+ coverageReportActionsWrapperSupplier.getCoverageArtifacts(
+ buildResultListener.getAnalyzedTargets(), buildResultListener.getAnalyzedTests());
+ eventBus.post(CoverageArtifactsKnownEvent.create(coverageArtifacts));
+ additionalArtifactsResult =
+ skyframeExecutor.evaluateSkyKeys(
+ eventHandler, Artifact.keys(coverageArtifacts), keepGoing);
+ eventBus.post(new CoverageActionFinishedEvent());
+ if (additionalArtifactsResult.hasError()) {
+ detailedExitCodes.add(
+ SkyframeErrorProcessor.processErrors(
+ additionalArtifactsResult,
+ skyframeExecutor.getCyclesReporter(),
+ eventHandler,
+ keepGoing,
+ skyframeExecutor.tracksStateForIncrementality(),
+ eventBus,
+ bugReporter,
+ /* includeExecutionPhase= */ true)
+ .executionDetailedExitCode());
+ }
  }
  } finally {
  // No more action execution beyond this point.
@@ -749,35 +756,35 @@ public SkyframeAnalysisResult analyzeAndExecuteTargets(
  resourceManager.resetResourceUsage();
  }
 
- if (!evaluationResult.hasError() && detailedExitCodes.isEmpty()) {
+ if (!mainEvaluationResult.hasError() && detailedExitCodes.isEmpty()) {
  ImmutableMap<AspectKey, ConfiguredAspect> successfulAspects =
  getSuccessfulAspectMap(
  topLevelAspectsKeys.size(),
- evaluationResult,
+ mainEvaluationResult,
  buildDriverAspectKeys,
- /*topLevelActionConflictReport=*/ null);
+ /* topLevelActionConflictReport= */ null);
  var targetsWithConfiguration =
  ImmutableList.<TargetAndConfiguration>builderWithExpectedSize(ctKeys.size());
  ImmutableSet<ConfiguredTarget> successfulConfiguredTargets =
  getSuccessfulConfiguredTargets(
  ctKeys.size(),
- evaluationResult,
+ mainEvaluationResult,
  buildDriverCTKeys,
  labelTargetMap,
  targetsWithConfiguration,
  /* topLevelActionConflictReport= */ null);
 
  return SkyframeAnalysisAndExecutionResult.success(
  successfulConfiguredTargets,
- evaluationResult.getWalkableGraph(),
+ mainEvaluationResult.getWalkableGraph(),
  successfulAspects,
  targetsWithConfiguration.build(),
  /* packageRoots= */ null);
  }
 
  ErrorProcessingResult errorProcessingResult =
  SkyframeErrorProcessor.processErrors(
- evaluationResult,
+ mainEvaluationResult,
  skyframeExecutor.getCyclesReporter(),
  eventHandler,
  keepGoing,
@@ -792,7 +799,7 @@ public SkyframeAnalysisResult analyzeAndExecuteTargets(
  foundActionConflictInLatestCheck
  ? handleActionConflicts(
  eventHandler,
- evaluationResult.getWalkableGraph(),
+ mainEvaluationResult.getWalkableGraph(),
  ctKeys,
  topLevelAspectsKeys,
  topLevelArtifactContext,
@@ -804,15 +811,15 @@ public SkyframeAnalysisResult analyzeAndExecuteTargets(
  ImmutableMap<AspectKey, ConfiguredAspect> successfulAspects =
  getSuccessfulAspectMap(
  topLevelAspectsKeys.size(),
- evaluationResult,
+ mainEvaluationResult,
  buildDriverAspectKeys,
  topLevelActionConflictReport);
  var targetsWithConfiguration =
  ImmutableList.<TargetAndConfiguration>builderWithExpectedSize(ctKeys.size());
  ImmutableSet<ConfiguredTarget> successfulConfiguredTargets =
  getSuccessfulConfiguredTargets(
  ctKeys.size(),
- evaluationResult,
+ mainEvaluationResult,
  buildDriverCTKeys,
  labelTargetMap,
  targetsWithConfiguration,
@@ -823,7 +830,7 @@ public SkyframeAnalysisResult analyzeAndExecuteTargets(
  /* hasAnalysisError= */ errorProcessingResult.hasAnalysisError(),
  /* hasActionConflicts= */ foundActionConflictInLatestCheck,
  successfulConfiguredTargets,
- evaluationResult.getWalkableGraph(),
+ mainEvaluationResult.getWalkableGraph(),
  successfulAspects,
  targetsWithConfiguration.build(),
  /* packageRoots= */ null,