Fix shared actions that generate tree artifacts after the recent change to make Artifact#equals take owner into account (unknown commit). In the case that shared actions generate tree artifacts, the outputs of the second action do not include all the artifacts present in the results of the first action, because those artifacts were not known until execution. However, corresponding artifacts can be created easily enough, with the proper owners.

Fixes #5396

PiperOrigin-RevId: 200640969

1 files changed, 43 insertions, 2 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 85664fd7bc..b571d5e1a3 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
@@ -21,10 +21,12 @@ 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.ActionInputHelper;
 import com.google.devtools.build.lib.actions.ActionLookupData;
 import com.google.devtools.build.lib.actions.ActionLookupValue;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.Artifact.OwnerlessArtifactWrapper;
+import com.google.devtools.build.lib.actions.ArtifactOwner;
 import com.google.devtools.build.lib.actions.FileArtifactValue;
 import com.google.devtools.build.lib.actions.FileStateValue;
 import com.google.devtools.build.lib.actions.FileValue;
@@ -240,9 +242,48 @@ public class ActionExecutionValue implements SkyValue {
     }
     ImmutableMap.Builder<Artifact, V> result = ImmutableMap.builderWithExpectedSize(data.size());
     for (Map.Entry<Artifact, V> entry : data.entrySet()) {
+      Artifact artifact = entry.getKey();
       Artifact transformedArtifact =
-          Preconditions.checkNotNull(
-              newArtifactMap.get(new OwnerlessArtifactWrapper(entry.getKey())), entry);
+          newArtifactMap.get(new OwnerlessArtifactWrapper(entry.getKey()));
+      if (transformedArtifact == null) {
+        // If this action generated a tree artifact, then the declared outputs of the action will
+        // not include the contents of the directory corresponding to that artifact, but the
+        // contents are present in this ActionExecutionValue as TreeFileArtifacts. We must create
+        // corresponding artifacts in the shared action's ActionExecutionValue. We can do that since
+        // a TreeFileArtifact is uniquely described by its parent, its owner, and its parent-
+        // relative path. Since the child was not a declared output, the child and parent must be
+        // generated by the same action, hence they have the same owner, and the parent was a
+        // declared output, so it is present in the shared action. Then we can create the new
+        // TreeFileArtifact to have the shared action's version of the parent artifact (instead of
+        // the original parent artifact); the same parent-relative path; and the new parent's
+        // ArtifactOwner.
+        Preconditions.checkState(
+            artifact.hasParent(),
+            "Output artifact %s from one shared action not present in another's outputs (%s)",
+            artifact,
+            newArtifactMap);
+        ArtifactOwner childOwner = artifact.getArtifactOwner();
+        Artifact parent = Preconditions.checkNotNull(artifact.getParent(), artifact);
+        ArtifactOwner parentOwner = parent.getArtifactOwner();
+        Preconditions.checkState(
+            parentOwner.equals(childOwner),
+            "A parent tree artifact %s has a different ArtifactOwner (%s) than its child %s (owned "
+                + "by %s), but both artifacts were generated by the same action",
+            parent,
+            parentOwner,
+            artifact,
+            childOwner);
+        Artifact newParent =
+            Preconditions.checkNotNull(
+                newArtifactMap.get(new OwnerlessArtifactWrapper(parent)),
+                "parent %s of %s was not present in shared action's data (%s)",
+                parent,
+                artifact,
+                newArtifactMap);
+        transformedArtifact =
+            ActionInputHelper.treeFileArtifact(
+                (Artifact.SpecialArtifact) newParent, artifact.getParentRelativePath());
+      }
       result.put(transformedArtifact, entry.getValue());
     }
     return result.build();