Change signatures to DerivedArtifact, and get Labels from non-Artifact sources when possible.
In tests, try to get artifacts directly from the actual configured target, rather than creating fresh ones.
This change should be a prod functional no-op, just changing signatures. Split out from the follow-up to reduce the diff.
PiperOrigin-RevId: 250527865
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 0cd2c56..0dce603 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
@@ -678,7 +678,7 @@
}
ImmutableList<FilesetOutputSymlink> mapping =
- ActionInputMapHelper.getFilesets(env, actionInput);
+ ActionInputMapHelper.getFilesets(env, (Artifact.SpecialArtifact) actionInput);
if (mapping == null) {
return null;
}
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 356ab9c..26ca7e2 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
@@ -50,7 +50,8 @@
Artifact artifact = entry.first;
inputMap.put(artifact, entry.second, /*depOwner=*/ key);
if (artifact.isFileset()) {
- ImmutableList<FilesetOutputSymlink> expandedFileset = getFilesets(env, artifact);
+ ImmutableList<FilesetOutputSymlink> expandedFileset =
+ getFilesets(env, (Artifact.SpecialArtifact) artifact);
if (expandedFileset != null) {
expandedFilesets.put(artifact, expandedFileset);
}
@@ -89,8 +90,8 @@
}
}
- static ImmutableList<FilesetOutputSymlink> getFilesets(Environment env,
- Artifact actionInput) throws InterruptedException {
+ static ImmutableList<FilesetOutputSymlink> getFilesets(
+ Environment env, Artifact.SpecialArtifact actionInput) throws InterruptedException {
Preconditions.checkState(actionInput.isFileset(), actionInput);
ActionLookupKey filesetActionLookupKey = (ActionLookupKey) actionInput.getArtifactOwner();
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
index f82d075..e4fd0fb 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
@@ -493,7 +493,7 @@
@Override
public void injectRemoteDirectory(
- Artifact output, Map<PathFragment, RemoteFileArtifactValue> children) {
+ SpecialArtifact output, Map<PathFragment, RemoteFileArtifactValue> children) {
Preconditions.checkArgument(
isKnownOutput(output), output + " is not a declared output of this action");
Preconditions.checkArgument(output.isTreeArtifact(), "output must be a tree artifact");
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 9d9cfeb..f3f5372 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
@@ -106,7 +106,7 @@
// failedAction, which the caller of getRewindPlan already knows must be restarted).
ImmutableList.Builder<Action> additionalActionsToRestart = ImmutableList.builder();
- HashMultimap<Artifact, ActionInput> lostInputsByDepOwners =
+ HashMultimap<Artifact.DerivedArtifact, ActionInput> lostInputsByDepOwners =
getLostInputsByDepOwners(
lostInputs,
lostInputsException.getInputOwners(),
@@ -114,9 +114,9 @@
ImmutableSet.copyOf(failedActionDeps),
failedAction);
- for (Map.Entry<Artifact, Collection<ActionInput>> entry :
+ for (Map.Entry<Artifact.DerivedArtifact, Collection<ActionInput>> entry :
lostInputsByDepOwners.asMap().entrySet()) {
- Artifact lostArtifact = entry.getKey();
+ Artifact.DerivedArtifact lostArtifact = entry.getKey();
checkIfLostArtifactIsSource(
lostArtifact, failedAction, lostInputsException, entry.getValue());
@@ -207,14 +207,15 @@
}
}
- private HashMultimap<Artifact, ActionInput> getLostInputsByDepOwners(
+ private HashMultimap<Artifact.DerivedArtifact, ActionInput> getLostInputsByDepOwners(
ImmutableList<ActionInput> lostInputs,
LostInputsExecException.InputOwners inputOwners,
ActionInputDepOwners runfilesDepOwners,
ImmutableSet<SkyKey> failedActionDeps,
Action failedActionForLogging) {
- HashMultimap<Artifact, ActionInput> lostInputsByDepOwners = HashMultimap.create();
+ HashMultimap<Artifact.DerivedArtifact, ActionInput> lostInputsByDepOwners =
+ HashMultimap.create();
for (ActionInput lostInput : lostInputs) {
boolean foundLostInputDepOwner = false;
Artifact owner = inputOwners.getOwner(lostInput);
@@ -231,7 +232,8 @@
if (failedActionDeps.contains(runfilesTransitiveOwner)) {
// The lost input's owning tree artifact or fileset is included in a runfiles middleman
// that the action directly depends on.
- lostInputsByDepOwners.put(runfilesTransitiveOwner, lostInput);
+ lostInputsByDepOwners.put(
+ (Artifact.DerivedArtifact) runfilesTransitiveOwner, lostInput);
foundLostInputDepOwner = true;
}
}
@@ -240,7 +242,7 @@
Collection<Artifact> runfilesOwners = runfilesDepOwners.getDepOwners(lostInput);
for (Artifact runfilesOwner : runfilesOwners) {
if (failedActionDeps.contains(runfilesOwner)) {
- lostInputsByDepOwners.put(runfilesOwner, lostInput);
+ lostInputsByDepOwners.put((Artifact.DerivedArtifact) runfilesOwner, lostInput);
foundLostInputDepOwner = true;
}
}
@@ -248,7 +250,7 @@
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);
+ lostInputsByDepOwners.put((Artifact.DerivedArtifact) owner, lostInput);
foundLostInputDepOwner = true;
}
@@ -259,7 +261,7 @@
+ "lostInput: %s, failedAction: %.10000s",
lostInput,
failedActionForLogging);
- lostInputsByDepOwners.put((Artifact) lostInput, lostInput);
+ lostInputsByDepOwners.put((Artifact.DerivedArtifact) lostInput, lostInput);
foundLostInputDepOwner = true;
}
@@ -302,7 +304,7 @@
ActionAndLookupData actionAndLookupData = uncheckedActions.removeFirst();
ActionLookupData actionKey = actionAndLookupData.lookupData();
Action action = actionAndLookupData.action();
- HashSet<Artifact> artifactsToCheck = new HashSet<>();
+ HashSet<Artifact.DerivedArtifact> artifactsToCheck = new HashSet<>();
if (action instanceof SkyframeAwareAction) {
// This action depends on more than just its input artifact values. We need to also restart
@@ -319,7 +321,7 @@
addPropagatingActionDepsAndGetNewlyVisitedArtifacts(rewindGraph, actionKey, action));
}
- for (Artifact artifact : artifactsToCheck) {
+ for (Artifact.DerivedArtifact artifact : artifactsToCheck) {
Map<ActionLookupData, Action> actionMap = getActionsForLostArtifact(artifact, env);
if (actionMap == null) {
continue;
@@ -338,7 +340,7 @@
*
* <p>Returns a list of artifacts which were newly added to the graph.
*/
- private ImmutableList<Artifact> addSkyframeAwareDepsAndGetNewlyVisitedArtifacts(
+ private ImmutableList<Artifact.DerivedArtifact> addSkyframeAwareDepsAndGetNewlyVisitedArtifacts(
MutableGraph<SkyKey> rewindGraph, ActionLookupData actionKey, SkyframeAwareAction action) {
ImmutableGraph<SkyKey> graph = action.getSkyframeDependenciesForRewinding(actionKey);
@@ -347,12 +349,12 @@
}
assertSkyframeAwareRewindingGraph(graph, actionKey);
- ImmutableList.Builder<Artifact> newlyVisitedArtifacts = ImmutableList.builder();
+ ImmutableList.Builder<Artifact.DerivedArtifact> newlyVisitedArtifacts = ImmutableList.builder();
Set<EndpointPair<SkyKey>> edges = graph.edges();
for (EndpointPair<SkyKey> edge : edges) {
SkyKey target = edge.target();
if (target instanceof Artifact && rewindGraph.addNode(target)) {
- newlyVisitedArtifacts.add(((Artifact) target));
+ newlyVisitedArtifacts.add(((Artifact.DerivedArtifact) target));
}
rewindGraph.putEdge(edge.source(), edge.target());
}
@@ -365,10 +367,11 @@
*
* <p>Returns a list of artifacts which were newly added to the graph.
*/
- private ImmutableList<Artifact> addPropagatingActionDepsAndGetNewlyVisitedArtifacts(
- MutableGraph<SkyKey> rewindGraph, ActionLookupData actionKey, Action action) {
+ private ImmutableList<Artifact.DerivedArtifact>
+ addPropagatingActionDepsAndGetNewlyVisitedArtifacts(
+ MutableGraph<SkyKey> rewindGraph, ActionLookupData actionKey, Action action) {
- ImmutableList.Builder<Artifact> newlyVisitedArtifacts = ImmutableList.builder();
+ ImmutableList.Builder<Artifact.DerivedArtifact> newlyVisitedArtifacts = ImmutableList.builder();
for (Artifact input : action.getInputs()) {
if (input.isSourceArtifact()) {
continue;
@@ -380,7 +383,7 @@
//
// Rewinding is expected to be rare, so refining this may not be necessary.
if (rewindGraph.addNode(input)) {
- newlyVisitedArtifacts.add(input);
+ newlyVisitedArtifacts.add((Artifact.DerivedArtifact) input);
}
rewindGraph.putEdge(actionKey, input);
}
@@ -432,7 +435,7 @@
*/
@Nullable
private Map<ActionLookupData, Action> getActionsForLostArtifact(
- Artifact lostInput, Environment env) throws InterruptedException {
+ Artifact.DerivedArtifact lostInput, Environment env) throws InterruptedException {
Set<ActionLookupData> actionExecutionDeps = getActionExecutionDeps(lostInput, env);
if (actionExecutionDeps == null) {
return null;
@@ -451,8 +454,8 @@
* or {@code null} if any of those dependencies are not done.
*/
@Nullable
- private Set<ActionLookupData> getActionExecutionDeps(Artifact lostInput, Environment env)
- throws InterruptedException {
+ private Set<ActionLookupData> getActionExecutionDeps(
+ Artifact.DerivedArtifact lostInput, Environment env) throws InterruptedException {
ArtifactFunction.ArtifactDependencies artifactDependencies =
ArtifactFunction.ArtifactDependencies.discoverDependencies(lostInput, env);
if (artifactDependencies == null) {
@@ -519,6 +522,13 @@
graph,
actionKey,
target);
+ Preconditions.checkState(
+ !(target instanceof Artifact) || target instanceof Artifact.DerivedArtifact,
+ "A non-source artifact must be derived. graph: %s, rootActionNode: %s, sourceArtifact:"
+ + " %s",
+ graph,
+ actionKey,
+ target);
}
}
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 71de21a..bfec907 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
@@ -83,7 +83,7 @@
}
ArtifactDependencies artifactDependencies =
- ArtifactDependencies.discoverDependencies(artifact, env);
+ ArtifactDependencies.discoverDependencies((Artifact.DerivedArtifact) artifact, env);
if (artifactDependencies == null) {
return null;
}
@@ -446,16 +446,13 @@
}
/**
- * Constructs an {@link ArtifactDependencies} for the provided {@code derivedArtifact}, which
- * must not be a source artifact. Returns {@code null} if any dependencies are not yet ready.
+ * Constructs an {@link ArtifactDependencies} for the provided {@code derivedArtifact}. Returns
+ * {@code null} if any dependencies are not yet ready.
*/
@Nullable
static ArtifactDependencies discoverDependencies(
- Artifact derivedArtifact, SkyFunction.Environment env) throws InterruptedException {
- Preconditions.checkArgument(
- !derivedArtifact.isSourceArtifact(),
- "derivedArtifact is not derived: %s",
- derivedArtifact);
+ Artifact.DerivedArtifact derivedArtifact, SkyFunction.Environment env)
+ throws InterruptedException {
ActionLookupKey actionLookupKey = ArtifactFunction.getActionLookupKey(derivedArtifact);
ActionLookupValue actionLookupValue =
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
index 10a63fb..44a9325 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
@@ -387,7 +387,8 @@
artifactValue,
env);
if (input.isFileset()) {
- expandedFilesets.put(input, ActionInputMapHelper.getFilesets(env, input));
+ expandedFilesets.put(
+ input, ActionInputMapHelper.getFilesets(env, (Artifact.SpecialArtifact) input));
}
}
} catch (MissingInputFileException e) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
index f2d6021..bc740eb 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -32,7 +32,6 @@
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.ArtifactFactory;
-import com.google.devtools.build.lib.actions.ArtifactOwner;
import com.google.devtools.build.lib.actions.ArtifactPrefixConflictException;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.actions.PackageRoots;
@@ -727,7 +726,7 @@
/** Returns null if any build-info values are not ready. */
@Nullable
CachingAnalysisEnvironment createAnalysisEnvironment(
- ArtifactOwner owner,
+ ActionLookupValue.ActionLookupKey owner,
boolean isSystemEnv,
ExtendedEventHandler eventHandler,
Environment env,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionFunction.java
index 503a124..112fd7e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionFunction.java
@@ -77,7 +77,7 @@
}
}
} else {
- Multimap<ActionLookupValue.ActionLookupKey, Artifact> keyToArtifactMap =
+ Multimap<ActionLookupValue.ActionLookupKey, Artifact.DerivedArtifact> keyToArtifactMap =
Multimaps.index(
TestProvider.getTestStatusArtifacts(ct), ArtifactFunction::getActionLookupKey);
Map<SkyKey, SkyValue> actionLookupValues = env.getValues(keyToArtifactMap.keySet());
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusValue.java
index d54379a4..55876e3 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusValue.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import com.google.devtools.build.lib.actions.Actions;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.BasicActionLookupValue;
import com.google.devtools.build.lib.analysis.WorkspaceStatusAction;
@@ -37,7 +38,8 @@
Artifact stableArtifact,
Artifact volatileArtifact,
WorkspaceStatusAction workspaceStatusAction) {
- super(workspaceStatusAction);
+ super(
+ Actions.GeneratingActions.fromSingleAction(workspaceStatusAction), /*nonceVersion=*/ null);
this.stableArtifact = stableArtifact;
this.volatileArtifact = volatileArtifact;
}
