Rename ActionExecutionValue#createSimpleFileArtifactValue and make it an instance method.
The name of the method is misleading because it doesn't create anything new, just gets it from a map. It also asserts that it exists, so rename it to getExistingFileArtifactValue.
Also, make it an instance method. A static method that requires passing in an instance of the class seems awkward.
RELNOTES: None.
PiperOrigin-RevId: 312712401
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 91b428d..291f42a 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
@@ -39,6 +39,7 @@
import com.google.devtools.build.lib.actions.ActionRewoundEvent;
import com.google.devtools.build.lib.actions.AlreadyReportedActionExecutionException;
import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact;
import com.google.devtools.build.lib.actions.ArtifactPathResolver;
import com.google.devtools.build.lib.actions.DiscoveredInputsEvent;
import com.google.devtools.build.lib.actions.FileArtifactValue;
@@ -1049,8 +1050,8 @@
} else if (retrievedMetadata instanceof ActionExecutionValue) {
inputData.putWithNoDepOwner(
input,
- ActionExecutionValue.createSimpleFileArtifactValue(
- (Artifact.DerivedArtifact) input, (ActionExecutionValue) retrievedMetadata));
+ ((ActionExecutionValue) retrievedMetadata)
+ .getExistingFileArtifactValue((DerivedArtifact) input));
} else if (retrievedMetadata instanceof MissingFileArtifactValue) {
inputData.putWithNoDepOwner(input, FileArtifactValue.MISSING_FILE_MARKER);
} else if (retrievedMetadata instanceof FileArtifactValue) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java
index 1a2587e..7116a0a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java
@@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact;
import com.google.devtools.build.lib.actions.Artifact.OwnerlessArtifactWrapper;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.actions.ArtifactOwner;
@@ -117,19 +118,22 @@
}
/**
- * Create {@link FileArtifactValue} for artifact that must be non-middleman non-tree derived
- * artifact.
+ * Retrieves a {@link FileArtifactValue} for a regular (non-middleman, non-tree) derived artifact.
+ *
+ * <p>The value for the given artifact must be present.
*/
- static FileArtifactValue createSimpleFileArtifactValue(
- Artifact.DerivedArtifact artifact, ActionExecutionValue actionValue) {
- Preconditions.checkState(!artifact.isMiddlemanArtifact(), "%s %s", artifact, actionValue);
- Preconditions.checkState(!artifact.isTreeArtifact(), "%s %s", artifact, actionValue);
+ FileArtifactValue getExistingFileArtifactValue(DerivedArtifact artifact) {
+ Preconditions.checkState(
+ !artifact.isMiddlemanArtifact() && !artifact.isTreeArtifact(),
+ "Cannot request %s from %s",
+ artifact,
+ this);
return Preconditions.checkNotNull(
- actionValue.getArtifactValue(artifact),
- "%s %s %s",
+ getArtifactValue(artifact),
+ "Missing artifact %s (generating action key %s) in %s",
artifact,
artifact.getGeneratingActionKey(),
- actionValue);
+ this);
}
/**
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 9e15389..9c7a570 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
@@ -22,6 +22,8 @@
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;
+import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact;
+import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.actions.FilesetOutputSymlink;
import com.google.devtools.build.lib.analysis.actions.SymlinkAction;
@@ -51,7 +53,7 @@
inputMap.put(artifact, entry.second, /*depOwner=*/ key);
if (artifact.isFileset()) {
ImmutableList<FilesetOutputSymlink> expandedFileset =
- getFilesets(env, (Artifact.SpecialArtifact) artifact);
+ getFilesets(env, (SpecialArtifact) artifact);
if (expandedFileset != null) {
filesetsInsideRunfiles.put(artifact, expandedFileset);
}
@@ -87,11 +89,10 @@
} else if (value instanceof ActionExecutionValue) {
inputMap.put(
key,
- ActionExecutionValue.createSimpleFileArtifactValue(
- (Artifact.DerivedArtifact) key, (ActionExecutionValue) value),
+ ((ActionExecutionValue) value).getExistingFileArtifactValue((DerivedArtifact) key),
key);
if (key.isFileset()) {
- topLevelFilesets.put(key, getFilesets(env, (Artifact.SpecialArtifact) key));
+ topLevelFilesets.put(key, getFilesets(env, (SpecialArtifact) key));
}
} else {
Preconditions.checkState(value instanceof FileArtifactValue);
@@ -100,7 +101,7 @@
}
static ImmutableList<FilesetOutputSymlink> getFilesets(
- Environment env, Artifact.SpecialArtifact actionInput) throws InterruptedException {
+ Environment env, SpecialArtifact actionInput) throws InterruptedException {
Preconditions.checkState(actionInput.isFileset(), actionInput);
ActionLookupData generatingActionKey = actionInput.getGeneratingActionKey();
ActionLookupKey filesetActionLookupKey = generatingActionKey.getActionLookupKey();
@@ -113,8 +114,8 @@
ActionLookupData filesetActionKey;
if (generatingAction instanceof SymlinkAction) {
- Artifact.DerivedArtifact outputManifest =
- (Artifact.DerivedArtifact) generatingAction.getInputs().getSingleton();
+ DerivedArtifact outputManifest =
+ (DerivedArtifact) generatingAction.getInputs().getSingleton();
ActionLookupData manifestGeneratingKey = outputManifest.getGeneratingActionKey();
Preconditions.checkState(
manifestGeneratingKey.getActionLookupKey().equals(filesetActionLookupKey),
@@ -125,8 +126,8 @@
manifestGeneratingKey);
ActionAnalysisMetadata symlinkTreeAction =
filesetActionLookupValue.getAction(manifestGeneratingKey.getActionIndex());
- Artifact.DerivedArtifact inputManifest =
- (Artifact.DerivedArtifact) symlinkTreeAction.getInputs().getSingleton();
+ DerivedArtifact inputManifest =
+ (DerivedArtifact) symlinkTreeAction.getInputs().getSingleton();
ActionLookupData inputManifestGeneratingKey = inputManifest.getGeneratingActionKey();
Preconditions.checkState(
inputManifestGeneratingKey.getActionLookupKey().equals(filesetActionLookupKey),
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 c582e7b..e2c4621 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
@@ -216,8 +216,7 @@
for (TreeFileArtifact treeFileArtifact : treeFileArtifacts) {
FileArtifactValue value =
- ActionExecutionValue.createSimpleFileArtifactValue(
- treeFileArtifact, actionExecutionValue);
+ actionExecutionValue.getExistingFileArtifactValue(treeFileArtifact);
if (FileArtifactValue.OMITTED_FILE_MARKER.equals(value)) {
omitted = true;
} else {
@@ -333,8 +332,8 @@
fileInputsBuilder.add(
Pair.of(
input,
- ActionExecutionValue.createSimpleFileArtifactValue(
- (DerivedArtifact) input, (ActionExecutionValue) inputValue)));
+ ((ActionExecutionValue) inputValue)
+ .getExistingFileArtifactValue((DerivedArtifact) input)));
} else if (inputValue instanceof TreeArtifactValue) {
directoryInputsBuilder.add(Pair.of(input, (TreeArtifactValue) inputValue));
} else {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java
index d636b21..f260515 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java
@@ -22,6 +22,7 @@
import com.google.common.collect.Collections2;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.actions.FileStateType;
@@ -278,9 +279,8 @@
fsVal = (HasDigest) value;
} else if (value instanceof ActionExecutionValue) {
fsVal =
- Preconditions.checkNotNull(
- ActionExecutionValue.createSimpleFileArtifactValue(
- (Artifact.DerivedArtifact) artifact, (ActionExecutionValue) value));
+ ((ActionExecutionValue) value)
+ .getExistingFileArtifactValue((DerivedArtifact) artifact);
} else {
return NON_EXISTENT_FILE_INFO;
}
