Skip to content

Commit 32563ca

Browse files
joelebacopybara-github
authored andcommitted
[Skymeld] Avoid printing extra WARNINGS for execution failures in -k.
This is a divergence from the non-skymeld behavior: we should only _log_ unusual exceptions in the execution phase and don't _print_ any warning. PiperOrigin-RevId: 566246387 Change-Id: I1fb26dab8f40cef2f179d54ccbf3365d71df478f
1 parent 11cf1b7 commit 32563ca

File tree

3 files changed

+44
-11
lines changed

3 files changed

+44
-11
lines changed

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

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ static ErrorProcessingResult processErrors(
311311
boolean isExecutionException = isExecutionException(cause);
312312
if (keepGoing) {
313313
aggregatingResultBuilder.aggregateSingleResult(individualErrorProcessingResult);
314-
printWarningMessage(isExecutionException, label, eventHandler);
314+
logOrPrintWarnings(isExecutionException, label, eventHandler, cause);
315315
} else {
316316
noKeepGoingAnalysisExceptionAspect =
317317
throwOrReturnAspectAnalysisException(
@@ -569,17 +569,29 @@ private static DetailedExitCode sendBugReportAndCreateUnknownExecutionDetailedEx
569569
return createDetailedExecutionExitCode(message, UNKNOWN_EXECUTION);
570570
}
571571

572-
private static void printWarningMessage(
572+
private static void logOrPrintWarnings(
573573
boolean isExecutionException,
574574
@Nullable Label topLevelLabel,
575-
ExtendedEventHandler eventHandler) {
576-
String warningMsg =
577-
isExecutionException
578-
? String.format("errors encountered while building target '%s'", topLevelLabel)
579-
: String.format(
575+
ExtendedEventHandler eventHandler,
576+
Exception cause) {
577+
// For execution exceptions, we don't print any extra warning.
578+
if (isExecutionException) {
579+
if (isExecutionCauseWorthLogging(cause)) {
580+
logger.atWarning().withCause(cause).log(
581+
"Non-action-execution/input-error exception while building target %s", topLevelLabel);
582+
}
583+
return;
584+
}
585+
eventHandler.handle(
586+
Event.warn(
587+
String.format(
580588
"errors encountered while analyzing target '%s': it will not be built",
581-
topLevelLabel);
582-
eventHandler.handle(Event.warn(warningMsg));
589+
topLevelLabel)));
590+
}
591+
592+
private static boolean isExecutionCauseWorthLogging(Throwable cause) {
593+
return !(cause instanceof ActionExecutionException)
594+
&& !(cause instanceof InputFileErrorException);
583595
}
584596

585597
private static boolean isValidErrorKeyType(Object errorKey) {
@@ -842,8 +854,7 @@ private static DetailedExitCode getDetailedExitCodeKeepGoing(EvaluationResult<?>
842854
detailedExitCode =
843855
DetailedExitCodeComparator.chooseMoreImportantWithFirstIfTie(
844856
detailedExitCode, ((DetailedException) cause).getDetailedExitCode());
845-
if (!(cause instanceof ActionExecutionException)
846-
&& !(cause instanceof InputFileErrorException)) {
857+
if (isExecutionCauseWorthLogging(cause)) {
847858
logger.atWarning().withCause(cause).log(
848859
"Non-action-execution/input-error exception for %s", error);
849860
}

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -628,6 +628,24 @@ public void targetAnalysisFailureNullBuild_correctErrorsPropagated(
628628
+ " exist");
629629
}
630630

631+
// Regression test for b/300391729.
632+
@Test
633+
public void executionFailure_keepGoing_doesNotSpamWarnings() throws Exception {
634+
addOptions("--keep_going");
635+
writeExecutionFailureAspectBzl();
636+
write(
637+
"foo/BUILD",
638+
"cc_library(name = 'foo', srcs = ['foo.cc'], deps = [':bar'])",
639+
"cc_library(name = 'bar', srcs = ['bar.cc'])");
640+
write("foo/foo.cc");
641+
write("foo/bar.cc");
642+
addOptions("--aspects=//foo:aspect.bzl%execution_err_aspect", "--output_groups=files");
643+
644+
assertThrows(BuildFailedException.class, () -> buildTarget("//foo/..."));
645+
// No warnings.
646+
events.assertNoWarnings();
647+
}
648+
631649
private void assertSingleAnalysisPhaseCompleteEventWithLabels(String... labels) {
632650
assertThat(eventsSubscriber.getAnalysisPhaseCompleteEvents()).hasSize(1);
633651
AnalysisPhaseCompleteEvent analysisPhaseCompleteEvent =

src/test/java/com/google/devtools/build/lib/events/util/EventCollectionApparatus.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,10 @@ public void assertNoWarningsOrErrors() {
159159
MoreAsserts.assertNoEvents(errors());
160160
}
161161

162+
public void assertNoWarnings() {
163+
MoreAsserts.assertNoEvents(warnings());
164+
}
165+
162166
/**
163167
* Utility method: Assert that the {@link #collector()} has received an
164168
* info message with the {@code expectedMessage}.

0 commit comments

Comments
 (0)