Move LoadingPhaseCompleteEvent posting to TargetPatternPhaseFunction

Also simplify LoadingPhaseCompleteEvent, and SkyframeExecutor, and remove
LoadingCallback, which is unnecessary now that we only have a single
implementation (previously LoadingPhaseRunner).

This also removes some of the excessive Skyframe calls introduced by
https://github.com/bazelbuild/bazel/commit/1067310e18cb9ac203110726de0be53bdc403cea, and prepares for interleaving target pattern eval and loading.

PiperOrigin-RevId: 205813197

3 files changed, 6 insertions, 50 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index f89fb2403a..378d4a980b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -104,15 +104,10 @@ import com.google.devtools.build.lib.packages.PackageFactory;
 import com.google.devtools.build.lib.packages.RuleClassProvider;
 import com.google.devtools.build.lib.packages.RuleVisibility;
 import com.google.devtools.build.lib.packages.SkylarkSemanticsOptions;
-import com.google.devtools.build.lib.packages.Target;
-import com.google.devtools.build.lib.pkgcache.LoadingCallback;
-import com.google.devtools.build.lib.pkgcache.LoadingFailedException;
 import com.google.devtools.build.lib.pkgcache.LoadingOptions;
-import com.google.devtools.build.lib.pkgcache.LoadingPhaseCompleteEvent;
 import com.google.devtools.build.lib.pkgcache.LoadingResult;
 import com.google.devtools.build.lib.pkgcache.PackageCacheOptions;
 import com.google.devtools.build.lib.pkgcache.PackageManager;
-import com.google.devtools.build.lib.pkgcache.PackageManager.PackageManagerStatistics;
 import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
 import com.google.devtools.build.lib.pkgcache.TargetParsingPhaseTimeEvent;
 import com.google.devtools.build.lib.pkgcache.TargetPatternEvaluator;
@@ -2206,9 +2201,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
       PathFragment relativeWorkingDirectory,
       LoadingOptions options,
       boolean keepGoing,
-      boolean determineTests,
-      @Nullable LoadingCallback callback)
-      throws TargetParsingException, LoadingFailedException, InterruptedException {
+      boolean determineTests)
+      throws TargetParsingException, InterruptedException {
     Stopwatch timer = Stopwatch.createStarted();
     SkyKey key =
         TargetPatternPhaseValue.key(
@@ -2256,19 +2250,9 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
       throw exc;
     }
     long timeMillis = timer.stop().elapsed(TimeUnit.MILLISECONDS);
+    eventHandler.post(new TargetParsingPhaseTimeEvent(timeMillis));
 
     TargetPatternPhaseValue patternParsingValue = evalResult.get(key);
-    eventHandler.post(new TargetParsingPhaseTimeEvent(timeMillis));
-    ImmutableSet<Target> targets = patternParsingValue.getTargets(eventHandler, packageManager);
-    if (callback != null) {
-      callback.notifyTargets(targets);
-    }
-    eventHandler.post(
-        new LoadingPhaseCompleteEvent(
-            targets,
-            patternParsingValue.getRemovedTargets(eventHandler, packageManager),
-            PackageManagerStatistics.ZERO,
-            /*timeInMs=*/ 0));
     return patternParsingValue.toLoadingResult(eventHandler, packageManager);
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java
index 1bcf79ee11..d5fdabb851 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java
@@ -31,6 +31,7 @@ import com.google.devtools.build.lib.packages.TargetUtils;
 import com.google.devtools.build.lib.pkgcache.AbstractRecursivePackageProvider.MissingDepException;
 import com.google.devtools.build.lib.pkgcache.CompileOneDependencyTransformer;
 import com.google.devtools.build.lib.pkgcache.FilteringPolicies;
+import com.google.devtools.build.lib.pkgcache.LoadingPhaseCompleteEvent;
 import com.google.devtools.build.lib.pkgcache.ParsingFailedEvent;
 import com.google.devtools.build.lib.pkgcache.TargetParsingCompleteEvent;
 import com.google.devtools.build.lib.pkgcache.TargetProvider;
@@ -202,7 +203,6 @@ final class TargetPatternPhaseFunction implements SkyFunction {
             testsToRunLabels,
             targets.hasError(),
             expandedTargets.hasError() || workspaceError,
-            removedTargetLabels,
             workspaceName);
 
     env.getListener()
@@ -214,6 +214,8 @@ final class TargetPatternPhaseFunction implements SkyFunction {
                 options.getTargetPatterns(),
                 expandedTargets.getTargets(),
                 failedPatterns));
