Skip to content

Commit

Permalink
feat: add option to exit early if analysis cache is discarded
Browse files Browse the repository at this point in the history
Adds `--allow_analysis_cache_discard` option that allows the build to exit early if the analysis cache would have been discarded.

The flag name is of course open to bikeshedding etc.

fixes bazelbuild#16804

Closes bazelbuild#16805.

PiperOrigin-RevId: 552575951
Change-Id: Ia336eb3a5b7d7e41665fd0e0adf3edc03ed50f18
  • Loading branch information
mattem authored and gregestren committed Sep 12, 2023
1 parent 484b305 commit 2cd5c3a
Show file tree
Hide file tree
Showing 11 changed files with 81 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,17 @@ public class AnalysisOptions extends OptionsBase {
)
public boolean discardAnalysisCache;

@Option(
name = "allow_analysis_cache_discard",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.EAGERNESS_TO_EXIT},
help =
"If discarding the analysis cache due to a change in the build system, setting this"
+ " option to false will cause bazel to exit, rather than continuing with the build."
+ " This option has no effect when 'discard_analysis_cache' is also set.")
public boolean allowAnalysisCacheDiscards;

@Option(
name = "max_config_changes_to_show",
defaultValue = "3",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,8 @@ public AnalysisResult update(
}

skyframeBuildView.setConfigurations(
eventHandler, configurations, viewOptions.maxConfigChangesToShow);
eventHandler, configurations, viewOptions.maxConfigChangesToShow,
viewOptions.allowAnalysisCacheDiscards);

eventBus.post(
new MakeEnvironmentEvent(configurations.getTargetConfiguration().getMakeEnvironment()));
Expand Down Expand Up @@ -535,7 +536,7 @@ private AnalysisResult createResult(
TopLevelTargetsAndConfigsResult topLevelTargetsWithConfigs,
boolean includeExecutionPhase)
throws InterruptedException {
Set<Label> testsToRun = loadingResult.getTestsToRunLabels();
ImmutableSet<Label> testsToRun = loadingResult.getTestsToRunLabels();
Set<ConfiguredTarget> configuredTargets =
Sets.newLinkedHashSet(skyframeAnalysisResult.getConfiguredTargets());
ImmutableMap<AspectKey, ConfiguredAspect> aspects = skyframeAnalysisResult.getAspects();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,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,9 +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 java.util.Collection;
import javax.annotation.Nullable;

/**
* Class all events completing a build inherit from.
Expand All @@ -27,18 +29,28 @@
* 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 +77,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 @@ -826,6 +826,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 @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import com.google.devtools.build.lib.analysis.config.BuildOptions;
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.InvalidConfigurationException;
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;
Expand All @@ -85,6 +86,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 @@ -275,7 +277,9 @@ private ImmutableSet<OptionDefinition> getNativeCacheInvalidatingDifferences(
public void setConfigurations(
EventHandler eventHandler,
BuildConfigurationCollection configurations,
int maxDifferencesToShow) {
int maxDifferencesToShow,
boolean allowAnalysisCacheDiscards)
throws InvalidConfigurationException {
if (skyframeAnalysisWasDiscarded) {
eventHandler.handle(
Event.info(
Expand All @@ -285,7 +289,17 @@ public void setConfigurations(
} else {
String diff = describeConfigurationDifference(configurations, maxDifferencesToShow);
if (diff != null) {
eventHandler.handle(Event.info(diff + ", discarding analysis cache."));
if (!allowAnalysisCacheDiscards) {
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.warn(
diff
+ ", discarding analysis cache (this can be expensive, see"
+ " https://bazel.build/advanced/performance/iteration-speed)."));
// Note that clearing the analysis cache is currently required for correctness. It is also
// helpful to save memory.
//
Expand Down
2 changes: 2 additions & 0 deletions src/main/protobuf/failure_details.proto
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,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 Expand Up @@ -1190,6 +1191,7 @@ message Analysis {
CONFIGURED_VALUE_CREATION_FAILED = 18 [(metadata) = { exit_code: 1 }];
INCOMPATIBLE_TARGET_REQUESTED = 19 [(metadata) = { exit_code: 1 }];
ANALYSIS_FAILURE_PROPAGATION_FAILED = 20 [(metadata) = { exit_code: 1 }];
ANALYSIS_CACHE_DISCARDED = 21 [(metadata) = { exit_code: 1 }];
}

Code code = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
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.InvalidConfigurationException;
import com.google.devtools.build.lib.analysis.config.RequiresOptions;
import com.google.devtools.build.lib.analysis.config.transitions.NoTransition;
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
Expand Down Expand Up @@ -1160,4 +1161,16 @@ public void cacheClearMessageAfterDiscardAnalysisCacheBuildWithRelevantOptionCha
assertDoesNotContainEvent("Build option");
assertContainsEvent("discarding analysis cache");
}

@Test
public void throwsIfAnalysisCacheIsDiscardedWhenOptionSet() throws Exception {
setupDiffResetTesting();
scratch.file("test/BUILD", "load(':lib.bzl', 'normal_lib')", "normal_lib(name='top')");
useConfiguration("--definitely_relevant=old");
update("//test:top");
useConfiguration("--noallow_analysis_cache_discard", "--definitely_relevant=new");

Throwable t = assertThrows(InvalidConfigurationException.class, () -> update("//test:top"));
assertThat(t.getMessage().contains("analysis cache would have been discarded")).isTrue();
}
}
1 change: 1 addition & 0 deletions src/test/java/com/google/devtools/build/lib/analysis/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_options",
"//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_registry",
"//src/main/java/com/google/devtools/build/lib/analysis:config/host_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/invalid_configuration_exception",
"//src/main/java/com/google/devtools/build/lib/analysis:config/per_label_options",
"//src/main/java/com/google/devtools/build/lib/analysis:config/run_under",
"//src/main/java/com/google/devtools/build/lib/analysis:config/run_under_converter",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,16 @@ public AnalysisResult update(
/** Sets the configurations. Not thread-safe. */
public void setConfigurationsForTesting(
EventHandler eventHandler, BuildConfigurationCollection configurations) {
skyframeBuildView.setConfigurations(
eventHandler, configurations, /* maxDifferencesToShow */ -1);
try {
skyframeBuildView.setConfigurations(
eventHandler, configurations, /* maxDifferencesToShow */ -1,
/* allowAnalysisCacheDiscards */ true);
} catch (InvalidConfigurationException e) {
throw new UnsupportedOperationException(
"InvalidConfigurationException was thrown and caught during a test, "
+ "this case is not yet handled",
e);
}
}

public ArtifactFactory getArtifactFactory() {
Expand Down

0 comments on commit 2cd5c3a

Please sign in to comment.