Permit rewinding with incrementality.
When rewinding prerequisites are not met, fail the build instead of ignoring the flag. Respecting the value of `--rewind_lost_inputs` instead of silently ignoring it allows for some cleanups to be made in followups.
PiperOrigin-RevId: 568724875
Change-Id: If7a1e8beec2b6753268258379521960c6d2d262c
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
index 308a97a..add7334 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
@@ -59,6 +59,8 @@
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.ProfilerTask;
import com.google.devtools.build.lib.repository.ExternalPackageHelper;
+import com.google.devtools.build.lib.server.FailureDetails.ActionRewinding;
+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.ExternalFilesHelper.ExternalFileAction;
import com.google.devtools.build.lib.skyframe.PackageFunction.ActionOnIOExceptionReadingBuildFile;
@@ -67,6 +69,7 @@
import com.google.devtools.build.lib.skyframe.actiongraph.v2.AqueryConsumingOutputHandler;
import com.google.devtools.build.lib.skyframe.rewinding.RewindableGraphInconsistencyReceiver;
import com.google.devtools.build.lib.util.AbruptExitException;
+import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.util.ResourceUsage;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.BatchStat;
@@ -244,8 +247,10 @@
QuiescingExecutors executors,
OptionsProvider options)
throws InterruptedException, AbruptExitException {
+ // TODO(b/228090759): Set the correct inconsistency receiver when evaluatorNeedsReset is false.
+ boolean rewindingEnabled = rewindingEnabled(options);
if (evaluatorNeedsReset) {
- if (rewindingPermitted(options)) {
+ if (rewindingEnabled) {
// Currently incompatible with Skymeld i.e. this code path won't be run in Skymeld mode. We
// may need to combine these GraphInconsistencyReceiver implementations in the future.
var rewindableReceiver = new RewindableGraphInconsistencyReceiver();
@@ -295,15 +300,22 @@
return workspaceInfo;
}
- private boolean rewindingPermitted(OptionsProvider options) {
- // Rewinding is only supported with no incremental state and no action cache.
- if (trackIncrementalState) {
+ private static boolean rewindingEnabled(OptionsProvider options) throws AbruptExitException {
+ var buildRequestOptions = options.getOptions(BuildRequestOptions.class);
+ if (buildRequestOptions == null || !buildRequestOptions.rewindLostInputs) {
return false;
}
- BuildRequestOptions buildRequestOptions = options.getOptions(BuildRequestOptions.class);
- return buildRequestOptions != null
- && !buildRequestOptions.useActionCache
- && buildRequestOptions.rewindLostInputs;
+ if (buildRequestOptions.useActionCache) {
+ throw new AbruptExitException(
+ DetailedExitCode.of(
+ FailureDetail.newBuilder()
+ .setMessage("--rewind_lost_inputs requires --nouse_action_cache")
+ .setActionRewinding(
+ ActionRewinding.newBuilder()
+ .setCode(ActionRewinding.Code.REWIND_LOST_INPUTS_PREREQ_UNMET))
+ .build()));
+ }
+ return true;
}
/**
diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto
index fb15475..3cc189c 100644
--- a/src/main/protobuf/failure_details.proto
+++ b/src/main/protobuf/failure_details.proto
@@ -1033,6 +1033,7 @@
ACTION_REWINDING_UNKNOWN = 0 [(metadata) = { exit_code: 37 }];
LOST_INPUT_TOO_MANY_TIMES = 1 [(metadata) = { exit_code: 1 }];
LOST_INPUT_IS_SOURCE = 2 [(metadata) = { exit_code: 1 }];
+ REWIND_LOST_INPUTS_PREREQ_UNMET = 3 [(metadata) = { exit_code: 2 }];
}
Code code = 1;
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java
index 4326b00..1d8a38c 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java
@@ -105,6 +105,7 @@
import com.google.devtools.build.lib.query2.common.QueryTransitivePackagePreloader;
import com.google.devtools.build.lib.runtime.KeepGoingOption;
import com.google.devtools.build.lib.runtime.QuiescingExecutorsImpl;
+import com.google.devtools.build.lib.server.FailureDetails.ActionRewinding;
import com.google.devtools.build.lib.server.FailureDetails.Crash;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.server.FailureDetails.Spawn;
@@ -2662,24 +2663,11 @@
}
@Test
- public void usesCorrectGraphInconsistencyReceiver(
- @TestParameter boolean trackIncrementalState,
- @TestParameter boolean useActionCache,
- @TestParameter boolean rewindLostInputs)
+ public void rewindingPrerequisites(
+ @TestParameter boolean trackIncrementalState, @TestParameter boolean useActionCache)
throws Exception {
- extraSkyFunctions.put(
- SkyFunctionName.FOR_TESTING,
- (key, env) -> {
- if (!trackIncrementalState && !useActionCache && rewindLostInputs) {
- assertThat(env.resetPermitted()).isTrue();
- } else {
- assertThat(env.resetPermitted()).isFalse();
- }
- return new SkyValue() {};
- });
initializeSkyframeExecutor();
- options.parse(
- "--use_action_cache=" + useActionCache, "--rewind_lost_inputs=" + rewindLostInputs);
+ options.parse("--rewind_lost_inputs", "--use_action_cache=" + useActionCache);
skyframeExecutor.setActive(false);
skyframeExecutor.decideKeepIncrementalState(
@@ -2690,10 +2678,14 @@
/* discardAnalysisCache= */ false,
reporter);
skyframeExecutor.setActive(true);
- syncSkyframeExecutor();
- EvaluationResult<?> result = evaluate(ImmutableList.of(GraphTester.skyKey("key")));
- assertThat(result.hasError()).isFalse();
+ if (useActionCache) {
+ AbruptExitException e = assertThrows(AbruptExitException.class, this::syncSkyframeExecutor);
+ assertThat(e.getDetailedExitCode().getFailureDetail().getActionRewinding().getCode())
+ .isEqualTo(ActionRewinding.Code.REWIND_LOST_INPUTS_PREREQ_UNMET);
+ } else {
+ syncSkyframeExecutor(); // Permitted.
+ }
}
private void syncSkyframeExecutor() throws InterruptedException, AbruptExitException {
