Add a reference to the pertinent RunfilesTree to RunfilesArtifactValue.
RELNOTES: None.
PiperOrigin-RevId: 599787221
Change-Id: Id0808314613f8b387fef59f83f76fe0e892fa20f
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
index 71dda42..5250c00 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
@@ -32,6 +32,7 @@
import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.actions.FilesetTraversalParams.DirectTraversalRoot;
import com.google.devtools.build.lib.actions.FilesetTraversalParams.PackageBoundaryMode;
+import com.google.devtools.build.lib.actions.MiddlemanAction;
import com.google.devtools.build.lib.bugreport.BugReport;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.Event;
@@ -148,7 +149,7 @@
artifactDependencies);
FileArtifactValue individualMetadata = actionValue.getExistingFileArtifactValue(artifact);
- return createRunfilesArtifactValue(artifact, action, individualMetadata, env);
+ return createRunfilesArtifactValue(artifact, (MiddlemanAction) action, individualMetadata, env);
}
private static void mkdirForTreeArtifact(
@@ -344,7 +345,7 @@
@Nullable
private static RunfilesArtifactValue createRunfilesArtifactValue(
Artifact artifact,
- ActionAnalysisMetadata action,
+ MiddlemanAction action,
FileArtifactValue value,
SkyFunction.Environment env)
throws InterruptedException {
@@ -388,7 +389,12 @@
}
return new RunfilesArtifactValue(
- value, files.build(), fileValues.build(), trees.build(), treeValues.build());
+ value,
+ action.getRunfilesTree(),
+ files.build(),
+ fileValues.build(),
+ trees.build(),
+ treeValues.build());
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
index 33e40a6..40907eb 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -598,6 +598,7 @@
":tree_artifact_value",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
+ "//src/main/java/com/google/devtools/build/lib/actions:runfiles_supplier",
"//src/main/java/com/google/devtools/build/lib/util:hash_codes",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//third_party:guava",
@@ -682,10 +683,8 @@
"//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
- "//src/main/java/com/google/devtools/build/lib/actions:middleman_type",
"//src/main/java/com/google/devtools/build/lib/bugreport",
"//src/main/java/com/google/devtools/build/lib/cmdline",
- "//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RunfilesArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/RunfilesArtifactValue.java
index ec76bfc..cc8e3f2 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RunfilesArtifactValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RunfilesArtifactValue.java
@@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.FileArtifactValue;
+import com.google.devtools.build.lib.actions.RunfilesSupplier.RunfilesTree;
import com.google.devtools.build.lib.util.HashCodes;
import com.google.devtools.build.skyframe.SkyValue;
@@ -32,6 +33,7 @@
}
private final FileArtifactValue metadata;
+ private final RunfilesTree runfilesTree;
// Parallel lists.
private final ImmutableList<Artifact> files;
@@ -43,11 +45,13 @@
RunfilesArtifactValue(
FileArtifactValue metadata,
+ RunfilesTree runfilesTree,
ImmutableList<Artifact> files,
ImmutableList<FileArtifactValue> fileValues,
ImmutableList<Artifact> trees,
ImmutableList<TreeArtifactValue> treeValues) {
this.metadata = checkNotNull(metadata);
+ this.runfilesTree = checkNotNull(runfilesTree);
this.files = checkNotNull(files);
this.fileValues = checkNotNull(fileValues);
this.trees = checkNotNull(trees);
@@ -63,6 +67,11 @@
return metadata;
}
+ /** Returns the runfiles tree this value represents. */
+ RunfilesTree getRunfilesTree() {
+ return runfilesTree;
+ }
+
/** Visits the file artifacts that this runfiles artifact expands to, together with their data. */
void forEachFile(RunfilesConsumer<FileArtifactValue> consumer) throws InterruptedException {
for (int i = 0; i < files.size(); i++) {
@@ -79,6 +88,22 @@
@Override
public boolean equals(Object o) {
+ // This method, seemingly erroneously, does not check whether the runfilesTree of the two
+ // objects is equivalent. This is because it's costly (it involves flattening nested sets and
+ // even if one caches a fingerprint, it's still a fair amount of CPU) and because it's
+ // currently not necessary: RunfilesArtifactValue is only ever created as the SkyValue of
+ // runfiles middlemen and those are special-cased in ActionCacheChecker (see
+ // ActionCacheChecker.checkMiddlemanAction()): the checksum of a middleman artifact is the
+ // function of the checksum of all the artifacts on the inputs of the middleman action, which
+ // includes both the artifacts the runfiles tree links to and the runfiles input manifest,
+ // which in turn encodes the structure of the runfiles tree. The checksum of the middleman
+ // artifact is here as the "metadata" field, which *is* compared here, so the
+ // RunfilesArtifactValues of two runfiles middlemen will be equals iff they represent the same
+ // runfiles tree.
+ //
+ // Eventually, if we ever do away with runfiles input manifests, it will be necessary to change
+ // this (it's weird that one needs to do a round-trip to the file system to determine the
+ // checksum of a runfiles tree), but that's not happening anytime soon.
if (this == o) {
return true;
}
