[7.0.0] Add more details to the warning message in --keep_going mode (#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 https://github.com/bazelbuild/bazel/issues/20443
Commit
https://github.com/bazelbuild/bazel/commit/c5493b9557502fe6ec48e78b0b5f06f4c1d75238
PiperOrigin-RevId: 588363260
Change-Id: I70e151cd9baa6a0dd7b307b7ddeb9f43ee30b526
Co-authored-by: Googler <leba@google.com>diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeErrorProcessor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeErrorProcessor.java
index b138996..5c51881 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeErrorProcessor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeErrorProcessor.java
@@ -270,8 +270,9 @@
} 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 @@
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,7 +392,7 @@
*/
private static ViewCreationFailedException throwOrReturnAspectAnalysisException(
EvaluationResult<? extends SkyValue> result,
- Exception cause,
+ @Nullable Exception cause,
BugReporter bugReporter,
SkyKey errorKey,
boolean isExecutionException,
@@ -399,6 +400,8 @@
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 @@
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 @@
}
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 @@
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
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
index 47feb01..e8058f8 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
@@ -1066,9 +1066,9 @@
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
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/SkymeldBuildIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/SkymeldBuildIntegrationTest.java
index e8dae66..2aead5d 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/SkymeldBuildIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/SkymeldBuildIntegrationTest.java
@@ -302,6 +302,30 @@
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");
