Automated rollback of commit 45b308a62f42c2c0bcfe79dcd4046c4025a31059.

*** Reason for rollback ***

Breaks Javascript compilation.

There's currently no way to iterate over a depset of Artifacts and deduplicate by identical paths in Skylark. This means that actions that want to do something once per Artifact in a depset (add a flag to the command line with the path of the Artifact for instance) will have duplicate entries if there are multiple Artifacts with the same path.

This is not a true automated rollback, since I tried to make this as minimal as possible, to avoid merge conflicts and keep some of the benefits of the rolled back CL. In particular, the tests that were changed in the original CL to give artifacts their correct owners did not need to be changed back, since the owners are still correct.

Moreover, this effectively contains rollbacks of unknown commit and https://github.com/bazelbuild/bazel/commit/39d6f89644107a8f7c080c4f4aec8527c1a73954, but keeps test coverage from those CLs as well. Comments added to CL thread where not a clean rollback (there are plenty of files not changed at all: ActionArtifactCycleReporter is the main wart, since it can still handle SkyKeys with Artifact as the argument, but Artifact is no longer the argument to any SkyKey).

RELNOTES: None

*** Original change description ***

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: 201464780

1 files changed, 0 insertions, 75 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 b571d5e1a3..37f8b2031f 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,14 +19,10 @@ 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.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;
@@ -38,8 +34,6 @@ 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;
 
 /**
@@ -234,73 +228,4 @@ public class ActionExecutionValue implements SkyValue {
       super(artifactData, treeArtifactData, additionalOutputData, outputSymlinks);
     }
   }
-
-  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 artifact = entry.getKey();
-      Artifact transformedArtifact =
-          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();
-  }
-
-  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 create(
-        transformKeys(artifactData, newArtifactMap),
-        transformKeys(treeArtifactData, newArtifactMap),
-        transformKeys(additionalOutputData, newArtifactMap),
-        outputSymlinks,
-        this instanceof CrossServerUnshareableActionExecutionValue);
-  }
 }