Remove ConfiguredTarget from TargetCompletionValue, since it is no longer needed: we can get the ConfiguredTargetKey directly from the TargetCompletionKey. Since that was the only use of the actual value in EvaluationProgressReceiver#evaluated, remove it, instead just provide a boolean enum that gives whether or not evaluation succeeded.

PiperOrigin-RevId: 199178047

5 files changed, 67 insertions, 56 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java
index 7dc9f959ef..6e62d1afdb 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java
@@ -23,7 +23,6 @@ import static com.google.devtools.build.lib.skyframe.RecursiveFilesystemTraversa
 import static com.google.devtools.build.lib.skyframe.RecursiveFilesystemTraversalValue.ResolvedFileFactoryForTesting.symlinkToFileForTesting;
 
 import com.google.common.base.Preconditions;
-import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
@@ -73,6 +72,7 @@ import java.util.Map;
 import java.util.Set;
 import java.util.UUID;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Supplier;
 import javax.annotation.Nullable;
 import org.junit.Before;
 import org.junit.Test;
@@ -326,7 +326,7 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe
   private static final class RecordingEvaluationProgressReceiver
       extends EvaluationProgressReceiver.NullEvaluationProgressReceiver {
     Set<SkyKey> invalidations;
-    Set<SkyValue> evaluations;
+    Set<SkyKey> evaluations;
 
     RecordingEvaluationProgressReceiver() {
       clear();
@@ -344,10 +344,11 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe
 
     @Override
     public void evaluated(
-        SkyKey skyKey, Supplier<SkyValue> skyValueSupplier, EvaluationState state) {
-      SkyValue value = skyValueSupplier.get();
-      if (value != null) {
-        evaluations.add(value);
+        SkyKey skyKey,
+        Supplier<EvaluationSuccessState> evaluationSuccessState,
+        EvaluationState state) {
+      if (evaluationSuccessState.get().succeeded()) {
+        evaluations.add(skyKey);
       }
     }
   }
@@ -382,14 +383,14 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe
     ResolvedFile expected = regularFileForTesting(rootedPath);
     RecursiveFilesystemTraversalValue v1 = traverseAndAssertFiles(traversalRoot, expected);
     assertThat(progressReceiver.invalidations).isEmpty();
-    assertThat(progressReceiver.evaluations).contains(v1);
+    assertThat(progressReceiver.evaluations).contains(traversalRoot);
     progressReceiver.clear();
 
     // Edit the file and verify that the value is rebuilt.
     appendToFile(rootArtifact, "bar");
     RecursiveFilesystemTraversalValue v2 = traverseAndAssertFiles(traversalRoot, expected);
     assertThat(progressReceiver.invalidations).contains(traversalRoot);
-    assertThat(progressReceiver.evaluations).contains(v2);
+    assertThat(progressReceiver.evaluations).contains(traversalRoot);
     assertThat(v2).isNotEqualTo(v1);
     assertTraversalRootHashesAreNotEqual(v1, v2);
 
@@ -422,14 +423,14 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe
     ResolvedFile expected = symlinkToFileForTesting(symlinkTargetPath, symlinkNamePath, linkValue);
     RecursiveFilesystemTraversalValue v1 = traverseAndAssertFiles(traversalRoot, expected);
     assertThat(progressReceiver.invalidations).isEmpty();
-    assertThat(progressReceiver.evaluations).contains(v1);
+    assertThat(progressReceiver.evaluations).contains(traversalRoot);
     progressReceiver.clear();
 
     // Edit the target of the symlink and verify that the value is rebuilt.
     appendToFile(linkTargetArtifact, "bar");
     RecursiveFilesystemTraversalValue v2 = traverseAndAssertFiles(traversalRoot, expected);
     assertThat(progressReceiver.invalidations).contains(traversalRoot);
-    assertThat(progressReceiver.evaluations).contains(v2);
+    assertThat(progressReceiver.evaluations).contains(traversalRoot);
     assertThat(v2).isNotEqualTo(v1);
     assertTraversalRootHashesAreNotEqual(v1, v2);
   }
@@ -477,7 +478,7 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe
     RecursiveFilesystemTraversalValue v1 =
         traverseAndAssertFiles(traversalRoot, expected1, expected2);
     assertThat(progressReceiver.invalidations).isEmpty();
-    assertThat(progressReceiver.evaluations).contains(v1);
+    assertThat(progressReceiver.evaluations).contains(traversalRoot);
     progressReceiver.clear();
 
     // Add a new file to the directory and see that the value is rebuilt.
@@ -493,7 +494,7 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe
     RecursiveFilesystemTraversalValue v2 =
         traverseAndAssertFiles(traversalRoot, expected1, expected2, expected3);
     assertThat(progressReceiver.invalidations).contains(traversalRoot);
-    assertThat(progressReceiver.evaluations).contains(v2);
+    assertThat(progressReceiver.evaluations).contains(traversalRoot);
     // Directories always have the same hash code, but that is fine because their contents are also
     // part of the RecursiveFilesystemTraversalValue, so v1 and v2 are unequal.
     assertThat(v2).isNotEqualTo(v1);
@@ -507,7 +508,7 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe
       appendToFile(file1, toInvalidate, "bar");
       v3 = traverseAndAssertFiles(traversalRoot, expected1, expected2, expected3);
       assertThat(progressReceiver.invalidations).contains(traversalRoot);
-      assertThat(progressReceiver.evaluations).contains(v3);
+      assertThat(progressReceiver.evaluations).contains(traversalRoot);
       assertThat(v3).isNotEqualTo(v2);
       // Directories always have the same hash code, but that is fine because their contents are
       // also part of the RecursiveFilesystemTraversalValue, so v2 and v3 are unequal.
@@ -602,7 +603,7 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe
     RecursiveFilesystemTraversalValue v1 =
         traverseAndAssertFiles(traversalRoot, expected1, expected2, expected3);
     assertThat(progressReceiver.invalidations).isEmpty();
-    assertThat(progressReceiver.evaluations).contains(v1);
+    assertThat(progressReceiver.evaluations).contains(traversalRoot);
     progressReceiver.clear();
 
     // Add a new file to the directory and see that the value is rebuilt.
@@ -612,7 +613,7 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe
     RecursiveFilesystemTraversalValue v2 =
         traverseAndAssertFiles(traversalRoot, expected1, expected2, expected3, expected4);
     assertThat(progressReceiver.invalidations).contains(traversalRoot);
-    assertThat(progressReceiver.evaluations).contains(v2);
+    assertThat(progressReceiver.evaluations).contains(traversalRoot);
     assertThat(v2).isNotEqualTo(v1);
     assertTraversalRootHashesAreNotEqual(v1, v2);
     progressReceiver.clear();
@@ -622,7 +623,7 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe
     RecursiveFilesystemTraversalValue v3 =
         traverseAndAssertFiles(traversalRoot, expected1, expected2, expected3, expected4);
     assertThat(progressReceiver.invalidations).contains(traversalRoot);
-    assertThat(progressReceiver.evaluations).contains(v3);
+    assertThat(progressReceiver.evaluations).contains(traversalRoot);
     assertThat(v3).isNotEqualTo(v2);
     assertTraversalRootHashesAreNotEqual(v2, v3);
     progressReceiver.clear();
@@ -805,7 +806,7 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe
     TraversalRequest params = fileLikeRoot(artifact, DONT_CROSS);
     ResolvedFile expected = regularFileForTesting(path);
     RecursiveFilesystemTraversalValue v1 = traverseAndAssertFiles(params, expected);
-    assertThat(progressReceiver.evaluations).contains(v1);
+    assertThat(progressReceiver.evaluations).contains(params);
     progressReceiver.clear();
 
     // Change the digest of the file. See that the value is rebuilt.
@@ -826,7 +827,7 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe
     TraversalRequest params = fileLikeRoot(artifact, DONT_CROSS);
     ResolvedFile expected = regularFileForTesting(path);
     RecursiveFilesystemTraversalValue v1 = traverseAndAssertFiles(params, expected);
-    assertThat(progressReceiver.evaluations).contains(v1);
+    assertThat(progressReceiver.evaluations).contains(params);
     progressReceiver.clear();
 
     // Change the mtime of the file but not the digest. See that the value is *not* rebuilt.
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeAwareActionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeAwareActionTest.java
index 5332637254..00cdc24dba 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeAwareActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeAwareActionTest.java
@@ -16,7 +16,6 @@ package com.google.devtools.build.lib.skyframe;
 import static com.google.common.truth.Truth.assertThat;
 
 import com.google.common.base.Objects;
-import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
@@ -42,7 +41,6 @@ import com.google.devtools.build.skyframe.EvaluationProgressReceiver;
 import com.google.devtools.build.skyframe.EvaluationProgressReceiver.EvaluationState;
 import com.google.devtools.build.skyframe.SkyFunction.Environment;
 import com.google.devtools.build.skyframe.SkyKey;
-import com.google.devtools.build.skyframe.SkyValue;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -50,6 +48,7 @@ import java.nio.charset.StandardCharsets;
 import java.util.Set;
 import java.util.concurrent.Callable;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Supplier;
 import javax.annotation.Nullable;
 import org.junit.Before;
 import org.junit.Test;
@@ -99,14 +98,14 @@ public class SkyframeAwareActionTest extends TimestampBuilderTestCase {
       }
     }
 