+    env.getListener()
+        .post(new LoadingPhaseCompleteEvent(result.getTargetLabels(), removedTargetLabels));
     return result;
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseValue.java
index ea7b37a16a..7670dc0588 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseValue.java
@@ -50,10 +50,6 @@ public final class TargetPatternPhaseValue implements SkyValue {
   @Nullable private final ImmutableSet<Label> testsToRunLabels;
   private final boolean hasError;
   private final boolean hasPostExpansionError;
-
-  // This field is only for the purposes of generating the LoadingPhaseCompleteEvent.
-  // TODO(ulfjack): Support EventBus event posting in Skyframe, and remove this code again.
-  private final ImmutableSet<Label> removedTargetLabels;
   private final String workspaceName;
 
   TargetPatternPhaseValue(
@@ -61,13 +57,11 @@ public final class TargetPatternPhaseValue implements SkyValue {
       ImmutableSet<Label> testsToRunLabels,
       boolean hasError,
       boolean hasPostExpansionError,
-      ImmutableSet<Label> removedTargetLabels,
       String workspaceName) {
     this.targetLabels = targetLabels;
     this.testsToRunLabels = testsToRunLabels;
     this.hasError = hasError;
     this.hasPostExpansionError = hasPostExpansionError;
-    this.removedTargetLabels = removedTargetLabels;
     this.workspaceName = workspaceName;
   }
 
@@ -105,28 +99,6 @@ public final class TargetPatternPhaseValue implements SkyValue {
     return hasPostExpansionError;
   }
 
-  /**
-   * Returns a set of targets that were present on the command line but got expanded during the
-   * loading phase (currently these are only test suites; this set is always empty when <code>
-   * --expand_test_suites=false</code>.
-   */
-  public ImmutableSet<Target> getRemovedTargets(
-      ExtendedEventHandler eventHandler, PackageManager packageManager) {
-    return removedTargetLabels
-        .stream()
-        .map(
-            (label) -> {
-              try {
-                return packageManager
-                    .getPackage(eventHandler, label.getPackageIdentifier())
-                    .getTarget(label.getName());
-              } catch (NoSuchPackageException | NoSuchTargetException | InterruptedException e) {
-                throw new RuntimeException(e);
-              }
-            })
-        .collect(ImmutableSet.toImmutableSet());
-  }
-
   public String getWorkspaceName() {
     return workspaceName;
   }
@@ -184,7 +156,6 @@ public final class TargetPatternPhaseValue implements SkyValue {
     TargetPatternPhaseValue that = (TargetPatternPhaseValue) obj;
     return Objects.equals(this.targetLabels, that.targetLabels)
         && Objects.equals(this.testsToRunLabels, that.testsToRunLabels)
-        && Objects.equals(this.removedTargetLabels, that.removedTargetLabels)
         && Objects.equals(this.workspaceName, that.workspaceName)
         && this.hasError == that.hasError
         && this.hasPostExpansionError == that.hasPostExpansionError;
@@ -195,7 +166,6 @@ public final class TargetPatternPhaseValue implements SkyValue {
     return Objects.hash(
         this.targetLabels,
         this.testsToRunLabels,
-        this.removedTargetLabels,
         this.workspaceName,
         this.hasError,
         this.hasPostExpansionError);