Correctly flag loading errors in the interleaved case.

In the interleaved case, loading errors can now also be discovered during the
analysis phase. Add a boolean flag to the SkyframeAnalysisResult to indicate
that such an error happened, and pass it through in BuildView.

Also refactor BuildView to simplify the code a bit - simply pass the
SkyframeAnalysisResult to the createResult method.

There is already an integration test for this - I'm adding a faster unit test
in BuildViewTest, as this is part of the BuildView contract.

--
MOS_MIGRATED_REVID=113716870

2 files changed, 21 insertions, 6 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAnalysisResult.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAnalysisResult.java
index 371ba8ff88..7db70f3e62 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAnalysisResult.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAnalysisResult.java
@@ -26,27 +26,39 @@ import java.util.Collection;
  *  Encapsulates the raw analysis result of top level targets and aspects coming from Skyframe.
  */
 public class SkyframeAnalysisResult {
-  private final boolean hasError;
+  private final boolean hasLoadingError;
+  private final boolean hasAnalysisError;
   private final ImmutableList<ConfiguredTarget> configuredTargets;
   private final WalkableGraph walkableGraph;
   private final ImmutableList<AspectValue> aspects;
   private final ImmutableMap<PackageIdentifier, Path> packageRoots;
 
   public SkyframeAnalysisResult(
-      boolean hasError,
+      boolean hasLoadingError,
+      boolean hasAnalysisError,
       ImmutableList<ConfiguredTarget> configuredTargets,
       WalkableGraph walkableGraph,
       ImmutableList<AspectValue> aspects,
       ImmutableMap<PackageIdentifier, Path> packageRoots) {
-    this.hasError = hasError;
+    this.hasLoadingError = hasLoadingError;
+    this.hasAnalysisError = hasAnalysisError;
     this.configuredTargets = configuredTargets;
     this.walkableGraph = walkableGraph;
     this.aspects = aspects;
     this.packageRoots = packageRoots;
   }
 
-  public boolean hasError() {
-    return hasError;
+  /**
+   * If the new simplified loading phase is enabled, then we can also see loading errors during the
+   * analysis phase. This method returns true if any such errors were encountered. However, you also
+   * always need to check if the loading result has an error! These will be merged eventually.
+   */
+  public boolean hasLoadingError() {
+    return hasLoadingError;
+  }
+
+  public boolean hasAnalysisError() {
+    return hasAnalysisError;
   }
 
   public Collection<ConfiguredTarget> getConfiguredTargets() {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
index 3f0aa18735..f97dae1f8a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -283,7 +283,7 @@ public final class SkyframeBuildView {
     if (!result.hasError() && badActions.isEmpty()) {
       setDeserializedArtifactOwners();
       return new SkyframeAnalysisResult(
-          false,
+          /*hasLoadingError=*/false, /*hasAnalysisError=*/false,
           ImmutableList.copyOf(goodCts),
           result.getWalkableGraph(),
           ImmutableList.copyOf(goodAspects),
@@ -336,6 +336,7 @@ public final class SkyframeBuildView {
       throw new ViewCreationFailedException(errorMsg);
     }
 
+    boolean hasLoadingError = false;
     // --keep_going : We notify the error and return a ConfiguredTargetValue
     for (Map.Entry<SkyKey, ErrorInfo> errorEntry : result.errorMap().entrySet()) {
       // Only handle errors of configured targets, not errors of top-level aspects.
@@ -356,6 +357,7 @@ public final class SkyframeBuildView {
       if (cause instanceof ConfiguredValueCreationException) {
         ConfiguredValueCreationException ctCause = (ConfiguredValueCreationException) cause;
         for (Label rootCause : ctCause.getRootCauses()) {
+          hasLoadingError = true;
           eventBus.post(new LoadingFailureEvent(topLevelLabel, rootCause));
         }
         analysisRootCause = ctCause.getAnalysisRootCause();
@@ -408,6 +410,7 @@ public final class SkyframeBuildView {
     }
     setDeserializedArtifactOwners();
     return new SkyframeAnalysisResult(
+        hasLoadingError,
         result.hasError() || !badActions.isEmpty(),
         ImmutableList.copyOf(goodCts),
         result.getWalkableGraph(),