Put ActionLookupData inside DerivedArtifact, and move ArtifactOwner into SourceArtifact.
ActionLookupData is set inside a DerivedArtifact at ConfiguredTarget creation time. In particular, it is now mutable. This means that it is no longer safe to call #getArtifactOwner, #getOwnerLabel, etc. on an artifact if it was created by the configured target that is currently being analyzed.
This allows all callers that were indirectly getting the ActionLookupData via retrieving an ActionLookupKey from the graph to just use it directly. For now, don't actually do very much with this new power. In particular, don't change ArtifactFunction to avoid that lookup.
In a follow-up, ActionExecutionFunction will stop requesting ArtifactValues for normal output artifacts, instead requesting ActionExecutionValues directly, thus avoiding the memory and CPU of creating ArtifactValues for the vast majority of artifacts.
Will also remove ArtifactOwner from ArtifactFactory#getDerivedArtifact in a follow-up.
PiperOrigin-RevId: 250560440
diff --git a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/JUnit4Bazel.java b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/JUnit4Bazel.java
index 5ea00cd..2443583 100644
--- a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/JUnit4Bazel.java
+++ b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/JUnit4Bazel.java
@@ -59,8 +59,8 @@
import org.junit.runner.notification.RunListener;
/**
- * Utility class to create a JUnit4Runner instance from a {@link Builder}. All required
- * dependencies are being injected automatically.
+ * Utility class to create a JUnit4Runner instance from a {@link Builder}. All required dependencies
+ * are being injected automatically.
*/
public final class JUnit4Bazel {
private Supplier<Class<?>> topLevelSuiteSupplier;
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..d320315 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
@@ -144,19 +144,9 @@
*/
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);
+ result.setGeneratingActionKey(parent.getGeneratingActionKey());
+ return result;
}
/**
@@ -168,6 +158,13 @@
return treeFileArtifact(parent, PathFragment.create(relativePath));
}
+ public static TreeFileArtifact treeFileArtifactWithNoGeneratingActionSet(
+ Artifact.SpecialArtifact parent, PathFragment relativePath) {
+ Preconditions.checkState(
+ parent.isTreeArtifact(), "Given parent %s must be a TreeArtifact", parent);
+ return new TreeFileArtifact(parent, relativePath);
+ }
+
/** 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/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/Actions.java b/src/main/java/com/google/devtools/build/lib/actions/Actions.java
index 4e9fb5e..6a44b93 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
@@ -31,9 +31,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;
/**
* Helper class for actions.
@@ -112,80 +109,131 @@
}
/**
- * 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 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 conflict, 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 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);
}
/**
- * 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,
+ ActionLookupValue.ActionLookupKey actionLookupKey)
+ throws ActionConflictException {
+ return Actions.assignOwnersAndMaybeFilterSharedActionsAndThrowIfConflict(
+ actionKeyContext, actions, actionLookupKey, /*allowSharedAction=*/ true);
+ }
+
+ 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>Also builds a map of package-relative paths to artifacts, for use by output file configured
+ * targets when they are retrieving their artifacts from their associated rule.
+ */
+ private static GeneratingActions assignOwnersAndMaybeFilterSharedActionsAndThrowIfConflict(
+ ActionKeyContext actionKeyContext,
+ ImmutableList<ActionAnalysisMetadata> actions,
+ ActionLookupValue.ActionLookupKey actionLookupKey,
boolean allowSharedAction)
throws ActionConflictException {
- Map<Artifact, Integer> generatingActions = new HashMap<>();
+ Map<PathFragment, Artifact.DerivedArtifact> seenArtifacts = new HashMap<>();
+ Map<PathFragment, Artifact> artifactsByPackageRelativePath = new HashMap<>();
+ // If the action owner has a label, we get the package directory so that we can store package-
+ // relative paths.
+ Label owningLabel = actionLookupKey.getLabel();
+ PathFragment packageDirectory =
+ owningLabel != null ? owningLabel.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 package-relative path map with this artifact if applicable.
+ PathFragment rootRelativePath = output.getRootRelativePath();
+ if (packageDirectory != null && rootRelativePath.startsWith(packageDirectory)) {
+ artifactsByPackageRelativePath.put(
+ rootRelativePath.relativeTo(packageDirectory), output);
}
}
+ // 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, ImmutableMap.copyOf(artifactsByPackageRelativePath));
}
/**
@@ -321,52 +369,50 @@
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<PathFragment, Artifact> artifactsByPackageRelativePath;
private GeneratingActions(
ImmutableList<ActionAnalysisMetadata> actions,
- ImmutableMap<Artifact, Integer> generatingActionIndex) {
+ ImmutableMap<PathFragment, Artifact> artifactsByPackageRelativePath) {
this.actions = actions;
- this.generatingActionIndex = generatingActionIndex;
+ this.artifactsByPackageRelativePath = artifactsByPackageRelativePath;
}
- public static GeneratingActions fromSingleAction(ActionAnalysisMetadata action) {
+ public static GeneratingActions fromSingleAction(
+ ActionAnalysisMetadata action, ActionLookupValue.ActionLookupKey actionLookupKey) {
+ Label owningLabel = actionLookupKey.getLabel();
+ Map<PathFragment, Artifact> artifactsByPackageRelativePath = new HashMap<>();
+ PathFragment packageDirectory = null;
+ if (owningLabel != null) {
+ packageDirectory = owningLabel.getPackageIdentifier().getSourceRoot();
+ }
+ ActionLookupData generatingActionKey = ActionLookupData.create(actionLookupKey, 0);
+ for (Artifact output : action.getOutputs()) {
+ ((Artifact.DerivedArtifact) output).setGeneratingActionKey(generatingActionKey);
+ PathFragment rootRelativePath = output.getRootRelativePath();
+ if (packageDirectory != null && rootRelativePath.startsWith(packageDirectory)) {
+ artifactsByPackageRelativePath.put(rootRelativePath.relativeTo(packageDirectory), output);
+ }
+ }
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;
+ ImmutableList.of(action), ImmutableMap.copyOf(artifactsByPackageRelativePath));
}
public ImmutableList<ActionAnalysisMetadata> getActions() {
return actions;
}
+
+ public ImmutableMap<PathFragment, Artifact> getArtifactsByPackageRelativePath() {
+ return artifactsByPackageRelativePath;
+ }
}
}
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..594247d 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
@@ -38,7 +38,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 +102,6 @@
* </ul>
*/
@Immutable
-@AutoCodec
public abstract class Artifact
implements FileType.HasFileType,
ActionInput,
@@ -223,45 +221,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 +237,102 @@
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 {
+ private static final Interner<DerivedArtifact> INTERNER = BlazeInterners.newWeakInterner();
+
private final PathFragment rootRelativePath;
+ private ActionLookupData generatingActionKey;
@VisibleForTesting
- public DerivedArtifact(ArtifactRoot root, PathFragment execPath, ArtifactOwner owner) {
- super(root, execPath, owner);
+ public DerivedArtifact(ArtifactRoot root, PathFragment execPath) {
+ super(root, execPath);
Preconditions.checkState(
!root.getExecPath().isEmpty(), "Derived root has no exec path: %s, %s", root, execPath);
this.rootRelativePath = execPath.relativeTo(root.getExecPath());
}
+ /** Called when a configured target's actions are being collected. */
+ @VisibleForTesting
+ public void setGeneratingActionKey(ActionLookupData generatingActionKey) {
+ Preconditions.checkState(
+ this.generatingActionKey == null,
+ "Already set generating action key: %s (%s %s)",
+ this,
+ this.generatingActionKey,
+ generatingActionKey);
+ this.generatingActionKey = Preconditions.checkNotNull(generatingActionKey, this);
+ }
+
+ boolean hasGeneratingActionKey() {
+ return this.generatingActionKey != null;
+ }
+
+ @Override
+ public boolean hasArtifactOwnerSet() {
+ return hasGeneratingActionKey();
+ }
+
+ /** Can only be called once {@link #setGeneratingActionKey} is called. */
+ public ActionLookupData getGeneratingActionKey() {
+ return Preconditions.checkNotNull(generatingActionKey, this);
+ }
+
+ @Override
+ public ActionLookupValue.ActionLookupKey getArtifactOwner() {
+ return getGeneratingActionKey().getActionLookupKey();
+ }
+
+ @Override
+ public Label getOwnerLabel() {
+ return getGeneratingActionKey().getLabel();
+ }
+
@Override
public PathFragment getRootRelativePath() {
return rootRelativePath;
}
+
+ @Override
+ boolean ownersEqual(Artifact other) {
+ DerivedArtifact that = (DerivedArtifact) other;
+ if (this.generatingActionKey == null || that.generatingActionKey == null) {
+ // Happens only intra-analysis of a configured target. Tolerate.
+ return true;
+ }
+ return this.generatingActionKey.equals(that.generatingActionKey);
+ }
+
+ /**
+ * 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));
+ artifact.setGeneratingActionKey(generatingActionKey);
+ return INTERNER.intern(artifact);
+ }
}
public final Path getPath() {
@@ -357,28 +398,36 @@
}
/**
- * Returns the artifact owner. May be null.
+ * Returns the artifact's owning label. May be null. Can only be called if {@link
+ * #hasArtifactOwnerSet} returns true.
*/
- @Nullable public final Label getOwner() {
+ @Nullable
+ public final Label getOwner() {
return getOwnerLabel();
}
/**
- * Gets the {@code ActionLookupKey} of the {@code ConfiguredTarget} that owns this artifact, if it
- * was set. Otherwise, this should be a dummy value -- either {@link
+ * Can only be called if {@link #hasArtifactOwnerSet} returns true.
+ *
+ * <p>Gets the {@code ActionLookupKey} of the {@code ConfiguredTarget} that owns this artifact, if
+ * it was set. Otherwise, this should be a dummy value -- either {@link
* ArtifactOwner.NullArtifactOwner#INSTANCE} or a dummy owner set in tests. Such a dummy value
* should only occur for source artifacts if created without specifying the owner, or for special
* derived artifacts, such as target completion middleman artifacts, build info artifacts, and the
* like.
+ *
+ * <p>For derived artifacts, this can only be called after the configured target that created the
+ * artifact has finished analyzing. Before that analysis is complete, this artifact should not be
+ * visible to any other configured target, so there is no need to call this method, since the
+ * configured target knows what it is.
*/
- public final ArtifactOwner getArtifactOwner() {
- return owner;
- }
+ public abstract ArtifactOwner getArtifactOwner();
- @Override
- public Label getOwnerLabel() {
- return owner.getLabel();
- }
+ /**
+ * Always true for source artifacts. True for derived artifacts after their configured target has
+ * finished analyzing.
+ */
+ public abstract boolean hasArtifactOwnerSet();
/**
* Returns the root beneath which this Artifact resides, if any. This may be one of the
@@ -471,18 +520,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 +542,23 @@
return getExecPath();
}
+ @Override
+ public ArtifactOwner getArtifactOwner() {
+ return owner;
+ }
+
+ @Override
+ public Label getOwnerLabel() {
+ return owner.getLabel();
+ }
+
+ @Override
+ public boolean hasArtifactOwnerSet() {
+ return true;
+ }
+
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 +586,24 @@
public static final class SpecialArtifact extends DerivedArtifact {
private final SpecialArtifactType type;
- @VisibleForSerialization
- public SpecialArtifact(
- ArtifactRoot root, PathFragment execPath, ArtifactOwner owner, SpecialArtifactType type) {
- super(root, execPath, owner);
+ @VisibleForTesting
+ public SpecialArtifact(ArtifactRoot root, PathFragment execPath, SpecialArtifactType type) {
+ super(root, execPath);
this.type = type;
}
+ @AutoCodec.VisibleForSerialization
+ @AutoCodec.Instantiator
+ static SpecialArtifact create(
+ ArtifactRoot root,
+ PathFragment execPath,
+ SpecialArtifactType type,
+ ActionLookupData generatingActionKey) {
+ SpecialArtifact result = new SpecialArtifact(root, execPath, type);
+ result.setGeneratingActionKey(generatingActionKey);
+ return result;
+ }
+
@Override
public final boolean isFileset() {
return type == SpecialArtifactType.FILESET;
@@ -584,25 +662,13 @@
/**
* Constructs a TreeFileArtifact with the given parent-relative path under the given parent
- * TreeArtifact. The {@link ArtifactOwner} of the TreeFileArtifact is the {@link ArtifactOwner}
- * of the parent TreeArtifact.
+ * TreeArtifact.
*/
@VisibleForTesting
- public TreeFileArtifact(SpecialArtifact parent, PathFragment parentRelativePath) {
- this(parent, parentRelativePath, parent.getArtifactOwner());
- }
-
- /**
- * 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) {
+ public TreeFileArtifact(SpecialArtifact parentTreeArtifact, PathFragment parentRelativePath) {
super(
parentTreeArtifact.getRoot(),
- parentTreeArtifact.getExecPath().getRelative(parentRelativePath),
- owner);
+ parentTreeArtifact.getExecPath().getRelative(parentRelativePath));
Preconditions.checkArgument(
parentTreeArtifact.isTreeArtifact(),
"The parent of TreeFileArtifact (parent-relative path: %s) is not a TreeArtifact: %s",
@@ -620,6 +686,17 @@
this.parentRelativePath = parentRelativePath;
}
+ @AutoCodec.VisibleForSerialization
+ @AutoCodec.Instantiator
+ static TreeFileArtifact createForSerialization(
+ SpecialArtifact parentTreeArtifact,
+ PathFragment parentRelativePath,
+ ActionLookupData generatingActionKey) {
+ TreeFileArtifact result = new TreeFileArtifact(parentTreeArtifact, parentRelativePath);
+ result.setGeneratingActionKey(generatingActionKey);
+ return result;
+ }
+
@Override
public SpecialArtifact getParent() {
return parentTreeArtifact;
@@ -683,7 +760,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 +771,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..45b0f02 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
@@ -325,9 +325,9 @@
if (type == null) {
return root.isSourceRoot()
? new Artifact.SourceArtifact(root, execPath, owner)
- : new Artifact.DerivedArtifact(root, execPath, owner);
+ : new Artifact.DerivedArtifact(root, execPath);
} else {
- return new Artifact.SpecialArtifact(root, execPath, owner, type);
+ return new Artifact.SpecialArtifact(root, execPath, 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..832c1ae 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,14 @@
AnalysisEnvironment analysisEnvironment = ruleContext.getAnalysisEnvironment();
GeneratingActions generatingActions =
- Actions.filterSharedActionsAndThrowActionConflict(
+ Actions.assignOwnersAndFilterSharedActionsAndThrowActionConflict(
analysisEnvironment.getActionKeyContext(),
- analysisEnvironment.getRegisteredActions());
+ analysisEnvironment.getRegisteredActions(),
+ ruleContext.getOwner());
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..7f76c22 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.getOutputByPackageRelativePath(outputFile.getLabel().getName());
+ 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..1d553fd 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
@@ -222,13 +222,16 @@
}
AnalysisEnvironment analysisEnvironment = ruleContext.getAnalysisEnvironment();
- GeneratingActions generatingActions = Actions.filterSharedActionsAndThrowActionConflict(
- analysisEnvironment.getActionKeyContext(), analysisEnvironment.getRegisteredActions());
+ GeneratingActions generatingActions =
+ Actions.assignOwnersAndFilterSharedActionsAndThrowActionConflict(
+ analysisEnvironment.getActionKeyContext(),
+ analysisEnvironment.getRegisteredActions(),
+ ruleContext.getOwner());
return new RuleConfiguredTarget(
ruleContext,
providers,
generatingActions.getActions(),
- generatingActions.getGeneratingActionIndex());
+ generatingActions.getArtifactsByPackageRelativePath());
}
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..c998e36 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
@@ -104,10 +104,9 @@
PathFragment parentRelativeOutputPath =
outputPathMapper.parentRelativeOutputPath(inputTreeFileArtifact);
- TreeFileArtifact outputTreeFileArtifact = ActionInputHelper.treeFileArtifact(
- outputTreeArtifact,
- parentRelativeOutputPath,
- artifactOwner);
+ TreeFileArtifact outputTreeFileArtifact =
+ ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
+ outputTreeArtifact, parentRelativeOutputPath);
expandedActions.add(createAction(inputTreeFileArtifact, outputTreeFileArtifact));
}
@@ -121,7 +120,7 @@
TreeFileArtifact inputTreeFileArtifact =
ActionInputHelper.treeFileArtifact(inputTreeArtifact, "dummy_for_key");
TreeFileArtifact outputTreeFileArtifact =
- ActionInputHelper.treeFileArtifact(
+ ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
outputTreeArtifact, outputPathMapper.parentRelativeOutputPath(inputTreeFileArtifact));
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..474c24a 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
@@ -48,6 +48,7 @@
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
import com.google.devtools.build.lib.syntax.Printer;
+import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.function.Consumer;
import javax.annotation.Nullable;
@@ -94,7 +95,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<PathFragment, Artifact> artifactsByPackageRelativePath;
@Instantiator
@VisibleForSerialization
@@ -107,8 +109,9 @@
ImmutableSet<ConfiguredTargetKey> implicitDeps,
String ruleClassString,
ImmutableList<ActionAnalysisMetadata> actions,
- ImmutableMap<Artifact, Integer> generatingActionIndex) {
+ ImmutableMap<PathFragment, Artifact> artifactsByPackageRelativePath) {
super(label, configurationKey, visibility);
+ this.artifactsByPackageRelativePath = artifactsByPackageRelativePath;
// We don't use ImmutableMap.Builder here to allow augmenting the initial list of 'default'
// providers by passing them in.
@@ -131,14 +134,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<PathFragment, Artifact> artifactsByPackageRelativePath) {
this(
ruleContext.getLabel(),
ruleContext.getConfigurationKey(),
@@ -148,7 +150,7 @@
Util.findImplicitDeps(ruleContext),
ruleContext.getRule().getRuleClass(),
actions,
- generatingActionIndex);
+ artifactsByPackageRelativePath);
// 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 +253,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 path relative to the package of this target, for use by output file
+ * configured targets.
*/
- public ImmutableMap<Artifact, Integer> getGeneratingActionIndex() {
- return generatingActionIndex;
+ public Artifact getOutputByPackageRelativePath(String packageRelativePath) {
+ return Preconditions.checkNotNull(
+ artifactsByPackageRelativePath.get(PathFragment.create(packageRelativePath)),
+ "%s %s %s",
+ packageRelativePath,
+ this,
+ this.artifactsByPackageRelativePath);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java
index e7e0586..2f995bd 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java
@@ -480,7 +480,10 @@
}
}
+ // If this executable is created by this rule, it may not have a label yet. That is fine. If it
+ // was created by another rule, then it will have a label by now, so we'll catch it.
if (executable != null
+ && executable.hasArtifactOwnerSet()
&& !executable.getArtifactOwner().equals(context.getRuleContext().getOwner())) {
throw new EvalException(
loc,
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..a8a8567 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
@@ -125,11 +125,11 @@
try {
String outputName = outputTreeFileArtifactName(inputTreeFileArtifact);
TreeFileArtifact outputTreeFileArtifact =
- ActionInputHelper.treeFileArtifact(
- outputTreeArtifact, PathFragment.create(outputName), artifactOwner);
+ ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
+ outputTreeArtifact, PathFragment.create(outputName));
TreeFileArtifact dotdFileArtifact =
- ActionInputHelper.treeFileArtifact(
- dotdTreeArtifact, PathFragment.create(outputName + ".d"), artifactOwner);
+ ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
+ dotdTreeArtifact, PathFragment.create(outputName + ".d"));
expandedActions.add(
createAction(
inputTreeFileArtifact, outputTreeFileArtifact, dotdFileArtifact, privateHeaders));
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/ActionInputMapHelper.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java
index 26ca7e2..05a496b 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;
@@ -93,23 +94,42 @@
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);
+ filesetActionLookupValue.getAction(generatingActionKey.getActionIndex());
int filesetManifestActionIndex;
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);
+ filesetManifestActionIndex = inputManifestGeneratingKey.getActionIndex();
} else {
- filesetManifestActionIndex = filesetActionLookupValue.getGeneratingActionIndex(actionInput);
+ filesetManifestActionIndex = generatingActionKey.getActionIndex();
}
SkyKey filesetActionKey =
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..913a7357 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
@@ -454,25 +454,19 @@
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);
+ derivedArtifact,
+ generatingActionKey.getActionLookupKey(),
+ actionLookupValue,
+ generatingActionKey.getActionIndex());
}
Artifact getArtifact() {
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..edcc5eb 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);
} 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 feeb02f..97a4347 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
@@ -896,9 +896,10 @@
// rule implementation).
try {
generatingActions =
- Actions.filterSharedActionsAndThrowActionConflict(
+ Actions.assignOwnersAndFilterSharedActionsAndThrowActionConflict(
analysisEnvironment.getActionKeyContext(),
- analysisEnvironment.getRegisteredActions());
+ analysisEnvironment.getRegisteredActions(),
+ configuredTargetKey);
} 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..731ba91 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,11 @@
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);
} 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 96c54a8..12c8762 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
@@ -355,12 +355,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());
@@ -374,31 +371,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 c1f6d2a..8768063 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;
}
diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index 79ba259..1f9eb3a 100644
--- a/src/test/java/com/google/devtools/build/lib/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -616,6 +616,7 @@
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs",
+ "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//src/main/java/com/google/devtools/common/options",
"//src/main/protobuf:action_cache_java_proto",
"//third_party:jsr305",
diff --git a/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java b/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java
index 1befe4c..5c0532b 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java
@@ -226,6 +226,8 @@
artifactFactory.getDerivedArtifact(fooRelative, outRoot, NULL_ARTIFACT_OWNER);
Artifact.DerivedArtifact b =
artifactFactory.getDerivedArtifact(barRelative, outRoot, NULL_ARTIFACT_OWNER);
+ a.setGeneratingActionKey(ActionsTestUtil.NULL_ACTION_LOOKUP_DATA);
+ b.setGeneratingActionKey(ActionsTestUtil.NULL_ACTION_LOOKUP_DATA);
MutableActionGraph actionGraph = new MapBasedActionGraph(actionKeyContext);
Action originalAction = new ActionsTestUtil.NullAction(NULL_ACTION_OWNER, a);
actionGraph.registerAction(originalAction);
diff --git a/src/test/java/com/google/devtools/build/lib/actions/ArtifactTest.java b/src/test/java/com/google/devtools/build/lib/actions/ArtifactTest.java
index aef63fe..01099d2 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/ArtifactTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/ArtifactTest.java
@@ -37,6 +37,7 @@
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
+import com.google.devtools.build.skyframe.SkyFunctionName;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
@@ -107,12 +108,6 @@
}
@Test
- public void testEmptyLabelIsNone() {
- Artifact artifact = ActionsTestUtil.createArtifact(rootDir, "src/a");
- assertThat(artifact.getOwnerLabel()).isNull();
- }
-
- @Test
public void testComparison() {
PathFragment aPath = PathFragment.create("src/a");
PathFragment bPath = PathFragment.create("src/b");
@@ -145,9 +140,7 @@
Path f1 = scratch.file("/exec/dir/file.ext");
assertThrows(
NullPointerException.class,
- () ->
- new Artifact.DerivedArtifact(
- null, f1.relativeTo(execDir), ArtifactOwner.NullArtifactOwner.INSTANCE));
+ () -> new Artifact.DerivedArtifact(null, f1.relativeTo(execDir)));
}
@Test
@@ -340,13 +333,12 @@
public void testCodec() throws Exception {
Artifact.DerivedArtifact artifact =
(Artifact.DerivedArtifact) ActionsTestUtil.createArtifact(rootDir, "src/a");
+ artifact.setGeneratingActionKey(ActionsTestUtil.NULL_ACTION_LOOKUP_DATA);
ArtifactRoot anotherRoot =
ArtifactRoot.asDerivedRoot(scratch.getFileSystem().getPath("/"), scratch.dir("/src"));
Artifact.DerivedArtifact anotherArtifact =
- new Artifact.DerivedArtifact(
- anotherRoot,
- anotherRoot.getExecPath().getRelative("src/c"),
- new LabelArtifactOwner(Label.parseAbsoluteUnchecked("//foo:bar")));
+ new Artifact.DerivedArtifact(anotherRoot, anotherRoot.getExecPath().getRelative("src/c"));
+ anotherArtifact.setGeneratingActionKey(ActionsTestUtil.NULL_ACTION_LOOKUP_DATA);
new SerializationTester(artifact, anotherArtifact)
.addDependency(FileSystem.class, scratch.getFileSystem())
.runTests();
@@ -443,12 +435,26 @@
public void hashCodeAndEquals() throws IOException {
Path execRoot = scratch.getFileSystem().getPath("/");
ArtifactRoot root = ArtifactRoot.asDerivedRoot(execRoot, scratch.dir("/newRoot"));
- ArtifactOwner firstOwner = () -> Label.parseAbsoluteUnchecked("//bar:bar");
- ArtifactOwner secondOwner = () -> Label.parseAbsoluteUnchecked("//foo:foo");
- Artifact derived1 =
- new Artifact.DerivedArtifact(root, PathFragment.create("newRoot/shared"), firstOwner);
- Artifact derived2 =
- new Artifact.DerivedArtifact(root, PathFragment.create("newRoot/shared"), secondOwner);
+ ActionLookupValue.ActionLookupKey firstOwner =
+ new ActionLookupValue.ActionLookupKey() {
+ @Override
+ public SkyFunctionName functionName() {
+ return null;
+ }
+ };
+ ActionLookupValue.ActionLookupKey secondOwner =
+ new ActionLookupValue.ActionLookupKey() {
+ @Override
+ public SkyFunctionName functionName() {
+ return null;
+ }
+ };
+ Artifact.DerivedArtifact derived1 =
+ new Artifact.DerivedArtifact(root, PathFragment.create("newRoot/shared"));
+ derived1.setGeneratingActionKey(ActionLookupData.create(firstOwner, 0));
+ Artifact.DerivedArtifact derived2 =
+ new Artifact.DerivedArtifact(root, PathFragment.create("newRoot/shared"));
+ derived2.setGeneratingActionKey(ActionLookupData.create(secondOwner, 0));
ArtifactRoot sourceRoot = ArtifactRoot.asSourceRoot(Root.fromPath(root.getRoot().asPath()));
Artifact source1 = new SourceArtifact(sourceRoot, PathFragment.create("shared"), firstOwner);
Artifact source2 = new SourceArtifact(sourceRoot, PathFragment.create("shared"), secondOwner);
diff --git a/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java b/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java
index 21ba9fc..db4e34a 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java
@@ -901,10 +901,12 @@
.addPlaceholderTreeArtifactExecPath("--argTwo", treeArtifactTwo)
.build();
- TreeFileArtifact treeFileArtifactOne = createTreeFileArtifact(
- treeArtifactOne, "children/child1");
- TreeFileArtifact treeFileArtifactTwo = createTreeFileArtifact(
- treeArtifactTwo, "children/child2");
+ TreeFileArtifact treeFileArtifactOne =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(
+ treeArtifactOne, "children/child1");
+ TreeFileArtifact treeFileArtifactTwo =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(
+ treeArtifactTwo, "children/child2");
CustomCommandLine commandLine = commandLineTemplate.evaluateTreeFileArtifacts(
ImmutableList.of(treeFileArtifactOne, treeFileArtifactTwo));
@@ -974,17 +976,9 @@
return new SpecialArtifact(
rootDir,
rootDir.getExecPath().getRelative(relpath),
- ArtifactOwner.NullArtifactOwner.INSTANCE,
SpecialArtifactType.TREE);
}
- private TreeFileArtifact createTreeFileArtifact(
- SpecialArtifact inputTreeArtifact, String parentRelativePath) {
- return ActionInputHelper.treeFileArtifact(
- inputTreeArtifact,
- PathFragment.create(parentRelativePath));
- }
-
private static <T> ImmutableList<T> list(T... objects) {
return ImmutableList.<T>builder().addAll(Arrays.asList(objects)).build();
}
diff --git a/src/test/java/com/google/devtools/build/lib/actions/ExecutableSymlinkActionTest.java b/src/test/java/com/google/devtools/build/lib/actions/ExecutableSymlinkActionTest.java
index 7721533..1ae3dcb 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/ExecutableSymlinkActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/ExecutableSymlinkActionTest.java
@@ -124,10 +124,12 @@
file.setExecutable(/*executable=*/ false);
Artifact.DerivedArtifact input =
(Artifact.DerivedArtifact) ActionsTestUtil.createArtifact(inputRoot, file);
+ input.setGeneratingActionKey(ActionsTestUtil.NULL_ACTION_LOOKUP_DATA);
Artifact.DerivedArtifact output =
(Artifact.DerivedArtifact)
ActionsTestUtil.createArtifact(
outputRoot, outputRoot.getRoot().getRelative("some-output"));
+ output.setGeneratingActionKey(ActionsTestUtil.NULL_ACTION_LOOKUP_DATA);
SymlinkAction action = SymlinkAction.toExecutable(NULL_ACTION_OWNER, input, output, "progress");
new SerializationTester(action)
.addDependency(FileSystem.class, scratch.getFileSystem())
diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
index 863f4cd..7d004da 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
@@ -36,6 +36,8 @@
import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ActionInputPrefetcher;
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.ActionOwner;
import com.google.devtools.build.lib.actions.ActionResult;
import com.google.devtools.build.lib.actions.Artifact;
@@ -82,6 +84,7 @@
import com.google.devtools.build.skyframe.EvaluationContext;
import com.google.devtools.build.skyframe.EvaluationResult;
import com.google.devtools.build.skyframe.SkyFunction;
+import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.ValueOrUntypedException;
@@ -224,7 +227,13 @@
public static Artifact createArtifactWithExecPath(ArtifactRoot root, PathFragment execPath) {
return root.isSourceRoot()
? new Artifact.SourceArtifact(root, execPath, ArtifactOwner.NullArtifactOwner.INSTANCE)
- : new Artifact.DerivedArtifact(root, execPath, ArtifactOwner.NullArtifactOwner.INSTANCE);
+ : new Artifact.DerivedArtifact(root, execPath);
+ }
+
+ public static TreeFileArtifact createTreeFileArtifactWithNoGeneratingAction(
+ SpecialArtifact parent, String relativePath) {
+ return ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
+ parent, PathFragment.create(relativePath));
}
public static void assertNoArtifactEndingWith(RuleConfiguredTarget target, String path) {
@@ -326,6 +335,22 @@
null,
null);
+ @AutoCodec
+ public static final ActionLookupData NULL_ACTION_LOOKUP_DATA =
+ ActionLookupData.create(
+ new ActionLookupValue.ActionLookupKey() {
+ @Override
+ public SkyFunctionName functionName() {
+ return null;
+ }
+
+ @Override
+ public Label getLabel() {
+ return NULL_LABEL;
+ }
+ },
+ 0);
+
/** An unchecked exception class for action conflicts. */
public static class UncheckedActionConflictException extends RuntimeException {
public UncheckedActionConflictException(ActionConflictException e) {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java
index 1bbcb9b..158236d 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java
@@ -27,7 +27,6 @@
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
-import com.google.devtools.build.lib.actions.ArtifactOwner;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.CommandLine;
import com.google.devtools.build.lib.actions.Executor;
@@ -132,15 +131,13 @@
return new SpecialArtifact(
rootDir,
rootDir.getExecPath().getRelative(relpath),
- ArtifactOwner.NullArtifactOwner.INSTANCE,
SpecialArtifactType.TREE);
}
private TreeFileArtifact createTreeFileArtifact(
SpecialArtifact inputTreeArtifact, String parentRelativePath) {
- return ActionInputHelper.treeFileArtifact(
- inputTreeArtifact,
- PathFragment.create(parentRelativePath));
+ return ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
+ inputTreeArtifact, PathFragment.create(parentRelativePath));
}
private ParameterFileWriteAction createParameterFileWriteAction(
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplateTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplateTest.java
index 2bdaed0..8c8b1e2 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplateTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplateTest.java
@@ -373,11 +373,11 @@
private SpecialArtifact createTreeArtifact(String rootRelativePath) {
PathFragment relpath = PathFragment.create(rootRelativePath);
- return new SpecialArtifact(
- root,
- root.getExecPath().getRelative(relpath),
- ArtifactOwner.NullArtifactOwner.INSTANCE,
- SpecialArtifactType.TREE);
+ SpecialArtifact result =
+ new SpecialArtifact(
+ root, root.getExecPath().getRelative(relpath), SpecialArtifactType.TREE);
+ result.setGeneratingActionKey(ActionsTestUtil.NULL_ACTION_LOOKUP_DATA);
+ return result;
}
private Artifact createDerivedArtifact(String rootRelativePath) {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java
index 444eb68..cd81206 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java
@@ -22,6 +22,7 @@
import com.google.devtools.build.lib.actions.ActionResult;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Executor;
+import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.exec.util.TestExecutorBuilder;
@@ -57,6 +58,7 @@
FileSystemUtils.createDirectoryAndParents(linkedInput.getParentDirectory());
linkedInput.createSymbolicLink(input);
outputArtifact = getBinArtifactWithNoOwner("destination.txt");
+ outputArtifact.setGeneratingActionKey(ActionsTestUtil.NULL_ACTION_LOOKUP_DATA);
output = outputArtifact.getPath();
FileSystemUtils.createDirectoryAndParents(output.getParentDirectory());
action = SymlinkAction.toArtifact(NULL_ACTION_OWNER,
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index 0a85f26..1193e2a 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -92,6 +92,7 @@
import com.google.devtools.build.lib.analysis.config.transitions.NullTransition;
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
import com.google.devtools.build.lib.analysis.configuredtargets.FileConfiguredTarget;
+import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.analysis.extra.ExtraAction;
import com.google.devtools.build.lib.analysis.skylark.StarlarkTransition;
@@ -789,6 +790,9 @@
mutableActionGraph.getGeneratingAction(artifact);
if (actionAnalysisMetadata == null) {
+ if (artifact.isSourceArtifact() || !artifact.hasArtifactOwnerSet()) {
+ return null;
+ }
actionAnalysisMetadata = getActionGraph().getGeneratingAction(artifact);
}
@@ -1228,31 +1232,20 @@
}
/**
- * Gets a tree artifact, creating it if necessary. {@code ArtifactOwner} should be a genuine
- * {@link ConfiguredTargetKey} corresponding to a {@link ConfiguredTarget}. If called from a test
- * that does not exercise the analysis phase, the convenience methods {@link
- * #getBinArtifactWithNoOwner} or {@link #getGenfilesArtifactWithNoOwner} should be used instead.
- */
- protected Artifact getTreeArtifact(
- PathFragment rootRelativePath, ArtifactRoot root, ArtifactOwner owner) {
- return view.getArtifactFactory().getTreeArtifact(rootRelativePath, root, owner);
- }
-
- /**
* Gets a Tree Artifact for testing in the subdirectory of the {@link
* BuildConfiguration#getBinDirectory} corresponding to the package of {@code owner}. So to
* specify a file foo/foo.o owned by target //foo:foo, {@code packageRelativePath} should just be
* "foo.o".
*/
protected final Artifact getTreeArtifact(String packageRelativePath, ConfiguredTarget owner) {
- return getPackageRelativeTreeArtifact(
- packageRelativePath,
+ ActionLookupValue.ActionLookupKey actionLookupKey =
+ ConfiguredTargetKey.of(owner, owner.getConfigurationKey(), /*isHostConfiguration=*/ false);
+ return getDerivedArtifact(
+ owner.getLabel().getPackageFragment().getRelative(packageRelativePath),
getConfiguration(owner).getBinDirectory(RepositoryName.MAIN),
- ConfiguredTargetKey.of(
- owner, skyframeExecutor.getConfiguration(reporter, owner.getConfigurationKey())));
+ actionLookupKey);
}
-
/**
* Gets a derived Artifact for testing with path of the form
* root/owner.getPackageFragment()/packageRelativePath.
@@ -1266,19 +1259,6 @@
root, owner);
}
- /**
- * Gets a tree Artifact for testing with path of the form
- * root/owner.getPackageFragment()/packageRelativePath.
- *
- * @see #getDerivedArtifact(PathFragment, ArtifactRoot, ArtifactOwner)
- */
- private Artifact getPackageRelativeTreeArtifact(
- String packageRelativePath, ArtifactRoot root, ArtifactOwner owner) {
- return getTreeArtifact(
- owner.getLabel().getPackageFragment().getRelative(packageRelativePath),
- root, owner);
- }
-
/** Returns the input {@link Artifact}s to the given {@link Action} with the given exec paths. */
protected List<Artifact> getInputs(Action owner, Collection<String> execPaths) {
Set<String> expectedPaths = new HashSet<>(execPaths);
@@ -2164,38 +2144,26 @@
return label.replace(':', '/').substring(1);
}
- protected Artifact getImplicitOutputArtifact(
+ protected final String getImplicitOutputPath(
ConfiguredTarget target, SafeImplicitOutputsFunction outputFunction) {
- return getImplicitOutputArtifact(target, getConfiguration(target), outputFunction);
- }
-
- protected final Artifact getImplicitOutputArtifact(
- ConfiguredTarget target,
- BuildConfiguration configuration,
- SafeImplicitOutputsFunction outputFunction) {
Rule rule;
try {
rule = (Rule) skyframeExecutor.getPackageManager().getTarget(reporter, target.getLabel());
} catch (NoSuchPackageException | NoSuchTargetException | InterruptedException e) {
throw new IllegalStateException(e);
}
- Rule associatedRule = rule.getAssociatedRule();
- RepositoryName repository = associatedRule.getRepository();
+ RawAttributeMapper attr = RawAttributeMapper.of(rule.getAssociatedRule());
- ArtifactRoot root;
- if (associatedRule.hasBinaryOutput()) {
- root = configuration.getBinDirectory(repository);
- } else {
- root = configuration.getGenfilesDirectory(repository);
+ return Iterables.getOnlyElement(outputFunction.getImplicitOutputs(eventCollector, attr));
+ }
+
+ protected final Artifact getImplicitOutputArtifact(
+ ConfiguredTarget target, SafeImplicitOutputsFunction outputFunction) {
+ if (target instanceof MergedConfiguredTarget) {
+ target = ((MergedConfiguredTarget) target).getBaseConfiguredTargetForTesting();
}
- ArtifactOwner owner = ConfiguredTargetKey.of(target.getLabel(), getConfiguration(target));
-
- RawAttributeMapper attr = RawAttributeMapper.of(associatedRule);
-
- String path = Iterables.getOnlyElement(outputFunction.getImplicitOutputs(eventCollector, attr));
-
- return view.getArtifactFactory()
- .getDerivedArtifact(target.getLabel().getPackageFragment().getRelative(path), root, owner);
+ return ((RuleConfiguredTarget) target)
+ .getOutputByPackageRelativePath(getImplicitOutputPath(target, outputFunction));
}
public Path getExecRoot() {
diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java
index fb5e5ad..20c8278 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java
@@ -29,7 +29,6 @@
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
-import com.google.devtools.build.lib.actions.ArtifactOwner;
import com.google.devtools.build.lib.actions.ArtifactPathResolver;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.EmptyRunfilesSupplier;
@@ -235,8 +234,10 @@
public void testRunfilesWithTreeArtifacts() throws Exception {
SpecialArtifact treeArtifact = createTreeArtifact("treeArtifact");
assertThat(treeArtifact.isTreeArtifact()).isTrue();
- TreeFileArtifact file1 = ActionInputHelper.treeFileArtifact(treeArtifact, "file1");
- TreeFileArtifact file2 = ActionInputHelper.treeFileArtifact(treeArtifact, "file2");
+ TreeFileArtifact file1 =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(treeArtifact, "file1");
+ TreeFileArtifact file2 =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(treeArtifact, "file2");
FileSystemUtils.writeContentAsLatin1(file1.getPath(), "foo");
FileSystemUtils.writeContentAsLatin1(file2.getPath(), "bar");
@@ -265,8 +266,10 @@
public void testRunfilesWithTreeArtifactsInSymlinks() throws Exception {
SpecialArtifact treeArtifact = createTreeArtifact("treeArtifact");
assertThat(treeArtifact.isTreeArtifact()).isTrue();
- TreeFileArtifact file1 = ActionInputHelper.treeFileArtifact(treeArtifact, "file1");
- TreeFileArtifact file2 = ActionInputHelper.treeFileArtifact(treeArtifact, "file2");
+ TreeFileArtifact file1 =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(treeArtifact, "file1");
+ TreeFileArtifact file2 =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(treeArtifact, "file2");
FileSystemUtils.writeContentAsLatin1(file1.getPath(), "foo");
FileSystemUtils.writeContentAsLatin1(file2.getPath(), "bar");
Runfiles runfiles =
@@ -298,8 +301,10 @@
public void testTreeArtifactsInInputs() throws Exception {
SpecialArtifact treeArtifact = createTreeArtifact("treeArtifact");
assertThat(treeArtifact.isTreeArtifact()).isTrue();
- TreeFileArtifact file1 = ActionInputHelper.treeFileArtifact(treeArtifact, "file1");
- TreeFileArtifact file2 = ActionInputHelper.treeFileArtifact(treeArtifact, "file2");
+ TreeFileArtifact file1 =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(treeArtifact, "file1");
+ TreeFileArtifact file2 =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(treeArtifact, "file2");
FileSystemUtils.writeContentAsLatin1(file1.getPath(), "foo");
FileSystemUtils.writeContentAsLatin1(file2.getPath(), "bar");
@@ -329,7 +334,6 @@
return new SpecialArtifact(
derivedRoot,
derivedRoot.getExecPath().getRelative(derivedRoot.getRoot().relativize(outputPath)),
- ArtifactOwner.NullArtifactOwner.INSTANCE,
SpecialArtifactType.TREE);
}
@@ -399,7 +403,6 @@
return new SpecialArtifact(
rootDir,
PathFragment.create(execPath),
- ArtifactOwner.NullArtifactOwner.INSTANCE,
SpecialArtifactType.FILESET);
}
diff --git a/src/test/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCacheTests.java b/src/test/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCacheTests.java
index 49fe5f3..9ddc38e 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCacheTests.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCacheTests.java
@@ -46,7 +46,6 @@
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.SpecialArtifactType;
-import com.google.devtools.build.lib.actions.ArtifactOwner;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue;
@@ -814,7 +813,6 @@
new SpecialArtifact(
artifactRoot,
PathFragment.create("outputs/dir"),
- ArtifactOwner.NullArtifactOwner.INSTANCE,
SpecialArtifactType.TREE);
MetadataInjector injector = mock(MetadataInjector.class);
@@ -882,7 +880,6 @@
new SpecialArtifact(
artifactRoot,
PathFragment.create("outputs/dir"),
- ArtifactOwner.NullArtifactOwner.INSTANCE,
SpecialArtifactType.TREE);
MetadataInjector injector = mock(MetadataInjector.class);
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidLibraryTest.java
index 59f60a4..96dd324 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidLibraryTest.java
@@ -43,6 +43,7 @@
import com.google.devtools.build.lib.analysis.config.CoreOptionConverters.StrictDepsMode;
import com.google.devtools.build.lib.analysis.configuredtargets.FileConfiguredTarget;
import com.google.devtools.build.lib.analysis.configuredtargets.OutputFileConfiguredTarget;
+import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
@@ -1881,7 +1882,6 @@
getGeneratingSpawnAction(
getImplicitOutputArtifact(
a.getConfiguredTarget(),
- a.getConfiguration(),
AndroidRuleClasses.ANDROID_COMPILED_SYMBOLS));
assertThat(compileAction).isNotNull();
@@ -1889,7 +1889,6 @@
getGeneratingSpawnAction(
getImplicitOutputArtifact(
a.getConfiguredTarget(),
- a.getConfiguration(),
AndroidRuleClasses.ANDROID_RESOURCES_AAPT2_LIBRARY_APK));
assertThat(linkAction).isNotNull();
@@ -1898,21 +1897,17 @@
sdk.getConfiguredTarget().get(AndroidSdkProvider.PROVIDER).getAndroidJar(),
getImplicitOutputArtifact(
a.getConfiguredTarget(),
- a.getConfiguration(),
AndroidRuleClasses.ANDROID_COMPILED_SYMBOLS),
getImplicitOutputArtifact(
b.getConfiguredTarget(),
- a.getConfiguration(),
AndroidRuleClasses.ANDROID_COMPILED_SYMBOLS));
assertThat(linkAction.getOutputs())
.containsAtLeast(
getImplicitOutputArtifact(
a.getConfiguredTarget(),
- a.getConfiguration(),
AndroidRuleClasses.ANDROID_RESOURCES_AAPT2_R_TXT),
getImplicitOutputArtifact(
a.getConfiguredTarget(),
- a.getConfiguration(),
AndroidRuleClasses.ANDROID_RESOURCES_AAPT2_SOURCE_JAR));
}
@@ -1927,16 +1922,11 @@
" resource_files = ['res/values/a.xml'],",
")");
- ConfiguredTarget a = getConfiguredTarget("//java/a:a");
- SpawnAction compileAction =
- getGeneratingSpawnAction(
- getImplicitOutputArtifact(a, AndroidRuleClasses.ANDROID_COMPILED_SYMBOLS));
- assertThat(compileAction).isNull();
-
- SpawnAction linkAction =
- getGeneratingSpawnAction(
- getImplicitOutputArtifact(a, AndroidRuleClasses.ANDROID_RESOURCES_AAPT2_LIBRARY_APK));
- assertThat(linkAction).isNull();
+ RuleConfiguredTarget a = (RuleConfiguredTarget) getConfiguredTarget("//java/a:a");
+ ActionsTestUtil.assertNoArtifactEndingWith(
+ a, getImplicitOutputPath(a, AndroidRuleClasses.ANDROID_COMPILED_SYMBOLS));
+ ActionsTestUtil.assertNoArtifactEndingWith(
+ a, getImplicitOutputPath(a, AndroidRuleClasses.ANDROID_RESOURCES_AAPT2_LIBRARY_APK));
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java
index 9992c70..7a04a66 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java
@@ -24,13 +24,11 @@
import com.google.common.primitives.Ints;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
-import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.Artifact;
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.SpecialArtifactType;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
-import com.google.devtools.build.lib.actions.ArtifactOwner;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.ResourceSet;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
@@ -901,7 +899,6 @@
return new SpecialArtifact(
ArtifactRoot.asDerivedRoot(execRoot, execRoot.getRelative("out")),
execPath,
- ArtifactOwner.NullArtifactOwner.INSTANCE,
SpecialArtifactType.TREE);
}
@@ -919,8 +916,12 @@
SpecialArtifact testTreeArtifact = createTreeArtifact("library_directory");
- TreeFileArtifact library0 = ActionInputHelper.treeFileArtifact(testTreeArtifact, "library0.o");
- TreeFileArtifact library1 = ActionInputHelper.treeFileArtifact(testTreeArtifact, "library1.o");
+ TreeFileArtifact library0 =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(
+ testTreeArtifact, "library0.o");
+ TreeFileArtifact library1 =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(
+ testTreeArtifact, "library1.o");
ArtifactExpander expander =
new ArtifactExpander() {
@@ -970,8 +971,12 @@
SpecialArtifact testTreeArtifact = createTreeArtifact("library_directory");
- TreeFileArtifact library0 = ActionInputHelper.treeFileArtifact(testTreeArtifact, "library0.o");
- TreeFileArtifact library1 = ActionInputHelper.treeFileArtifact(testTreeArtifact, "library1.o");
+ TreeFileArtifact library0 =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(
+ testTreeArtifact, "library0.o");
+ TreeFileArtifact library1 =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(
+ testTreeArtifact, "library1.o");
ArtifactExpander expander =
(artifact, output) -> {
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscoveryTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscoveryTest.java
index 9c621f7..9daa6cd 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscoveryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscoveryTest.java
@@ -21,7 +21,6 @@
import com.google.devtools.build.lib.actions.ActionExecutionException;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
-import com.google.devtools.build.lib.actions.ArtifactOwner;
import com.google.devtools.build.lib.actions.ArtifactResolver;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
@@ -78,7 +77,6 @@
return new SpecialArtifact(
artifactRoot,
artifactRoot.getExecPath().getRelative(artifactRoot.getRoot().relativize(path)),
- ArtifactOwner.NullArtifactOwner.INSTANCE,
Artifact.SpecialArtifactType.TREE);
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/HeaderThinningTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/HeaderThinningTest.java
index 50614c4..aa96424 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/HeaderThinningTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/HeaderThinningTest.java
@@ -217,7 +217,6 @@
return new SpecialArtifact(
treeArtifactBase.getRoot(),
treeArtifactBase.getExecPath(),
- treeArtifactBase.getArtifactOwner(),
SpecialArtifactType.TREE);
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java
index aefb20d..d2836c3 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java
@@ -23,6 +23,7 @@
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactRoot;
+import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.actions.util.LabelArtifactOwner;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
@@ -144,8 +145,7 @@
/* toolchainInvocations= */ ImmutableList.of(),
"bazel-out",
protoInfo(
- /* directProtos */ ImmutableList.of(
- derivedArtifact("//:dont-care", "source_file.proto")),
+ /* directProtos */ ImmutableList.of(derivedArtifact("source_file.proto")),
/* transitiveProtos */ NestedSetBuilder.emptySet(STABLE_ORDER),
/* transitiveProtoSourceRoots= */ NestedSetBuilder.emptySet(STABLE_ORDER),
/* strictImportableProtoSourceRoots= */ NestedSetBuilder.create(
@@ -367,35 +367,31 @@
assertThat(
protoArgv(
null /* directDependencies */,
- ImmutableList.of(derivedArtifact("//:dont-care", "foo.proto")),
+ ImmutableList.of(derivedArtifact("foo.proto")),
ImmutableList.of(".")))
.containsExactly("-Ifoo.proto=out/foo.proto");
assertThat(
protoArgv(
ImmutableList.of() /* directDependencies */,
- ImmutableList.of(derivedArtifact("//:dont-care", "foo.proto")),
+ ImmutableList.of(derivedArtifact("foo.proto")),
ImmutableList.of(".")))
.containsExactly("-Ifoo.proto=out/foo.proto", "--direct_dependencies=");
assertThat(
protoArgv(
ImmutableList.of(
- Pair.of(
- derivedArtifact("//:dont-care", "foo.proto"),
- null)) /* directDependencies */,
- ImmutableList.of(derivedArtifact("//:dont-care", "foo.proto")),
+ Pair.of(derivedArtifact("foo.proto"), null)) /* directDependencies */,
+ ImmutableList.of(derivedArtifact("foo.proto")),
ImmutableList.of(".")))
.containsExactly("-Ifoo.proto=out/foo.proto", "--direct_dependencies", "foo.proto");
assertThat(
protoArgv(
ImmutableList.of(
- Pair.of(derivedArtifact("//:dont-care", "foo.proto"), null),
- Pair.of(
- derivedArtifact("//:dont-care", "bar.proto"),
- null)) /* directDependencies */,
- ImmutableList.of(derivedArtifact("//:dont-care", "foo.proto")),
+ Pair.of(derivedArtifact("foo.proto"), null),
+ Pair.of(derivedArtifact("bar.proto"), null)) /* directDependencies */,
+ ImmutableList.of(derivedArtifact("foo.proto")),
ImmutableList.of(".")))
.containsExactly(
"-Ifoo.proto=out/foo.proto", "--direct_dependencies", "foo.proto:bar.proto");
@@ -436,11 +432,11 @@
}
/** Creates a dummy artifact with the given path, that actually resides in /out/<path>. */
- private Artifact derivedArtifact(String ownerLabel, String path) {
- return new Artifact.DerivedArtifact(
- derivedRoot,
- derivedRoot.getExecPath().getRelative(path),
- new LabelArtifactOwner(Label.parseAbsoluteUnchecked(ownerLabel)));
+ private Artifact derivedArtifact(String path) {
+ Artifact.DerivedArtifact derivedArtifact =
+ new Artifact.DerivedArtifact(derivedRoot, derivedRoot.getExecPath().getRelative(path));
+ derivedArtifact.setGeneratingActionKey(ActionsTestUtil.NULL_ACTION_LOOKUP_DATA);
+ return derivedArtifact;
}
private static Iterable<String> protoArgv(
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
index 5fc2f17..bca8dfa 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
@@ -25,7 +25,6 @@
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
-import com.google.devtools.build.lib.actions.ArtifactOwner;
import com.google.devtools.build.lib.actions.ArtifactPathResolver;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.FileArtifactValue;
@@ -189,9 +188,7 @@
@Test
public void withUnknownOutputArtifactMissingAllowedTreeArtifact() throws Exception {
PathFragment path = PathFragment.create("bin/foo/bar");
- SpecialArtifact treeArtifact =
- new SpecialArtifact(
- outputRoot, path, ArtifactOwner.NullArtifactOwner.INSTANCE, SpecialArtifactType.TREE);
+ SpecialArtifact treeArtifact = new SpecialArtifact(outputRoot, path, SpecialArtifactType.TREE);
Artifact artifact = new TreeFileArtifact(treeArtifact, PathFragment.create("baz"));
ActionInputMap map = new ActionInputMap(1);
ActionMetadataHandler handler = new ActionMetadataHandler(
@@ -208,9 +205,7 @@
public void withUnknownOutputArtifactStatsFileTreeArtifact() throws Exception {
scratch.file("/output/bin/foo/bar/baz", "not empty");
PathFragment path = PathFragment.create("bin/foo/bar");
- SpecialArtifact treeArtifact =
- new SpecialArtifact(
- outputRoot, path, ArtifactOwner.NullArtifactOwner.INSTANCE, SpecialArtifactType.TREE);
+ SpecialArtifact treeArtifact = new SpecialArtifact(outputRoot, path, SpecialArtifactType.TREE);
Artifact artifact = new TreeFileArtifact(treeArtifact, PathFragment.create("baz"));
assertThat(artifact.getPath().exists()).isTrue();
ActionInputMap map = new ActionInputMap(1);
@@ -227,9 +222,7 @@
@Test
public void withUnknownOutputArtifactMissingDisallowedTreeArtifact() throws Exception {
PathFragment path = PathFragment.create("bin/foo/bar");
- SpecialArtifact treeArtifact =
- new SpecialArtifact(
- outputRoot, path, ArtifactOwner.NullArtifactOwner.INSTANCE, SpecialArtifactType.TREE);
+ SpecialArtifact treeArtifact = new SpecialArtifact(outputRoot, path, SpecialArtifactType.TREE);
Artifact artifact = new TreeFileArtifact(treeArtifact, PathFragment.create("baz"));
ActionInputMap map = new ActionInputMap(1);
ActionMetadataHandler handler = new ActionMetadataHandler(
@@ -297,9 +290,8 @@
@Test
public void injectRemoteTreeArtifactMetadata() throws Exception {
PathFragment path = PathFragment.create("bin/dir");
- SpecialArtifact treeArtifact =
- new SpecialArtifact(
- outputRoot, path, ArtifactOwner.NullArtifactOwner.INSTANCE, SpecialArtifactType.TREE);
+ SpecialArtifact treeArtifact = new SpecialArtifact(outputRoot, path, SpecialArtifactType.TREE);
+ treeArtifact.setGeneratingActionKey(ActionsTestUtil.NULL_ACTION_LOOKUP_DATA);
OutputStore store = new OutputStore();
ActionMetadataHandler handler =
new ActionMetadataHandler(
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java
index 2596d8f..4c46e27 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java
@@ -21,7 +21,6 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Action;
-import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.ActionTemplate;
@@ -213,7 +212,7 @@
ActionTemplate<?> actionTemplate) {
return new NonRuleConfiguredTargetValue(
Mockito.mock(ConfiguredTarget.class),
- Actions.GeneratingActions.fromSingleAction(actionTemplate),
+ Actions.GeneratingActions.fromSingleAction(actionTemplate, CTKEY),
NestedSetBuilder.<Package>stableOrder().build(),
/*nonceVersion=*/ null);
}
@@ -223,7 +222,6 @@
return new SpecialArtifact(
ArtifactRoot.asDerivedRoot(rootDirectory, rootDirectory.getRelative("out")),
execPath,
- ArtifactOwner.NullArtifactOwner.INSTANCE,
SpecialArtifactType.TREE);
}
@@ -233,8 +231,9 @@
Map<TreeFileArtifact, FileArtifactValue> treeFileArtifactMap = new LinkedHashMap<>();
for (String childRelativePath : childRelativePaths) {
- TreeFileArtifact treeFileArtifact = ActionInputHelper.treeFileArtifact(
- treeArtifact, PathFragment.create(childRelativePath));
+ TreeFileArtifact treeFileArtifact =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(
+ treeArtifact, childRelativePath);
scratch.file(treeFileArtifact.getPath().toString(), childRelativePath);
// We do not care about the FileArtifactValues in this test.
treeFileArtifactMap.put(treeFileArtifact, FileArtifactValue.create(treeFileArtifact));
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java
index f6f8a8b..a58db17 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java
@@ -32,7 +32,6 @@
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
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;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.ArtifactSkyKey;
import com.google.devtools.build.lib.actions.BasicActionLookupValue;
@@ -192,10 +191,8 @@
@Test
public void testActionTreeArtifactOutput() throws Throwable {
SpecialArtifact artifact = createDerivedTreeArtifactWithAction("treeArtifact");
- TreeFileArtifact treeFileArtifact1 =
- createFakeTreeFileArtifact(artifact, ALL_OWNER, "child1", "hello1");
- TreeFileArtifact treeFileArtifact2 =
- createFakeTreeFileArtifact(artifact, ALL_OWNER, "child2", "hello2");
+ TreeFileArtifact treeFileArtifact1 = createFakeTreeFileArtifact(artifact, "child1", "hello1");
+ TreeFileArtifact treeFileArtifact2 = createFakeTreeFileArtifact(artifact, "child2", "hello2");
TreeArtifactValue value = (TreeArtifactValue) evaluateArtifactValue(artifact);
assertThat(value.getChildValues()).containsKey(treeFileArtifact1);
@@ -216,15 +213,11 @@
TreeFileArtifact treeFileArtifact1 =
createFakeTreeFileArtifact(
artifact2,
- ActionTemplateExpansionValue.ActionTemplateExpansionKey.of(
- (ActionLookupValue.ActionLookupKey) artifact2.getArtifactOwner(), 1),
"child1",
"hello1");
TreeFileArtifact treeFileArtifact2 =
createFakeTreeFileArtifact(
artifact2,
- ActionTemplateExpansionValue.ActionTemplateExpansionKey.of(
- (ActionLookupValue.ActionLookupKey) artifact2.getArtifactOwner(), 1),
"child2",
"hello2");
@@ -257,15 +250,11 @@
TreeFileArtifact treeFileArtifact1 =
createFakeTreeFileArtifact(
artifact3,
- ActionTemplateExpansionValue.ActionTemplateExpansionKey.of(
- (ActionLookupValue.ActionLookupKey) artifact2.getArtifactOwner(), 2),
"child1",
"hello1");
TreeFileArtifact treeFileArtifact2 =
createFakeTreeFileArtifact(
artifact3,
- ActionTemplateExpansionValue.ActionTemplateExpansionKey.of(
- (ActionLookupValue.ActionLookupKey) artifact2.getArtifactOwner(), 2),
"child2",
"hello2");
actions.add(
@@ -280,17 +269,23 @@
@Test
public void actionExecutionValueSerialization() throws Exception {
- Artifact artifact1 = createDerivedArtifact("one");
- Artifact artifact2 = createDerivedArtifact("two");
+ Artifact.DerivedArtifact artifact1 = createDerivedArtifact("one");
+ artifact1.setGeneratingActionKey(ActionsTestUtil.NULL_ACTION_LOOKUP_DATA);
+ Artifact.DerivedArtifact artifact2 = createDerivedArtifact("two");
+ artifact2.setGeneratingActionKey(ActionsTestUtil.NULL_ACTION_LOOKUP_DATA);
ArtifactFileMetadata metadata1 =
ActionMetadataHandler.fileMetadataFromArtifact(artifact1, null, null);
SpecialArtifact treeArtifact = createDerivedTreeArtifactOnly("tree");
- TreeFileArtifact treeFileArtifact =
- createFakeTreeFileArtifact(treeArtifact, "subpath", "content");
+ treeArtifact.setGeneratingActionKey(ActionsTestUtil.NULL_ACTION_LOOKUP_DATA);
+ TreeFileArtifact treeFileArtifact = ActionInputHelper.treeFileArtifact(treeArtifact, "subpath");
+ Path path = treeFileArtifact.getPath();
+ FileSystemUtils.createDirectoryAndParents(path.getParentDirectory());
+ writeFile(path, "contents");
TreeArtifactValue treeArtifactValue =
TreeArtifactValue.create(
ImmutableMap.of(treeFileArtifact, FileArtifactValue.create(treeFileArtifact)));
- Artifact artifact3 = createDerivedArtifact("three");
+ Artifact.DerivedArtifact artifact3 = createDerivedArtifact("three");
+ artifact3.setGeneratingActionKey(ActionsTestUtil.NULL_ACTION_LOOKUP_DATA);
FilesetOutputSymlink filesetOutputSymlink =
FilesetOutputSymlink.createForTesting(
PathFragment.EMPTY_FRAGMENT, PathFragment.EMPTY_FRAGMENT, PathFragment.EMPTY_FRAGMENT);
@@ -326,11 +321,11 @@
ArtifactRoot.asSourceRoot(Root.fromPath(root)), PathFragment.create(path));
}
- private Artifact createDerivedArtifact(String path) {
+ private Artifact.DerivedArtifact createDerivedArtifact(String path) {
PathFragment execPath = PathFragment.create("out").getRelative(path);
- Artifact output =
+ Artifact.DerivedArtifact output =
new Artifact.DerivedArtifact(
- ArtifactRoot.asDerivedRoot(root, root.getRelative("out")), execPath, ALL_OWNER);
+ ArtifactRoot.asDerivedRoot(root, root.getRelative("out")), execPath);
actions.add(new DummyAction(ImmutableList.<Artifact>of(), output));
return output;
}
@@ -339,7 +334,7 @@
ArtifactRoot middlemanRoot =
ArtifactRoot.middlemanRoot(middlemanPath, middlemanPath.getRelative("out"));
return new Artifact.DerivedArtifact(
- middlemanRoot, middlemanRoot.getExecPath().getRelative(path), ALL_OWNER);
+ middlemanRoot, middlemanRoot.getExecPath().getRelative(path));
}
private SpecialArtifact createDerivedTreeArtifactWithAction(String path) {
@@ -353,29 +348,17 @@
return new SpecialArtifact(
ArtifactRoot.asDerivedRoot(root, root.getRelative("out")),
execPath,
- ALL_OWNER,
SpecialArtifactType.TREE);
}
private TreeFileArtifact createFakeTreeFileArtifact(
- SpecialArtifact treeArtifact, String parentRelativePath, String content) throws Exception {
- return createFakeTreeFileArtifact(
- treeArtifact,
- ActionTemplateExpansionValue.ActionTemplateExpansionKey.of(
- (ActionLookupValue.ActionLookupKey) treeArtifact.getArtifactOwner(), 0),
- parentRelativePath,
- content);
- }
-
- private TreeFileArtifact createFakeTreeFileArtifact(
SpecialArtifact treeArtifact,
- ArtifactOwner artifactOwner,
String parentRelativePath,
String content)
throws Exception {
TreeFileArtifact treeFileArtifact =
- ActionInputHelper.treeFileArtifact(
- treeArtifact, PathFragment.create(parentRelativePath), artifactOwner);
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(
+ treeArtifact, parentRelativePath);
Path path = treeFileArtifact.getPath();
FileSystemUtils.createDirectoryAndParents(path.getParentDirectory());
writeFile(path, content);
@@ -416,8 +399,8 @@
ImmutableMap.of(
ALL_OWNER,
new BasicActionLookupValue(
- Actions.filterSharedActionsAndThrowActionConflict(
- actionKeyContext, ImmutableList.copyOf(actions)),
+ Actions.assignOwnersAndFilterSharedActionsAndThrowActionConflict(
+ actionKeyContext, ImmutableList.copyOf(actions), ALL_OWNER),
/*nonceVersion=*/ null)));
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
index 1a9da7a..f9719a4 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
@@ -15,7 +15,6 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
-import static com.google.devtools.build.lib.actions.ActionInputHelper.treeFileArtifact;
import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
import com.google.common.collect.ImmutableList;
@@ -31,7 +30,6 @@
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
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;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue;
@@ -425,21 +423,23 @@
batchStatter, ModifiedFileSet.NOTHING_MODIFIED)).isEmpty();
}
- public void checkDirtyTreeArtifactActions(BatchStat batchStatter)
- throws Exception {
+ private void checkDirtyTreeArtifactActions(BatchStat batchStatter) throws Exception {
// Normally, an Action specifies the contents of a TreeArtifact when it executes.
// To decouple FileSystemValueTester checking from Action execution, we inject TreeArtifact
// contents into ActionExecutionValues.
SpecialArtifact out1 = createTreeArtifact("one");
- TreeFileArtifact file11 = treeFileArtifact(out1, "fizz");
+ TreeFileArtifact file11 =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(out1, "fizz");
FileSystemUtils.createDirectoryAndParents(out1.getPath());
FileSystemUtils.writeContentAsLatin1(file11.getPath(), "buzz");
SpecialArtifact out2 = createTreeArtifact("two");
FileSystemUtils.createDirectoryAndParents(out2.getPath().getChild("subdir"));
- TreeFileArtifact file21 = treeFileArtifact(out2, "moony");
- TreeFileArtifact file22 = treeFileArtifact(out2, "subdir/wormtail");
+ TreeFileArtifact file21 =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(out2, "moony");
+ TreeFileArtifact file22 =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(out2, "subdir/wormtail");
FileSystemUtils.writeContentAsLatin1(file21.getPath(), "padfoot");
FileSystemUtils.writeContentAsLatin1(file22.getPath(), "prongs");
@@ -540,11 +540,13 @@
.containsExactly(actionKey1);
// Test that directory contents (and nested contents) matter
- Artifact out1new = treeFileArtifact(out1, "julius/caesar");
+ Artifact out1new =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(out1, "julius/caesar");
FileSystemUtils.createDirectoryAndParents(out1.getPath().getChild("julius"));
FileSystemUtils.writeContentAsLatin1(out1new.getPath(), "octavian");
// even for empty directories
- Artifact outEmptyNew = treeFileArtifact(outEmpty, "marcus");
+ Artifact outEmptyNew =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(outEmpty, "marcus");
FileSystemUtils.writeContentAsLatin1(outEmptyNew.getPath(), "aurelius");
// so does removing
file21.getPath().delete();
@@ -636,7 +638,6 @@
return new SpecialArtifact(
derivedRoot,
derivedRoot.getExecPath().getRelative(derivedRoot.getRoot().relativize(outputPath)),
- ArtifactOwner.NullArtifactOwner.INSTANCE,
SpecialArtifactType.TREE);
}
@@ -802,7 +803,8 @@
ImmutableMap.builder();
for (Map.Entry<PathFragment, RemoteFileArtifactValue> child : children.entrySet()) {
childFileValues.put(
- ActionInputHelper.treeFileArtifact(output, child.getKey()), child.getValue());
+ ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(output, child.getKey()),
+ child.getValue());
}
TreeArtifactValue treeArtifactValue = TreeArtifactValue.create(childFileValues.build());
return ActionExecutionValue.create(
@@ -929,7 +931,8 @@
.isEmpty();
// Create dir/foo on the local disk and test that it invalidates the associated sky key.
- TreeFileArtifact fooArtifact = treeFileArtifact(treeArtifact, "foo");
+ TreeFileArtifact fooArtifact =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(treeArtifact, "foo");
FileSystemUtils.writeContentAsLatin1(fooArtifact.getPath(), "new-foo-content");
assertThat(
new FilesystemValueChecker(/* tsgm= */ null, /* lastExecutionTimeRange= */ null)
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java
index 7ab9ea3..cbebee6 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java
@@ -28,12 +28,10 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
-import com.google.devtools.build.lib.actions.ActionInputHelper;
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.SpecialArtifactType;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
-import com.google.devtools.build.lib.actions.ArtifactOwner;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.ArtifactSkyKey;
import com.google.devtools.build.lib.actions.FileArtifactValue;
@@ -201,7 +199,6 @@
SpecialArtifact treeArtifact = new SpecialArtifact(
ArtifactRoot.asDerivedRoot(rootDirectory, rootDirectory.getRelative("out")),
PathFragment.create("out/" + path),
- ArtifactOwner.NullArtifactOwner.INSTANCE,
SpecialArtifactType.TREE);
assertThat(treeArtifact.isTreeArtifact()).isTrue();
return treeArtifact;
@@ -209,7 +206,8 @@
private void addNewTreeFileArtifact(SpecialArtifact parent, String relatedPath)
throws IOException {
- TreeFileArtifact treeFileArtifact = ActionInputHelper.treeFileArtifact(parent, relatedPath);
+ TreeFileArtifact treeFileArtifact =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(parent, relatedPath);
artifactFunction.addNewTreeFileArtifact(treeFileArtifact);
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
index 08fd7de..e60b386 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
@@ -271,8 +271,8 @@
ImmutableMap.of(
ACTION_LOOKUP_KEY,
new BasicActionLookupValue(
- Actions.filterSharedActionsAndThrowActionConflict(
- actionKeyContext, ImmutableList.copyOf(actions)),
+ Actions.assignOwnersAndFilterSharedActionsAndThrowActionConflict(
+ actionKeyContext, ImmutableList.copyOf(actions), ACTION_LOOKUP_KEY),
/*nonceVersion=*/ null)));
}
}
@@ -384,9 +384,7 @@
Path execRoot = fs.getPath(TestUtils.tmpDir());
PathFragment execPath = PathFragment.create("out").getRelative(name);
return new Artifact.DerivedArtifact(
- ArtifactRoot.asDerivedRoot(execRoot, execRoot.getRelative("out")),
- execPath,
- ACTION_LOOKUP_KEY);
+ ArtifactRoot.asDerivedRoot(execRoot, execRoot.getRelative("out")), execPath);
}
/**
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
index b990867..f22184c 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
@@ -33,6 +33,8 @@
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.ActionInputHelper;
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.ActionResult;
import com.google.devtools.build.lib.actions.Actions;
import com.google.devtools.build.lib.actions.Artifact;
@@ -106,17 +108,23 @@
writeFile(in, "input_content");
outOne = createTreeArtifact("outputOne");
- outOneFileOne = treeFileArtifact(outOne, "out_one_file_one");
- outOneFileTwo = treeFileArtifact(outOne, "out_one_file_two");
+ outOneFileOne =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(outOne, "out_one_file_one");
+ outOneFileTwo =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(outOne, "out_one_file_two");
outTwo = createTreeArtifact("outputTwo");
- outTwoFileOne = treeFileArtifact(outTwo, "out_one_file_one");
- outTwoFileTwo = treeFileArtifact(outTwo, "out_one_file_two");
+ outTwoFileOne =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(outTwo, "out_one_file_one");
+ outTwoFileTwo =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(outTwo, "out_one_file_two");
}
@Test
public void testCodec() throws Exception {
- new SerializationTester(outOne, outOneFileOne)
+ SpecialArtifact parent = createTreeArtifact("parent");
+ parent.setGeneratingActionKey(ActionsTestUtil.NULL_ACTION_LOOKUP_DATA);
+ new SerializationTester(parent, ActionInputHelper.treeFileArtifact(parent, "child"))
.addDependency(FileSystem.class, scratch.getFileSystem())
.runTests();
}
@@ -185,9 +193,11 @@
@Test
public void testCacheCheckingForTreeArtifactsDoesNotCauseReexecution() throws Exception {
SpecialArtifact outOne = createTreeArtifact("outputOne");
+ outOne.setGeneratingActionKey(ActionLookupData.create(ACTION_LOOKUP_KEY, 0));
Button buttonOne = new Button();
SpecialArtifact outTwo = createTreeArtifact("outputTwo");
+ outTwo.setGeneratingActionKey(ActionLookupData.create(ACTION_LOOKUP_KEY, 1));
Button buttonTwo = new Button();
TouchingTestAction actionOne = new TouchingTestAction(
@@ -404,6 +414,10 @@
reporter.removeHandler(failFastHandler);
reporter.addHandler(storingEventHandler);
+ SpecialArtifact outOne = createTreeArtifact("outputOne");
+ outOne.setGeneratingActionKey(ActionLookupData.create(ACTION_LOOKUP_KEY, 0));
+ TreeFileArtifact outOneFileOne = ActionInputHelper.treeFileArtifact(outOne, "out_one_file_one");
+ TreeFileArtifact outOneFileTwo = ActionInputHelper.treeFileArtifact(outOne, "out_one_file_two");
TreeArtifactTestAction failureOne = new TreeArtifactTestAction(
Runnables.doNothing(), outOneFileOne, outOneFileTwo) {
@Override
@@ -428,6 +442,10 @@
assertThat(errors.get(0).getMessage()).contains("not present on disk");
assertThat(errors.get(1).getMessage()).contains("not all outputs were created or valid");
+ SpecialArtifact outTwo = createTreeArtifact("outputTwo");
+ outTwo.setGeneratingActionKey(ActionLookupData.create(ACTION_LOOKUP_KEY, 1));
+ TreeFileArtifact outTwoFileOne = ActionInputHelper.treeFileArtifact(outOne, "out_one_file_one");
+ TreeFileArtifact outTwoFileTwo = ActionInputHelper.treeFileArtifact(outOne, "out_one_file_two");
TreeArtifactTestAction failureTwo = new TreeArtifactTestAction(
Runnables.doNothing(), outTwoFileOne, outTwoFileTwo) {
@Override
@@ -747,6 +765,7 @@
public void testExpandedActionsBuildInActionTemplate() throws Throwable {
// artifact1 is a tree artifact generated by a TouchingTestAction.
SpecialArtifact artifact1 = createTreeArtifact("treeArtifact1");
+ artifact1.setGeneratingActionKey(ActionLookupData.create(ACTION_LOOKUP_KEY, 0));
TreeFileArtifact treeFileArtifactA = ActionInputHelper.treeFileArtifact(
artifact1, PathFragment.create("child1"));
TreeFileArtifact treeFileArtifactB = ActionInputHelper.treeFileArtifact(
@@ -761,10 +780,10 @@
// We mock out the action template function to expand into two actions that just touch the
// output files.
- TreeFileArtifact expectedOutputTreeFileArtifact1 = ActionInputHelper.treeFileArtifact(
- artifact2, PathFragment.create("child1"));
- TreeFileArtifact expectedOutputTreeFileArtifact2 = ActionInputHelper.treeFileArtifact(
- artifact2, PathFragment.create("child2"));
+ TreeFileArtifact expectedOutputTreeFileArtifact1 =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(artifact2, "child1");
+ TreeFileArtifact expectedOutputTreeFileArtifact2 =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(artifact2, "child2");
Action generateOutputAction = new DummyAction(
ImmutableList.<Artifact>of(treeFileArtifactA), expectedOutputTreeFileArtifact1);
Action noGenerateOutputAction = new DummyAction(
@@ -784,6 +803,7 @@
// artifact1 is a tree artifact generated by a TouchingTestAction.
SpecialArtifact artifact1 = createTreeArtifact("treeArtifact1");
+ artifact1.setGeneratingActionKey(ActionLookupData.create(ACTION_LOOKUP_KEY, 0));
TreeFileArtifact treeFileArtifactA = ActionInputHelper.treeFileArtifact(
artifact1, PathFragment.create("child1"));
TreeFileArtifact treeFileArtifactB = ActionInputHelper.treeFileArtifact(
@@ -799,10 +819,10 @@
// We mock out the action template function to expand into two actions:
// One Action that touches the output file.
// The other action that does not generate the output file.
- TreeFileArtifact expectedOutputTreeFileArtifact1 = ActionInputHelper.treeFileArtifact(
- artifact2, PathFragment.create("child1"));
- TreeFileArtifact expectedOutputTreeFileArtifact2 = ActionInputHelper.treeFileArtifact(
- artifact2, PathFragment.create("child2"));
+ TreeFileArtifact expectedOutputTreeFileArtifact1 =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(artifact2, "child1");
+ TreeFileArtifact expectedOutputTreeFileArtifact2 =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(artifact2, "child2");
Action generateOutputAction = new DummyAction(
ImmutableList.<Artifact>of(treeFileArtifactA), expectedOutputTreeFileArtifact1);
Action noGenerateOutputAction = new NoOpDummyAction(
@@ -825,6 +845,7 @@
// artifact1 is a tree artifact generated by a TouchingTestAction.
SpecialArtifact artifact1 = createTreeArtifact("treeArtifact1");
+ artifact1.setGeneratingActionKey(ActionLookupData.create(ACTION_LOOKUP_KEY, 0));
TreeFileArtifact treeFileArtifactA = ActionInputHelper.treeFileArtifact(
artifact1, PathFragment.create("child1"));
TreeFileArtifact treeFileArtifactB = ActionInputHelper.treeFileArtifact(
@@ -840,10 +861,10 @@
// We mock out the action template function to expand into two actions:
// One Action that touches the output file.
// The other action that just throws when executed.
- TreeFileArtifact expectedOutputTreeFileArtifact1 = ActionInputHelper.treeFileArtifact(
- artifact2, PathFragment.create("child1"));
- TreeFileArtifact expectedOutputTreeFileArtifact2 = ActionInputHelper.treeFileArtifact(
- artifact2, PathFragment.create("child2"));
+ TreeFileArtifact expectedOutputTreeFileArtifact1 =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(artifact2, "child1");
+ TreeFileArtifact expectedOutputTreeFileArtifact2 =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(artifact2, "child2");
Action generateOutputAction = new DummyAction(
ImmutableList.<Artifact>of(treeFileArtifactA), expectedOutputTreeFileArtifact1);
Action throwingAction = new ThrowingDummyAction(
@@ -866,6 +887,7 @@
// artifact1 is a tree artifact generated by a TouchingTestAction.
SpecialArtifact artifact1 = createTreeArtifact("treeArtifact1");
+ artifact1.setGeneratingActionKey(ActionLookupData.create(ACTION_LOOKUP_KEY, 0));
TreeFileArtifact treeFileArtifactA = ActionInputHelper.treeFileArtifact(
artifact1, PathFragment.create("child1"));
TreeFileArtifact treeFileArtifactB = ActionInputHelper.treeFileArtifact(
@@ -879,10 +901,10 @@
registerAction(actionTemplate);
// We mock out the action template function to expand into two actions that throw when executed.
- TreeFileArtifact expectedOutputTreeFileArtifact1 = ActionInputHelper.treeFileArtifact(
- artifact2, PathFragment.create("child1"));
- TreeFileArtifact expectedOutputTreeFileArtifact2 = ActionInputHelper.treeFileArtifact(
- artifact2, PathFragment.create("child2"));
+ TreeFileArtifact expectedOutputTreeFileArtifact1 =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(artifact2, "child1");
+ TreeFileArtifact expectedOutputTreeFileArtifact2 =
+ ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(artifact2, "child2");
Action throwingAction = new ThrowingDummyAction(
ImmutableList.<Artifact>of(treeFileArtifactA),
ImmutableList.<Artifact>of(expectedOutputTreeFileArtifact1));
@@ -1186,11 +1208,12 @@
Path execRoot =
fs.getPath(TestUtils.tmpDir()).getRelative("execroot").getRelative("default-exec-root");
PathFragment execPath = PathFragment.create("out").getRelative(name);
- return new SpecialArtifact(
- ArtifactRoot.asDerivedRoot(execRoot, execRoot.getRelative("out")),
- execPath,
- ACTION_LOOKUP_KEY,
- SpecialArtifactType.TREE);
+ SpecialArtifact result =
+ new SpecialArtifact(
+ ArtifactRoot.asDerivedRoot(execRoot, execRoot.getRelative("out")),
+ execPath,
+ SpecialArtifactType.TREE);
+ return result;
}
private void buildArtifact(Artifact artifact) throws Exception {
@@ -1246,7 +1269,8 @@
public SkyValue compute(SkyKey skyKey, Environment env) {
try {
return new ActionTemplateExpansionValue(
- Actions.filterSharedActionsAndThrowActionConflict(actionKeyContext, actions));
+ Actions.assignOwnersAndFilterSharedActionsAndThrowActionConflict(
+ actionKeyContext, actions, (ActionLookupValue.ActionLookupKey) skyKey));
} catch (ActionConflictException e) {
throw new IllegalStateException(e);
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java
index a08c30e..2fa0ee8 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java
@@ -233,7 +233,6 @@
new SpecialArtifact(
ArtifactRoot.asDerivedRoot(root, root.getRelative("out")),
execPath,
- ALL_OWNER,
SpecialArtifactType.TREE);
actions.add(new DummyAction(ImmutableList.<Artifact>of(), output));
FileSystemUtils.createDirectoryAndParents(fullPath);
@@ -255,8 +254,8 @@
ImmutableMap.of(
ALL_OWNER,
new BasicActionLookupValue(
- Actions.filterSharedActionsAndThrowActionConflict(
- actionKeyContext, ImmutableList.copyOf(actions)),
+ Actions.assignOwnersAndFilterSharedActionsAndThrowActionConflict(
+ actionKeyContext, ImmutableList.copyOf(actions), ALL_OWNER),
/*nonceVersion=*/ null)));
}
}
