Support rewinding of inputs in runfiles
An action that fails because it depends on a generated file in the
action's runfiles will now rewind the appropriate Skyframe nodes.
Previously the checkInputs method lost the relationship between
"middleman" artifacts, used by runfiles and directly depended on by
actions that include runfiles, to the artifacts they aggregate.
This CL refactors checkInputs and the ActionInputMap it populates so
that it can be rerun to reestablish those relationships if the
executing action fails due to lost inputs, without incurring additional
costs in the normal case.
RELNOTES: None.
PiperOrigin-RevId: 222296167
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionInputDepOwnerMap.java b/src/main/java/com/google/devtools/build/lib/actions/ActionInputDepOwnerMap.java
new file mode 100644
index 0000000..b198e3c
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionInputDepOwnerMap.java
@@ -0,0 +1,46 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.actions;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Maps;
+import java.util.Collection;
+import java.util.Map;
+import javax.annotation.Nullable;
+
+/** A {@link ActionInputDepOwners} which, as a {@link ActionInputMapSink}, is mutable. */
+public class ActionInputDepOwnerMap implements ActionInputMapSink, ActionInputDepOwners {
+
+ private final ImmutableSet<ActionInput> inputsOfInterest;
+ private final Map<ActionInput, Artifact> depOwnersByInputs;
+
+ public ActionInputDepOwnerMap(Collection<ActionInput> inputsOfInterest) {
+ this.inputsOfInterest = ImmutableSet.copyOf(inputsOfInterest);
+ this.depOwnersByInputs = Maps.newHashMapWithExpectedSize(inputsOfInterest.size());
+ }
+
+ @Override
+ public boolean put(ActionInput input, FileArtifactValue metadata, @Nullable Artifact depOwner) {
+ if (depOwner == null || !inputsOfInterest.contains(input)) {
+ return false;
+ }
+ return depOwnersByInputs.put(input, depOwner) == null;
+ }
+
+ @Override
+ @Nullable
+ public Artifact getDepOwner(ActionInput input) {
+ return depOwnersByInputs.get(input);
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionInputDepOwners.java b/src/main/java/com/google/devtools/build/lib/actions/ActionInputDepOwners.java
new file mode 100644
index 0000000..9ea612e
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionInputDepOwners.java
@@ -0,0 +1,33 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.actions;
+
+import javax.annotation.Nullable;
+
+/**
+ * Association between {@link ActionInput}s and the {@link Artifact}s, directly depended on by an
+ * action, that are responsible for that action's inclusion of those inputs.
+ */
+public interface ActionInputDepOwners {
+
+ /** An {@link ActionInputDepOwners} without any ownership associations. */
+ ActionInputDepOwners EMPTY_INSTANCE = input -> null;
+
+ /**
+ * Returns the {@link Artifact} associated with {@code input}, or {@code null} if no such
+ * association exists.
+ */
+ @Nullable
+ Artifact getDepOwner(ActionInput input);
+}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionInputMap.java b/src/main/java/com/google/devtools/build/lib/actions/ActionInputMap.java
index a1f3a93..99c8e0c 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionInputMap.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionInputMap.java
@@ -30,7 +30,7 @@
*
* <p>This class is thread-compatible.
*/
-public final class ActionInputMap implements MetadataProvider {
+public final class ActionInputMap implements MetadataProvider, ActionInputMapSink {
/** The number of elements contained in this map. */
int size;
@@ -116,8 +116,12 @@
return size;
}
- /** @return true if an entry was added, false if the map already contains {@code input} */
- public boolean put(ActionInput input, FileArtifactValue metadata) {
+ @Override
+ public boolean put(ActionInput input, FileArtifactValue metadata, @Nullable Artifact depOwner) {
+ return putWithNoDepOwner(input, metadata);
+ }
+
+ public boolean putWithNoDepOwner(ActionInput input, FileArtifactValue metadata) {
Preconditions.checkNotNull(input);
if (size >= keys.length) {
resize();
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionInputMapSink.java b/src/main/java/com/google/devtools/build/lib/actions/ActionInputMapSink.java
new file mode 100644
index 0000000..8d598d6
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionInputMapSink.java
@@ -0,0 +1,27 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.actions;
+
+import javax.annotation.Nullable;
+
+/**
+ * Provides a method for registering associations between {@link ActionInput}s, their {@link
+ * FileArtifactValue}s, and (optionally) the {@link Artifact} dependency responsible for their
+ * inclusion in an action's inputs.
+ */
+public interface ActionInputMapSink {
+
+ /** Returns true if an entry was added, false if the map already contains {@code input}. */
+ boolean put(ActionInput input, FileArtifactValue metadata, @Nullable Artifact depOwner);
+}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/LostInputsExecException.java b/src/main/java/com/google/devtools/build/lib/actions/LostInputsExecException.java
index 442b69f..3b2ff23 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/LostInputsExecException.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/LostInputsExecException.java
@@ -81,7 +81,7 @@
@Nullable
Artifact getOwner(ActionInput input);
- /** Returns the lost {@link ActionInput}s that came from runfiles. */
- Set<ActionInput> getRunfilesInputs();
+ /** Returns the lost {@link ActionInput}s that came from runfiles along with their owners. */
+ Set<ActionInput> getRunfilesInputsAndOwners();
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/MiddlemanAction.java b/src/main/java/com/google/devtools/build/lib/actions/MiddlemanAction.java
index d95bbfb..ee76215 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/MiddlemanAction.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/MiddlemanAction.java
@@ -100,6 +100,11 @@
return MIDDLEMAN_MNEMONIC;
}
+ @Override
+ public boolean mayInsensitivelyPropagateInputs() {
+ return true;
+ }
+
/**
* Creates a new middleman action.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
index 41cadcc..9cca974 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
@@ -25,7 +25,11 @@
import com.google.devtools.build.lib.actions.ActionCacheChecker.Token;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionExecutionException;
+import com.google.devtools.build.lib.actions.ActionInput;
+import com.google.devtools.build.lib.actions.ActionInputDepOwnerMap;
+import com.google.devtools.build.lib.actions.ActionInputDepOwners;
import com.google.devtools.build.lib.actions.ActionInputMap;
+import com.google.devtools.build.lib.actions.ActionInputMapSink;
import com.google.devtools.build.lib.actions.ActionLookupData;
import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.AlreadyReportedActionExecutionException;
@@ -70,6 +74,7 @@
import java.util.Set;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.IntFunction;
import javax.annotation.Nullable;
/**
@@ -214,7 +219,28 @@
} catch (LostInputsActionExecutionException e) {
// Remove action from state map in case it's there (won't be unless it discovers inputs).
stateMap.remove(action);
- RewindPlan rewindPlan = actionRewindStrategy.getRewindPlan(action, inputDepKeys, e, env);
+
+ // Reconstruct the relationship between lost inputs and this action's direct deps if any of
+ // the lost inputs came from runfiles:
+ ActionInputDepOwners runfilesDepOwners;
+ Set<ActionInput> lostRunfiles = e.getInputOwners().getRunfilesInputsAndOwners();
+ if (!lostRunfiles.isEmpty()) {
+ try {
+ runfilesDepOwners = getInputDepOwners(env, action, inputDeps, lostRunfiles);
+ } catch (ActionExecutionException unexpected) {
+ // getInputDepOwners should not be able to throw, because it does the same work as
+ // checkInputs, so if getInputDepOwners throws then checkInputs should have thrown, and if
+ // checkInputs threw then we shouldn't have reached this point in action execution.
+ throw new IllegalStateException(unexpected);
+ }
+ } else {
+ runfilesDepOwners = ActionInputDepOwners.EMPTY_INSTANCE;
+ }
+
+ // TODO(b/19539699): discovered deps aren't necessarily in inputDepKeys. They're discovered
+ // under checkCacheAndExecuteIfNeeded. Thread them through to here.
+ RewindPlan rewindPlan =
+ actionRewindStrategy.getRewindPlan(action, inputDepKeys, e, runfilesDepOwners, env);
for (Action actionToRestart : rewindPlan.getActionsToRestart()) {
skyframeActionExecutor.resetActionExecution(actionToRestart);
}
@@ -559,7 +585,7 @@
// execution, and found some new ones, but the new ones were already present in the graph.
// We must therefore cache the metadata for those new ones.
for (Map.Entry<SkyKey, SkyValue> entry : metadataFoundDuringActionExecution.entrySet()) {
- state.inputArtifactData.put(
+ state.inputArtifactData.putWithNoDepOwner(
ArtifactSkyKey.artifact(entry.getKey()), (FileArtifactValue) entry.getValue());
}
// TODO(ulfjack): This causes information loss about omitted and injected outputs. Also see
@@ -620,11 +646,11 @@
expandedArtifacts.put(input, ImmutableSet.<Artifact>copyOf(treeValue.getChildren()));
for (Map.Entry<Artifact.TreeFileArtifact, FileArtifactValue> child :
treeValue.getChildValues().entrySet()) {
- inputData.put(child.getKey(), child.getValue());
+ inputData.putWithNoDepOwner(child.getKey(), child.getValue());
}
- inputData.put(input, treeValue.getSelfData());
+ inputData.putWithNoDepOwner(input, treeValue.getSelfData());
} else {
- inputData.put(input, (FileArtifactValue) entry.getValue());
+ inputData.putWithNoDepOwner(input, (FileArtifactValue) entry.getValue());
}
}
}
@@ -695,6 +721,13 @@
}
}
+ private interface AccumulateInputResultsFactory<S extends ActionInputMapSink, R> {
+ R create(
+ S actionInputMapSink,
+ Map<Artifact, Collection<Artifact>> expandedArtifacts,
+ Map<Artifact, ImmutableList<FilesetOutputSymlink>> expandedFilesets);
+ }
+
/**
* Declare dependency on all known inputs of action. Throws exception if any are known to be
* missing. Some inputs may not yet be in the graph, in which case the builder should abort.
@@ -704,6 +737,33 @@
Action action,
Map<SkyKey, ValueOrException2<MissingInputFileException, ActionExecutionException>> inputDeps)
throws ActionExecutionException, InterruptedException {
+ return accumulateInputs(env, action, inputDeps, ActionInputMap::new, CheckInputResults::new);
+ }
+
+ /**
+ * Reconstructs the relationships between lost inputs and the direct deps responsible for them.
+ */
+ private ActionInputDepOwners getInputDepOwners(
+ Environment env,
+ Action action,
+ Map<SkyKey, ValueOrException2<MissingInputFileException, ActionExecutionException>> inputDeps,
+ Collection<ActionInput> lostInputs)
+ throws ActionExecutionException, InterruptedException {
+ return accumulateInputs(
+ env,
+ action,
+ inputDeps,
+ ignoredInputDepsSize -> new ActionInputDepOwnerMap(lostInputs),
+ (actionInputMapSink, expandedArtifacts, expandedFilesets) -> actionInputMapSink);
+ }
+
+ private <S extends ActionInputMapSink, R> R accumulateInputs(
+ Environment env,
+ Action action,
+ Map<SkyKey, ValueOrException2<MissingInputFileException, ActionExecutionException>> inputDeps,
+ IntFunction<S> actionInputMapSinkFactory,
+ AccumulateInputResultsFactory<S, R> accumulateInputResultsFactory)
+ throws ActionExecutionException, InterruptedException {
int missingCount = 0;
int actionFailures = 0;
// Only populate input data if we have the input values, otherwise they'll just go unused.
@@ -712,7 +772,7 @@
// some deps are still missing.
boolean populateInputData = !env.valuesMissing();
NestedSetBuilder<Cause> rootCauses = NestedSetBuilder.stableOrder();
- ActionInputMap inputArtifactData = new ActionInputMap(populateInputData ? inputDeps.size() : 0);
+ S inputArtifactData = actionInputMapSinkFactory.apply(populateInputData ? inputDeps.size() : 0);
Map<Artifact, Collection<Artifact>> expandedArtifacts =
new HashMap<>(populateInputData ? 128 : 0);
Map<Artifact, ImmutableList<FilesetOutputSymlink>> expandedFilesets = new HashMap<>();
@@ -779,7 +839,8 @@
rootCauses.build(),
/*catastrophe=*/ false);
}
- return new CheckInputResults(inputArtifactData, expandedArtifacts, expandedFilesets);
+ return accumulateInputResultsFactory.create(
+ inputArtifactData, expandedArtifacts, expandedFilesets);
}
private static Iterable<Artifact> filterKnownInputs(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java
index bbfb9e2..356ab9c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java
@@ -18,7 +18,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
-import com.google.devtools.build.lib.actions.ActionInputMap;
+import com.google.devtools.build.lib.actions.ActionInputMapSink;
import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
import com.google.devtools.build.lib.actions.Artifact;
@@ -37,17 +37,18 @@
// Adds a value obtained by an Artifact skyvalue lookup to the action input map. May do Skyframe
// lookups.
static void addToMap(
- ActionInputMap inputMap,
+ ActionInputMapSink inputMap,
Map<Artifact, Collection<Artifact>> expandedArtifacts,
Map<Artifact, ImmutableList<FilesetOutputSymlink>> expandedFilesets,
Artifact key,
SkyValue value,
- Environment env) throws InterruptedException {
+ Environment env)
+ throws InterruptedException {
if (value instanceof AggregatingArtifactValue) {
AggregatingArtifactValue aggregatingValue = (AggregatingArtifactValue) value;
for (Pair<Artifact, FileArtifactValue> entry : aggregatingValue.getFileArtifacts()) {
Artifact artifact = entry.first;
- inputMap.put(artifact, entry.second);
+ inputMap.put(artifact, entry.second, /*depOwner=*/ key);
if (artifact.isFileset()) {
ImmutableList<FilesetOutputSymlink> expandedFileset = getFilesets(env, artifact);
if (expandedFileset != null) {
@@ -60,11 +61,12 @@
entry.getFirst(),
Preconditions.checkNotNull(entry.getSecond()),
expandedArtifacts,
- inputMap);
+ inputMap,
+ /*depOwner=*/ key);
}
// We have to cache the "digest" of the aggregating value itself,
// because the action cache checker may want it.
- inputMap.put(key, aggregatingValue.getSelfData());
+ inputMap.put(key, aggregatingValue.getSelfData(), /*depOwner=*/ key);
// While not obvious at all this code exists to ensure that we don't expand the
// .runfiles/MANIFEST file into the inputs. The reason for that being that the MANIFEST
// file contains absolute paths that don't work with remote execution.
@@ -80,11 +82,10 @@
}
} else if (value instanceof TreeArtifactValue) {
expandTreeArtifactAndPopulateArtifactData(
- key, (TreeArtifactValue) value, expandedArtifacts, inputMap);
-
+ key, (TreeArtifactValue) value, expandedArtifacts, inputMap, /*depOwner=*/ key);
} else {
Preconditions.checkState(value instanceof FileArtifactValue);
- inputMap.put(key, (FileArtifactValue) value);
+ inputMap.put(key, (FileArtifactValue) value, /*depOwner=*/ key);
}
}
@@ -126,15 +127,16 @@
Artifact treeArtifact,
TreeArtifactValue value,
Map<Artifact, Collection<Artifact>> expandedArtifacts,
- ActionInputMap inputMap) {
+ ActionInputMapSink inputMap,
+ Artifact depOwner) {
ImmutableSet.Builder<Artifact> children = ImmutableSet.builder();
for (Map.Entry<Artifact.TreeFileArtifact, FileArtifactValue> child :
value.getChildValues().entrySet()) {
children.add(child.getKey());
- inputMap.put(child.getKey(), child.getValue());
+ inputMap.put(child.getKey(), child.getValue(), depOwner);
}
expandedArtifacts.put(treeArtifact, children.build());
// Again, we cache the "digest" of the value for cache checking.
- inputMap.put(treeArtifact, value.getSelfData());
+ inputMap.put(treeArtifact, value.getSelfData(), depOwner);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindStrategy.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindStrategy.java
index ef12703..5a3b23f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindStrategy.java
@@ -21,6 +21,7 @@
import com.google.common.collect.Maps;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionInput;
+import com.google.devtools.build.lib.actions.ActionInputDepOwners;
import com.google.devtools.build.lib.actions.ActionLookupData;
import com.google.devtools.build.lib.actions.AlreadyReportedActionExecutionException;
import com.google.devtools.build.lib.actions.Artifact;
@@ -68,6 +69,7 @@
Action failedAction,
Iterable<SkyKey> failedActionDeps,
LostInputsActionExecutionException lostInputsException,
+ ActionInputDepOwners runfilesDepOwners,
Environment env)
throws ActionExecutionFunctionException, InterruptedException {
ImmutableList<ActionInput> lostInputs = lostInputsException.getLostInputs();
@@ -80,15 +82,16 @@
ImmutableList.Builder<Action> actionsToRestart = ImmutableList.builder();
actionsToRestart.add(failedAction);
- HashMultimap<Artifact, ActionInput> lostInputsByOwners =
- getLostInputsByOwners(
+ HashMultimap<Artifact, ActionInput> lostInputsByDepOwners =
+ getLostInputsByDepOwners(
lostInputs,
lostInputsException.getInputOwners(),
+ runfilesDepOwners,
ImmutableSet.copyOf(failedActionDeps),
failedAction);
for (Map.Entry<Artifact, Collection<ActionInput>> entry :
- lostInputsByOwners.asMap().entrySet()) {
+ lostInputsByDepOwners.asMap().entrySet()) {
Artifact lostArtifact = entry.getKey();
if (lostArtifact.isSourceArtifact()) {
@@ -140,13 +143,14 @@
return actionsToCheckForPropagation.build();
}
- private HashMultimap<Artifact, ActionInput> getLostInputsByOwners(
+ private HashMultimap<Artifact, ActionInput> getLostInputsByDepOwners(
ImmutableList<ActionInput> lostInputs,
LostInputsExecException.InputOwners inputOwners,
+ ActionInputDepOwners runfilesDepOwners,
ImmutableSet<SkyKey> failedActionDeps,
Action failedActionForLogging) {
- HashMultimap<Artifact, ActionInput> lostInputsByOwners = HashMultimap.create();
+ HashMultimap<Artifact, ActionInput> lostInputsByDepOwners = HashMultimap.create();
for (ActionInput lostInput : lostInputs) {
if (failedActionDeps.contains(lostInput)) {
Preconditions.checkState(
@@ -155,30 +159,56 @@
+ "lostInput: %s\nfailedAction: %s",
lostInput,
failedActionForLogging);
- lostInputsByOwners.put((Artifact) lostInput, lostInput);
+ lostInputsByDepOwners.put((Artifact) lostInput, lostInput);
continue;
}
- Artifact lostInputOwner = inputOwners.getOwner(lostInput);
- if (lostInputOwner == null || !failedActionDeps.contains(lostInputOwner)) {
- // Rewinding can't do anything about a lost input that isn't a dep and that either has no
- // owner or has an owner that also isn't a dep. This may happen if the action consists of
- // a sequence of spawns where an output generated by one spawn is consumed by another but
- // was lost in-between. In this case, reevaluating the failed action (and no other deps) may
- // help, because doing so may rerun the generating spawn.
- //
- // It may also happen because of a bug, so we log that this has occurred.
- logger.info(
- String.format(
- "lostInput not a dep, and has no owner or its owner is not a dep.\nlostInput: %s\n"
- + "lostInputOwner: %s\nfailedAction: %s",
- lostInput, lostInputOwner, failedActionForLogging));
+ Artifact owner = inputOwners.getOwner(lostInput);
+ if (owner != null && failedActionDeps.contains(owner)) {
+ // The lost input is included in a tree artifact or fileset that the action directly depends
+ // on.
+ lostInputsByDepOwners.put(owner, lostInput);
continue;
}
- lostInputsByOwners.put(lostInputOwner, lostInput);
+ Artifact runfilesDepOwner = runfilesDepOwners.getDepOwner(lostInput);
+ if (runfilesDepOwner != null && failedActionDeps.contains(runfilesDepOwner)) {
+ // The lost input is included in a runfiles middleman that the action directly depends on.
+ lostInputsByDepOwners.put(runfilesDepOwner, lostInput);
+ continue;
+ }
+
+ Artifact runfilesDepTransitiveOwner = null;
+ if (owner != null) {
+ runfilesDepTransitiveOwner = runfilesDepOwners.getDepOwner(owner);
+ if (runfilesDepTransitiveOwner != null
+ && failedActionDeps.contains(runfilesDepTransitiveOwner)) {
+ // The lost input is included in a tree artifact or fileset which is included in a
+ // runfiles middleman that the action directly depends on.
+ lostInputsByDepOwners.put(runfilesDepTransitiveOwner, lostInput);
+ continue;
+ }
+ }
+
+ // Rewinding can't do anything about a lost input that can't be associated with a direct dep
+ // of the failed action. This may happen if the action consists of a sequence of spawns where
+ // an output generated by one spawn is consumed by another but was lost in-between. In this
+ // case, reevaluating the failed action (and no other deps) may help, because doing so may
+ // rerun the generating spawn.
+ //
+ // It may also happen because of a bug, so we log that this has occurred.
+ logger.info(
+ String.format(
+ "lostInput not a dep of the failed action, and can't be associated with such a dep.\n"
+ + "lostInput: %s\nowner: %s\nrunfilesDepOwner: %s\nrunfilesDepTransitiveOwner: %s"
+ + "\nfailedAction: %s",
+ lostInput,
+ owner,
+ runfilesDepOwner,
+ runfilesDepTransitiveOwner,
+ failedActionForLogging));
}
- return lostInputsByOwners;
+ return lostInputsByDepOwners;
}
private void recurseAcrossPropagatingActions(
diff --git a/src/test/java/com/google/devtools/build/lib/actions/ActionInputMapTest.java b/src/test/java/com/google/devtools/build/lib/actions/ActionInputMapTest.java
index 2f3313e..5402175 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/ActionInputMapTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/ActionInputMapTest.java
@@ -96,7 +96,7 @@
Collections.shuffle(data);
for (int i = 0; i < data.size(); ++i) {
TestEntry entry = data.get(i);
- assertThat(map.put(entry.input, entry.metadata)).isTrue();
+ assertThat(map.putWithNoDepOwner(entry.input, entry.metadata)).isTrue();
}
assertThat(map.size()).isEqualTo(data.size());
for (int i = 0; i < data.size(); ++i) {
@@ -107,7 +107,7 @@
}
private boolean put(String execPath, int value) {
- return map.put(new TestInput(execPath), new TestMetadata(value));
+ return map.putWithNoDepOwner(new TestInput(execPath), new TestMetadata(value));
}
private void assertContains(String execPath, int value) {
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
index 9c2e5ef..89ac11c 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
@@ -56,7 +56,7 @@
ActionInput input = ActionInputHelper.fromPath("foo/bar");
FileArtifactValue metadata = FileArtifactValue.createNormalFile(new byte[] {1, 2, 3}, 10);
ActionInputMap map = new ActionInputMap(1);
- map.put(input, metadata);
+ map.putWithNoDepOwner(input, metadata);
assertThat(map.getMetadata(input)).isEqualTo(metadata);
ActionMetadataHandler handler = new ActionMetadataHandler(
map,
@@ -74,7 +74,7 @@
Artifact artifact = new Artifact(path, sourceRoot);
FileArtifactValue metadata = FileArtifactValue.createNormalFile(new byte[] {1, 2, 3}, 10);
ActionInputMap map = new ActionInputMap(1);
- map.put(artifact, metadata);
+ map.putWithNoDepOwner(artifact, metadata);
ActionMetadataHandler handler = new ActionMetadataHandler(
map,
/* missingArtifactsAllowed= */ false,