-    public static final class EvaluatedEntry {
+    private static final class EvaluatedEntry {
       public final SkyKey skyKey;
-      public final SkyValue value;
+      final EvaluationSuccessState successState;
       public final EvaluationState state;
 
-      EvaluatedEntry(SkyKey skyKey, SkyValue value, EvaluationState state) {
+      EvaluatedEntry(SkyKey skyKey, EvaluationSuccessState successState, EvaluationState state) {
         this.skyKey = skyKey;
-        this.value = value;
+        this.successState = successState;
         this.state = state;
       }
 
@@ -114,13 +113,13 @@ public class SkyframeAwareActionTest extends TimestampBuilderTestCase {
       public boolean equals(Object obj) {
         return obj instanceof EvaluatedEntry
             && this.skyKey.equals(((EvaluatedEntry) obj).skyKey)
-            && this.value.equals(((EvaluatedEntry) obj).value)
+            && this.successState.equals(((EvaluatedEntry) obj).successState)
             && this.state.equals(((EvaluatedEntry) obj).state);
       }
 
       @Override
       public int hashCode() {
-        return Objects.hashCode(skyKey, value, state);
+        return Objects.hashCode(skyKey, successState, state);
       }
     }
 
@@ -164,8 +163,10 @@ public class SkyframeAwareActionTest extends TimestampBuilderTestCase {
 
     @Override
     public void evaluated(
-        SkyKey skyKey, Supplier<SkyValue> skyValueSupplier, EvaluationState state) {
-      evaluated.add(new EvaluatedEntry(skyKey, skyValueSupplier.get(), state));
+        SkyKey skyKey,
+        Supplier<EvaluationSuccessState> evaluationSuccessState,
+        EvaluationState state) {
+      evaluated.add(new EvaluatedEntry(skyKey, evaluationSuccessState.get(), state));
     }
   }
 
@@ -417,7 +418,6 @@ public class SkyframeAwareActionTest extends TimestampBuilderTestCase {
     TrackingEvaluationProgressReceiver.EvaluatedEntry evaluatedAction =
         progressReceiver.getEvalutedEntry(actionKey);
     assertThat(evaluatedAction).isNotNull();
-    SkyValue actionValue = evaluatedAction.value;
 
     // Mutate the action input if requested.
     maybeChangeFile(actionInput, changeActionInput);
@@ -451,12 +451,10 @@ public class SkyframeAwareActionTest extends TimestampBuilderTestCase {
       if (expectActionIs.actuallyClean()) {
         // Action was dirtied but verified clean.
         assertThat(newEntry.state).isEqualTo(EvaluationState.CLEAN);
-        assertThat(newEntry.value).isEqualTo(actionValue);
       } else {
         // Action was dirtied and rebuilt. It was either reexecuted or was an action cache hit,
         // doesn't matter here.
         assertThat(newEntry.state).isEqualTo(EvaluationState.BUILT);
-        assertThat(newEntry.value).isNotEqualTo(actionValue);
       }
     } else {
       // Action was not dirtied.
diff --git a/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java b/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
index ef7ada37b2..2be4f1b477 100644
--- a/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
@@ -26,7 +26,6 @@ import static org.junit.Assert.fail;
 
 import com.google.common.base.Preconditions;
 import com.google.common.base.Predicates;
-import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -62,6 +61,7 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Supplier;
 import javax.annotation.Nullable;
 import org.junit.After;
 import org.junit.Before;
@@ -4336,7 +4336,9 @@ public class MemoizingEvaluatorTest {
         new TrackingProgressReceiver() {
           @Override
           public void evaluated(
-              SkyKey skyKey, Supplier<SkyValue> skyValueSupplier, EvaluationState state) {
+              SkyKey skyKey,
+              Supplier<EvaluationSuccessState> evaluationSuccessState,
+              EvaluationState state) {
             evaluated.add(skyKey);
           }
         });
@@ -4472,7 +4474,9 @@ public class MemoizingEvaluatorTest {
         new TrackingProgressReceiver() {
           @Override
           public void evaluated(
-              SkyKey skyKey, Supplier<SkyValue> skyValueSupplier, EvaluationState state) {
+              SkyKey skyKey,
+              Supplier<EvaluationSuccessState> evaluationSuccessState,
+              EvaluationState state) {
             evaluated.add(skyKey);
           }
         });
diff --git a/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java b/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java
index 8fd7287431..c702bae0bd 100644
--- a/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java
@@ -21,7 +21,6 @@ import static com.google.devtools.build.skyframe.EvaluationResultSubjectFactory.
 import static com.google.devtools.build.skyframe.GraphTester.CONCATENATE;
 import static org.junit.Assert.fail;
 
-import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -55,6 +54,7 @@ import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Supplier;
 import javax.annotation.Nullable;
 import org.junit.After;
 import org.junit.Before;
@@ -256,14 +256,17 @@ public class ParallelEvaluatorTest {
       eval(/*keepGoing=*/false, fastKey);
     }
     final Set<SkyKey> receivedValues = Sets.newConcurrentHashSet();
-    revalidationReceiver = new DirtyTrackingProgressReceiver(
-        new EvaluationProgressReceiver.NullEvaluationProgressReceiver() {
-      @Override
-      public void evaluated(SkyKey skyKey, Supplier<SkyValue> skyValueSupplier,
-          EvaluationState state) {
-        receivedValues.add(skyKey);
-      }
-    });
+    revalidationReceiver =
+        new DirtyTrackingProgressReceiver(
+            new EvaluationProgressReceiver.NullEvaluationProgressReceiver() {
+              @Override
+              public void evaluated(
+                  SkyKey skyKey,
+                  Supplier<EvaluationSuccessState> evaluationSuccessState,
+                  EvaluationState state) {
+                receivedValues.add(skyKey);
+              }
+            });
     TestThread evalThread = new TestThread() {
       @Override
       public void runTest() throws Exception {
@@ -1910,17 +1913,19 @@ public class ParallelEvaluatorTest {
     final Set<SkyKey> evaluatedValues = Sets.newConcurrentHashSet();
     EvaluationProgressReceiver progressReceiver =
         new EvaluationProgressReceiver.NullEvaluationProgressReceiver() {
-      @Override
-      public void enqueueing(SkyKey skyKey) {
-        enqueuedValues.add(skyKey);
-      }
+          @Override
+          public void enqueueing(SkyKey skyKey) {
+            enqueuedValues.add(skyKey);
+          }
 
-      @Override
-      public void evaluated(SkyKey skyKey, Supplier<SkyValue> skyValueSupplier,
-          EvaluationState state) {
-        evaluatedValues.add(skyKey);
-      }
-    };
+          @Override
+          public void evaluated(
+              SkyKey skyKey,
+              Supplier<EvaluationSuccessState> evaluationSuccessState,
+              EvaluationState state) {
+            evaluatedValues.add(skyKey);
+          }
+        };
 
     ExtendedEventHandler reporter =
         new Reporter(
diff --git a/src/test/java/com/google/devtools/build/skyframe/TrackingProgressReceiver.java b/src/test/java/com/google/devtools/build/skyframe/TrackingProgressReceiver.java
index 133add71f0..627460c555 100644
--- a/src/test/java/com/google/devtools/build/skyframe/TrackingProgressReceiver.java
+++ b/src/test/java/com/google/devtools/build/skyframe/TrackingProgressReceiver.java
@@ -14,9 +14,9 @@
 package com.google.devtools.build.skyframe;
 
 import com.google.common.base.Preconditions;
-import com.google.common.base.Supplier;
 import com.google.common.collect.Sets;
 import java.util.Set;
+import java.util.function.Supplier;
 
 /**
  * A testing utility to keep track of evaluation.
@@ -50,9 +50,12 @@ public class TrackingProgressReceiver
   }
 
   @Override
-  public void evaluated(SkyKey skyKey, Supplier<SkyValue> skyValueSupplier, EvaluationState state) {
+  public void evaluated(
+      SkyKey skyKey,
+      Supplier<EvaluationSuccessState> evaluationSuccessState,
+      EvaluationState state) {
     evaluated.add(skyKey);
-    if (skyValueSupplier.get() != null) {
+    if (evaluationSuccessState.get().succeeded()) {
       deleted.remove(skyKey);
       if (state.equals(EvaluationState.CLEAN)) {
         dirty.remove(skyKey);