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/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);
}
}
