Skip to content

Commit

Permalink
throw InvalidConfigurationException and add FailureDetail to `Bui…
Browse files Browse the repository at this point in the history
…ldCompletingEvent`
  • Loading branch information
mattem committed Mar 25, 2023
1 parent cd08c0a commit 8313dd0
Show file tree
Hide file tree
Showing 15 changed files with 101 additions and 60 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -27,18 +30,29 @@
* 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;

private final Collection<BuildEventId> children;

public BuildCompletingEvent(
ExitCode exitCode, long finishTimeMillis, Collection<BuildEventId> children) {
this.detailedExitCode = null;
this.exitCode = exitCode;
this.finishTimeMillis = finishTimeMillis;
this.children = children;
}

public BuildCompletingEvent(
DetailedExitCode detailedExitCode, long finishTimeMillis, Collection<BuildEventId> 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());
}
Expand All @@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -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<Label> explicitTargetPatterns =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,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.AspectKeyCreator.AspectKey;
import com.google.devtools.build.lib.skyframe.BuildConfigurationKey;
import com.google.devtools.build.lib.skyframe.BuildInfoCollectionFunction;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.RepositoryMappingValue.RepositoryMappingResolutionException;
import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue;
import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.AspectAnalyzedEvent;
import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.TestAnalyzedEvent;
import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.TopLevelTargetAnalyzedEvent;
Expand Down Expand Up @@ -84,7 +87,7 @@ public static AnalysisResult execute(
TargetValidator validator)
throws BuildFailedException, InterruptedException, ViewCreationFailedException,
TargetParsingException, LoadingFailedException, AbruptExitException,
InvalidConfigurationException, RepositoryMappingResolutionException, AnalysisCacheDiscardedException {
InvalidConfigurationException, RepositoryMappingResolutionException {

// Target pattern evaluation.
TargetPatternPhaseValue loadingResult;
Expand Down Expand Up @@ -216,7 +219,7 @@ private static AnalysisResult runAnalysisPhase(
TargetPatternPhaseValue loadingResult,
BuildOptions targetOptions)
throws InterruptedException, InvalidConfigurationException,
RepositoryMappingResolutionException, ViewCreationFailedException, AnalysisCacheDiscardedException {
RepositoryMappingResolutionException, ViewCreationFailedException {
Stopwatch timer = Stopwatch.createStarted();
env.getReporter().handle(Event.progress("Loading complete. Analyzing..."));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
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.FailureDetail;
import com.google.devtools.build.lib.skyframe.AnalysisCacheDiscardedException;
import com.google.devtools.build.lib.skyframe.BuildResultListener;
import com.google.devtools.build.lib.skyframe.RepositoryMappingValue.RepositoryMappingResolutionException;
import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor;
import com.google.devtools.build.lib.skyframe.SkyframeBuildView.BuildDriverKeyTestContext;
Expand Down Expand Up @@ -151,7 +151,7 @@ public void buildTargets(BuildRequest request, BuildResult result, TargetValidat
throws BuildFailedException, InterruptedException, ViewCreationFailedException,
TargetParsingException, LoadingFailedException, AbruptExitException,
InvalidConfigurationException, TestExecException, ExitException,
PostExecutionActionGraphDumpException, RepositoryMappingResolutionException, AnalysisCacheDiscardedException {
PostExecutionActionGraphDumpException, RepositoryMappingResolutionException {
try (SilentCloseable c = Profiler.instance().profile("validateOptions")) {
validateOptions(request);
}
Expand Down Expand Up @@ -311,8 +311,7 @@ private void buildTargetsWithMergedAnalysisExecution(
BuildOptions buildOptions)
throws InterruptedException, TargetParsingException, LoadingFailedException,
AbruptExitException, ViewCreationFailedException, BuildFailedException, TestExecException,
InvalidConfigurationException, RepositoryMappingResolutionException, AnalysisCacheDiscardedException {

InvalidConfigurationException, RepositoryMappingResolutionException {
// Target pattern evaluation.
TargetPatternPhaseValue loadingResult;
Profiler.instance().markPhase(ProfilePhase.TARGET_PATTERN_EVAL);
Expand Down Expand Up @@ -573,7 +572,7 @@ public BuildResult processRequest(
} else {
result.setCatastrophe();
}
} catch (TargetParsingException | LoadingFailedException | AnalysisCacheDiscardedException e) {
} catch (TargetParsingException | LoadingFailedException e) {
detailedExitCode = e.getDetailedExitCode();
reportExceptionError(e);
} catch (RepositoryMappingResolutionException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public final class BuildCompleteEvent extends BuildCompletingEvent {

/** Construct the BuildCompleteEvent. */
public BuildCompleteEvent(BuildResult result, Collection<BuildEventId> children) {
super(result.getDetailedExitCode().getExitCode(), result.getStopTime(), children);
super(result.getDetailedExitCode(), result.getStopTime(), children);
this.result = checkNotNull(result);
}

Expand Down
1 change: 0 additions & 1 deletion src/main/java/com/google/devtools/build/lib/skyframe/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ java_library(
"ActionLookupConflictFindingFunction.java",
"ActionLookupValuesTraversal.java",
"ActionOutputDirectoryHelper.java",
"AnalysisCacheDiscardedException.java",
"AspectCompletor.java",
"AspectFunction.java",
"BazelSkyframeExecutorConstants.java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,8 @@
import com.google.devtools.build.lib.analysis.ToolchainCollection;
import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
import com.google.devtools.build.lib.analysis.ViewCreationFailedException;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.*;
import com.google.devtools.build.lib.analysis.config.BuildOptions.OptionsDiff;
import com.google.devtools.build.lib.analysis.config.ConfigConditions;
import com.google.devtools.build.lib.analysis.config.StarlarkTransitionCache;
import com.google.devtools.build.lib.analysis.test.AnalysisFailurePropagationException;
import com.google.devtools.build.lib.bugreport.BugReport;
import com.google.devtools.build.lib.bugreport.BugReporter;
Expand All @@ -84,6 +81,7 @@
import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.SilentCloseable;
import com.google.devtools.build.lib.server.FailureDetails;
import com.google.devtools.build.lib.skyframe.ArtifactConflictFinder.ConflictException;
import com.google.devtools.build.lib.skyframe.AspectKeyCreator.AspectKey;
import com.google.devtools.build.lib.skyframe.AspectKeyCreator.TopLevelAspectsKey;
Expand Down Expand Up @@ -269,11 +267,8 @@ private ImmutableSet<OptionDefinition> getNativeCacheInvalidatingDifferences(

/** Sets the configuration. Not thread-safe. DO NOT CALL except from tests! */
@VisibleForTesting
public void setConfigurations(
EventHandler eventHandler,
BuildConfigurationCollection configurations,
int maxDifferencesToShow,
boolean failOnAnalysisCacheDiscard) throws AnalysisCacheDiscardedException {
public void setConfiguration(
EventHandler eventHandler, BuildConfigurationValue configuration, int maxDifferencesToShow, boolean failOnAnalysisCacheDiscard) throws InvalidConfigurationException {
if (skyframeAnalysisWasDiscarded) {
eventHandler.handle(
Event.info(
Expand All @@ -284,9 +279,10 @@ public void setConfigurations(
String diff = describeConfigurationDifference(configuration, maxDifferencesToShow);
if (diff != null) {
if (failOnAnalysisCacheDiscard) {
throw new AnalysisCacheDiscardedException(diff);
String message = String.format("%s, analysis cache would have been discarded.", diff);
throw new InvalidConfigurationException(
message, FailureDetails.BuildConfiguration.Code.CONFIGURATION_DISCARDED_ANALYSIS_CACHE);
}

eventHandler.handle(Event.info(diff + ", discarding analysis cache."));
// Note that clearing the analysis cache is currently required for correctness. It is also
// helpful to save memory.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,6 @@ public final class ExitCode {
ExitCode.create(45, "PERSISTENT_BUILD_EVENT_SERVICE_UPLOAD_ERROR");
public static final ExitCode EXTERNAL_DEPS_ERROR = ExitCode.create(48, "EXTERNAL_DEPS_ERROR");

public static final ExitCode ANALYSIS_CACHE_DISCARD_FAILURE =
ExitCode.create(49, "ANALYSIS_CACHE_DISCARD_FAILURE");

/**
* Creates and returns an ExitCode. Requires a unique exit code number.
*
Expand Down
1 change: 1 addition & 0 deletions src/main/protobuf/failure_details.proto
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,7 @@ message BuildConfiguration {
// possibilities in Bazel, so we go with the more straightforward
// command-line error exit code 2.
INVALID_OUTPUT_DIRECTORY_MNEMONIC = 11 [(metadata) = { exit_code: 2 }];
CONFIGURATION_DISCARDED_ANALYSIS_CACHE = 12 [(metadata) = { exit_code: 2 }];
}

Code code = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,14 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ObjectArrays;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.BuildOptionsView;
import com.google.devtools.build.lib.analysis.config.Fragment;
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
import com.google.devtools.build.lib.analysis.config.RequiresOptions;
import com.google.devtools.build.lib.analysis.config.*;
import com.google.devtools.build.lib.analysis.config.transitions.NoTransition;
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
import com.google.devtools.build.lib.analysis.util.AnalysisCachingTestBase;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.rules.java.JavaInfo;
import com.google.devtools.build.lib.rules.java.JavaSourceJarsProvider;
import com.google.devtools.build.lib.skyframe.AnalysisCacheDiscardedException;
import com.google.devtools.build.lib.testutil.TestConstants.InternalTestExecutionMode;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
import com.google.devtools.common.options.Option;
Expand Down Expand Up @@ -1186,7 +1181,7 @@ public void throwsIfAnalysisCacheIsDiscardedWhenOptionSet() throws Exception {
update("//test:top");
useConfiguration("--fail_on_analysis_cache_discard", "--definitely_relevant=new");

Throwable t = assertThrows(AnalysisCacheDiscardedException.class, () -> update("//test:top"));
Throwable t = assertThrows(InvalidConfigurationException.class, () -> update("//test:top"));
assertTrue(t.getMessage().contains("analysis cache would have been discarded"));
}
}
Loading

0 comments on commit 8313dd0

Please sign in to comment.