Skip to content

Commit

Permalink
Add error reporting capabilities.
Browse files Browse the repository at this point in the history
The prototype can now handle loading/analysis and execution phase errors gracefully. Also works with --keep_going mode.

PiperOrigin-RevId: 410233322
  • Loading branch information
joeleba authored and Copybara-Service committed Nov 16, 2021
1 parent fd727ec commit f7b8b72
Show file tree
Hide file tree
Showing 17 changed files with 537 additions and 134 deletions.
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/util:custom_failure_detail_publisher",
"//src/main/java/com/google/devtools/build/lib/util:debug-logger-configurator",
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
"//src/main/java/com/google/devtools/build/lib/util:execution_detailed_exit_code_helper",
"//src/main/java/com/google/devtools/build/lib/util:exit_code",
"//src/main/java/com/google/devtools/build/lib/util:interrupted_failure_details",
"//src/main/java/com/google/devtools/build/lib/util:logging",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableSortedSet;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.skyframe.AspectKeyCreator.AspectKey;
import java.util.Collection;
import javax.annotation.Nullable;
Expand All @@ -37,6 +38,7 @@ public final class AnalysisAndExecutionResult extends AnalysisResult {
ImmutableMap<AspectKey, ConfiguredAspect> aspects,
@Nullable ImmutableList<ConfiguredTarget> targetsToTest,
ImmutableSet<ConfiguredTarget> targetsToSkip,
@Nullable FailureDetail failureDetail,
ImmutableSet<Artifact> artifactsToBuild,
ImmutableSet<ConfiguredTarget> parallelTests,
ImmutableSet<ConfiguredTarget> exclusiveTests,
Expand All @@ -50,7 +52,7 @@ public final class AnalysisAndExecutionResult extends AnalysisResult {
aspects,
targetsToTest,
targetsToSkip,
/*failureDetail=*/ null,
failureDetail,
/*actionGraph=*/ null,
artifactsToBuild,
parallelTests,
Expand Down
41 changes: 28 additions & 13 deletions src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
import com.google.devtools.build.lib.skyframe.CoverageReportValue;
import com.google.devtools.build.lib.skyframe.PrepareAnalysisPhaseValue;
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.SkyframeExecutor;
Expand Down Expand Up @@ -360,23 +361,25 @@ public AnalysisResult update(
getArtifactFactory().noteAnalysisStarting();
SkyframeAnalysisResult skyframeAnalysisResult;
try {
Supplier<Map<BuildConfigurationKey, BuildConfigurationValue>> configurationLookupSupplier =
() -> {
Map<BuildConfigurationKey, BuildConfigurationValue> result = new HashMap<>();
for (TargetAndConfiguration node : topLevelTargetsWithConfigs) {
if (node.getConfiguration() != null) {
result.put(node.getConfiguration().getKey(), node.getConfiguration());
}
}
return result;
};
Supplier<Map<BuildConfigurationKey, BuildConfigurationValue>>
memoizedConfigurationLookupSupplier =
Suppliers.memoize(
() -> {
Map<BuildConfigurationKey, BuildConfigurationValue> result = new HashMap<>();
for (TargetAndConfiguration node : topLevelTargetsWithConfigs) {
if (node.getConfiguration() != null) {
result.put(node.getConfiguration().getKey(), node.getConfiguration());
}
}
return result;
});
if (!includeExecutionPhase) {
skyframeAnalysisResult =
skyframeBuildView.configureTargets(
eventHandler,
topLevelCtKeys,
aspectsKeys.build(),
Suppliers.memoize(configurationLookupSupplier),
memoizedConfigurationLookupSupplier,
topLevelOptions,
eventBus,
keepGoing,
Expand All @@ -391,7 +394,9 @@ public AnalysisResult update(
eventHandler,
topLevelCtKeys,
aspectsKeys.build(),
memoizedConfigurationLookupSupplier,
topLevelOptions,
eventBus,
keepGoing,
loadingPhaseThreads,
viewOptions.cpuHeavySkyKeysThreadPoolSize,
Expand Down Expand Up @@ -558,13 +563,16 @@ private AnalysisResult createResult(
ImmutableSet<ConfiguredTarget> parallelTests = testsPair.first;
ImmutableSet<ConfiguredTarget> exclusiveTests = testsPair.second;

FailureDetail failureDetail =
createFailureDetail(loadingResult, skyframeAnalysisResult, topLevelTargetsWithConfigs);
if (includeExecutionPhase) {
return new AnalysisAndExecutionResult(
configurations,
ImmutableSet.copyOf(configuredTargets),
aspects,
allTargetsToTest == null ? null : ImmutableList.copyOf(allTargetsToTest),
ImmutableSet.copyOf(targetsToSkip),
failureDetail,
artifactsToBuild.build(),
parallelTests,
exclusiveTests,
Expand All @@ -574,8 +582,6 @@ private AnalysisResult createResult(
loadingResult.getNotSymlinkedInExecrootDirectories());
}

FailureDetail failureDetail =
createFailureDetail(loadingResult, skyframeAnalysisResult, topLevelTargetsWithConfigs);

WalkableGraph graph = skyframeAnalysisResult.getWalkableGraph();
ActionGraph actionGraph =
Expand Down Expand Up @@ -659,6 +665,15 @@ public static FailureDetail createFailureDetail(
.setAnalysis(Analysis.newBuilder().setCode(Analysis.Code.NOT_ALL_TARGETS_ANALYZED))
.build();
}
if (skyframeAnalysisResult instanceof SkyframeAnalysisAndExecutionResult) {
SkyframeAnalysisAndExecutionResult skyframeAnalysisAndExecutionResult =
(SkyframeAnalysisAndExecutionResult) skyframeAnalysisResult;
if (skyframeAnalysisAndExecutionResult.getRepresentativeExecutionExitCode() != null) {
return skyframeAnalysisAndExecutionResult
.getRepresentativeExecutionExitCode()
.getFailureDetail();
}
}
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,11 @@
// limitations under the License.
package com.google.devtools.build.lib.buildtool;

import com.google.common.base.Stopwatch;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.flogger.GoogleLogger;
import com.google.devtools.build.lib.actions.BuildFailedException;
import com.google.devtools.build.lib.analysis.AnalysisAndExecutionResult;
import com.google.devtools.build.lib.analysis.AnalysisPhaseCompleteEvent;
import com.google.devtools.build.lib.analysis.BuildView;
import com.google.devtools.build.lib.analysis.ViewCreationFailedException;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
Expand Down Expand Up @@ -47,7 +45,6 @@
import java.util.Collection;
import java.util.List;
import java.util.Set;
import java.util.concurrent.TimeUnit;

/**
* Intended drop-in replacement for AnalysisPhaseRunner after we're done with merging Skyframe's
Expand Down Expand Up @@ -170,7 +167,6 @@ private static AnalysisAndExecutionResult runAnalysisAndExecutionPhase(
BuildOptions targetOptions,
Set<String> multiCpu)
throws InterruptedException, InvalidConfigurationException, ViewCreationFailedException {
Stopwatch timer = Stopwatch.createStarted();
env.getReporter().handle(Event.progress("Loading complete. Analyzing..."));

ImmutableSet<String> explicitTargetPatterns =
Expand Down Expand Up @@ -200,16 +196,6 @@ private static AnalysisAndExecutionResult runAnalysisAndExecutionPhase(
/*includeExecutionPhase=*/ true,
request.getBuildOptions().jobs);

// TODO(bazel-team): Merge these into one event.
env.getEventBus()
.post(
new AnalysisPhaseCompleteEvent(
analysisAndExecutionResult.getTargetsToBuild(),
view.getEvaluatedCounts(),
view.getEvaluatedActionsCounts(),
timer.stop().elapsed(TimeUnit.MILLISECONDS),
view.getAndClearPkgManagerStatistics(),
env.getSkyframeExecutor().wasAnalysisCacheDiscardedAndResetBit()));
// TODO(b/199053098) TestFilteringCompleteEvent.
return analysisAndExecutionResult;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,10 @@
import com.google.devtools.build.lib.runtime.BlazeRuntime;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.server.FailureDetails.ActionQuery;
import com.google.devtools.build.lib.server.FailureDetails.BuildConfiguration;
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.AspectKeyCreator.AspectKey;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor;
import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue;
import com.google.devtools.build.lib.skyframe.WorkspaceInfoFromDiff;
Expand All @@ -73,6 +75,8 @@
import java.io.IOException;
import java.io.OutputStream;
import java.io.PrintStream;
import java.util.HashSet;
import java.util.Set;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -157,7 +161,7 @@ public void buildTargets(BuildRequest request, BuildResult result, TargetValidat
throw new InvalidConfigurationException(
"The experimental setting to select multiple CPUs is only supported for 'build' and "
+ "'test' right now!",
BuildConfiguration.Code.MULTI_CPU_PREREQ_UNMET);
Code.MULTI_CPU_PREREQ_UNMET);
}
}

Expand All @@ -178,9 +182,11 @@ public void buildTargets(BuildRequest request, BuildResult result, TargetValidat

if (request.getBuildOptions().performAnalysisPhase) {
executionTool = new ExecutionTool(env, request);
Set<ConfiguredTargetKey> builtTargets = new HashSet<>();
Set<AspectKey> builtAspects = new HashSet<>();

try (SilentCloseable c = Profiler.instance().profile("ExecutionTool.init")) {
executionTool.prepareForExecution(request.getId());
executionTool.prepareForExecution(request.getId(), builtTargets, builtAspects);
}

// TODO(b/199053098): implement support for --nobuild.
Expand All @@ -191,6 +197,27 @@ public void buildTargets(BuildRequest request, BuildResult result, TargetValidat
analysisAndExecutionResult.getConfigurationCollection());
result.setActualTargets(analysisAndExecutionResult.getTargetsToBuild());
result.setTestTargets(analysisAndExecutionResult.getTargetsToTest());
try (SilentCloseable c = Profiler.instance().profile("Show results")) {
result.setSuccessfulTargets(
ExecutionTool.determineSuccessfulTargets(
analysisAndExecutionResult.getTargetsToBuild(), builtTargets));
result.setSuccessfulAspects(
ExecutionTool.determineSuccessfulAspects(
analysisAndExecutionResult.getAspectsMap().keySet(), builtAspects));
result.setSkippedTargets(analysisAndExecutionResult.getTargetsToSkip());
BuildResultPrinter buildResultPrinter = new BuildResultPrinter(env);
buildResultPrinter.showBuildResult(
request,
result,
analysisAndExecutionResult.getTargetsToBuild(),
analysisAndExecutionResult.getTargetsToSkip(),
analysisAndExecutionResult.getAspectsMap());
}
FailureDetail delayedFailureDetail = analysisAndExecutionResult.getFailureDetail();
if (delayedFailureDetail != null) {
throw new BuildFailedException(
delayedFailureDetail.getMessage(), DetailedExitCode.of(delayedFailureDetail));
}
}
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.google.common.flogger.GoogleLogger;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionCacheChecker;
import com.google.devtools.build.lib.actions.ActionExecutionStatusReporter;
import com.google.devtools.build.lib.actions.ActionGraph;
import com.google.devtools.build.lib.actions.ActionInputPrefetcher;
import com.google.devtools.build.lib.actions.ArtifactFactory;
Expand Down Expand Up @@ -55,6 +56,7 @@
import com.google.devtools.build.lib.buildtool.buildevent.ConvenienceSymlinksIdentifiedEvent;
import com.google.devtools.build.lib.buildtool.buildevent.ExecRootPreparedEvent;
import com.google.devtools.build.lib.buildtool.buildevent.ExecutionPhaseCompleteEvent;
import com.google.devtools.build.lib.buildtool.buildevent.ExecutionProgressReceiverAvailableEvent;
import com.google.devtools.build.lib.buildtool.buildevent.ExecutionStartingEvent;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.events.Event;
Expand Down Expand Up @@ -105,7 +107,6 @@
import java.time.Duration;
import java.util.Collection;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
Expand Down Expand Up @@ -244,7 +245,8 @@ TestActionContext getTestActionContext() {
* scattered over several classes. We need this in order to merge analysis & execution phases.
* TODO(b/199053098): Minimize code duplication with the main code path.
*/
public void prepareForExecution(UUID buildId)
public void prepareForExecution(
UUID buildId, Set<ConfiguredTargetKey> builtTargets, Set<AspectKey> builtAspects)
throws AbruptExitException, BuildFailedException, InterruptedException {
init();

Expand Down Expand Up @@ -304,6 +306,24 @@ public void prepareForExecution(UUID buildId)
skyframeBuilder.getActionCacheChecker(),
skyframeBuilder.getTopDownActionCache());
}

// Note that executionProgressReceiver accesses builtTargets concurrently (after wrapping in a
// synchronized collection), so unsynchronized access to this variable is unsafe while it runs.
// TODO(leba): count test actions
ExecutionProgressReceiver executionProgressReceiver =
new ExecutionProgressReceiver(
Preconditions.checkNotNull(builtTargets), Preconditions.checkNotNull(builtAspects), 0);
skyframeExecutor
.getEventBus()
.post(new ExecutionProgressReceiverAvailableEvent(executionProgressReceiver));

ActionExecutionStatusReporter statusReporter =
ActionExecutionStatusReporter.create(env.getReporter(), skyframeExecutor.getEventBus());
// TODO(leba): Add watchdog support.
skyframeExecutor.setActionExecutionProgressReportingObjects(
executionProgressReceiver, executionProgressReceiver, statusReporter);
skyframeExecutor.setExecutionProgressReceiver(executionProgressReceiver);

for (ExecutorLifecycleListener executorLifecycleListener : executorLifecycleListeners) {
try (SilentCloseable c =
Profiler.instance().profile(executorLifecycleListener + ".executionPhaseStarting")) {
Expand Down Expand Up @@ -796,11 +816,11 @@ public void handle(Event event) {
*
* @param configuredTargets The configured targets whose artifacts are to be built.
*/
private static Collection<ConfiguredTarget> determineSuccessfulTargets(
static ImmutableSet<ConfiguredTarget> determineSuccessfulTargets(
Collection<ConfiguredTarget> configuredTargets, Set<ConfiguredTargetKey> builtTargets) {
// Maintain the ordering by copying builtTargets into a LinkedHashSet in the same iteration
// order as configuredTargets.
Collection<ConfiguredTarget> successfulTargets = new LinkedHashSet<>();
// Maintain the ordering by copying builtTargets into an ImmutableSet.Builder in the same
// iteration order as configuredTargets.
ImmutableSet.Builder<ConfiguredTarget> successfulTargets = ImmutableSet.builder();
for (ConfiguredTarget target : configuredTargets) {
if (builtTargets.contains(
ConfiguredTargetKey.builder()
Expand All @@ -810,10 +830,10 @@ private static Collection<ConfiguredTarget> determineSuccessfulTargets(
successfulTargets.add(target);
}
}
return successfulTargets;
return successfulTargets.build();
}

private static ImmutableSet<AspectKey> determineSuccessfulAspects(
static ImmutableSet<AspectKey> determineSuccessfulAspects(
ImmutableSet<AspectKey> aspects, Set<AspectKey> builtAspects) {
// Maintain the ordering.
return aspects.stream().filter(builtAspects::contains).collect(ImmutableSet.toImmutableSet());
Expand Down

0 comments on commit f7b8b72

Please sign in to comment.