Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add option to exit early if analysis cache is discarded #16805

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -255,7 +255,7 @@ public AnalysisResult update(
}

skyframeBuildView.setConfiguration(
eventHandler, topLevelConfig, viewOptions.maxConfigChangesToShow);
eventHandler, topLevelConfig, viewOptions.maxConfigChangesToShow, viewOptions.allowAnalysisCacheDiscards);

eventBus.post(new MakeEnvironmentEvent(topLevelConfig.getMakeEnvironment()));
eventBus.post(topLevelConfig.toBuildEvent());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration",
"//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 @@ -836,6 +836,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 @@ -66,6 +66,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.analysis.test.CoverageActionFinishedEvent;
Expand All @@ -89,6 +90,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 @@ -279,7 +281,7 @@ private ImmutableSet<OptionDefinition> getNativeCacheInvalidatingDifferences(
/** Sets the configuration. Not thread-safe. DO NOT CALL except from tests! */
@VisibleForTesting
public void setConfiguration(
EventHandler eventHandler, BuildConfigurationValue configuration, int maxDifferencesToShow) {
EventHandler eventHandler, BuildConfigurationValue configuration, int maxDifferencesToShow, boolean allowAnalysisCacheDiscards) throws InvalidConfigurationException {
if (skyframeAnalysisWasDiscarded) {
eventHandler.handle(
Event.warn(
Expand All @@ -290,6 +292,11 @@ public void setConfiguration(
} else {
String diff = describeConfigurationDifference(configuration, maxDifferencesToShow);
if (diff != null) {
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
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 @@ -608,6 +608,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 @@ -1197,6 +1198,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 @@ -16,6 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;

import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
Expand All @@ -25,6 +26,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 @@ -1163,4 +1165,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"));
assertTrue(t.getMessage().contains("analysis cache would have been discarded"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/analysis:config/fragment",
"//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/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 @@ -258,7 +258,13 @@ public AnalysisResult update(
/** Sets the configuration. Not thread-safe. */
public void setConfigurationForTesting(
EventHandler eventHandler, BuildConfigurationValue configuration) {
skyframeBuildView.setConfiguration(eventHandler, configuration, /* maxDifferencesToShow= */ -1);
try {
skyframeBuildView.setConfiguration(eventHandler, configuration,
/* 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