Allow tree artifacts to be omitted, take 2. PiperOrigin-RevId: 314357168

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 b465a6e..1c63e3e 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

@@ -73,8 +73,12 @@
     }
 
     for (Map.Entry<Artifact, TreeArtifactValue> tree : treeArtifactData.entrySet()) {
+      TreeArtifactValue treeArtifact = tree.getValue();
+      if (TreeArtifactValue.OMITTED_TREE_MARKER.equals(treeArtifact)) {
+        continue;
+      }
       for (Map.Entry<TreeFileArtifact, FileArtifactValue> file :
-          tree.getValue().getChildValues().entrySet()) {
+          treeArtifact.getChildValues().entrySet()) {
         // We should only have RegularFileValue instances in here, but apparently tree artifacts
         // sometimes store their own root directory in here. Sad.
         // https://github.com/bazelbuild/bazel/issues/9058

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 9c7a570..03fed1a 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

@@ -157,6 +157,10 @@
       Map<Artifact, Collection<Artifact>> expandedArtifacts,
       ActionInputMapSink inputMap,
       Artifact depOwner) {
+    if (TreeArtifactValue.OMITTED_TREE_MARKER.equals(value)) {
+      inputMap.put(treeArtifact, FileArtifactValue.OMITTED_FILE_MARKER, depOwner);
+      return;
+    }
     ImmutableSet.Builder<Artifact> children = ImmutableSet.builder();
     for (Map.Entry<Artifact.TreeFileArtifact, FileArtifactValue> child :
         value.getChildValues().entrySet()) {

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
index 0d9de61..1a65a9c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java

@@ -424,9 +424,19 @@
 
   @Override
   public void markOmitted(Artifact output) {
-    Preconditions.checkState(executionMode.get());
-    Preconditions.checkState(omittedOutputs.add(output), "%s marked as omitted twice", output);
-    store.putArtifactData(output, FileArtifactValue.OMITTED_FILE_MARKER);
+    Preconditions.checkState(
+        executionMode.get(), "Tried to mark %s omitted outside of execution", output);
+    boolean newlyOmitted = omittedOutputs.add(output);
+    if (output.isTreeArtifact()) {
+      // Tolerate marking a tree artifact as omitted multiple times so that callers don't have to
+      // deduplicate when a tree artifact has several omitted children.
+      if (newlyOmitted) {
+        store.putTreeArtifactData((SpecialArtifact) output, TreeArtifactValue.OMITTED_TREE_MARKER);
+      }
+    } else {
+      Preconditions.checkState(newlyOmitted, "%s marked as omitted twice", output);
+      store.putArtifactData(output, FileArtifactValue.OMITTED_FILE_MARKER);
+    }
   }
 
   @Override

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 e2c4621..d9f50b4 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

@@ -161,7 +161,7 @@
     }
   }
 
-  private static SkyValue createTreeArtifactValueFromActionKey(
+  private static TreeArtifactValue createTreeArtifactValueFromActionKey(
       ArtifactDependencies artifactDependencies, Environment env) throws InterruptedException {
     // Request the list of expanded actions from the ActionTemplate.
     ActionTemplateExpansion actionTemplateExpansion =
@@ -232,7 +232,7 @@
           children.isEmpty(),
           "Action template expansion has some but not all outputs omitted, present outputs: %s",
           children);
-      return FileArtifactValue.OMITTED_FILE_MARKER;
+      return TreeArtifactValue.OMITTED_TREE_MARKER;
     }
     return TreeArtifactValue.create(children);
   }

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
index 26b90ab..baf24a3 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD

