Make Artifact#equals take the owner into account for derived artifacts.

Derived artifacts' owners are important because they are used to determine the artifact's generating action. Source artifacts' owners are not used in this way, so I left them alone.

This allows us to get rid of most uses of ArtifactSkyKey. We may be able to delete it entirely in a follow-up.

PiperOrigin-RevId: 199836436

1 files changed, 33 insertions, 0 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java
index 8ddb52a905..687a787628 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java
@@ -19,10 +19,12 @@ import com.google.common.base.Objects;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Maps;
 import com.google.devtools.build.lib.actions.ActionLookupData;
 import com.google.devtools.build.lib.actions.ActionLookupValue;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.Artifact.OwnerlessArtifactWrapper;
 import com.google.devtools.build.lib.actions.FileArtifactValue;
 import com.google.devtools.build.lib.actions.FileStateValue;
 import com.google.devtools.build.lib.actions.FileValue;
@@ -33,6 +35,8 @@ import com.google.devtools.build.lib.vfs.RootedPath;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 import java.util.Map;
+import java.util.function.Function;
+import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 
 /**
@@ -196,4 +200,33 @@ public class ActionExecutionValue implements SkyValue {
     }
     return value;
   }
+
+  private static <V> ImmutableMap<Artifact, V> transformKeys(
+      ImmutableMap<Artifact, V> data, Map<OwnerlessArtifactWrapper, Artifact> newArtifactMap) {
+    if (data.isEmpty()) {
+      return data;
+    }
+    ImmutableMap.Builder<Artifact, V> result = ImmutableMap.builderWithExpectedSize(data.size());
+    for (Map.Entry<Artifact, V> entry : data.entrySet()) {
+      Artifact transformedArtifact =
+          Preconditions.checkNotNull(
+              newArtifactMap.get(new OwnerlessArtifactWrapper(entry.getKey())), entry);
+      result.put(transformedArtifact, entry.getValue());
+    }
+    return result.build();
+  }
+
+  ActionExecutionValue transformForSharedAction(ImmutableSet<Artifact> outputs) {
+    Map<OwnerlessArtifactWrapper, Artifact> newArtifactMap =
+        outputs
+            .stream()
+            .collect(Collectors.toMap(OwnerlessArtifactWrapper::new, Function.identity()));
+    // This is only called for shared actions, so we'll almost certainly have to transform all keys
+    // in all sets.
+    return new ActionExecutionValue(
+        transformKeys(artifactData, newArtifactMap),
+        transformKeys(treeArtifactData, newArtifactMap),
+        transformKeys(additionalOutputData, newArtifactMap),
+        outputSymlinks);
+  }
 }