commit 9d4193b2cede75de398e737fa620f8d09a8f57fb
author Googler <lberki@google.com> Fri Oct 25 08:06:01 2024 -0700
committer Copybara-Service <copybara-worker@google.com> Fri Oct 25 08:07:18 2024 -0700
tree eaafeaf8e7742f9e543441d3aed16738166ba7c2
parent 5357798b160f4054ade31d109cae4021ff552e26

Track metadata for omitted artifacts also.

This was already the case in some scenarios and since we always have the metadata available, it doesn't make sense to keep hold it back. It's useful, for example, for populating the local action cache for actions with omitted outputs.

I briefly considered adding an isOmitted() bit to FileArtifactValue but decided against it because it would mean that "omitted-ness" is propagated further than ActionOutputMetadataStore, and why do that if it's not necessary?

This means that OMITTED_FILE_MARKER and OMITTED_TREE_MARKER are mostly obsolete. They will be deleted in a followup. change

RELNOTES: None.
PiperOrigin-RevId: 689791065
Change-Id: I2269bb40495da0e9c54c9155a18cacbd34970eca

src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java
index da85124..d9f9c53 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java
@@ -380,17 +380,7 @@
   @Override
   public void markOmitted(Artifact output) {
     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) {
-        treeArtifactData.put((SpecialArtifact) output, TreeArtifactValue.OMITTED_TREE_MARKER);
-      }
-    } else {
-      checkState(newlyOmitted, "%s marked as omitted twice", output);
-      putArtifactData(output, FileArtifactValue.OMITTED_FILE_MARKER);
-    }
+    omittedOutputs.add(output);
   }
 
   @Override

src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStoreTest.java

diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStoreTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStoreTest.java
index eae6cde..e846dec 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStoreTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStoreTest.java
@@ -611,8 +611,6 @@
 
     assertThat(store.artifactOmitted(omitted)).isTrue();
     assertThat(store.artifactOmitted(consumed)).isFalse();
-    assertThat(store.getAllArtifactData())
-        .containsExactly(omitted, FileArtifactValue.OMITTED_FILE_MARKER);
     assertThat(store.getAllTreeArtifactData()).isEmpty();
     assertThat(chmodCalls).isEmpty();
   }
@@ -634,8 +632,6 @@
 
     assertThat(store.artifactOmitted(omittedTree)).isTrue();
     assertThat(store.artifactOmitted(consumedTree)).isFalse();
-    assertThat(store.getAllTreeArtifactData())
-        .containsExactly(omittedTree, TreeArtifactValue.OMITTED_TREE_MARKER);
     assertThat(store.getAllArtifactData()).isEmpty();
     assertThat(chmodCalls).isEmpty();
   }