@@ -2634,6 +2634,7 @@
     deps = [
         "//src/main/java/com/google/devtools/build/lib/actions",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
+        "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant",
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
         "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
index 7ac6c27..01920ca 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java

@@ -26,6 +26,7 @@
 import com.google.devtools.build.lib.actions.HasDigest;
 import com.google.devtools.build.lib.actions.cache.DigestUtils;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
 import com.google.devtools.build.lib.vfs.Dirent;
 import com.google.devtools.build.lib.vfs.Dirent.Type;
 import com.google.devtools.build.lib.vfs.Path;
@@ -43,14 +44,12 @@
  * Value for TreeArtifacts, which contains a digest and the {@link FileArtifactValue}s of its child
  * {@link TreeFileArtifact}s.
  */
-@AutoCodec
 public class TreeArtifactValue implements HasDigest, SkyValue {
 
-  private static final TreeArtifactValue EMPTY =
+  @SerializationConstant @AutoCodec.VisibleForSerialization
+  static final TreeArtifactValue EMPTY =
       new TreeArtifactValue(
-          DigestUtils.fromMetadata(ImmutableMap.of()),
-          ImmutableSortedMap.of(),
-          /* remote= */ false);
+          DigestUtils.fromMetadata(ImmutableMap.of()), ImmutableSortedMap.of(), /*remote=*/ false);
 
   private final byte[] digest;
   private final ImmutableSortedMap<TreeFileArtifact, FileArtifactValue> childData;
@@ -161,57 +160,67 @@
   }
 
   /**
+   * Represents a tree artifact that was intentionally omitted, similar to {@link
+   * FileArtifactValue#OMITTED_FILE_MARKER}.
+   */
+  @SerializationConstant
+  public static final TreeArtifactValue OMITTED_TREE_MARKER = createMarker("OMITTED_TREE_MARKER");
+
+  /**
    * A TreeArtifactValue that represents a missing TreeArtifact. This is occasionally useful because
    * Java's concurrent collections disallow null members.
    */
-  static final TreeArtifactValue MISSING_TREE_ARTIFACT =
-      new TreeArtifactValue(null, ImmutableSortedMap.of(), /* remote= */ false) {
-        @Override
-        FileArtifactValue getSelfData() {
-          throw new UnsupportedOperationException();
-        }
+  static final TreeArtifactValue MISSING_TREE_ARTIFACT = createMarker("MISSING_TREE_ARTIFACT");
 
-        @Override
-        public ImmutableSet<TreeFileArtifact> getChildren() {
-          throw new UnsupportedOperationException();
-        }
+  private static TreeArtifactValue createMarker(String toStringRepresentation) {
+    return new TreeArtifactValue(null, ImmutableSortedMap.of(), /*remote=*/ false) {
+      @Override
+      FileArtifactValue getSelfData() {
+        throw new UnsupportedOperationException(toString());
+      }
 
-        @Override
-        ImmutableMap<TreeFileArtifact, FileArtifactValue> getChildValues() {
-          throw new UnsupportedOperationException();
-        }
+      @Override
+      public ImmutableSet<TreeFileArtifact> getChildren() {
+        throw new UnsupportedOperationException(toString());
+      }
 
-        @Override
-        FileArtifactValue getMetadata() {
-          throw new UnsupportedOperationException();
-        }
+      @Override
+      ImmutableMap<TreeFileArtifact, FileArtifactValue> getChildValues() {
+        throw new UnsupportedOperationException(toString());
+      }
 
-        @Override
-        ImmutableSet<PathFragment> getChildPaths() {
-          throw new UnsupportedOperationException();
-        }
+      @Override
+      FileArtifactValue getMetadata() {
+        throw new UnsupportedOperationException(toString());
+      }
 
-        @Nullable
-        @Override
-        public byte[] getDigest() {
-          throw new UnsupportedOperationException();
-        }
+      @Override
+      ImmutableSet<PathFragment> getChildPaths() {
+        throw new UnsupportedOperationException(toString());
+      }
 
-        @Override
-        public int hashCode() {
-          return 24; // my favorite number
-        }
+      @Nullable
+      @Override
+      public byte[] getDigest() {
+        throw new UnsupportedOperationException(toString());
+      }
 
-        @Override
-        public boolean equals(Object other) {
-          return this == other;
-        }
+      @Override
+      public int hashCode() {
+        return System.identityHashCode(this);
+      }
 
-        @Override
-        public String toString() {
-          return "MISSING_TREE_ARTIFACT";
-        }
-      };
+      @Override
+      public boolean equals(Object other) {
+        return this == other;
+      }
+
+      @Override
+      public String toString() {
+        return toStringRepresentation;
+      }
+    };
+  }
 
   private static void explodeDirectory(
       Path treeArtifactPath,