Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.0.0] Add more details to the warning message in --keep_going mode #20450

Merged
merged 1 commit into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Loading