Change WorkspaceStatusAction incrementality logic. We no longer manually invalidate the BUILD_INFO_KEY node on --workspace_status_command and related flag changes. Instead, the action has a supplier that allows it to retrieve the correct values at execution time.

This does not sacrifice correctness because the action executes unconditionally on every build, so it will never have stale data.

PiperOrigin-RevId: 200265375

3 files changed, 4 insertions, 49 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java
index 41968ff240..9d14119cd7 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java
@@ -451,22 +451,6 @@ public class AnalysisCachingTest extends AnalysisCachingTestBase {
     update();
     assertContainsEvent("Heuristic sharding is intended as a one-off experimentation tool");
   }
-  
-  @Test
-  public void testWorkspaceStatusCommandIsNotCachedForNullBuild() throws Exception {
-    if (getInternalTestExecutionMode() != InternalTestExecutionMode.NORMAL) {
-      // TODO(b/66477180): maybe just ignore.
-      return;
-    }
-    update();
-    WorkspaceStatusAction actionA = getView().getLastWorkspaceBuildInfoActionForTesting();
-    assertThat(actionA.getMnemonic()).isEqualTo("DummyBuildInfoAction");
-
-    workspaceStatusActionFactory.setKey("Second");
-    update();
-    WorkspaceStatusAction actionB = getView().getLastWorkspaceBuildInfoActionForTesting();
-    assertThat(actionB.getMnemonic()).isEqualTo("DummyBuildInfoActionSecond");
-  }
 
   @Test
   public void testSkyframeCacheInvalidationBuildFileChange() throws Exception {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java
index 51c0fcf6e3..1464e08091 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java
@@ -13,7 +13,6 @@
 // limitations under the License.
 package com.google.devtools.build.lib.analysis.util;
 
-import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -63,7 +62,6 @@ import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.UUID;
 import java.util.regex.Pattern;
 
 /**
@@ -207,17 +205,14 @@ public final class AnalysisTestUtil {
   /** A dummy WorkspaceStatusAction. */
   @Immutable
   public static final class DummyWorkspaceStatusAction extends WorkspaceStatusAction {
-    private final String key;
     private final Artifact stableStatus;
     private final Artifact volatileStatus;
 
-    public DummyWorkspaceStatusAction(String key,
-        Artifact stableStatus, Artifact volatileStatus) {
+    public DummyWorkspaceStatusAction(Artifact stableStatus, Artifact volatileStatus) {
       super(
           ActionOwner.SYSTEM_ACTION_OWNER,
           ImmutableList.<Artifact>of(),
           ImmutableList.of(stableStatus, volatileStatus));
-      this.key = key;
       this.stableStatus = stableStatus;
       this.volatileStatus = volatileStatus;
     }
@@ -236,7 +231,7 @@ public final class AnalysisTestUtil {
 
     @Override
     public String getMnemonic() {
-      return "DummyBuildInfoAction" + key;
+      return "DummyBuildInfoAction";
     }
 
     @Override
@@ -251,21 +246,6 @@ public final class AnalysisTestUtil {
     public Artifact getStableStatus() {
       return stableStatus;
     }
-
-    @Override
-    public boolean equals(Object o) {
-      if (!(o instanceof DummyWorkspaceStatusAction)) {
-        return false;
-      }
-
-      DummyWorkspaceStatusAction that = (DummyWorkspaceStatusAction) o;
-      return that.key.equals(this.key);
-    }
-
-    @Override
-    public int hashCode() {
-      return key.hashCode();
-    }
   }
 
   /** A WorkspaceStatusAction.Context that has no stable keys and no volatile keys. */
@@ -287,28 +267,21 @@ public final class AnalysisTestUtil {
    */
   public static class DummyWorkspaceStatusActionFactory implements WorkspaceStatusAction.Factory {
     private final BlazeDirectories directories;
-    private String key;
 
     public DummyWorkspaceStatusActionFactory(BlazeDirectories directories) {
       this.directories = directories;
-      this.key = "";
-    }
-
-    public void setKey(String key) {
-      this.key = key;
     }
 
     @Override
     public WorkspaceStatusAction createWorkspaceStatusAction(
-        ArtifactFactory artifactFactory, ArtifactOwner artifactOwner, Supplier<UUID> buildId,
-        String workspaceName) {
+        ArtifactFactory artifactFactory, ArtifactOwner artifactOwner, String workspaceName) {
       Artifact stableStatus = artifactFactory.getDerivedArtifact(
           PathFragment.create("build-info.txt"),
           directories.getBuildDataDirectory(workspaceName), artifactOwner);
       Artifact volatileStatus = artifactFactory.getConstantMetadataArtifact(
           PathFragment.create("build-changelist.txt"),
           directories.getBuildDataDirectory(workspaceName), artifactOwner);
-      return new DummyWorkspaceStatusAction(key, stableStatus, volatileStatus);
+      return new DummyWorkspaceStatusAction(stableStatus, volatileStatus);
     }
 
     @Override
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index 45e39349d6..a243790ca3 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -900,8 +900,6 @@ public abstract class BuildViewTestCase extends FoundationTestCase {
             .build());
 
     invalidatePackages();
-    // Need to re-initialize the workspace status.
-    getSkyframeExecutor().maybeInvalidateWorkspaceStatusValue("test");
   }
 
   /**