Put ActionLookupData inside DerivedArtifact, and move ArtifactOwner i…

…nto 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

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;

src/main/java/com/google/devtools/build/lib/actions/ActionInputHelper.java

@@ -144,19 +144,9 @@ public static Collection<ActionInput> fromPaths(Collection<String> paths) {
    */
   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 @@ public static TreeFileArtifact treeFileArtifact(
     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) {

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;
 
 /**
@@ -38,38 +34,6 @@ protected ActionLookupValue(@Nullable BigInteger nonceVersion) {
   /** Returns a list of actions registered by this {@link SkyValue}. */
   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}
@@ -98,17 +62,6 @@ public boolean isActionTemplate(int index) {
     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.
    */

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 @@ private static boolean artifactsEqualWithoutOwner(
   }
 
   /**
-   * 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)
+  public static GeneratingActions assignOwnersAndFilterSharedActionsAndThrowActionConflict(
+      ActionKeyContext actionKeyContext,
+      ImmutableList<ActionAnalysisMetadata> actions,
+      ActionLookupValue.ActionLookupKey actionLookupKey)
       throws ActionConflictException {
-    return Actions.maybeFilterSharedActionsAndThrowIfConflict(
-        actionKeyContext, actions, /*allowSharedAction=*/ true);
+    return Actions.assignOwnersAndMaybeFilterSharedActionsAndThrowIfConflict(
+        actionKeyContext, actions, actionLookupKey, /*allowSharedAction=*/ true);
   }
 
-  private static GeneratingActions maybeFilterSharedActionsAndThrowIfConflict(
+  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 @@ public static String escapeLabel(Label label) {
     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;
+    }
   }
 }