From 8313dd076130935f6b596ec81b230cadf9ec31ef Mon Sep 17 00:00:00 2001 From: Matt Mackay Date: Mon, 16 Jan 2023 16:29:50 -0500 Subject: [PATCH] throw `InvalidConfigurationException` and add `FailureDetail` to `BuildCompletingEvent` --- .../build/lib/analysis/BuildView.java | 27 +++++++++++---- .../devtools/build/lib/buildeventstream/BUILD | 1 + .../BuildCompletingEvent.java | 26 ++++++++++++--- .../proto/build_event_stream.proto | 3 ++ .../AnalysisAndExecutionPhaseRunner.java | 33 +++++++++++++------ .../lib/buildtool/AnalysisPhaseRunner.java | 9 +++-- .../build/lib/buildtool/BuildTool.java | 9 +++-- .../buildevent/BuildCompleteEvent.java | 2 +- .../google/devtools/build/lib/skyframe/BUILD | 1 - .../build/lib/skyframe/SkyframeBuildView.java | 18 ++++------ .../devtools/build/lib/util/ExitCode.java | 3 -- src/main/protobuf/failure_details.proto | 1 + .../lib/analysis/AnalysisCachingTest.java | 9 ++--- .../google/devtools/build/lib/analysis/BUILD | 1 + .../analysis/util/BuildViewForTesting.java | 18 +++++----- 15 files changed, 101 insertions(+), 60 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java index f0e0443bc04fa1..948601cbf01768 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java @@ -81,10 +81,20 @@ import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.server.FailureDetails.TargetPatterns; import com.google.devtools.build.lib.server.FailureDetails.TargetPatterns.Code; -import com.google.devtools.build.lib.skyframe.*; +import com.google.devtools.build.lib.skyframe.AspectKeyCreator; import com.google.devtools.build.lib.skyframe.AspectKeyCreator.AspectKey; import com.google.devtools.build.lib.skyframe.AspectKeyCreator.TopLevelAspectsKey; +import com.google.devtools.build.lib.skyframe.BuildConfigurationKey; +import com.google.devtools.build.lib.skyframe.BuildResultListener; +import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; +import com.google.devtools.build.lib.skyframe.CoverageReportValue; import com.google.devtools.build.lib.skyframe.RepositoryMappingValue.RepositoryMappingResolutionException; +import com.google.devtools.build.lib.skyframe.SkyframeAnalysisAndExecutionResult; +import com.google.devtools.build.lib.skyframe.SkyframeAnalysisResult; +import com.google.devtools.build.lib.skyframe.SkyframeBuildView; +import com.google.devtools.build.lib.skyframe.SkyframeBuildView.BuildDriverKeyTestContext; +import com.google.devtools.build.lib.skyframe.SkyframeExecutor; +import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue; import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.RegexFilter; @@ -213,9 +223,14 @@ public AnalysisResult update( @Nullable ResourceManager resourceManager, @Nullable BuildResultListener buildResultListener, @Nullable ExecutionSetup executionSetupCallback, - @Nullable BuildConfigurationsCreated buildConfigurationsCreatedCallback) - throws ViewCreationFailedException, InvalidConfigurationException, InterruptedException, - BuildFailedException, TestExecException, AbruptExitException, AnalysisCacheDiscardedException { + @Nullable BuildConfigurationsCreated buildConfigurationsCreatedCallback, + @Nullable BuildDriverKeyTestContext buildDriverKeyTestContext) + throws ViewCreationFailedException, + InvalidConfigurationException, + InterruptedException, + BuildFailedException, + TestExecException, + AbruptExitException { logger.atInfo().log("Starting analysis"); pollInterruptedStatus(); @@ -253,8 +268,8 @@ public AnalysisResult update( skyframeExecutor); } - skyframeBuildView.setConfigurations( - eventHandler, configurations, viewOptions.maxConfigChangesToShow, viewOptions.failOnAnalysisCacheDiscard); + skyframeBuildView.setConfiguration( + eventHandler, configuration, viewOptions.maxConfigChangesToShow, viewOptions.failOnAnalysisCacheDiscard); eventBus.post(new MakeEnvironmentEvent(configuration.getMakeEnvironment())); eventBus.post(configuration.toBuildEvent()); diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/BUILD b/src/main/java/com/google/devtools/build/lib/buildeventstream/BUILD index 1ec4cfe5c51e56..a714fb12a259b2 100644 --- a/src/main/java/com/google/devtools/build/lib/buildeventstream/BUILD +++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/BUILD @@ -27,6 +27,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", "//src/main/java/com/google/devtools/build/lib/util", + "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code", "//src/main/java/com/google/devtools/build/lib/util:exit_code", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildCompletingEvent.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildCompletingEvent.java index 8390012989bfc8..0c0c1d516759e2 100644 --- a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildCompletingEvent.java +++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildCompletingEvent.java @@ -16,8 +16,11 @@ import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildEventId; +import com.google.devtools.build.lib.util.DetailedExitCode; import com.google.devtools.build.lib.util.ExitCode; import com.google.protobuf.util.Timestamps; + +import javax.annotation.Nullable; import java.util.Collection; /** @@ -27,6 +30,8 @@ * However, subclasses do not have to implement anything. */ public abstract class BuildCompletingEvent implements BuildEvent { + @Nullable + private final DetailedExitCode detailedExitCode; private final ExitCode exitCode; private final long finishTimeMillis; @@ -34,11 +39,20 @@ public abstract class BuildCompletingEvent implements BuildEvent { public BuildCompletingEvent( ExitCode exitCode, long finishTimeMillis, Collection children) { + this.detailedExitCode = null; this.exitCode = exitCode; this.finishTimeMillis = finishTimeMillis; this.children = children; } + public BuildCompletingEvent( + DetailedExitCode detailedExitCode, long finishTimeMillis, Collection children) { + this.detailedExitCode = detailedExitCode; + this.exitCode = detailedExitCode.getExitCode(); + this.finishTimeMillis = finishTimeMillis; + this.children = children; + } + public BuildCompletingEvent(ExitCode exitCode, long finishTimeMillis) { this(exitCode, finishTimeMillis, ImmutableList.of()); } @@ -65,13 +79,17 @@ public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventContext convert .setCode(exitCode.getNumericExitCode()) .build(); - BuildEventStreamProtos.BuildFinished finished = + BuildEventStreamProtos.BuildFinished.Builder finished = BuildEventStreamProtos.BuildFinished.newBuilder() .setOverallSuccess(ExitCode.SUCCESS.equals(exitCode)) .setExitCode(protoExitCode) .setFinishTime(Timestamps.fromMillis(finishTimeMillis)) - .setFinishTimeMillis(finishTimeMillis) - .build(); - return GenericBuildEvent.protoChaining(this).setFinished(finished).build(); + .setFinishTimeMillis(finishTimeMillis); + + if (detailedExitCode != null && detailedExitCode.getFailureDetail() != null) { + finished.setFailureDetail(detailedExitCode.getFailureDetail()); + } + + return GenericBuildEvent.protoChaining(this).setFinished(finished.build()).build(); } } diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto index 6af667e3377751..66339bb8808361 100644 --- a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto +++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto @@ -825,6 +825,9 @@ message BuildFinished { google.protobuf.Timestamp finish_time = 5; AnomalyReport anomaly_report = 4 [deprecated = true]; + + // Only populated if success = false, and sometimes not even then. + failure_details.FailureDetail failure_detail = 6; } message BuildMetrics { diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisAndExecutionPhaseRunner.java b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisAndExecutionPhaseRunner.java index 52c1016e5f3798..ca3269e37f08bd 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisAndExecutionPhaseRunner.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisAndExecutionPhaseRunner.java @@ -45,8 +45,12 @@ import com.google.devtools.build.lib.runtime.CommandEnvironment; import com.google.devtools.build.lib.server.FailureDetails.BuildConfiguration.Code; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; -import com.google.devtools.build.lib.skyframe.*; +import com.google.devtools.build.lib.skyframe.BuildInfoCollectionFunction; +import com.google.devtools.build.lib.skyframe.BuildResultListener; +import com.google.devtools.build.lib.skyframe.PrecomputedValue; import com.google.devtools.build.lib.skyframe.RepositoryMappingValue.RepositoryMappingResolutionException; +import com.google.devtools.build.lib.skyframe.SkyframeBuildView.BuildDriverKeyTestContext; +import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue; import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.TopLevelTargetAnalyzedEvent; import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.util.DetailedExitCode; @@ -75,11 +79,15 @@ static AnalysisAndExecutionResult execute( BuildOptions buildOptions, TargetPatternPhaseValue loadingResult, ExecutionSetup executionSetupCallback, - BuildConfigurationsCreated buildConfigurationCreatedCallback) - throws BuildFailedException, InterruptedException, ViewCreationFailedException, - TargetParsingException, LoadingFailedException, AbruptExitException, - InvalidConfigurationException, TestExecException, RepositoryMappingResolutionException, - AnalysisCacheDiscardedException { + BuildConfigurationsCreated buildConfigurationCreatedCallback, + BuildDriverKeyTestContext buildDriverKeyTestContext) + throws BuildFailedException, + InterruptedException, + ViewCreationFailedException, + AbruptExitException, + InvalidConfigurationException, + TestExecException, + RepositoryMappingResolutionException { // Compute the heuristic instrumentation filter if needed. if (request.needsInstrumentationFilter()) { @@ -199,10 +207,15 @@ private static AnalysisAndExecutionResult runAnalysisAndExecutionPhase( TargetPatternPhaseValue loadingResult, BuildOptions targetOptions, ExecutionSetup executionSetupCallback, - BuildConfigurationsCreated buildConfigurationCreatedCallback) - throws InterruptedException, InvalidConfigurationException, ViewCreationFailedException, - BuildFailedException, TestExecException, RepositoryMappingResolutionException, - AbruptExitException, AnalysisCacheDiscardedException { + BuildConfigurationsCreated buildConfigurationCreatedCallback, + BuildDriverKeyTestContext buildDriverKeyTestContext) + throws InterruptedException, + InvalidConfigurationException, + ViewCreationFailedException, + BuildFailedException, + TestExecException, + RepositoryMappingResolutionException, + AbruptExitException { env.getReporter().handle(Event.progress("Loading complete. Analyzing...")); ImmutableSet