[7.0.0] Add more details to the warning message in --keep_going mode (b…

…azelbuild#20450)

The current message is too generic and not helpful for the user.

```
Before:
WARNING: errors encountered while analyzing target '//:bin': it will not be built

---
After:
WARNING: errors encountered while analyzing target '//:bin', it will not be built.
Target //:bin is incompatible and cannot be built, but was explicitly requested.
Dependency chain:
    //:bin (8250b5)   <-- target platform (@@local_config_platform//:host) didn't satisfy constraint @@platforms//:incompatible

```

Fixes bazelbuild#20443

Commit
bazelbuild@c5493b9

PiperOrigin-RevId: 588363260
Change-Id: I70e151cd9baa6a0dd7b307b7ddeb9f43ee30b526

Co-authored-by: Googler <[email protected]>

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

@@ -270,8 +270,9 @@ static ErrorProcessingResult processErrors(
       } else {
         assertValidAnalysisException(errorInfo, errorKey, result.getWalkableGraph(), keepEdges);
       }
-      Exception cause = errorInfo.getException();
-      Preconditions.checkState(cause != null || !errorInfo.getCycleInfo().isEmpty(), errorInfo);
+      Exception nullableCause = errorInfo.getException();
+      Preconditions.checkState(
+          nullableCause != null || !errorInfo.getCycleInfo().isEmpty(), errorInfo);
 
       if (inBuildViewTest && !isValidErrorKeyType(errorKey.argument())) {
         // This means that we are in a BuildViewTestCase.
@@ -308,15 +309,15 @@ static ErrorProcessingResult processErrors(
           label,
           individualErrorProcessingResult);
 
-      boolean isExecutionException = isExecutionException(cause);
+      boolean isExecutionException = isExecutionException(nullableCause);
       if (keepGoing) {
         aggregatingResultBuilder.aggregateSingleResult(individualErrorProcessingResult);
-        logOrPrintWarnings(isExecutionException, label, eventHandler, cause);
+        logOrPrintWarningsKeepGoing(isExecutionException, label, eventHandler, nullableCause);
       } else {
         noKeepGoingAnalysisExceptionAspect =
             throwOrReturnAspectAnalysisException(
                 result,
-                cause,
+                nullableCause,
                 bugReporter,
                 errorKey,
                 isExecutionException,
@@ -391,14 +392,16 @@ private static void maybePostFailureEventsForNonConflictError(
    */
   private static ViewCreationFailedException throwOrReturnAspectAnalysisException(
       EvaluationResult<? extends SkyValue> result,
-      Exception cause,
+      @Nullable Exception cause,
       BugReporter bugReporter,
       SkyKey errorKey,
       boolean isExecutionException,
       boolean hasExecutionCycle)
       throws BuildFailedException, TestExecException, ViewCreationFailedException {
     // If the error is execution-related: straightaway rethrow. No further steps required.
     if (isExecutionException) {
+      // cause is not null for execution exceptions.
+      Preconditions.checkNotNull(cause);
       rethrow(cause, bugReporter, result);
     }
     // If a --nokeep_going build found a cycle, that means there were no other errors thrown
@@ -569,11 +572,11 @@ private static DetailedExitCode sendBugReportAndCreateUnknownExecutionDetailedEx
     return createDetailedExecutionExitCode(message, UNKNOWN_EXECUTION);
   }
 
-  private static void logOrPrintWarnings(
+  private static void logOrPrintWarningsKeepGoing(
       boolean isExecutionException,
       @Nullable Label topLevelLabel,
       ExtendedEventHandler eventHandler,
-      Exception cause) {
+      @Nullable Exception cause) {
     // For execution exceptions, we don't print any extra warning.
     if (isExecutionException) {
       if (isExecutionCauseWorthLogging(cause)) {
@@ -582,11 +585,13 @@ private static void logOrPrintWarnings(
       }
       return;
     }
-    eventHandler.handle(
-        Event.warn(
-            String.format(
-                "errors encountered while analyzing target '%s': it will not be built",
-                topLevelLabel)));
+    var message =
+        String.format(
+            "errors encountered while analyzing target '%s', it will not be built.", topLevelLabel);
+    if (cause != null) {
+      message += String.format("\n%s", cause.getMessage());
+    }
+    eventHandler.handle(Event.warn(message));
   }
 
   private static boolean isExecutionCauseWorthLogging(Throwable cause) {
@@ -777,7 +782,7 @@ private static DetailedException convertToAnalysisException(Throwable cause) {
     return null;
   }
 
-  private static boolean isExecutionException(Throwable cause) {
+  private static boolean isExecutionException(@Nullable Throwable cause) {
     return cause instanceof ActionExecutionException
         || cause instanceof InputFileErrorException
         || cause instanceof TestExecException

src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java

@@ -1066,9 +1066,9 @@ public void testCircularDependencyWithKeepGoing() throws Exception {
     assertContainsEvent("in cc_library rule //cycle:foo: cycle in dependency graph:");
     assertContainsEvent("in cc_library rule //cycle:bas: cycle in dependency graph:");
     assertContainsEvent(
-        "errors encountered while analyzing target '//cycle:foo': it will not be built");
+        "errors encountered while analyzing target '//cycle:foo', it will not be built");
     assertContainsEvent(
-        "errors encountered while analyzing target '//cycle:bat': it will not be built");
+        "errors encountered while analyzing target '//cycle:bat', it will not be built");
     // With interleaved loading and analysis, we can no longer distinguish loading-phase cycles
     // and analysis-phase cycles. This was previously reported as a loading-phase cycle, as it
     // happens with any configuration (cycle is hard-coded in the BUILD files). Also see the

src/test/java/com/google/devtools/build/lib/buildtool/SkymeldBuildIntegrationTest.java

@@ -302,6 +302,30 @@ public void targetAnalysisFailure_consistentWithNonSkymeld(
     events.assertContainsError("rule '//foo:missing' does not exist");
   }
 
+  // Regression test for https://github.com/bazelbuild/bazel/issues/20443
+  @Test
+  public void testKeepGoingWarningContainsDetails() throws Exception {
+    addOptions("--keep_going");
+    write(
+        "foo/BUILD",
+        "constraint_setting(name = 'incompatible_setting')",
+        "constraint_value(",
+        "    name = 'incompatible',",
+        "    constraint_setting = ':incompatible_setting',",
+        "    visibility = ['//visibility:public']",
+        ")",
+        "cc_library(",
+        "    name = 'foo',",
+        "    srcs = ['foo.cc'],",
+        "    target_compatible_with = ['//foo:incompatible']",
+        ")");
+    assertThrows(BuildFailedException.class, () -> buildTarget("//foo:foo"));
+    events.assertContainsWarning(
+        "errors encountered while analyzing target '//foo:foo', it will not be built.");
+    // The details.
+    events.assertContainsWarning("Dependency chain:");
+  }
+
   @Test
   public void analysisAndExecutionFailure_keepGoing_bothReported() throws Exception {
     addOptions("--keep_going");