Fix build results for aspect builds.

The current output was pretty much completely incorrect. However since the result output was always hidden for the default value of --show_result, users simply didn't see the incorrect output (instead getting no output at all).

This CL fixes both the --show_result problem and makes the output correct.

RELNOTES: Print correct build result for builds with --aspects flag.
PiperOrigin-RevId: 191456352

9 files changed, 187 insertions, 111 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResult.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResult.java
index b8dedd1973..5ca0204cf6 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResult.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResult.java
@@ -19,6 +19,7 @@ import com.google.common.base.MoreObjects;
 import com.google.common.base.Preconditions;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
+import com.google.devtools.build.lib.skyframe.AspectValue;
 import com.google.devtools.build.lib.util.ExitCode;
 import java.util.Collection;
 import java.util.Collections;
@@ -42,6 +43,7 @@ public final class BuildResult {
   private Collection<ConfiguredTarget> testTargets;
   private Collection<ConfiguredTarget> successfulTargets;
   private Collection<ConfiguredTarget> skippedTargets;
+  private Collection<AspectValue> successfulAspects;
 
   public BuildResult(long startTimeMillis) {
     this.startTimeMillis = startTimeMillis;
@@ -194,20 +196,36 @@ public final class BuildResult {
     this.successfulTargets = successfulTargets;
   }
 
+  /** @see #getSuccessfulAspects */
+  void setSuccessfulAspects(Collection<AspectValue> successfulAspects) {
+    this.successfulAspects = successfulAspects;
+  }
+
   /**
-   * Returns the set of targets which successfully built.  This value
-   * is set at the end of the build, after the target patterns have been parsed
-   * and resolved and after attempting to build the targets.  If --keep_going
-   * is specified, this set may exclude targets that could not be found or
-   * successfully analyzed, or could not be built.  It may be examined after
-   * the build.  May be null if the execution phase was not attempted, as
-   * may happen if there are errors in the loading phase, for example.
+   * Returns the set of targets that were successfully built. This value is set at the end of the
+   * build, after the target patterns have been parsed and resolved and after attempting to build
+   * the targets. If --keep_going is specified, this set may exclude targets that could not be found
+   * or successfully analyzed, or could not be built. It may be examined after the build. May be
+   * null if the execution phase was not attempted, as may happen if there are errors in the loading
+   * phase, for example.
    */
   public Collection<ConfiguredTarget> getSuccessfulTargets() {
     return successfulTargets;
   }
 
   /**
+   * Returns the set of aspects that were successfully built. This value is set at the end of the
+   * build, after the target patterns have been parsed and resolved and after attempting to build
+   * the targets. If --keep_going is specified, this set may exclude targets that could not be found
+   * or successfully analyzed, or could not be built. It may be examined after the build. May be
+   * null if the execution phase was not attempted, as may happen if there are errors in the loading
+   * phase, for example.
+   */
+  public Collection<AspectValue> getSuccessfulAspects() {
+    return successfulAspects;
+  }
+
+  /**
    * See {@link #getSkippedTargets()}.
    */
   void setSkippedTargets(Collection<ConfiguredTarget> skippedTargets) {
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java
index d0b8b0c11c..005b1858a3 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java
@@ -66,92 +66,111 @@ class BuildResultPrinter {
     // problem where the summary message and the exit code disagree.  The logic
     // here is already complex.
 
+    OutErr outErr = request.getOutErr();
     Collection<ConfiguredTarget> targetsToPrint = filterTargetsToPrint(configuredTargets);
     Collection<AspectValue> aspectsToPrint = filterAspectsToPrint(aspects);
-
-    // Filter the targets we care about into two buckets:
-    Collection<ConfiguredTarget> succeeded = new ArrayList<>();
-    Collection<ConfiguredTarget> failed = new ArrayList<>();
-    for (ConfiguredTarget target : targetsToPrint) {
+    final boolean success;
+    if (aspectsToPrint.isEmpty()) {
+      // Suppress summary if --show_result value is exceeded:
+      if (targetsToPrint.size() > request.getBuildOptions().maxResultTargets) {
+        return;
+      }
+      // Filter the targets we care about into two buckets:
+      Collection<ConfiguredTarget> succeeded = new ArrayList<>();
+      Collection<ConfiguredTarget> failed = new ArrayList<>();
       Collection<ConfiguredTarget> successfulTargets = result.getSuccessfulTargets();
-      (successfulTargets.contains(target) ? succeeded : failed).add(target);
-    }
-
-    // TODO(bazel-team): convert these to a new "SKIPPED" status when ready: b/62191890.
-    failed.addAll(configuredTargetsToSkip);
-
-    // Suppress summary if --show_result value is exceeded:
-    if (succeeded.size() + failed.size() + aspectsToPrint.size()
-        > request.getBuildOptions().maxResultTargets) {
-      return;
-    }
+      for (ConfiguredTarget target : targetsToPrint) {
+        (successfulTargets.contains(target) ? succeeded : failed).add(target);
+      }
 
-    OutErr outErr = request.getOutErr();
+      // TODO(bazel-team): convert these to a new "SKIPPED" status when ready: b/62191890.
+      failed.addAll(configuredTargetsToSkip);
 
-    TopLevelArtifactContext context = request.getTopLevelArtifactContext();
-    for (ConfiguredTarget target : succeeded) {
-      Label label = target.getLabel();
-      // For up-to-date targets report generated artifacts, but only
-      // if they have associated action and not middleman artifacts.
-      boolean headerFlag = true;
-      for (Artifact artifact :
-          TopLevelArtifactHelper.getAllArtifactsToBuild(target, context).getImportantArtifacts()) {
-        if (shouldPrint(artifact)) {
-          if (headerFlag) {
-            outErr.printErr("Target " + label + " up-to-date:\n");
-            headerFlag = false;
+      TopLevelArtifactContext context = request.getTopLevelArtifactContext();
+      for (ConfiguredTarget target : succeeded) {
+        Label label = target.getLabel();
+        // For up-to-date targets report generated artifacts, but only
+        // if they have associated action and not middleman artifacts.
+        boolean headerFlag = true;
+        for (Artifact artifact :
+            TopLevelArtifactHelper.getAllArtifactsToBuild(target, context)
+                .getImportantArtifacts()) {
+          if (shouldPrint(artifact)) {
+            if (headerFlag) {
+              outErr.printErr("Target " + label + " up-to-date:\n");
+              headerFlag = false;
+            }
+            outErr.printErrLn(formatArtifactForShowResults(artifact, request));
           }
-          outErr.printErrLn(formatArtifactForShowResults(artifact, request));
         }
-      }
-      if (headerFlag) {
-        outErr.printErr("Target " + label + " up-to-date (nothing to build)\n");
-      }
-    }
-
-    for (AspectValue aspect : aspectsToPrint) {
-      Label label = aspect.getLabel();
-      String aspectName = aspect.getConfiguredAspect().getName();
-      boolean headerFlag = true;
-      NestedSet<Artifact> importantArtifacts =
-          TopLevelArtifactHelper.getAllArtifactsToBuild(aspect, context).getImportantArtifacts();
-      for (Artifact importantArtifact : importantArtifacts) {
         if (headerFlag) {
-          outErr.printErr("Aspect " + aspectName + " of " + label + " up-to-date:\n");
-          headerFlag = false;
+          outErr.printErr("Target " + label + " up-to-date (nothing to build)\n");
         }
-        if (shouldPrint(importantArtifact)) {
-          outErr.printErrLn(formatArtifactForShowResults(importantArtifact, request));
+      }
+      for (ConfiguredTarget target : failed) {
+        outErr.printErr("Target " + target.getLabel() + " failed to build\n");
+
+        // For failed compilation, it is still useful to examine temp artifacts,
+        // (ie, preprocessed and assembler files).
+        OutputGroupInfo topLevelProvider = OutputGroupInfo.get(target);
+        String productName = env.getRuntime().getProductName();
+        if (topLevelProvider != null) {
+          for (Artifact temp : topLevelProvider.getOutputGroup(OutputGroupInfo.TEMP_FILES)) {
+            if (temp.getPath().exists()) {
+              outErr.printErrLn(
+                  "  See temp at "
+                      + OutputDirectoryLinksUtils.getPrettyPath(
+                          temp.getPath(),
+                          env.getWorkspaceName(),
+                          env.getWorkspace(),
+                          request.getBuildOptions().getSymlinkPrefix(productName),
+                          productName));
+            }
+          }
         }
       }
-      if (headerFlag) {
-        outErr.printErr(
-            "Aspect " + aspectName + " of " + label + " up-to-date (nothing to build)\n");
+      success = failed.isEmpty();
+    } else {
+      // Suppress summary if --show_result value is exceeded:
+      if (aspectsToPrint.size() > request.getBuildOptions().maxResultTargets) {
+        return;
       }
-    }
-
-    for (ConfiguredTarget target : failed) {
-      outErr.printErr("Target " + target.getLabel() + " failed to build\n");
-
-      // For failed compilation, it is still useful to examine temp artifacts,
-      // (ie, preprocessed and assembler files).
-      OutputGroupInfo topLevelProvider =
-          OutputGroupInfo.get(target);
-      String productName = env.getRuntime().getProductName();
-      if (topLevelProvider != null) {
-        for (Artifact temp : topLevelProvider.getOutputGroup(OutputGroupInfo.TEMP_FILES)) {
-          if (temp.getPath().exists()) {
-            outErr.printErrLn("  See temp at "
-                + OutputDirectoryLinksUtils.getPrettyPath(temp.getPath(),
-                    env.getWorkspaceName(),
-                    env.getWorkspace(),
-                    request.getBuildOptions().getSymlinkPrefix(productName),
-                    productName));
+      // Filter the targets we care about into two buckets:
+      Collection<AspectValue> succeeded = new ArrayList<>();
+      Collection<AspectValue> failed = new ArrayList<>();
+      Collection<AspectValue> successfulAspects = result.getSuccessfulAspects();
+      for (AspectValue aspect : aspectsToPrint) {
+        (successfulAspects.contains(aspect) ? succeeded : failed).add(aspect);
+      }
+      TopLevelArtifactContext context = request.getTopLevelArtifactContext();
+      for (AspectValue aspect : succeeded) {
+        Label label = aspect.getLabel();
+        String aspectName = aspect.getConfiguredAspect().getName();
+        boolean headerFlag = true;
+        NestedSet<Artifact> importantArtifacts =
+            TopLevelArtifactHelper.getAllArtifactsToBuild(aspect, context).getImportantArtifacts();
+        for (Artifact importantArtifact : importantArtifacts) {
+          if (headerFlag) {
+            outErr.printErr("Aspect " + aspectName + " of " + label + " up-to-date:\n");
+            headerFlag = false;
+          }
+          if (shouldPrint(importantArtifact)) {
+            outErr.printErrLn(formatArtifactForShowResults(importantArtifact, request));
           }
         }
+        if (headerFlag) {
+          outErr.printErr(
+              "Aspect " + aspectName + " of " + label + " up-to-date (nothing to build)\n");
+        }
+      }
+      for (AspectValue aspect : failed) {
+        Label label = aspect.getLabel();
+        String aspectName = aspect.getConfiguredAspect().getName();
+        outErr.printErr("Aspect " + aspectName + " of " + label + " failed to build\n");
       }
+      success = failed.isEmpty();
     }
-    if (!failed.isEmpty() && !request.getOptions(ExecutionOptions.class).verboseFailures) {
+    if (!success && !request.getOptions(ExecutionOptions.class).verboseFailures) {
       outErr.printErr("Use --verbose_failures to see the command lines of failed build steps.\n");
     }
   }
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java
index ef4319573a..1becb5bc93 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java
@@ -22,6 +22,9 @@ import com.google.devtools.build.lib.actions.ActionLookupData;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.clock.BlazeClock;
 import com.google.devtools.build.lib.skyframe.ActionExecutionInactivityWatchdog;
+import com.google.devtools.build.lib.skyframe.AspectCompletionValue;
+import com.google.devtools.build.lib.skyframe.AspectCompletionValue.AspectCompletionKey;
+import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
 import com.google.devtools.build.lib.skyframe.SkyFunctions;
 import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor;
 import com.google.devtools.build.lib.skyframe.TargetCompletionValue;
@@ -47,6 +50,7 @@ public final class ExecutionProgressReceiver
 
   // Must be thread-safe!
   private final Set<ConfiguredTarget> builtTargets;
+  private final Set<AspectKey> builtAspects;
   private final Set<ActionLookupData> enqueuedActions = Sets.newConcurrentHashSet();
   private final Set<ActionLookupData> completedActions = Sets.newConcurrentHashSet();
   private final Set<ActionLookupData> ignoredActions = Sets.newConcurrentHashSet();
@@ -65,9 +69,9 @@ public final class ExecutionProgressReceiver
    * permitted while this receiver is active.
    */
   ExecutionProgressReceiver(
-      Set<ConfiguredTarget> builtTargets,
-      int exclusiveTestsCount) {
+      Set<ConfiguredTarget> builtTargets, Set<AspectKey> builtAspects, int exclusiveTestsCount) {
     this.builtTargets = Collections.synchronizedSet(builtTargets);
+    this.builtAspects = Collections.synchronizedSet(builtAspects);
     this.exclusiveTestsCount = exclusiveTestsCount;
   }
 
@@ -103,9 +107,15 @@ public final class ExecutionProgressReceiver
       if (value == null) {
         return;
       }
-
       ConfiguredTarget target = value.getConfiguredTarget();
       builtTargets.add(target);
+    } else if (type.equals(SkyFunctions.ASPECT_COMPLETION)) {
+      AspectCompletionValue value = (AspectCompletionValue) skyValueSupplier.get();
+      if (value == null) {
+        return;
+      }
+      AspectKey aspectKey = ((AspectCompletionKey) skyKey).aspectKey();
+      builtAspects.add(aspectKey);
     } else if (type.equals(SkyFunctions.ACTION_EXECUTION)) {
       // Remember all completed actions, even those in error, regardless of having been cached or
       // really executed.
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
index 3323f58f26..6971aaca39 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
@@ -14,7 +14,6 @@
 package com.google.devtools.build.lib.buildtool;
 
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
-import static java.util.concurrent.TimeUnit.MILLISECONDS;
 
 import com.google.common.base.Preconditions;
 import com.google.common.base.Predicate;
@@ -75,6 +74,7 @@ import com.google.devtools.build.lib.runtime.BlazeModule;
 import com.google.devtools.build.lib.runtime.BlazeRuntime;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
 import com.google.devtools.build.lib.skyframe.AspectValue;
+import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
 import com.google.devtools.build.lib.skyframe.Builder;
 import com.google.devtools.build.lib.skyframe.OutputService;
 import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
@@ -295,6 +295,7 @@ public class ExecutionTool {
                                   request.getOptionsDescription());
 
     Set<ConfiguredTarget> builtTargets = new HashSet<>();
+    Set<AspectKey> builtAspects = new HashSet<>();
     Collection<AspectValue> aspects = analysisResult.getAspects();
 
     Iterable<Artifact> allArtifactsForProviders =
@@ -345,6 +346,7 @@ public class ExecutionTool {
           analysisResult.getAspects(),
           executor,
           builtTargets,
+          builtAspects,
           request.getBuildOptions().explanationPath != null,
           env.getBlazeWorkspace().getLastExecutionTimeRange(),
           topLevelArtifactContext);
@@ -377,9 +379,13 @@ public class ExecutionTool {
         saveActionCache(actionCache);
       }
 
+      env.getEventBus()
+          .post(new ExecutionPhaseCompleteEvent(timer.stop().elapsed(TimeUnit.MILLISECONDS)));
+
       try (AutoProfiler p = AutoProfiler.profiled("Show results", ProfilerTask.INFO)) {
         buildResult.setSuccessfulTargets(
-            determineSuccessfulTargets(configuredTargets, builtTargets, timer));
+            determineSuccessfulTargets(configuredTargets, builtTargets));
+        buildResult.setSuccessfulAspects(determineSuccessfulAspects(aspects, builtAspects));
         buildResult.setSkippedTargets(analysisResult.getTargetsToSkip());
         BuildResultPrinter buildResultPrinter = new BuildResultPrinter(env);
         buildResultPrinter.showBuildResult(request, buildResult, configuredTargets,
@@ -529,16 +535,13 @@ public class ExecutionTool {
   }
 
   /**
-   * Computes the result of the build. Sets the list of successful (up-to-date)
-   * targets in the request object.
+   * Computes the result of the build. Sets the list of successful (up-to-date) targets in the
+   * request object.
    *
-   * @param configuredTargets The configured targets whose artifacts are to be
-   *                          built.
-   * @param timer A timer that was started when the execution phase started.
+   * @param configuredTargets The configured targets whose artifacts are to be built.
    */
   private Collection<ConfiguredTarget> determineSuccessfulTargets(
-      Collection<ConfiguredTarget> configuredTargets, Set<ConfiguredTarget> builtTargets,
-      Stopwatch timer) {
+      Collection<ConfiguredTarget> configuredTargets, Set<ConfiguredTarget> builtTargets) {
     // Maintain the ordering by copying builtTargets into a LinkedHashSet in the same iteration
     // order as configuredTargets.
     Collection<ConfiguredTarget> successfulTargets = new LinkedHashSet<>();
@@ -547,11 +550,22 @@ public class ExecutionTool {
         successfulTargets.add(target);
       }
     }
-    env.getEventBus().post(
-        new ExecutionPhaseCompleteEvent(timer.stop().elapsed(MILLISECONDS)));
     return successfulTargets;
   }
 
+  private Collection<AspectValue> determineSuccessfulAspects(
+      Collection<AspectValue> aspects, Set<AspectKey> builtAspects) {
+    // Maintain the ordering by copying builtTargets into a LinkedHashSet in the same iteration
+    // order as configuredTargets.
+    Collection<AspectValue> successfulAspects = new LinkedHashSet<>();
+    for (AspectValue aspect : aspects) {
+      if (builtAspects.contains(aspect.getKey())) {
+        successfulAspects.add(aspect);
+      }
+    }
+    return successfulAspects;
+  }
+
   /** Get action cache if present or reload it from the on-disk cache. */
   private ActionCache getActionCache() throws LocalEnvironmentException {
     try {
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java b/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java
index e72eda9ed0..f557e62ffe 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java
@@ -39,6 +39,7 @@ import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.packages.BuildFileNotFoundException;
 import com.google.devtools.build.lib.skyframe.ActionExecutionInactivityWatchdog;
 import com.google.devtools.build.lib.skyframe.AspectValue;
+import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
 import com.google.devtools.build.lib.skyframe.Builder;
 import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
 import com.google.devtools.build.lib.util.AbruptExitException;
@@ -103,6 +104,7 @@ public class SkyframeBuilder implements Builder {
       Collection<AspectValue> aspects,
       Executor executor,
       Set<ConfiguredTarget> builtTargets,
+      Set<AspectKey> builtAspects,
       boolean explain,
       @Nullable Range<Long> lastExecutionTimeRange,
       TopLevelArtifactContext topLevelArtifactContext)
@@ -114,6 +116,7 @@ public class SkyframeBuilder implements Builder {
     ExecutionProgressReceiver executionProgressReceiver =
         new ExecutionProgressReceiver(
             Preconditions.checkNotNull(builtTargets),
+            Preconditions.checkNotNull(builtAspects),
             countTestActions(exclusiveTests));
     skyframeExecutor
         .getEventBus()
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletionValue.java
index 7b33d0bfe4..64066cf948 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletionValue.java
@@ -37,8 +37,9 @@ public class AspectCompletionValue implements SkyValue {
         targets, aspectValue -> AspectCompletionKey.create(aspectValue.getKey(), ctx));
   }
 
+  /** The key of an AspectCompletionValue. */
   @AutoValue
-  abstract static class AspectCompletionKey implements SkyKey {
+  public abstract static class AspectCompletionKey implements SkyKey {
     public static AspectCompletionKey create(
         AspectKey aspectKey, TopLevelArtifactContext topLevelArtifactContext) {
       return new AutoValue_AspectCompletionValue_AspectCompletionKey(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/Builder.java b/src/main/java/com/google/devtools/build/lib/skyframe/Builder.java
index 1b88199614..5ce1016200 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/Builder.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/Builder.java
@@ -23,6 +23,7 @@ import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
 import com.google.devtools.build.lib.events.Reporter;
+import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
 import com.google.devtools.build.lib.util.AbruptExitException;
 import java.util.Collection;
 import java.util.Set;
@@ -43,33 +44,35 @@ public interface Builder {
 
   /**
    * Transitively build all given artifacts, targets, and tests, and all necessary prerequisites
-   * thereof. For sequential implementations of this interface, the top-level requests will be
-   * built in the iteration order of the Set provided; for concurrent implementations, the order
-   * is undefined.
+   * thereof. For sequential implementations of this interface, the top-level requests will be built
+   * in the iteration order of the Set provided; for concurrent implementations, the order is
+   * undefined.
    *
    * <p>This method should not be invoked more than once concurrently on the same Builder instance.
    *
    * @param artifacts the set of Artifacts to build
    * @param parallelTests tests to execute in parallel with the other top-level targetsToBuild and
-   *        artifacts.
+   *     artifacts.
    * @param exclusiveTests are executed one at a time, only after all other tasks have completed
    * @param targetsToBuild Set of targets which will be built
    * @param targetsToSkip Set of targets which should be skipped (they still show up in build
-   *        results, but with a "SKIPPED" status and without the cost of any actual build work)
+   *     results, but with a "SKIPPED" status and without the cost of any actual build work)
    * @param aspects Set of aspects that will be built
-   * @param executor an opaque application-specific value that will be
-   *        passed down to the execute() method of any Action executed during
-   *        this call
+   * @param executor an opaque application-specific value that will be passed down to the execute()
+   *     method of any Action executed during this call
    * @param builtTargets (out) set of successfully built subset of targetsToBuild. This set is
-   *        populated immediately upon confirmation that artifact is built so it will be
-   *        valid even if a future action throws ActionExecutionException
-   * @param lastExecutionTimeRange If not null, the start/finish time of the last build that
-   *        run the execution phase.
-   * @param topLevelArtifactContext contains the options which determine the artifacts to build
-   *        for the top-level targets.
+   *     populated immediately upon confirmation that artifact is built so it will be valid even if
+   *     a future action throws ActionExecutionException
+   * @param builtAspects (out) set of successfully built subset of targetsToBuild with the passed
+   *     aspects applied. This set is populated immediately upon confirmation that artifact is built
+   *     so it will be valid even if a future action throws ActionExecutionException
+   * @param lastExecutionTimeRange If not null, the start/finish time of the last build that run the
+   *     execution phase.
+   * @param topLevelArtifactContext contains the options which determine the artifacts to build for
+   *     the top-level targets.
    * @throws BuildFailedException if there were problems establishing the action execution
-   *         environment, if the metadata of any file  during the build could not be obtained,
-   *         if any input files are missing, or if an action fails during execution
+   *     environment, if the metadata of any file during the build could not be obtained, if any
+   *     input files are missing, or if an action fails during execution
    * @throws InterruptedException if there was an asynchronous stop request
    * @throws TestExecException if any test fails
    */
@@ -84,6 +87,7 @@ public interface Builder {
       Collection<AspectValue> aspects,
       Executor executor,
       Set<ConfiguredTarget> builtTargets,
+      Set<AspectKey> builtAspects,
       boolean explain,
       @Nullable Range<Long> lastExecutionTimeRange,
       TopLevelArtifactContext topLevelArtifactContext)
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 0a83696c54..5332637254 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
@@ -407,6 +407,7 @@ public class SkyframeAwareActionTest extends TimestampBuilderTestCase {
         null,
         executor,
         null,
+        null,
         false,
         null,
         null);
@@ -436,6 +437,7 @@ public class SkyframeAwareActionTest extends TimestampBuilderTestCase {
         null,
         executor,
         null,
+        null,
         false,
         null,
         null);
@@ -816,6 +818,7 @@ public class SkyframeAwareActionTest extends TimestampBuilderTestCase {
         null,
         executor,
         null,
+        null,
         false,
         null,
         null);
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
index 94ab81df76..b327612ad6 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
@@ -64,6 +64,7 @@ import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.events.StoredEventHandler;
 import com.google.devtools.build.lib.exec.SingleBuildFileCache;
 import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
+import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
 import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction;
 import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy;
 import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ActionCompletedReceiver;
@@ -259,6 +260,7 @@ public abstract class TimestampBuilderTestCase extends FoundationTestCase {
           Collection<AspectValue> aspects,
           Executor executor,
           Set<ConfiguredTarget> builtTargets,
+          Set<AspectKey> builtAspects,
           boolean explain,
           Range<Long> lastExecutionTimeRange,
           TopLevelArtifactContext topLevelArtifactContext)
@@ -400,7 +402,8 @@ public abstract class TimestampBuilderTestCase extends FoundationTestCase {
 
     tsgm.setCommandStartTime();
     Set<Artifact> artifactsToBuild = Sets.newHashSet(artifacts);
-    Set<ConfiguredTarget> builtArtifacts = new HashSet<>();
+    Set<ConfiguredTarget> builtTargets = new HashSet<>();
+    Set<AspectKey> builtAspects = new HashSet<>();
     try {
       builder.buildArtifacts(
           reporter,
@@ -411,8 +414,9 @@ public abstract class TimestampBuilderTestCase extends FoundationTestCase {
           null,
           null,
           executor,
-          builtArtifacts, /*explain=*/
-          false,
+          builtTargets,
+          builtAspects,
+          false /*explain*/,
           null,
           null);
     } finally {