Automated rollback of commit 3290e22356b59371274849ee51297635b9435285.
*** Reason for rollback ***
Rolling forward with fix: keep passing artifact owner into generated artifact, sacrificing a bit of type safety for actual safety. This effectively reverts a few files from the original CL.
Also fix memory regression: key map of output file artifacts by label, since we can reuse existing labels. Moreover, this allows us to have an empty map for many configured targets: a build of a large internal target had the following numbers of targets per # of output files (apologies for the only partially sorted map):
{0=63649, 1=67148, 2=89175, 3=49347, 4=12712, 5=3027, 261=1, 6=247, 7=9, 775=1, 8=10, 9=82, 10=6, 11=3, 12=54, 13=4, 14=2, 15=24, 655=1, 271=2, 144=1, 17=2, 18=24, 276=1, 21=9, 3990=2, 24=18, 27=11, 156=3, 30=6, 31=2, 33=12, 34=1, 163=1, 36=5, 37=1, 38=2, 294=1, 39=5, 41=1, 42=3, 43=2, 45=9, 302=1, 47=1, 48=4, 561=1, 306=2, 54=2, 183=1, 57=1, 573=2, 318=2, 63=2, 65=1, 66=1, 198=1, 71=1, 72=2, 330=2, 75=2, 204=1, 76=1, 77=2, 80=1, 82=1, 84=3, 87=2, 346=2, 90=4, 480=1, 99=5, 102=1, 231=2, 363=1, 624=2, 117=1, 120=2, 123=2, 252=2}
So those are the sizes of the maps that are actually needed. With the regression fixed, this change actually has a significant progression because we no longer create a duplicate derived artifact (with PathFragments, etc.) per OutputFileConfiguredTarget. Moreover, we don't need to keep a map with every artifact as keys inside ActionLookupValue and RuleConfiguredTarget. The only regression remaining is that we eagerly create ActionLookupData objects for every artifact, versus only on demand during execution. That's not a major issue, and it's ameliorated by no longer needing to intern ActionLookupData objects, which has memory and CPU benefits. Total memory diff for analysis of our large internal target looks very nice (net -125M out of 5.88G, or 2% gain):
objsize chg instances space KB class name
------------------------------------------------------
24 +0 1345958 31545 KB com.google.devtools.build.lib.actions.ActionLookupData
70 +0 -334 1992 KB [Ljava.lang.Object;
16 +0 -19073 -298 KB java.lang.Integer
40 +0 -12795 -499 KB com.google.common.collect.SingletonImmutableBiMap
40 +0 -45433 -1774 KB com.google.common.collect.MapMakerInternalMap$WeakKeyDummyValueEntry
32 -8 0 -1852 KB com.google.devtools.build.lib.analysis.ConfiguredAspect
24 +0 -79943 -1873 KB com.google.common.collect.SingletonImmutableSet
24 +0 -79943 -1873 KB com.google.common.collect.ImmutableEntry
40 +0 -61756 -2412 KB com.google.common.collect.RegularImmutableMap
16 +0 -216644 -3385 KB com.google.common.collect.RegularImmutableList
24 +0 -216650 -5077 KB com.google.common.collect.ImmutableMapEntrySet$RegularEntrySet
44 -4 -61755 -7376 KB [Ljava.util.Map$Entry;
44 -3 -61757 -7409 KB [Lcom.google.common.collect.ImmutableMapEntry;
32 +0 -259112 -8097 KB com.google.devtools.build.lib.actions.Artifact$DerivedArtifact
24 +0 -522911 -12255 KB com.google.devtools.build.lib.vfs.PathFragment
24 +0 -525254 -12310 KB java.lang.String
24 +0 -541285 -12686 KB com.google.common.collect.ImmutableMapEntry$NonTerminalImmutableMapEntry
24 +0 -999865 -23434 KB com.google.common.collect.ImmutableMapEntry
85 +0 -524326 -56276 KB [B
------------------------------------------------------
total change: -125514 KB
PiperOrigin-RevId: 251755552
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionInputHelper.java b/src/main/java/com/google/devtools/build/lib/actions/ActionInputHelper.java
index 2d2ffd7..e7ef58c 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionInputHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionInputHelper.java
@@ -21,7 +21,9 @@
import com.google.common.collect.Collections2;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
+import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -144,19 +146,10 @@
*/
public static TreeFileArtifact treeFileArtifact(
Artifact.SpecialArtifact parent, PathFragment relativePath) {
- Preconditions.checkState(parent.isTreeArtifact(),
- "Given parent %s must be a TreeArtifact", parent);
- return new TreeFileArtifact(parent, relativePath);
- }
-
- public static TreeFileArtifact treeFileArtifact(
- Artifact.SpecialArtifact parent, PathFragment relativePath, ArtifactOwner artifactOwner) {
- Preconditions.checkState(parent.isTreeArtifact(),
- "Given parent %s must be a TreeArtifact", parent);
- return new TreeFileArtifact(
- parent,
- relativePath,
- artifactOwner);
+ TreeFileArtifact result =
+ treeFileArtifactWithNoGeneratingActionSet(parent, relativePath, parent.getArtifactOwner());
+ result.setGeneratingActionKey(parent.getGeneratingActionKey());
+ return result;
}
/**
@@ -168,6 +161,13 @@
return treeFileArtifact(parent, PathFragment.create(relativePath));
}
+ public static TreeFileArtifact treeFileArtifactWithNoGeneratingActionSet(
+ SpecialArtifact parent, PathFragment relativePath, ActionLookupKey artifactOwner) {
+ Preconditions.checkState(
+ parent.isTreeArtifact(), "Given parent %s must be a TreeArtifact", parent);
+ return new TreeFileArtifact(parent, relativePath, artifactOwner);
+ }
+
/** Returns an Iterable of TreeFileArtifacts with the given parent and parent relative paths. */
public static Iterable<TreeFileArtifact> asTreeFileArtifacts(
final Artifact.SpecialArtifact parent, Iterable<? extends PathFragment> parentRelativePaths) {
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupData.java b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupData.java
index b71f22d..fc33a23 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupData.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupData.java
@@ -13,12 +13,11 @@
// limitations under the License.
package com.google.devtools.build.lib.actions;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
-import com.google.common.collect.Interner;
import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.concurrent.BlazeInterners;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.skyframe.ShareabilityOfValue;
import com.google.devtools.build.skyframe.SkyFunctionName;
@@ -27,7 +26,6 @@
/** Data that uniquely identifies an action. */
@AutoCodec
public class ActionLookupData implements SkyKey {
- private static final Interner<ActionLookupData> INTERNER = BlazeInterners.newWeakInterner();
// Test actions are not shareable.
// Action execution can be nondeterministic, so is semi-hermetic.
public static final SkyFunctionName NAME = SkyFunctionName.createSemiHermetic("ACTION_EXECUTION");
@@ -40,9 +38,14 @@
this.actionIndex = actionIndex;
}
+ /**
+ * Creates a key for the result of action execution. Does <i>not</i> intern its results, so should
+ * only be called once per {@code (actionLookupKey, actionIndex)} pair.
+ */
+ @VisibleForTesting
@AutoCodec.Instantiator
public static ActionLookupData create(ActionLookupKey actionLookupKey, int actionIndex) {
- return INTERNER.intern(new ActionLookupData(actionLookupKey, actionIndex));
+ return new ActionLookupData(actionLookupKey, actionIndex);
}
public ActionLookupKey getActionLookupKey() {
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java
index 6611deb..278d9e8 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java
@@ -15,15 +15,11 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Maps;
import com.google.devtools.build.lib.actions.Artifact.SourceArtifact;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import java.math.BigInteger;
-import java.util.List;
-import java.util.Map;
import javax.annotation.Nullable;
/**
@@ -39,38 +35,6 @@
public abstract ImmutableList<ActionAnalysisMetadata> getActions();
/**
- * Returns a map where keys are artifacts generated by this {@link SkyValue}, and values are
- * the index of the action which generates this artifact.
- */
- protected abstract ImmutableMap<Artifact, Integer> getGeneratingActionIndex();
-
- /**
- * Returns the index of the action that generates {@code artifact} in this value, or null if this
- * value does not have a generating action for this artifact. The index together with the key for
- * this {@link ActionLookupValue} uniquely identifies the action.
- *
- * <p>Unlike {@link #getAction}, this may be called after action execution.
- */
- @Nullable
- public Integer getGeneratingActionIndex(Artifact artifact) {
- return getGeneratingActionIndex().get(artifact);
- }
-
- /**
- * Returns the action that generates {@code artifact}, if known to this value, or null. This
- * method should be avoided. Call it only when the action is really needed, and it is known to be
- * present.
- */
- @Nullable
- public ActionAnalysisMetadata getGeneratingActionDangerousReadJavadoc(Artifact artifact) {
- Integer actionIndex = getGeneratingActionIndex(artifact);
- if (actionIndex == null) {
- return null;
- }
- return getActions().get(actionIndex);
- }
-
- /**
* Returns the {@link Action} with index {@code index} in this value. Never null. Should only be
* called during action execution by {@code ArtifactFunction} and {@code ActionExecutionFunction}
* -- after an action has executed, calling this with its index may crash.
@@ -98,17 +62,6 @@
return getActions().get(index) instanceof ActionTemplate;
}
- /** To be used only when checking consistency of the action graph -- not by other values. */
- public Map<Artifact, ActionAnalysisMetadata> getMapForConsistencyCheck() {
- return getMapForConsistencyCheck(getGeneratingActionIndex(), getActions());
- }
-
- public static Map<Artifact, ActionAnalysisMetadata> getMapForConsistencyCheck(
- Map<Artifact, Integer> generatingActionIndex,
- final List<? extends ActionAnalysisMetadata> actions) {
- return Maps.transformValues(generatingActionIndex, actions::get);
- }
-
/**
* Returns the number of {@link Action} objects present in this value.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionTemplate.java b/src/main/java/com/google/devtools/build/lib/actions/ActionTemplate.java
index 335b5d4..df4e219 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionTemplate.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionTemplate.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.actions;
+import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
import javax.annotation.Nullable;
@@ -78,7 +79,7 @@
* TreeFileArtifact}
*/
Iterable<T> generateActionForInputArtifacts(
- Iterable<TreeFileArtifact> inputTreeFileArtifacts, ArtifactOwner artifactOwner)
+ Iterable<TreeFileArtifact> inputTreeFileArtifacts, ActionLookupKey artifactOwner)
throws ActionTemplateExpansionException;
/** Returns the input TreeArtifact. */
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Actions.java b/src/main/java/com/google/devtools/build/lib/actions/Actions.java
index 4e9fb5e..1726fc9 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Actions.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Actions.java
@@ -19,9 +19,11 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.escape.Escaper;
import com.google.common.escape.Escapers;
+import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
+import com.google.devtools.build.lib.packages.OutputFile;
import com.google.devtools.build.lib.vfs.OsPathPolicy;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.Collection;
@@ -31,8 +33,6 @@
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.SortedMap;
-import java.util.TreeMap;
-import java.util.function.Function;
import javax.annotation.Nullable;
/**
@@ -112,80 +112,154 @@
}
/**
- * Finds action conflicts. An action conflict happens if two actions generate the same output
- * artifact. Shared actions are tolerated. See {@link #canBeShared} for details.
+ * Assigns generating action keys to artifacts, and finds action conflicts. An action conflict
+ * happens if two actions generate the same output artifact. Shared actions are not allowed. See
+ * {@link #canBeShared} for details. Should only be called for special action lookup values: does
+ * not handle normal configured targets. In particular, {@link
+ * GeneratingActions#getArtifactsByOutputLabel} will be empty.
*
- * @param actions a list of actions to check for action conflicts
- * @return a structure giving the mapping between artifacts and generating actions, with a level
- * of indirection.
+ * @param actions a list of actions to check for action conflict.
+ * @return a structure giving the actions, with a level of indirection.
* @throws ActionConflictException iff there are two actions generate the same output
*/
- public static GeneratingActions findAndThrowActionConflict(
- ActionKeyContext actionKeyContext, ImmutableList<ActionAnalysisMetadata> actions)
+ public static GeneratingActions assignOwnersAndFindAndThrowActionConflict(
+ ActionKeyContext actionKeyContext,
+ ImmutableList<ActionAnalysisMetadata> actions,
+ ActionLookupValue.ActionLookupKey actionLookupKey)
throws ActionConflictException {
- return Actions.maybeFilterSharedActionsAndThrowIfConflict(
- actionKeyContext, actions, /*allowSharedAction=*/ false);
+ return Actions.assignOwnersAndMaybeFilterSharedActionsAndThrowIfConflict(
+ actionKeyContext,
+ actions,
+ actionLookupKey,
+ /*allowSharedAction=*/ false,
+ /*outputFiles=*/ null);
}
/**
- * Finds action conflicts. An action conflict happens if two actions generate the same output
- * artifact. Shared actions are tolerated. See {@link #canBeShared} for details.
+ * Assigns generating action keys to artifacts and finds action conflicts. An action conflict
+ * happens if two actions generate the same output artifact. Shared actions are tolerated. See
+ * {@link #canBeShared} for details. Should be called by a configured target/aspect on the actions
+ * it owns. Should not be used for "global" checks of multiple configured targets: use {@link
+ * #findArtifactPrefixConflicts} for that.
*
- * @param actions a list of actions to check for action conflicts
- * @return a structure giving the mapping between artifacts and generating actions, with a level
- * of indirection.
+ * @param actions a list of actions to check for action conflicts, all generated by the same
+ * configured target/aspect.
+ * @return a structure giving the actions, with a level of indirection.
* @throws ActionConflictException iff there are two unshareable actions generating the same
* output
*/
- public static GeneratingActions filterSharedActionsAndThrowActionConflict(
- ActionKeyContext actionKeyContext, ImmutableList<ActionAnalysisMetadata> actions)
- throws ActionConflictException {
- return Actions.maybeFilterSharedActionsAndThrowIfConflict(
- actionKeyContext, actions, /*allowSharedAction=*/ true);
- }
-
- private static GeneratingActions maybeFilterSharedActionsAndThrowIfConflict(
+ public static GeneratingActions assignOwnersAndFilterSharedActionsAndThrowActionConflict(
ActionKeyContext actionKeyContext,
ImmutableList<ActionAnalysisMetadata> actions,
- boolean allowSharedAction)
+ ActionLookupKey actionLookupKey,
+ @Nullable Collection<OutputFile> outputFiles)
throws ActionConflictException {
- Map<Artifact, Integer> generatingActions = new HashMap<>();
+ return Actions.assignOwnersAndMaybeFilterSharedActionsAndThrowIfConflict(
+ actionKeyContext, actions, actionLookupKey, /*allowSharedAction=*/ true, outputFiles);
+ }
+
+ private static void verifyGeneratingActionKeys(
+ Artifact.DerivedArtifact output,
+ ActionLookupData otherKey,
+ boolean allowSharedAction,
+ ActionKeyContext actionKeyContext,
+ ImmutableList<ActionAnalysisMetadata> actions)
+ throws ActionConflictException {
+ ActionLookupData firstKey = output.getGeneratingActionKey();
+ Preconditions.checkState(
+ firstKey.getActionLookupKey().equals(otherKey.getActionLookupKey()),
+ "Mismatched lookup keys? %s %s %s",
+ output,
+ firstKey,
+ otherKey);
+ int actionIndex = firstKey.getActionIndex();
+ int otherIndex = otherKey.getActionIndex();
+ if (actionIndex != otherIndex
+ && (!allowSharedAction
+ || !Actions.canBeShared(
+ actionKeyContext, actions.get(actionIndex), actions.get(otherIndex)))) {
+ throw new ActionConflictException(
+ actionKeyContext, output, actions.get(actionIndex), actions.get(otherIndex));
+ }
+ }
+
+ /**
+ * Checks {@code actions} for conflicts and sets each artifact's generating action key.
+ *
+ * <p>Conflicts can happen in one of two ways: the same artifact can be the output of multiple
+ * unshareable actions (or shareable actions if {@code allowSharedAction} is false), or two
+ * artifacts with the same execPath can be the outputs of different unshareable actions.
+ *
+ * <p>If {@code outputFiles} is non-null, also builds a map of output-file labels to artifacts,
+ * for use by output file configured targets when they are retrieving their artifacts from this
+ * associated rule configured target.
+ */
+ private static GeneratingActions assignOwnersAndMaybeFilterSharedActionsAndThrowIfConflict(
+ ActionKeyContext actionKeyContext,
+ ImmutableList<ActionAnalysisMetadata> actions,
+ ActionLookupKey actionLookupKey,
+ boolean allowSharedAction,
+ @Nullable Collection<OutputFile> outputFiles)
+ throws ActionConflictException {
+ Map<PathFragment, Artifact.DerivedArtifact> seenArtifacts = new HashMap<>();
+ @Nullable ImmutableMap<String, Label> outputFileNames = null;
+ if (outputFiles != null && !outputFiles.isEmpty()) {
+ ImmutableMap.Builder<String, Label> outputFileNamesBuilder =
+ ImmutableMap.builderWithExpectedSize(outputFiles.size());
+ outputFiles.forEach(o -> outputFileNamesBuilder.put(o.getLabel().getName(), o.getLabel()));
+ outputFileNames = outputFileNamesBuilder.build();
+ }
+ @Nullable
+ ImmutableMap.Builder<Label, Artifact> artifactsByOutputLabel =
+ outputFileNames != null ? ImmutableMap.builderWithExpectedSize(outputFiles.size()) : null;
+ @Nullable Label label = actionLookupKey.getLabel();
+ @Nullable
+ PathFragment packageDirectory =
+ outputFileNames != null
+ ? Preconditions.checkNotNull(label, actionLookupKey)
+ .getPackageIdentifier()
+ .getSourceRoot()
+ : null;
+ // Loop over the actions, looking at all outputs for conflicts.
int actionIndex = 0;
for (ActionAnalysisMetadata action : actions) {
+ ActionLookupData generatingActionKey = ActionLookupData.create(actionLookupKey, actionIndex);
for (Artifact artifact : action.getOutputs()) {
- Integer previousIndex = generatingActions.put(artifact, actionIndex);
- if (previousIndex != null && previousIndex != actionIndex) {
- if (!allowSharedAction
- || !Actions.canBeShared(actionKeyContext, actions.get(previousIndex), action)) {
- throw new ActionConflictException(
- actionKeyContext, artifact, actions.get(previousIndex), action);
+ Artifact.DerivedArtifact output = (Artifact.DerivedArtifact) artifact;
+ // Has an artifact with this execPath been seen before?
+ Artifact.DerivedArtifact equalOutput =
+ seenArtifacts.putIfAbsent(output.getExecPath(), output);
+ if (equalOutput != null) {
+ // Yes: assert that its generating action and this artifact's are compatible.
+ verifyGeneratingActionKeys(
+ equalOutput, generatingActionKey, allowSharedAction, actionKeyContext, actions);
+ } else {
+ // No: populate the output label map with this artifact if applicable: if this
+ // artifact corresponds to a target that is an OutputFile with associated rule this label.
+ PathFragment rootRelativePath = output.getRootRelativePath();
+ if (packageDirectory != null && rootRelativePath.startsWith(packageDirectory)) {
+ PathFragment packageRelativePath = rootRelativePath.relativeTo(packageDirectory);
+ Label outputLabel = outputFileNames.get(packageRelativePath.getPathString());
+ if (outputLabel != null) {
+ artifactsByOutputLabel.put(outputLabel, artifact);
+ }
}
}
+ // Was this output already seen, so it has a generating action key set?
+ if (!output.hasGeneratingActionKey()) {
+ // Common case: artifact hasn't been seen before.
+ output.setGeneratingActionKey(generatingActionKey);
+ } else {
+ // Key is already set: verify that the generating action and this action are compatible.
+ verifyGeneratingActionKeys(
+ output, generatingActionKey, allowSharedAction, actionKeyContext, actions);
+ }
}
actionIndex++;
}
- return new GeneratingActions(actions, ImmutableMap.copyOf(generatingActions));
- }
-
- /**
- * Finds Artifact prefix conflicts between generated artifacts. An artifact prefix conflict
- * happens if one action generates an artifact whose path is a prefix of another artifact's path.
- * Those two artifacts cannot exist simultaneously in the output tree.
- *
- * @param generatingActions a map between generated artifacts and their associated generating
- * actions.
- * @return a map between actions that generated the conflicting artifacts and their associated
- * {@link ArtifactPrefixConflictException}.
- */
- public static Map<ActionAnalysisMetadata, ArtifactPrefixConflictException>
- findArtifactPrefixConflicts(Map<Artifact, ActionAnalysisMetadata> generatingActions) {
- TreeMap<PathFragment, Artifact> artifactPathMap = new TreeMap<>(comparatorForPrefixConflicts());
- for (Artifact artifact : generatingActions.keySet()) {
- artifactPathMap.put(artifact.getExecPath(), artifact);
- }
-
- return findArtifactPrefixConflicts(
- new MapBasedImmutableActionGraph(generatingActions), artifactPathMap);
+ return new GeneratingActions(
+ actions,
+ artifactsByOutputLabel != null ? artifactsByOutputLabel.build() : ImmutableMap.of());
}
/**
@@ -321,52 +395,41 @@
return PATH_ESCAPER.escape(label.getPackageName() + ":" + label.getName());
}
- private static class MapBasedImmutableActionGraph implements ActionGraph {
- private final Map<Artifact, ActionAnalysisMetadata> generatingActions;
-
- MapBasedImmutableActionGraph(
- Map<Artifact, ActionAnalysisMetadata> generatingActions) {
- this.generatingActions = ImmutableMap.copyOf(generatingActions);
- }
-
- @Nullable
- @Override
- public ActionAnalysisMetadata getGeneratingAction(Artifact artifact) {
- return generatingActions.get(artifact);
- }
- }
-
- /** Container class for actions and the artifacts they generate. */
+ /**
+ * Container class for actions, ensuring they have already been checked for conflicts and their
+ * generated artifacts have had owners assigned.
+ */
public static class GeneratingActions {
public static final GeneratingActions EMPTY =
new GeneratingActions(ImmutableList.of(), ImmutableMap.of());
private final ImmutableList<ActionAnalysisMetadata> actions;
- private final ImmutableMap<Artifact, Integer> generatingActionIndex;
+ private final ImmutableMap<Label, Artifact> artifactsByOutputLabel;
private GeneratingActions(
ImmutableList<ActionAnalysisMetadata> actions,
- ImmutableMap<Artifact, Integer> generatingActionIndex) {
+ ImmutableMap<Label, Artifact> artifactsByOutputLabel) {
this.actions = actions;
- this.generatingActionIndex = generatingActionIndex;
+ this.artifactsByOutputLabel = artifactsByOutputLabel;
}
- public static GeneratingActions fromSingleAction(ActionAnalysisMetadata action) {
- return new GeneratingActions(
- ImmutableList.of(action),
- ImmutableMap.copyOf(
- action
- .getOutputs()
- .stream()
- .collect(ImmutableMap.toImmutableMap(Function.identity(), (a) -> 0))));
- }
-
- public ImmutableMap<Artifact, Integer> getGeneratingActionIndex() {
- return generatingActionIndex;
+ /** Used only for the workspace status action. Does not handle duplicate artifacts. */
+ public static GeneratingActions fromSingleAction(
+ ActionAnalysisMetadata action, ActionLookupValue.ActionLookupKey actionLookupKey) {
+ Preconditions.checkState(actionLookupKey.getLabel() == null, actionLookupKey);
+ ActionLookupData generatingActionKey = ActionLookupData.create(actionLookupKey, 0);
+ for (Artifact output : action.getOutputs()) {
+ ((Artifact.DerivedArtifact) output).setGeneratingActionKey(generatingActionKey);
+ }
+ return new GeneratingActions(ImmutableList.of(action), ImmutableMap.of());
}
public ImmutableList<ActionAnalysisMetadata> getActions() {
return actions;
}
+
+ public ImmutableMap<Label, Artifact> getArtifactsByOutputLabel() {
+ return artifactsByOutputLabel;
+ }
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
index 673143b..66abc0b 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
@@ -27,6 +27,7 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata.MiddlemanType;
+import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
import com.google.devtools.build.lib.actions.ArtifactResolver.ArtifactResolverSupplier;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelConstants;
@@ -38,7 +39,6 @@
import com.google.devtools.build.lib.skyframe.serialization.SerializationContext;
import com.google.devtools.build.lib.skyframe.serialization.SerializationException;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
-import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
import com.google.devtools.build.lib.skylarkbuildapi.FileApi;
import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
import com.google.devtools.build.lib.util.FileType;
@@ -103,7 +103,6 @@
* </ul>
*/
@Immutable
-@AutoCodec
public abstract class Artifact
implements FileType.HasFileType,
ActionInput,
@@ -223,45 +222,11 @@
/** A Predicate that evaluates to true if the Artifact is not a middleman artifact. */
public static final Predicate<Artifact> MIDDLEMAN_FILTER = input -> !input.isMiddlemanArtifact();
- private static final Interner<DerivedArtifact> ARTIFACT_INTERNER =
- BlazeInterners.newWeakInterner();
-
private final int hashCode;
private final ArtifactRoot root;
private final PathFragment execPath;
- private final ArtifactOwner owner;
- /**
- * The {@code rootRelativePath is a few characters shorter than the {@code execPath} for derived
- * artifacts, so we save a few bytes by serializing it rather than the {@code execPath},
- * especially when the {@code root} is common to many artifacts and therefore memoized.
- */
- @AutoCodec.VisibleForSerialization
- @AutoCodec.Instantiator
- static Artifact createForSerialization(
- ArtifactRoot root, ArtifactOwner owner, PathFragment rootRelativePath) {
- if (rootRelativePath == null || rootRelativePath.isAbsolute() != root.getRoot().isAbsolute()) {
- throw new IllegalArgumentException(
- rootRelativePath
- + ": illegal rootRelativePath for "
- + rootRelativePath
- + " (root: "
- + root
- + ")");
- }
- PathFragment rootExecPath = root.getExecPath();
- if (rootExecPath.isEmpty()) {
- return new Artifact.SourceArtifact(root, rootRelativePath, owner);
- } else {
- return ARTIFACT_INTERNER.intern(
- new Artifact.DerivedArtifact(root, rootExecPath.getRelative(rootRelativePath), owner));
- }
- }
-
- private Artifact(
- ArtifactRoot root,
- PathFragment execPath,
- ArtifactOwner owner) {
+ private Artifact(ArtifactRoot root, PathFragment execPath) {
Preconditions.checkNotNull(root);
if (execPath.isEmpty()) {
throw new IllegalArgumentException(
@@ -273,25 +238,129 @@
this.hashCode = execPath.hashCode();
this.root = root;
this.execPath = execPath;
- this.owner = Preconditions.checkNotNull(owner);
}
/** An artifact corresponding to a file in the output tree, generated by an {@link Action}. */
+ @AutoCodec
public static class DerivedArtifact extends Artifact {
+ /** Only used for deserializing artifacts. */
+ private static final Interner<DerivedArtifact> INTERNER = BlazeInterners.newWeakInterner();
+
private final PathFragment rootRelativePath;
+ /**
+ * An {@link ActionLookupKey} until {@link #setGeneratingActionKey} is set, at which point it is
+ * an {@link ActionLookupData}, whose {@link ActionLookupData#getActionLookupKey} will be the
+ * same as the original value of owner.
+ *
+ * <p>We overload this field in order to save memory.
+ */
+ private Object owner;
@VisibleForTesting
- public DerivedArtifact(ArtifactRoot root, PathFragment execPath, ArtifactOwner owner) {
- super(root, execPath, owner);
+ public DerivedArtifact(ArtifactRoot root, PathFragment execPath, ActionLookupKey owner) {
+ super(root, execPath);
Preconditions.checkState(
!root.getExecPath().isEmpty(), "Derived root has no exec path: %s, %s", root, execPath);
this.rootRelativePath = execPath.relativeTo(root.getExecPath());
+ this.owner = owner;
+ }
+
+ /**
+ * Called when a configured target's actions are being collected. {@code generatingActionKey}
+ * must have the same owner as this artifact's current {@link #getArtifactOwner}.
+ */
+ @VisibleForTesting
+ public void setGeneratingActionKey(ActionLookupData generatingActionKey) {
+ Preconditions.checkState(
+ this.owner instanceof ArtifactOwner,
+ "Already set generating action key: %s (%s %s)",
+ this,
+ this.owner,
+ generatingActionKey);
+ Preconditions.checkState(
+ Preconditions.checkNotNull(generatingActionKey, this).getActionLookupKey().equals(owner),
+ "Owner of generating action key not same as artifact's owner: %s (%s %s)",
+ this,
+ this.owner,
+ generatingActionKey);
+ this.owner = Preconditions.checkNotNull(generatingActionKey, this);
+ }
+
+ @VisibleForTesting
+ public boolean hasGeneratingActionKey() {
+ return this.owner instanceof ActionLookupData;
+ }
+
+ /** Can only be called once {@link #setGeneratingActionKey} is called. */
+ public ActionLookupData getGeneratingActionKey() {
+ Preconditions.checkState(owner instanceof ActionLookupData, "Bad owner: %s %s", this, owner);
+ return (ActionLookupData) owner;
+ }
+
+ @Override
+ public ActionLookupValue.ActionLookupKey getArtifactOwner() {
+ return owner instanceof ActionLookupData
+ ? getGeneratingActionKey().getActionLookupKey()
+ : (ActionLookupKey) owner;
+ }
+
+ @Override
+ public Label getOwnerLabel() {
+ return getArtifactOwner().getLabel();
}
@Override
public PathFragment getRootRelativePath() {
return rootRelativePath;
}
+
+ @Override
+ boolean ownersEqual(Artifact other) {
+ DerivedArtifact that = (DerivedArtifact) other;
+ if (!(this.owner instanceof ActionLookupData) || !(that.owner instanceof ActionLookupData)) {
+ // Happens only intra-analysis of a configured target. Tolerate.
+ Preconditions.checkState(
+ this.getArtifactOwner().equals(that.getArtifactOwner()),
+ "Mismatched owners: %s %s %s %s",
+ this,
+ that,
+ this.getArtifactOwner(),
+ that.getArtifactOwner());
+ return true;
+ }
+ return this.owner.equals(that.owner);
+ }
+
+ /**
+ * The {@code rootRelativePath is a few characters shorter than the {@code execPath}, so we save
+ * a few bytes by serializing it rather than the {@code execPath}, especially when the {@code
+ * root} is common to many artifacts and therefore memoized.
+ */
+ @AutoCodec.VisibleForSerialization
+ @AutoCodec.Instantiator
+ static DerivedArtifact createForSerialization(
+ ArtifactRoot root, PathFragment rootRelativePath, ActionLookupData generatingActionKey) {
+ if (rootRelativePath == null
+ || rootRelativePath.isAbsolute() != root.getRoot().isAbsolute()) {
+ throw new IllegalArgumentException(
+ rootRelativePath
+ + ": illegal rootRelativePath for "
+ + root
+ + " (generatingActionKey: "
+ + generatingActionKey
+ + ")");
+ }
+ Preconditions.checkState(
+ !root.isSourceRoot(), "Root not derived: %s %s", root, rootRelativePath);
+ PathFragment rootExecPath = root.getExecPath();
+ DerivedArtifact artifact =
+ new DerivedArtifact(
+ root,
+ rootExecPath.getRelative(rootRelativePath),
+ generatingActionKey.getActionLookupKey());
+ artifact.setGeneratingActionKey(generatingActionKey);
+ return INTERNER.intern(artifact);
+ }
}
public final Path getPath() {
@@ -356,10 +425,9 @@
return getExecPathString();
}
- /**
- * Returns the artifact owner. May be null.
- */
- @Nullable public final Label getOwner() {
+ /** Returns the artifact's owning label. May be null. */
+ @Nullable
+ public final Label getOwner() {
return getOwnerLabel();
}
@@ -371,14 +439,7 @@
* derived artifacts, such as target completion middleman artifacts, build info artifacts, and the
* like.
*/
- public final ArtifactOwner getArtifactOwner() {
- return owner;
- }
-
- @Override
- public Label getOwnerLabel() {
- return owner.getLabel();
- }
+ public abstract ArtifactOwner getArtifactOwner();
/**
* Returns the root beneath which this Artifact resides, if any. This may be one of the
@@ -471,18 +532,21 @@
* TODO(shahan): move {@link Artifact#getPath} to this subclass.
* */
public static final class SourceArtifact extends Artifact {
+ private final ArtifactOwner owner;
+
@VisibleForTesting
public SourceArtifact(ArtifactRoot root, PathFragment execPath, ArtifactOwner owner) {
- super(root, execPath, owner);
+ super(root, execPath);
+ this.owner = owner;
}
/**
- * SourceArtifacts are compared without their owners, since owners do not affect behavior,
- * unlike with derived artifacts, whose owners determine their generating actions.
+ * Source artifacts do not consider their owners in equality checks, since their owners are
+ * purely cosmetic.
*/
@Override
- public boolean equals(Object other) {
- return other instanceof SourceArtifact && equalsWithoutOwner((SourceArtifact) other);
+ boolean ownersEqual(Artifact other) {
+ return true;
}
@Override
@@ -490,8 +554,18 @@
return getExecPath();
}
+ @Override
+ public ArtifactOwner getArtifactOwner() {
+ return owner;
+ }
+
+ @Override
+ public Label getOwnerLabel() {
+ return owner.getLabel();
+ }
+
boolean differentOwnerOrRoot(ArtifactOwner owner, ArtifactRoot root) {
- return !this.getArtifactOwner().equals(owner) || !this.getRoot().equals(root);
+ return !this.owner.equals(owner) || !this.getRoot().equals(root);
}
}
@@ -519,13 +593,26 @@
public static final class SpecialArtifact extends DerivedArtifact {
private final SpecialArtifactType type;
- @VisibleForSerialization
+ @VisibleForTesting
public SpecialArtifact(
- ArtifactRoot root, PathFragment execPath, ArtifactOwner owner, SpecialArtifactType type) {
+ ArtifactRoot root, PathFragment execPath, ActionLookupKey owner, SpecialArtifactType type) {
super(root, execPath, owner);
this.type = type;
}
+ @AutoCodec.VisibleForSerialization
+ @AutoCodec.Instantiator
+ static SpecialArtifact create(
+ ArtifactRoot root,
+ PathFragment execPath,
+ SpecialArtifactType type,
+ ActionLookupData generatingActionKey) {
+ SpecialArtifact result =
+ new SpecialArtifact(root, execPath, generatingActionKey.getActionLookupKey(), type);
+ result.setGeneratingActionKey(generatingActionKey);
+ return result;
+ }
+
@Override
public final boolean isFileset() {
return type == SpecialArtifactType.FILESET;
@@ -596,9 +683,10 @@
* Constructs a TreeFileArtifact with the given parent-relative path under the given parent
* TreeArtifact, owned by the given {@code artifactOwner}.
*/
- @AutoCodec.Instantiator
TreeFileArtifact(
- SpecialArtifact parentTreeArtifact, PathFragment parentRelativePath, ArtifactOwner owner) {
+ SpecialArtifact parentTreeArtifact,
+ PathFragment parentRelativePath,
+ ActionLookupKey owner) {
super(
parentTreeArtifact.getRoot(),
parentTreeArtifact.getExecPath().getRelative(parentRelativePath),
@@ -620,6 +708,19 @@
this.parentRelativePath = parentRelativePath;
}
+ @AutoCodec.VisibleForSerialization
+ @AutoCodec.Instantiator
+ static TreeFileArtifact createForSerialization(
+ SpecialArtifact parentTreeArtifact,
+ PathFragment parentRelativePath,
+ ActionLookupData generatingActionKey) {
+ TreeFileArtifact result =
+ new TreeFileArtifact(
+ parentTreeArtifact, parentRelativePath, generatingActionKey.getActionLookupKey());
+ result.setGeneratingActionKey(generatingActionKey);
+ return result;
+ }
+
@Override
public SpecialArtifact getParent() {
return parentTreeArtifact;
@@ -683,7 +784,7 @@
@SuppressWarnings("EqualsGetClass") // Distinct classes of Artifact are never equal.
@Override
- public boolean equals(Object other) {
+ public final boolean equals(Object other) {
if (this == other) {
return true;
}
@@ -694,13 +795,15 @@
return false;
}
Artifact that = (Artifact) other;
- return equalsWithoutOwner(that) && owner.equals(that.owner);
+ return equalsWithoutOwner(that) && ownersEqual(that);
}
- public boolean equalsWithoutOwner(Artifact other) {
+ final boolean equalsWithoutOwner(Artifact other) {
return hashCode == other.hashCode && execPath.equals(other.execPath) && root.equals(other.root);
}
+ abstract boolean ownersEqual(Artifact other);
+
@Override
public final int hashCode() {
// This is just execPath.hashCode() (along with the class). We cache a copy in the Artifact
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java b/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java
index 3cc5e3b..c565492 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java
@@ -17,6 +17,7 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
import com.google.common.util.concurrent.Striped;
+import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
import com.google.devtools.build.lib.actions.Artifact.SourceArtifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
@@ -325,9 +326,9 @@
if (type == null) {
return root.isSourceRoot()
? new Artifact.SourceArtifact(root, execPath, owner)
- : new Artifact.DerivedArtifact(root, execPath, owner);
+ : new Artifact.DerivedArtifact(root, execPath, (ActionLookupKey) owner);
} else {
- return new Artifact.SpecialArtifact(root, execPath, owner, type);
+ return new Artifact.SpecialArtifact(root, execPath, (ActionLookupKey) owner, type);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/BasicActionLookupValue.java b/src/main/java/com/google/devtools/build/lib/actions/BasicActionLookupValue.java
index 3b4a153..2e3eef9 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/BasicActionLookupValue.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/BasicActionLookupValue.java
@@ -17,8 +17,6 @@
import com.google.common.base.MoreObjects;
import com.google.common.base.MoreObjects.ToStringHelper;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
import java.math.BigInteger;
import javax.annotation.Nullable;
@@ -28,22 +26,18 @@
*/
public class BasicActionLookupValue extends ActionLookupValue {
protected final ImmutableList<ActionAnalysisMetadata> actions;
- @VisibleForSerialization protected final ImmutableMap<Artifact, Integer> generatingActionIndex;
protected BasicActionLookupValue(
ImmutableList<ActionAnalysisMetadata> actions,
- ImmutableMap<Artifact, Integer> generatingActionIndex,
@Nullable BigInteger nonceVersion) {
super(nonceVersion);
this.actions = actions;
- this.generatingActionIndex = generatingActionIndex;
}
@VisibleForTesting
public BasicActionLookupValue(
Actions.GeneratingActions generatingActions, @Nullable BigInteger nonceVersion) {
- this(
- generatingActions.getActions(), generatingActions.getGeneratingActionIndex(), nonceVersion);
+ this(generatingActions.getActions(), nonceVersion);
}
@Override
@@ -51,14 +45,7 @@
return actions;
}
- @Override
- protected ImmutableMap<Artifact, Integer> getGeneratingActionIndex() {
- return generatingActionIndex;
- }
-
protected ToStringHelper getStringHelper() {
- return MoreObjects.toStringHelper(this)
- .add("actions", actions)
- .add("generatingActionIndex", generatingActionIndex);
+ return MoreObjects.toStringHelper(this).add("actions", actions);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index 9e82277..96a7745 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -29,10 +29,11 @@
import com.google.common.eventbus.EventBus;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionGraph;
+import com.google.devtools.build.lib.actions.ActionLookupData;
import com.google.devtools.build.lib.actions.ActionLookupValue;
+import com.google.devtools.build.lib.actions.Actions;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactFactory;
-import com.google.devtools.build.lib.actions.ArtifactOwner;
import com.google.devtools.build.lib.actions.PackageRoots;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
@@ -77,7 +78,6 @@
import com.google.devtools.build.lib.syntax.SkylarkImport.SkylarkImportSyntaxException;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.util.RegexFilter;
-import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.WalkableGraph;
import java.util.ArrayList;
import java.util.Collection;
@@ -489,10 +489,11 @@
allTargetsToTest,
baselineCoverageArtifacts,
getArtifactFactory(),
+ skyframeExecutor.getActionKeyContext(),
CoverageReportValue.COVERAGE_REPORT_KEY,
loadingResult.getWorkspaceName());
if (actionsWrapper != null) {
- ImmutableList<ActionAnalysisMetadata> actions = actionsWrapper.getActions();
+ Actions.GeneratingActions actions = actionsWrapper.getActions();
skyframeExecutor.injectCoverageReportData(actions);
actionsWrapper.getCoverageOutputs().forEach(artifactsToOwnerLabelsBuilder::addArtifact);
}
@@ -516,19 +517,25 @@
@Nullable
@Override
public ActionAnalysisMetadata getGeneratingAction(Artifact artifact) {
- ArtifactOwner artifactOwner = artifact.getArtifactOwner();
- if (artifactOwner instanceof ActionLookupValue.ActionLookupKey) {
- SkyKey key = (ActionLookupValue.ActionLookupKey) artifactOwner;
- ActionLookupValue val;
- try {
- val = (ActionLookupValue) graph.getValue(key);
- } catch (InterruptedException e) {
- throw new IllegalStateException(
- "Interruption not expected from this graph: " + key, e);
- }
- return val == null ? null : val.getGeneratingActionDangerousReadJavadoc(artifact);
+ if (artifact.isSourceArtifact()) {
+ return null;
}
- return null;
+ ActionLookupData generatingActionKey =
+ ((Artifact.DerivedArtifact) artifact).getGeneratingActionKey();
+ ActionLookupValue val;
+ try {
+ val = (ActionLookupValue) graph.getValue(generatingActionKey.getActionLookupKey());
+ } catch (InterruptedException e) {
+ throw new IllegalStateException(
+ "Interruption not expected from this graph: " + generatingActionKey, e);
+ }
+ if (val == null) {
+ return null;
+ }
+ int actionIndex = generatingActionKey.getActionIndex();
+ return val.isActionTemplate(actionIndex)
+ ? val.getActionTemplate(actionIndex)
+ : val.getAction(actionIndex);
}
};
return new AnalysisResult(
@@ -666,8 +673,8 @@
ImmutableList.Builder<Artifact> artifacts = ImmutableList.builder();
// Add to 'artifacts' all extra-actions which were registered by aspects which 'topLevel'
// might have injected.
- for (Artifact artifact : provider.getTransitiveExtraActionArtifacts()) {
- ArtifactOwner owner = artifact.getArtifactOwner();
+ for (Artifact.DerivedArtifact artifact : provider.getTransitiveExtraActionArtifacts()) {
+ ActionLookupValue.ActionLookupKey owner = artifact.getArtifactOwner();
if (owner instanceof AspectKey) {
if (aspectClasses.contains(((AspectKey) owner).getAspectClass())) {
artifacts.add(artifact);
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java b/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java
index d66dee7..b31208d 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java
@@ -37,6 +37,7 @@
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.WorkspaceStatusValue;
import com.google.devtools.build.lib.syntax.StarlarkSemantics;
+import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.SkyFunction;
import java.io.PrintWriter;
@@ -76,7 +77,15 @@
private MiddlemanFactory middlemanFactory;
private ExtendedEventHandler errorEventListener;
private SkyFunction.Environment skyframeEnv;
- private Map<Artifact, String> artifacts;
+ /**
+ * Map of artifacts to either themselves or to {@code Pair<Artifact, String>} if
+ * --experimental_extended_sanity_checks is enabled. In the latter case, the string will contain
+ * the stack trace of where the artifact was created. In the former case, we'll construct a
+ * generic message in case of error.
+ *
+ * <p>The artifact is stored so that we can deduplicate artifacts created multiple times.
+ */
+ private Map<Artifact, Object> artifacts;
/**
* The list of actions registered by the configured target this analysis environment is
@@ -198,12 +207,21 @@
// The order of the artifacts.entrySet iteration is unspecified - we use a TreeMap here to
// guarantee that the return value of this method is deterministic.
Map<Artifact, String> orphanArtifacts = new TreeMap<>(Artifact.EXEC_PATH_COMPARATOR);
- for (Map.Entry<Artifact, String> entry : artifacts.entrySet()) {
+ for (Map.Entry<Artifact, Object> entry : artifacts.entrySet()) {
Artifact a = entry.getKey();
if (!a.isSourceArtifact() && !artifactsWithActions.contains(a)) {
- orphanArtifacts.put(a, String.format("%s\n%s",
- a.getExecPathString(), // uncovered artifact
- entry.getValue())); // origin of creation
+ Object value = entry.getValue();
+ if (value instanceof Artifact) {
+ value = "No origin, run with --experimental_extended_sanity_checks";
+ } else {
+ value = ((Pair<?, ?>) value).second;
+ }
+ orphanArtifacts.put(
+ a,
+ String.format(
+ "%s\n%s",
+ a.getExecPathString(), // uncovered artifact
+ value)); // origin of creation
}
}
return orphanArtifacts;
@@ -240,14 +258,23 @@
* sealed (disable()). For performance reasons we only track the originating stacktrace when
* running with --experimental_extended_sanity_checks.
*/
- private Artifact.DerivedArtifact trackArtifactAndOrigin(
+ @SuppressWarnings("unchecked") // Cast of artifacts map's value to Pair.
+ private Artifact.DerivedArtifact dedupAndTrackArtifactAndOrigin(
Artifact.DerivedArtifact a, @Nullable Throwable e) {
- if ((e != null) && !artifacts.containsKey(a)) {
+ if (artifacts.containsKey(a)) {
+ Object value = artifacts.get(a);
+ if (e == null) {
+ return (Artifact.DerivedArtifact) value;
+ } else {
+ return ((Pair<Artifact.DerivedArtifact, String>) value).first;
+ }
+ }
+ if ((e != null)) {
StringWriter sw = new StringWriter();
e.printStackTrace(new PrintWriter(sw));
- artifacts.put(a, sw.toString());
+ artifacts.put(a, Pair.of(a, sw.toString()));
} else {
- artifacts.put(a, "No origin, run with --experimental_extended_sanity_checks");
+ artifacts.put(a, a);
}
return a;
}
@@ -256,7 +283,7 @@
public Artifact.DerivedArtifact getDerivedArtifact(
PathFragment rootRelativePath, ArtifactRoot root) {
Preconditions.checkState(enabled);
- return trackArtifactAndOrigin(
+ return dedupAndTrackArtifactAndOrigin(
artifactFactory.getDerivedArtifact(rootRelativePath, root, getOwner()),
extendedSanityChecks ? new Throwable() : null);
}
@@ -265,7 +292,7 @@
public SpecialArtifact getTreeArtifact(PathFragment rootRelativePath, ArtifactRoot root) {
Preconditions.checkState(enabled);
return (SpecialArtifact)
- trackArtifactAndOrigin(
+ dedupAndTrackArtifactAndOrigin(
artifactFactory.getTreeArtifact(rootRelativePath, root, getOwner()),
extendedSanityChecks ? new Throwable() : null);
}
@@ -274,7 +301,7 @@
public Artifact.DerivedArtifact getFilesetArtifact(
PathFragment rootRelativePath, ArtifactRoot root) {
Preconditions.checkState(enabled);
- return trackArtifactAndOrigin(
+ return dedupAndTrackArtifactAndOrigin(
artifactFactory.getFilesetArtifact(rootRelativePath, root, getOwner()),
extendedSanityChecks ? new Throwable() : null);
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java
index 5c01533..86dd39e 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java
@@ -64,18 +64,15 @@
public final class ConfiguredAspect {
private final AspectDescriptor descriptor;
private final ImmutableList<ActionAnalysisMetadata> actions;
- private final ImmutableMap<Artifact, Integer> generatingActionIndex;
private final TransitiveInfoProviderMap providers;
@AutoCodec.VisibleForSerialization
ConfiguredAspect(
AspectDescriptor descriptor,
ImmutableList<ActionAnalysisMetadata> actions,
- ImmutableMap<Artifact, Integer> generatingActionIndex,
TransitiveInfoProviderMap providers) {
this.descriptor = descriptor;
this.actions = actions;
- this.generatingActionIndex = generatingActionIndex;
this.providers = providers;
// Initialize every SkylarkApiProvider
@@ -105,14 +102,6 @@
return actions;
}
- /**
- * Returns a map where keys are artifacts that are action outputs of this rule, and values are the
- * index of the action that generates that artifact.
- */
- public ImmutableMap<Artifact, Integer> getGeneratingActionIndex() {
- return generatingActionIndex;
- }
-
/** Returns the providers created by the aspect. */
public TransitiveInfoProviderMap getProviders() {
return providers;
@@ -145,15 +134,13 @@
}
public static ConfiguredAspect forAlias(ConfiguredAspect real) {
- return new ConfiguredAspect(
- real.descriptor, real.getActions(), real.getGeneratingActionIndex(), real.getProviders());
+ return new ConfiguredAspect(real.descriptor, real.getActions(), real.getProviders());
}
public static ConfiguredAspect forNonapplicableTarget(AspectDescriptor descriptor) {
return new ConfiguredAspect(
descriptor,
ImmutableList.of(),
- ImmutableMap.of(),
new TransitiveInfoProviderMapBuilder().add().build());
}
@@ -289,14 +276,15 @@
AnalysisEnvironment analysisEnvironment = ruleContext.getAnalysisEnvironment();
GeneratingActions generatingActions =
- Actions.filterSharedActionsAndThrowActionConflict(
+ Actions.assignOwnersAndFilterSharedActionsAndThrowActionConflict(
analysisEnvironment.getActionKeyContext(),
- analysisEnvironment.getRegisteredActions());
+ analysisEnvironment.getRegisteredActions(),
+ ruleContext.getOwner(),
+ /*outputFiles=*/ null);
return new ConfiguredAspect(
descriptor,
generatingActions.getActions(),
- generatingActions.getGeneratingActionIndex(),
providers.build());
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
index d8096c6..ad4f8da 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
@@ -24,8 +24,6 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SourceArtifact;
import com.google.devtools.build.lib.actions.ArtifactFactory;
-import com.google.devtools.build.lib.actions.ArtifactOwner;
-import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.FailAction;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.analysis.DependencyResolver.DependencyKind;
@@ -36,6 +34,7 @@
import com.google.devtools.build.lib.analysis.configuredtargets.InputFileConfiguredTarget;
import com.google.devtools.build.lib.analysis.configuredtargets.OutputFileConfiguredTarget;
import com.google.devtools.build.lib.analysis.configuredtargets.PackageGroupConfiguredTarget;
+import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.analysis.skylark.SkylarkRuleConfiguredTargetUtil;
import com.google.devtools.build.lib.analysis.test.AnalysisFailure;
import com.google.devtools.build.lib.analysis.test.AnalysisFailureInfo;
@@ -70,7 +69,6 @@
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
import com.google.devtools.build.lib.util.OrderedSetMultimap;
-import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
@@ -160,29 +158,6 @@
return null;
}
- /** Returns the output artifact for the given file, or null if Skyframe deps are missing. */
- private Artifact getOutputArtifact(
- OutputFile outputFile,
- BuildConfiguration configuration,
- boolean isFileset,
- ArtifactFactory artifactFactory) {
- Rule rule = outputFile.getAssociatedRule();
- ArtifactRoot root =
- rule.hasBinaryOutput()
- ? configuration.getBinDirectory(rule.getRepository())
- : configuration.getGenfilesDirectory(rule.getRepository());
- ArtifactOwner owner = ConfiguredTargetKey.of(rule.getLabel(), configuration);
- PathFragment rootRelativePath =
- outputFile.getLabel().getPackageIdentifier().getSourceRoot().getRelative(
- outputFile.getLabel().getName());
- Artifact result = isFileset
- ? artifactFactory.getFilesetArtifact(rootRelativePath, root, owner)
- : artifactFactory.getDerivedArtifact(rootRelativePath, root, owner);
- // The associated rule should have created the artifact.
- Preconditions.checkNotNull(result, "no artifact for %s", rootRelativePath);
- return result;
- }
-
/**
* Invokes the appropriate constructor to create a {@link ConfiguredTarget} instance.
*
@@ -231,14 +206,15 @@
config,
prerequisiteMap.get(DependencyResolver.OUTPUT_FILE_RULE_DEPENDENCY),
visibility);
- boolean isFileset = outputFile.getGeneratingRule().getRuleClass().equals("Fileset");
- Artifact artifact = getOutputArtifact(outputFile, config, isFileset, artifactFactory);
if (analysisEnvironment.getSkyframeEnv().valuesMissing()) {
return null;
}
- TransitiveInfoCollection rule = targetContext.findDirectPrerequisite(
- outputFile.getGeneratingRule().getLabel(), config);
- return new OutputFileConfiguredTarget(targetContext, outputFile, rule, artifact);
+ RuleConfiguredTarget rule =
+ (RuleConfiguredTarget)
+ targetContext.findDirectPrerequisite(
+ outputFile.getGeneratingRule().getLabel(), config);
+ Artifact artifact = rule.getArtifactByOutputLabel(outputFile.getLabel());
+ return new OutputFileConfiguredTarget(targetContext, outputFile, rule, artifact);
} else if (target instanceof InputFile) {
InputFile inputFile = (InputFile) target;
TargetContext targetContext =
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
index 15f7937..910c5a9 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
@@ -49,6 +49,7 @@
import com.google.devtools.build.lib.packages.InfoInterface;
import com.google.devtools.build.lib.packages.NativeProvider;
import com.google.devtools.build.lib.packages.Provider;
+import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.Type;
@@ -222,13 +223,17 @@
}
AnalysisEnvironment analysisEnvironment = ruleContext.getAnalysisEnvironment();
- GeneratingActions generatingActions = Actions.filterSharedActionsAndThrowActionConflict(
- analysisEnvironment.getActionKeyContext(), analysisEnvironment.getRegisteredActions());
+ GeneratingActions generatingActions =
+ Actions.assignOwnersAndFilterSharedActionsAndThrowActionConflict(
+ analysisEnvironment.getActionKeyContext(),
+ analysisEnvironment.getRegisteredActions(),
+ ruleContext.getOwner(),
+ ((Rule) ruleContext.getTarget()).getOutputFiles());
return new RuleConfiguredTarget(
ruleContext,
providers,
generatingActions.getActions(),
- generatingActions.getGeneratingActionIndex());
+ generatingActions.getArtifactsByOutputLabel());
}
private NestedSet<Label> transitiveLabels() {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java
index 4ae6ca2..28a7993 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java
@@ -21,12 +21,12 @@
import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ActionKeyCacher;
import com.google.devtools.build.lib.actions.ActionKeyContext;
+import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.ActionTemplate;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
-import com.google.devtools.build.lib.actions.ArtifactOwner;
import com.google.devtools.build.lib.actions.CommandLine;
import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
@@ -98,16 +98,15 @@
@Override
public Iterable<SpawnAction> generateActionForInputArtifacts(
- Iterable<TreeFileArtifact> inputTreeFileArtifacts, ArtifactOwner artifactOwner) {
+ Iterable<TreeFileArtifact> inputTreeFileArtifacts, ActionLookupKey artifactOwner) {
ImmutableList.Builder<SpawnAction> expandedActions = new ImmutableList.Builder<>();
for (TreeFileArtifact inputTreeFileArtifact : inputTreeFileArtifacts) {
PathFragment parentRelativeOutputPath =
outputPathMapper.parentRelativeOutputPath(inputTreeFileArtifact);
- TreeFileArtifact outputTreeFileArtifact = ActionInputHelper.treeFileArtifact(
- outputTreeArtifact,
- parentRelativeOutputPath,
- artifactOwner);
+ TreeFileArtifact outputTreeFileArtifact =
+ ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
+ outputTreeArtifact, parentRelativeOutputPath, artifactOwner);
expandedActions.add(createAction(inputTreeFileArtifact, outputTreeFileArtifact));
}
@@ -121,8 +120,10 @@
TreeFileArtifact inputTreeFileArtifact =
ActionInputHelper.treeFileArtifact(inputTreeArtifact, "dummy_for_key");
TreeFileArtifact outputTreeFileArtifact =
- ActionInputHelper.treeFileArtifact(
- outputTreeArtifact, outputPathMapper.parentRelativeOutputPath(inputTreeFileArtifact));
+ ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
+ outputTreeArtifact,
+ outputPathMapper.parentRelativeOutputPath(inputTreeFileArtifact),
+ outputTreeArtifact.getArtifactOwner());
SpawnAction dummyAction = createAction(inputTreeFileArtifact, outputTreeFileArtifact);
dummyAction.computeKey(actionKeyContext, fp);
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/MergedConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/MergedConfiguredTarget.java
index 18038f2..93c9f9f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/MergedConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/MergedConfiguredTarget.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.analysis.configuredtargets;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
@@ -225,4 +226,9 @@
public void repr(SkylarkPrinter printer) {
printer.append("<merged target " + getLabel() + ">");
}
+
+ @VisibleForTesting
+ public ConfiguredTarget getBaseConfiguredTargetForTesting() {
+ return base;
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java
index d5c5a17..9eba156 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java
@@ -94,7 +94,8 @@
private final ImmutableMap<Label, ConfigMatchingProvider> configConditions;
private final String ruleClassString;
private final ImmutableList<ActionAnalysisMetadata> actions;
- private final ImmutableMap<Artifact, Integer> generatingActionIndex;
+ // TODO(b/133160730): can we only populate this map for outputs that have labels?
+ private final ImmutableMap<Label, Artifact> artifactsByOutputLabel;
@Instantiator
@VisibleForSerialization
@@ -107,8 +108,9 @@
ImmutableSet<ConfiguredTargetKey> implicitDeps,
String ruleClassString,
ImmutableList<ActionAnalysisMetadata> actions,
- ImmutableMap<Artifact, Integer> generatingActionIndex) {
+ ImmutableMap<Label, Artifact> artifactsByOutputLabel) {
super(label, configurationKey, visibility);
+ this.artifactsByOutputLabel = artifactsByOutputLabel;
// We don't use ImmutableMap.Builder here to allow augmenting the initial list of 'default'
// providers by passing them in.
@@ -131,14 +133,13 @@
this.implicitDeps = IMPLICIT_DEPS_INTERNER.intern(implicitDeps);
this.ruleClassString = ruleClassString;
this.actions = actions;
- this.generatingActionIndex = generatingActionIndex;
}
public RuleConfiguredTarget(
RuleContext ruleContext,
TransitiveInfoProviderMap providers,
ImmutableList<ActionAnalysisMetadata> actions,
- ImmutableMap<Artifact, Integer> generatingActionIndex) {
+ ImmutableMap<Label, Artifact> artifactsByOutputLabel) {
this(
ruleContext.getLabel(),
ruleContext.getConfigurationKey(),
@@ -148,7 +149,7 @@
Util.findImplicitDeps(ruleContext),
ruleContext.getRule().getRuleClass(),
actions,
- generatingActionIndex);
+ artifactsByOutputLabel);
// If this rule is the run_under target, then check that we have an executable; note that
// run_under is only set in the target configuration, and the target must also be analyzed for
@@ -251,10 +252,15 @@
}
/**
- * Returns a map where keys are artifacts that are action outputs of this rule, and values are the
- * index of the action that generates that artifact.
+ * Provide an artifact by its corresponding output label, for use by output file configured
+ * targets.
*/
- public ImmutableMap<Artifact, Integer> getGeneratingActionIndex() {
- return generatingActionIndex;
+ public Artifact getArtifactByOutputLabel(Label outputLabel) {
+ return Preconditions.checkNotNull(
+ artifactsByOutputLabel.get(outputLabel),
+ "%s %s %s",
+ outputLabel,
+ this,
+ this.artifactsByOutputLabel);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/CoverageReportActionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/test/CoverageReportActionFactory.java
index 32b36e7..f8099db 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/CoverageReportActionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/CoverageReportActionFactory.java
@@ -18,12 +18,16 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.eventbus.EventBus;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
+import com.google.devtools.build.lib.actions.ActionKeyContext;
+import com.google.devtools.build.lib.actions.ActionLookupValue;
+import com.google.devtools.build.lib.actions.Actions;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactFactory;
-import com.google.devtools.build.lib.actions.ArtifactOwner;
+import com.google.devtools.build.lib.actions.MutableActionGraph;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.skyframe.CoverageReportValue;
import java.util.Collection;
import javax.annotation.Nullable;
@@ -32,27 +36,37 @@
*/
public interface CoverageReportActionFactory {
/**
- * Wraps the necessary actions to get a coverage report as well as the final output artifacts.
- * The lcovWriteAction creates a file containing a set of lcov files. This file is used as an
- * input artifact for coverageReportAction. We are only interested about the output artifacts from
+ * Wraps the necessary actions to get a coverage report as well as the final output artifacts. The
+ * lcovWriteAction creates a file containing a set of lcov files. This file is used as an input
+ * artifact for coverageReportAction. We are only interested about the output artifacts from
* coverageReportAction.
*/
- public static final class CoverageReportActionsWrapper {
- private final ActionAnalysisMetadata lcovWriteAction;
+ final class CoverageReportActionsWrapper {
private final ActionAnalysisMetadata coverageReportAction;
+ private final Actions.GeneratingActions processedActions;
- public CoverageReportActionsWrapper (
- ActionAnalysisMetadata lcovWriteAction, ActionAnalysisMetadata coverageReportAction) {
- this.lcovWriteAction = lcovWriteAction;
+ public CoverageReportActionsWrapper(
+ ActionAnalysisMetadata lcovWriteAction,
+ ActionAnalysisMetadata coverageReportAction,
+ ActionKeyContext actionKeyContext) {
this.coverageReportAction = coverageReportAction;
+ try {
+ this.processedActions =
+ Actions.assignOwnersAndFindAndThrowActionConflict(
+ actionKeyContext,
+ ImmutableList.of(lcovWriteAction, coverageReportAction),
+ CoverageReportValue.COVERAGE_REPORT_KEY);
+ } catch (MutableActionGraph.ActionConflictException e) {
+ throw new IllegalStateException(e);
+ }
}
public ActionAnalysisMetadata getCoverageReportAction() {
return coverageReportAction;
}
- public ImmutableList<ActionAnalysisMetadata> getActions() {
- return ImmutableList.of(lcovWriteAction, coverageReportAction);
+ public Actions.GeneratingActions getActions() {
+ return processedActions;
}
public ImmutableSet<Artifact> getCoverageOutputs() {
@@ -73,6 +87,7 @@
Collection<ConfiguredTarget> targetsToTest,
Iterable<Artifact> baselineCoverageArtifacts,
ArtifactFactory artifactFactory,
- ArtifactOwner artifactOwner,
+ ActionKeyContext actionKeyContext,
+ ActionLookupValue.ActionLookupKey actionLookupKey,
String workspaceName);
}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/coverage/BazelCoverageReportModule.java b/src/main/java/com/google/devtools/build/lib/bazel/coverage/BazelCoverageReportModule.java
index ba74cb0..81c0042 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/coverage/BazelCoverageReportModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/coverage/BazelCoverageReportModule.java
@@ -17,9 +17,10 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.eventbus.EventBus;
+import com.google.devtools.build.lib.actions.ActionKeyContext;
+import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactFactory;
-import com.google.devtools.build.lib.actions.ArtifactOwner;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.test.CoverageReportActionFactory;
@@ -85,7 +86,8 @@
Collection<ConfiguredTarget> targetsToTest,
Iterable<Artifact> baselineCoverageArtifacts,
ArtifactFactory artifactFactory,
- ArtifactOwner artifactOwner,
+ ActionKeyContext actionKeyContext,
+ ActionLookupValue.ActionLookupKey actionLookupKey,
String workspaceName) {
if (options == null || options.combinedReport == ReportType.NONE) {
return null;
@@ -98,7 +100,8 @@
targetsToTest,
baselineCoverageArtifacts,
artifactFactory,
- artifactOwner,
+ actionKeyContext,
+ actionLookupKey,
workspaceName,
this::getArgs,
this::getLocationMessage,
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java b/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java
index ba1ef1d..aefa76f 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java
@@ -161,6 +161,7 @@
Collection<ConfiguredTarget> targetsToTest,
Iterable<Artifact> baselineCoverageArtifacts,
ArtifactFactory factory,
+ ActionKeyContext actionKeyContext,
ArtifactOwner artifactOwner,
String workspaceName,
ArgsFunc argsFunction,
@@ -193,7 +194,8 @@
CoverageArgs.create(directories, coverageArtifacts, lcovArtifact, factory, artifactOwner,
reportGenerator, workspaceName, htmlReport),
argsFunction, locationFunc);
- return new CoverageReportActionsWrapper(lcovFileAction, coverageReportAction);
+ return new CoverageReportActionsWrapper(
+ lcovFileAction, coverageReportAction, actionKeyContext);
} else {
reporter.handle(
Event.error("Cannot generate coverage report - no coverage information was collected"));
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index 43679ed..a27a2c0 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -33,9 +33,6 @@
import com.google.devtools.build.lib.actions.ActionExecutionException;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.ActionKeyContext;
-import com.google.devtools.build.lib.actions.ActionLookupData;
-import com.google.devtools.build.lib.actions.ActionLookupValue;
-import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.ActionResult;
import com.google.devtools.build.lib.actions.Artifact;
@@ -56,7 +53,6 @@
import com.google.devtools.build.lib.actions.extra.CppCompileInfo;
import com.google.devtools.build.lib.actions.extra.EnvironmentVariable;
import com.google.devtools.build.lib.actions.extra.ExtraActionInfo;
-import com.google.devtools.build.lib.bugreport.BugReport;
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.collect.CollectionUtils;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
@@ -1570,62 +1566,26 @@
private static Map<Artifact, NestedSet<? extends Artifact>> computeTransitivelyUsedModules(
SkyFunction.Environment env, Collection<Artifact.DerivedArtifact> usedModules)
throws InterruptedException {
- // ActionLookupKey → ActionLookupValue
- Map<SkyKey, SkyValue> actionLookupValues =
+ Map<SkyKey, SkyValue> actionExecutionValues =
env.getValues(
- Iterables.transform(
- usedModules, module -> (ActionLookupKey) module.getArtifactOwner()));
- if (env.valuesMissing()) {
- ImmutableList<SkyKey> missingKeys =
- actionLookupValues.entrySet().stream()
- .filter(e -> e.getValue() == null)
- .map(Map.Entry::getKey)
- .collect(ImmutableList.toImmutableList());
- BugReport.sendBugReport(
- new IllegalStateException("Missing keys: " + missingKeys + ". Modules " + usedModules));
- return null;
- }
- ArrayList<ActionLookupData> executionValueLookups = new ArrayList<>(usedModules.size());
- for (Artifact module : usedModules) {
- ActionLookupData lookupData = lookupDataFromModule(actionLookupValues, module);
- executionValueLookups.add(Preconditions.checkNotNull(lookupData, module));
- }
-
- // ActionLookupData → ActionExecutionValue
- Map<SkyKey, SkyValue> actionExecutionValues = env.getValues(executionValueLookups);
+ Iterables.transform(usedModules, Artifact.DerivedArtifact::getGeneratingActionKey));
if (env.valuesMissing()) {
return null;
}
ImmutableMap.Builder<Artifact, NestedSet<? extends Artifact>> transitivelyUsedModules =
ImmutableMap.builderWithExpectedSize(usedModules.size());
- int pos = 0;
- for (Artifact module : usedModules) {
- ActionLookupData lookup = executionValueLookups.get(pos++);
- ActionExecutionValue value = (ActionExecutionValue) actionExecutionValues.get(lookup);
+ for (Artifact.DerivedArtifact module : usedModules) {
+ Preconditions.checkState(
+ module.isFileType(CppFileTypes.CPP_MODULE), "Non-module? %s", module);
+ ActionExecutionValue value =
+ Preconditions.checkNotNull(
+ (ActionExecutionValue) actionExecutionValues.get(module.getGeneratingActionKey()),
+ module);
transitivelyUsedModules.put(module, value.getDiscoveredModules());
}
return transitivelyUsedModules.build();
}
- @Nullable
- private static ActionLookupData lookupDataFromModule(
- Map<SkyKey, SkyValue> actionLookupValues, Artifact module) {
- ActionLookupKey lookupKey = (ActionLookupKey) module.getArtifactOwner();
- ActionLookupValue lookupValue = (ActionLookupValue) actionLookupValues.get(lookupKey);
- if (lookupValue == null) {
- return null;
- }
- Preconditions.checkState(
- module.isFileType(CppFileTypes.CPP_MODULE), "Non-module? %s (%s)", module, lookupValue);
- return ActionLookupData.create(
- lookupKey,
- Preconditions.checkNotNull(
- lookupValue.getGeneratingActionIndex(module),
- "%s missing action index for module %s",
- lookupValue,
- module));
- }
-
private static NestedSetBuilder<Artifact> createInputsBuilder(
NestedSet<Artifact> mandatoryInputs, Iterable<Artifact> inputsForInvalidation) {
return NestedSetBuilder.<Artifact>stableOrder()
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java
index de30e93..efcbed6 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java
@@ -22,11 +22,11 @@
import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ActionKeyCacher;
import com.google.devtools.build.lib.actions.ActionKeyContext;
+import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.ActionTemplate;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
-import com.google.devtools.build.lib.actions.ArtifactOwner;
import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
@@ -89,7 +89,7 @@
@Override
public Iterable<CppCompileAction> generateActionForInputArtifacts(
- Iterable<TreeFileArtifact> inputTreeFileArtifacts, ArtifactOwner artifactOwner)
+ Iterable<TreeFileArtifact> inputTreeFileArtifacts, ActionLookupKey artifactOwner)
throws ActionTemplateExpansionException {
ImmutableList.Builder<CppCompileAction> expandedActions = new ImmutableList.Builder<>();
@@ -125,10 +125,10 @@
try {
String outputName = outputTreeFileArtifactName(inputTreeFileArtifact);
TreeFileArtifact outputTreeFileArtifact =
- ActionInputHelper.treeFileArtifact(
+ ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
outputTreeArtifact, PathFragment.create(outputName), artifactOwner);
TreeFileArtifact dotdFileArtifact =
- ActionInputHelper.treeFileArtifact(
+ ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
dotdTreeArtifact, PathFragment.create(outputName + ".d"), artifactOwner);
expandedActions.add(
createAction(
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CriticalPathComputer.java b/src/main/java/com/google/devtools/build/lib/runtime/CriticalPathComputer.java
index fedcaa9..35a7f5e 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/CriticalPathComputer.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/CriticalPathComputer.java
@@ -37,7 +37,6 @@
import java.time.Duration;
import java.util.Comparator;
import java.util.List;
-import java.util.Objects;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
@@ -361,7 +360,9 @@
// shared action that is in the midst of being an action cache hit.
for (Artifact actionOutput : action.getOutputs()) {
if (input.equals(actionOutput)
- && Objects.equals(input.getArtifactOwner(), actionOutput.getArtifactOwner())) {
+ && input
+ .getGeneratingActionKey()
+ .equals(((Artifact.DerivedArtifact) actionOutput).getGeneratingActionKey())) {
// As far as we can tell, this (currently running) action is the same action that
// produced input, not another shared action. This should be impossible.
throw new IllegalStateException(
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 3be7aad..f589d15 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
@@ -13,7 +13,6 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
-import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import com.google.common.base.Objects;
import com.google.common.base.Preconditions;
@@ -22,8 +21,6 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
import com.google.devtools.build.lib.actions.ActionInputHelper;
-import com.google.devtools.build.lib.actions.ActionLookupData;
-import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.OwnerlessArtifactWrapper;
import com.google.devtools.build.lib.actions.ArtifactFileMetadata;
@@ -37,7 +34,6 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.util.BigIntegerFingerprint;
import com.google.devtools.build.lib.vfs.PathFragment;
-import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import java.math.BigInteger;
import java.util.Comparator;
@@ -236,18 +232,6 @@
.sorted(Comparator.comparing(Entry::getKey, Artifact.EXEC_PATH_COMPARATOR));
}
- /**
- * @param lookupKey A {@link SkyKey} whose argument is an {@code ActionLookupKey}, whose
- * corresponding {@code ActionLookupValue} contains the action to be executed.
- * @param index the index of the action to be executed in the {@code ActionLookupValue}, to be
- * passed to {@code ActionLookupValue#getAction}.
- */
- @ThreadSafe
- @VisibleForTesting
- public static ActionLookupData key(ActionLookupValue.ActionLookupKey lookupKey, int index) {
- return ActionLookupData.create(lookupKey, index);
- }
-
@Override
public String toString() {
return MoreObjects.toStringHelper(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 26ca7e2..d5771fc 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
@@ -19,6 +19,7 @@
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
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.ActionLookupValue.ActionLookupKey;
import com.google.devtools.build.lib.actions.Artifact;
@@ -27,7 +28,6 @@
import com.google.devtools.build.lib.analysis.actions.SymlinkAction;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
-import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import java.util.Collection;
import java.util.Map;
@@ -93,27 +93,44 @@
static ImmutableList<FilesetOutputSymlink> getFilesets(
Environment env, Artifact.SpecialArtifact actionInput) throws InterruptedException {
Preconditions.checkState(actionInput.isFileset(), actionInput);
- ActionLookupKey filesetActionLookupKey = (ActionLookupKey) actionInput.getArtifactOwner();
+ ActionLookupData generatingActionKey = actionInput.getGeneratingActionKey();
+ ActionLookupKey filesetActionLookupKey = generatingActionKey.getActionLookupKey();
ActionLookupValue filesetActionLookupValue =
(ActionLookupValue) env.getValue(filesetActionLookupKey);
ActionAnalysisMetadata generatingAction =
- filesetActionLookupValue.getGeneratingActionDangerousReadJavadoc(actionInput);
- int filesetManifestActionIndex;
+ filesetActionLookupValue.getAction(generatingActionKey.getActionIndex());
+ ActionLookupData filesetActionKey;
if (generatingAction instanceof SymlinkAction) {
- Artifact outputManifest = Iterables.getOnlyElement(generatingAction.getInputs());
+ Artifact.DerivedArtifact outputManifest =
+ (Artifact.DerivedArtifact) Iterables.getOnlyElement(generatingAction.getInputs());
+ ActionLookupData manifestGeneratingKey = outputManifest.getGeneratingActionKey();
+ Preconditions.checkState(
+ manifestGeneratingKey.getActionLookupKey().equals(filesetActionLookupKey),
+ "Mismatched actions and artifacts: %s %s %s %s",
+ actionInput,
+ outputManifest,
+ filesetActionLookupKey,
+ manifestGeneratingKey);
ActionAnalysisMetadata symlinkTreeAction =
- filesetActionLookupValue.getGeneratingActionDangerousReadJavadoc(outputManifest);
- Artifact inputManifest = Iterables.getOnlyElement(symlinkTreeAction.getInputs());
- filesetManifestActionIndex = filesetActionLookupValue.getGeneratingActionIndex(inputManifest);
+ filesetActionLookupValue.getAction(manifestGeneratingKey.getActionIndex());
+ Artifact.DerivedArtifact inputManifest =
+ (Artifact.DerivedArtifact) Iterables.getOnlyElement(symlinkTreeAction.getInputs());
+ ActionLookupData inputManifestGeneratingKey = inputManifest.getGeneratingActionKey();
+ Preconditions.checkState(
+ inputManifestGeneratingKey.getActionLookupKey().equals(filesetActionLookupKey),
+ "Mismatched actions and artifacts: %s %s %s %s",
+ actionInput,
+ inputManifest,
+ filesetActionLookupKey,
+ inputManifestGeneratingKey);
+ filesetActionKey = inputManifestGeneratingKey;
} else {
- filesetManifestActionIndex = filesetActionLookupValue.getGeneratingActionIndex(actionInput);
+ filesetActionKey = generatingActionKey;
}
- SkyKey filesetActionKey =
- ActionExecutionValue.key(filesetActionLookupKey, filesetManifestActionIndex);
ActionExecutionValue filesetValue = (ActionExecutionValue) env.getValue(filesetActionKey);
if (filesetValue == null) {
// At this point skyframe does not guarantee that the filesetValue will be ready, since
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java
index d8b5b5f..41bed76 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java
@@ -14,14 +14,18 @@
package com.google.devtools.build.lib.skyframe;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
+import com.google.devtools.build.lib.actions.ActionGraph;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.ActionTemplate;
import com.google.devtools.build.lib.actions.ActionTemplate.ActionTemplateExpansionException;
import com.google.devtools.build.lib.actions.Actions;
import com.google.devtools.build.lib.actions.Actions.GeneratingActions;
+import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.actions.ArtifactPrefixConflictException;
import com.google.devtools.build.lib.actions.ArtifactSkyKey;
@@ -29,11 +33,15 @@
import com.google.devtools.build.lib.bugreport.BugReport;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.skyframe.ActionTemplateExpansionValue.ActionTemplateExpansionKey;
+import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
+import java.util.HashMap;
+import java.util.List;
import java.util.Map;
+import java.util.TreeMap;
import javax.annotation.Nullable;
/**
@@ -80,7 +88,7 @@
// of the ActionTemplate.
generatingActions =
checkActionAndArtifactConflicts(
- actionTemplate.generateActionForInputArtifacts(inputTreeFileArtifacts, key));
+ actionTemplate.generateActionForInputArtifacts(inputTreeFileArtifacts, key), key);
} catch (ActionConflictException e) {
e.reportTo(env.getListener());
throw new ActionTemplateExpansionFunctionException(e);
@@ -110,14 +118,14 @@
}
}
- private GeneratingActions checkActionAndArtifactConflicts(Iterable<? extends Action> actions)
+ private GeneratingActions checkActionAndArtifactConflicts(
+ Iterable<? extends Action> actions, ActionLookupValue.ActionLookupKey actionLookupKey)
throws ActionConflictException, ArtifactPrefixConflictException {
GeneratingActions generatingActions =
- Actions.findAndThrowActionConflict(actionKeyContext, ImmutableList.copyOf(actions));
+ Actions.assignOwnersAndFindAndThrowActionConflict(
+ actionKeyContext, ImmutableList.copyOf(actions), actionLookupKey);
Map<ActionAnalysisMetadata, ArtifactPrefixConflictException> artifactPrefixConflictMap =
- Actions.findArtifactPrefixConflicts(
- ActionLookupValue.getMapForConsistencyCheck(
- generatingActions.getGeneratingActionIndex(), generatingActions.getActions()));
+ findArtifactPrefixConflicts(getMapForConsistencyCheck(generatingActions.getActions()));
if (!artifactPrefixConflictMap.isEmpty()) {
throw artifactPrefixConflictMap.values().iterator().next();
@@ -125,6 +133,57 @@
return generatingActions;
}
+ private static Map<Artifact, ActionAnalysisMetadata> getMapForConsistencyCheck(
+ List<? extends ActionAnalysisMetadata> actions) {
+ if (actions.isEmpty()) {
+ return ImmutableMap.of();
+ }
+ HashMap<Artifact, ActionAnalysisMetadata> result =
+ Maps.newHashMapWithExpectedSize(actions.size() * actions.get(0).getOutputs().size());
+ for (ActionAnalysisMetadata action : actions) {
+ for (Artifact output : action.getOutputs()) {
+ result.put(output, action);
+ }
+ }
+ return result;
+ }
+
+ /**
+ * Finds Artifact prefix conflicts between generated artifacts. An artifact prefix conflict
+ * happens if one action generates an artifact whose path is a prefix of another artifact's path.
+ * Those two artifacts cannot exist simultaneously in the output tree.
+ *
+ * @param generatingActions a map between generated artifacts and their associated generating
+ * actions.
+ * @return a map between actions that generated the conflicting artifacts and their associated
+ * {@link ArtifactPrefixConflictException}.
+ */
+ private static Map<ActionAnalysisMetadata, ArtifactPrefixConflictException>
+ findArtifactPrefixConflicts(Map<Artifact, ActionAnalysisMetadata> generatingActions) {
+ TreeMap<PathFragment, Artifact> artifactPathMap =
+ new TreeMap<>(Actions.comparatorForPrefixConflicts());
+ for (Artifact artifact : generatingActions.keySet()) {
+ artifactPathMap.put(artifact.getExecPath(), artifact);
+ }
+
+ return Actions.findArtifactPrefixConflicts(
+ new MapBasedImmutableActionGraph(generatingActions), artifactPathMap);
+ }
+
+ private static class MapBasedImmutableActionGraph implements ActionGraph {
+ private final Map<Artifact, ActionAnalysisMetadata> generatingActions;
+
+ MapBasedImmutableActionGraph(Map<Artifact, ActionAnalysisMetadata> generatingActions) {
+ this.generatingActions = ImmutableMap.copyOf(generatingActions);
+ }
+
+ @Nullable
+ @Override
+ public ActionAnalysisMetadata getGeneratingAction(Artifact artifact) {
+ return generatingActions.get(artifact);
+ }
+ }
+
@Nullable
@Override
public String extractTag(SkyKey skyKey) {
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 bfec907..b2b4d96 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
@@ -27,6 +27,7 @@
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.TreeFileArtifact;
import com.google.devtools.build.lib.actions.ArtifactFileMetadata;
import com.google.devtools.build.lib.actions.ArtifactOwner;
@@ -428,21 +429,12 @@
// TODO(b/19539699): extend this to comprehensively support all special artifact types (e.g.
// middleman, etc).
static class ArtifactDependencies {
-
- private final Artifact artifact;
- private final ActionLookupKey actionLookupKey;
+ private final DerivedArtifact artifact;
private final ActionLookupValue actionLookupValue;
- private final int actionIndex;
- private ArtifactDependencies(
- Artifact artifact,
- ActionLookupKey actionLookupKey,
- ActionLookupValue actionLookupValue,
- int actionIndex) {
+ private ArtifactDependencies(DerivedArtifact artifact, ActionLookupValue actionLookupValue) {
this.artifact = artifact;
- this.actionLookupKey = actionLookupKey;
this.actionLookupValue = actionLookupValue;
- this.actionIndex = actionIndex;
}
/**
@@ -454,25 +446,15 @@
Artifact.DerivedArtifact derivedArtifact, SkyFunction.Environment env)
throws InterruptedException {
- ActionLookupKey actionLookupKey = ArtifactFunction.getActionLookupKey(derivedArtifact);
+ ActionLookupData generatingActionKey = derivedArtifact.getGeneratingActionKey();
ActionLookupValue actionLookupValue =
- ArtifactFunction.getActionLookupValue(actionLookupKey, env, derivedArtifact);
+ ArtifactFunction.getActionLookupValue(
+ generatingActionKey.getActionLookupKey(), env, derivedArtifact);
if (actionLookupValue == null) {
return null;
}
- Integer actionIndex = actionLookupValue.getGeneratingActionIndex(derivedArtifact);
- if (derivedArtifact.hasParent() && actionIndex == null) {
- // If a TreeFileArtifact is created by a templated action, then it should have the proper
- // reference to its owner. However, if it was created as part of a directory, by the first
- // TreeArtifact-generating action in a chain, then its parent's generating action also
- // generated it. This catches that case.
- actionIndex = actionLookupValue.getGeneratingActionIndex(derivedArtifact.getParent());
- }
- Preconditions.checkNotNull(
- actionIndex, "%s %s %s", derivedArtifact, actionLookupKey, actionLookupValue);
- return new ArtifactDependencies(
- derivedArtifact, actionLookupKey, actionLookupValue, actionIndex);
+ return new ArtifactDependencies(derivedArtifact, actionLookupValue);
}
Artifact getArtifact() {
@@ -480,21 +462,21 @@
}
ActionLookupKey getActionLookupKey() {
- return actionLookupKey;
+ return artifact.getArtifactOwner();
}
int getActionIndex() {
- return actionIndex;
+ return artifact.getGeneratingActionKey().getActionIndex();
}
boolean isTemplateActionForTreeArtifact() {
- return artifact.isTreeArtifact() && actionLookupValue.isActionTemplate(actionIndex);
+ return artifact.isTreeArtifact() && actionLookupValue.isActionTemplate(getActionIndex());
}
ActionLookupData getNontemplateActionExecutionKey() {
Preconditions.checkState(
!isTemplateActionForTreeArtifact(), "Action is unexpectedly template: %s", this);
- return ActionExecutionValue.key(actionLookupKey, actionIndex);
+ return artifact.getGeneratingActionKey();
}
/**
@@ -509,7 +491,7 @@
Preconditions.checkState(
isTemplateActionForTreeArtifact(), "Action is unexpectedly non-template: %s", this);
ActionTemplateExpansionValue.ActionTemplateExpansionKey key =
- ActionTemplateExpansionValue.key(actionLookupKey, actionIndex);
+ ActionTemplateExpansionValue.key(getActionLookupKey(), getActionIndex());
ActionTemplateExpansionValue value = (ActionTemplateExpansionValue) env.getValue(key);
if (value == null) {
return null;
@@ -518,16 +500,15 @@
}
Action getAction() {
- return actionLookupValue.getAction(actionIndex);
+ return actionLookupValue.getAction(getActionIndex());
}
@Override
public String toString() {
return MoreObjects.toStringHelper(this)
.add("artifact", artifact)
- .add("actionLookupKey", actionLookupKey)
+ .add("generatingActionKey", artifact.getGeneratingActionKey())
.add("actionLookupValue", actionLookupValue)
- .add("actionIndex", actionIndex)
.toString();
}
}
@@ -555,8 +536,9 @@
int numActions = value.getNumActions();
ImmutableList.Builder<ActionLookupData> expandedActionExecutionKeys =
ImmutableList.builderWithExpectedSize(numActions);
- for (int i = 0; i < numActions; i++) {
- expandedActionExecutionKeys.add(ActionExecutionValue.key(key, i));
+ for (ActionAnalysisMetadata action : value.getActions()) {
+ expandedActionExecutionKeys.add(
+ ((DerivedArtifact) action.getPrimaryOutput()).getGeneratingActionKey());
}
return expandedActionExecutionKeys.build();
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
index b46fd13..50843d2 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
@@ -444,7 +444,7 @@
ConfiguredAspect configuredAspect,
NestedSet<Package> transitivePackagesForPackageRootResolution,
BigInteger nonceVersion) {
- super(configuredAspect.getActions(), configuredAspect.getGeneratingActionIndex(), nonceVersion);
+ super(configuredAspect.getActions(), nonceVersion);
this.label = Preconditions.checkNotNull(label, actions);
this.aspect = Preconditions.checkNotNull(aspect, label);
this.location = Preconditions.checkNotNull(location, label);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java
index 641e01b..3425c32 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java
@@ -98,8 +98,8 @@
GeneratingActions generatingActions;
try {
generatingActions =
- Actions.filterSharedActionsAndThrowActionConflict(
- actionKeyContext, collection.getActions());
+ Actions.assignOwnersAndFilterSharedActionsAndThrowActionConflict(
+ actionKeyContext, collection.getActions(), keyAndConfig, /*outputFiles=*/ null);
} catch (ActionConflictException e) {
throw new IllegalStateException("Action conflicts not expected in build info: " + skyKey, e);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
index a26a891..94231f6 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -942,9 +942,11 @@
// rule implementation).
try {
generatingActions =
- Actions.filterSharedActionsAndThrowActionConflict(
+ Actions.assignOwnersAndFilterSharedActionsAndThrowActionConflict(
analysisEnvironment.getActionKeyContext(),
- analysisEnvironment.getRegisteredActions());
+ analysisEnvironment.getRegisteredActions(),
+ configuredTargetKey,
+ /*outputFiles=*/ null);
} catch (ActionConflictException e) {
throw new ConfiguredTargetFunctionException(e);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportFunction.java
index c83069e..9a5ca17 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportFunction.java
@@ -16,12 +16,10 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.Actions;
import com.google.devtools.build.lib.actions.Actions.GeneratingActions;
-import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyKey;
@@ -50,16 +48,14 @@
return null;
}
- ImmutableSet.Builder<Artifact> outputs = new ImmutableSet.Builder<>();
-
- for (ActionAnalysisMetadata action : actions) {
- outputs.addAll(action.getOutputs());
- }
-
GeneratingActions generatingActions;
try {
generatingActions =
- Actions.filterSharedActionsAndThrowActionConflict(actionKeyContext, actions);
+ Actions.assignOwnersAndFilterSharedActionsAndThrowActionConflict(
+ actionKeyContext,
+ actions,
+ CoverageReportValue.COVERAGE_REPORT_KEY,
+ /*outputFiles=*/ null);
} catch (ActionConflictException e) {
throw new IllegalStateException("Action conflicts not expected in coverage: " + skyKey, e);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/NonRuleConfiguredTargetValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/NonRuleConfiguredTargetValue.java
index 9b70e17..46151e5 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/NonRuleConfiguredTargetValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/NonRuleConfiguredTargetValue.java
@@ -16,10 +16,8 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.Actions.GeneratingActions;
-import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SourceArtifact;
import com.google.devtools.build.lib.actions.BasicActionLookupValue;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
@@ -53,9 +51,8 @@
@VisibleForSerialization
NonRuleConfiguredTargetValue(
ImmutableList<ActionAnalysisMetadata> actions,
- ImmutableMap<Artifact, Integer> generatingActionIndex,
ConfiguredTarget configuredTarget) {
- super(actions, generatingActionIndex, /*nonceVersion=*/ null);
+ super(actions, /*nonceVersion=*/ null);
this.configuredTarget = configuredTarget;
// Transitive packages are not serialized.
this.transitivePackagesForPackageRootResolution = null;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RuleConfiguredTargetValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/RuleConfiguredTargetValue.java
index 4c8a431..1542b9e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RuleConfiguredTargetValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RuleConfiguredTargetValue.java
@@ -16,10 +16,8 @@
import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionLookupValue;
-import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
@@ -41,7 +39,6 @@
// clear(true) is called.
@Nullable private RuleConfiguredTarget configuredTarget;
private final ImmutableList<ActionAnalysisMetadata> actions;
- private final ImmutableMap<Artifact, Integer> generatingActionIndex;
// May be null either after clearing or because transitive packages are not tracked.
@Nullable private NestedSet<Package> transitivePackagesForPackageRootResolution;
@@ -64,7 +61,6 @@
this.transitivePackagesForPackageRootResolution = transitivePackagesForPackageRootResolution;
// These are specifically *not* copied to save memory.
this.actions = configuredTarget.getActions();
- this.generatingActionIndex = configuredTarget.getGeneratingActionIndex();
}
@Override
@@ -79,11 +75,6 @@
}
@Override
- protected ImmutableMap<Artifact, Integer> getGeneratingActionIndex() {
- return generatingActionIndex;
- }
-
- @Override
public NestedSet<Package> getTransitivePackagesForPackageRootResolution() {
return Preconditions.checkNotNull(transitivePackagesForPackageRootResolution);
}
@@ -100,7 +91,6 @@
@Override
public String toString() {
return MoreObjects.toStringHelper(this)
- .add("generatingActionIndex", generatingActionIndex)
.add("actions", actions)
.add("configuredTarget", configuredTarget)
.toString();
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
index e3cc635..f24127a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
@@ -356,12 +356,9 @@
ExecutorService executor = Executors.newFixedThreadPool(
numJobs,
new ThreadFactoryBuilder().setNameFormat("ActionLookupValue Processor %d").build());
- Set<ActionAnalysisMetadata> registeredActions = Sets.newConcurrentHashSet();
for (List<ActionLookupValue> shard : actionShards) {
executor.execute(
- wrapper.wrap(
- actionRegistration(
- shard, actionGraph, artifactPathMap, badActionMap, registeredActions)));
+ wrapper.wrap(actionRegistration(shard, actionGraph, artifactPathMap, badActionMap)));
}
boolean interrupted = ExecutorUtil.interruptibleShutdown(executor);
Throwables.propagateIfPossible(wrapper.getFirstThrownError());
@@ -375,31 +372,22 @@
final List<ActionLookupValue> values,
final MutableActionGraph actionGraph,
final ConcurrentMap<PathFragment, Artifact> artifactPathMap,
- final ConcurrentMap<ActionAnalysisMetadata, ConflictException> badActionMap,
- final Set<ActionAnalysisMetadata> registeredActions) {
- return new Runnable() {
- @Override
- public void run() {
- for (ActionLookupValue value : values) {
- for (Map.Entry<Artifact, ActionAnalysisMetadata> entry :
- value.getMapForConsistencyCheck().entrySet()) {
- ActionAnalysisMetadata action = entry.getValue();
- // We have an entry for each <action, artifact> pair. Only try to register each action
- // once.
- if (registeredActions.add(action)) {
- try {
- actionGraph.registerAction(action);
- } catch (ActionConflictException e) {
- Exception oldException = badActionMap.put(action, new ConflictException(e));
- Preconditions.checkState(
- oldException == null, "%s | %s | %s", action, e, oldException);
- // We skip the rest of the loop, and do not add the path->artifact mapping for this
- // artifact below -- we don't need to check it since this action is already in
- // error.
- continue;
- }
- }
- artifactPathMap.put(entry.getKey().getExecPath(), entry.getKey());
+ final ConcurrentMap<ActionAnalysisMetadata, ConflictException> badActionMap) {
+ return () -> {
+ for (ActionLookupValue value : values) {
+ for (ActionAnalysisMetadata action : value.getActions()) {
+ try {
+ actionGraph.registerAction(action);
+ } catch (ActionConflictException e) {
+ Exception oldException = badActionMap.put(action, new ConflictException(e));
+ Preconditions.checkState(oldException == null, "%s | %s | %s", action, e, oldException);
+ // We skip the rest of the loop, and do not add the path->artifact mapping for this
+ // artifact below -- we don't need to check it since this action is already in
+ // error.
+ continue;
+ }
+ for (Artifact output : action.getOutputs()) {
+ artifactPathMap.put(output.getExecPath(), output);
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index a8448c1..39e483e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -44,11 +44,12 @@
import com.google.devtools.build.lib.actions.ActionInputPrefetcher;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLogBufferPathGenerator;
+import com.google.devtools.build.lib.actions.ActionLookupData;
import com.google.devtools.build.lib.actions.ActionLookupValue;
+import com.google.devtools.build.lib.actions.Actions;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SourceArtifact;
import com.google.devtools.build.lib.actions.ArtifactFactory;
-import com.google.devtools.build.lib.actions.ArtifactOwner;
import com.google.devtools.build.lib.actions.ArtifactPathResolver;
import com.google.devtools.build.lib.actions.ArtifactResolver.ArtifactResolverSupplier;
import com.google.devtools.build.lib.actions.ArtifactRoot;
@@ -1113,8 +1114,8 @@
: (WorkspaceStatusAction) workspaceStatusValue.getAction(0);
}
- public void injectCoverageReportData(ImmutableList<ActionAnalysisMetadata> actions) {
- PrecomputedValue.COVERAGE_REPORT_KEY.set(injectable(), actions);
+ public void injectCoverageReportData(Actions.GeneratingActions actions) {
+ PrecomputedValue.COVERAGE_REPORT_KEY.set(injectable(), actions.getActions());
}
private void setDefaultVisibility(RuleVisibility defaultVisibility) {
@@ -2457,13 +2458,8 @@
return null;
}
- ArtifactOwner artifactOwner = artifact.getArtifactOwner();
- Preconditions.checkState(
- artifactOwner instanceof ActionLookupValue.ActionLookupKey,
- "%s %s",
- artifact,
- artifactOwner);
- SkyKey actionLookupKey = (ActionLookupValue.ActionLookupKey) artifactOwner;
+ ActionLookupData generatingActionKey =
+ ((Artifact.DerivedArtifact) artifact).getGeneratingActionKey();
synchronized (valueLookupLock) {
// Note that this will crash (attempting to run a configured target value builder after
@@ -2472,13 +2468,18 @@
// this action. We don't expect callers to query generating actions in such cases.
EvaluationResult<ActionLookupValue> result =
evaluate(
- ImmutableList.of(actionLookupKey),
+ ImmutableList.of(generatingActionKey.getActionLookupKey()),
/*keepGoing=*/ false,
/*numThreads=*/ ResourceUsage.getAvailableProcessors(),
eventHandler);
- return result.hasError()
- ? null
- : result.get(actionLookupKey).getGeneratingActionDangerousReadJavadoc(artifact);
+ if (result.hasError()) {
+ return null;
+ }
+ ActionLookupValue actionLookupValue = result.get(generatingActionKey.getActionLookupKey());
+ int actionIndex = generatingActionKey.getActionIndex();
+ return actionLookupValue.isActionTemplate(actionIndex)
+ ? actionLookupValue.getActionTemplate(actionIndex)
+ : actionLookupValue.getAction(actionIndex);
}
}
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 112fd7e..0524167 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
@@ -13,21 +13,15 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Multimap;
-import com.google.common.collect.Multimaps;
-import com.google.devtools.build.lib.actions.ActionLookupData;
-import com.google.devtools.build.lib.actions.ActionLookupValue;
+import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
import com.google.devtools.build.lib.analysis.test.TestProvider;
-import com.google.devtools.build.lib.bugreport.BugReport;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
-import java.util.Map;
/**
* TestCompletionFunction builds all relevant test artifacts of a {@link
@@ -53,49 +47,17 @@
ConfiguredTarget ct = ctValue.getConfiguredTarget();
if (key.exclusiveTesting()) {
// Request test execution iteratively if testing exclusively.
- for (Artifact testArtifact : TestProvider.getTestStatusArtifacts(ct)) {
- ActionLookupValue.ActionLookupKey actionLookupKey =
- ArtifactFunction.getActionLookupKey(testArtifact);
- ActionLookupValue actionLookupValue =
- ArtifactFunction.getActionLookupValue(actionLookupKey, env, testArtifact);
- if (actionLookupValue == null) {
- BugReport.sendBugReport(
- new IllegalStateException(
- "Unexpected absent value for "
- + actionLookupKey
- + " from "
- + testArtifact
- + " and "
- + ct
- + " for "
- + skyKey));
- return null;
- }
- env.getValue(getActionLookupData(testArtifact, actionLookupKey, actionLookupValue));
+ for (Artifact.DerivedArtifact testArtifact : TestProvider.getTestStatusArtifacts(ct)) {
+ env.getValue(testArtifact.getGeneratingActionKey());
if (env.valuesMissing()) {
return null;
}
}
} else {
- Multimap<ActionLookupValue.ActionLookupKey, Artifact.DerivedArtifact> keyToArtifactMap =
- Multimaps.index(
- TestProvider.getTestStatusArtifacts(ct), ArtifactFunction::getActionLookupKey);
- Map<SkyKey, SkyValue> actionLookupValues = env.getValues(keyToArtifactMap.keySet());
- if (env.valuesMissing()) {
- return null;
- }
env.getValues(
- keyToArtifactMap
- .entries()
- .stream()
- .map(
- entry ->
- getActionLookupData(
- entry.getValue(),
- entry.getKey(),
- (ActionLookupValue) actionLookupValues.get(entry.getKey())))
- .distinct()
- .collect(ImmutableSet.toImmutableSet()));
+ Iterables.transform(
+ TestProvider.getTestStatusArtifacts(ct),
+ Artifact.DerivedArtifact::getGeneratingActionKey));
if (env.valuesMissing()) {
return null;
}
@@ -103,14 +65,6 @@
return TestCompletionValue.TEST_COMPLETION_MARKER;
}
- private static ActionLookupData getActionLookupData(
- Artifact artifact,
- ActionLookupValue.ActionLookupKey actionLookupKey,
- ActionLookupValue actionLookupValue) {
- return ActionExecutionValue.key(
- actionLookupKey, actionLookupValue.getGeneratingActionIndex(artifact));
- }
-
@Override
public String extractTag(SkyKey skyKey) {
return Label.print(((ConfiguredTargetKey) skyKey.argument()).getLabel());
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 55876e3..da22b10 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
@@ -39,7 +39,8 @@
Artifact volatileArtifact,
WorkspaceStatusAction workspaceStatusAction) {
super(
- Actions.GeneratingActions.fromSingleAction(workspaceStatusAction), /*nonceVersion=*/ null);
+ Actions.GeneratingActions.fromSingleAction(workspaceStatusAction, BUILD_INFO_KEY),
+ /*nonceVersion=*/ null);
this.stableArtifact = stableArtifact;
this.volatileArtifact = volatileArtifact;
}
