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, 16 insertions, 44 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
index 949f658abc..b583a185a4 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
@@ -354,8 +354,7 @@ public abstract class AnalysisTestCase extends FoundationTestCase {
             PathFragment.EMPTY_FRAGMENT,
             loadingOptions,
             keepGoing,
-            /*determineTests=*/ false,
-            /*callback=*/ null);
+            /*determineTests=*/ false);
 
     BuildRequestOptions requestOptions = optionsParser.getOptions(BuildRequestOptions.class);
     ImmutableSortedSet<String> multiCpu = ImmutableSortedSet.copyOf(requestOptions.multiCpus);
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 a26174a992..82f041c1e7 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
@@ -1736,8 +1736,7 @@ public abstract class BuildViewTestCase extends FoundationTestCase {
             PathFragment.EMPTY_FRAGMENT,
             loadingOptions,
             keepGoing,
-            /*determineTests=*/ false,
-            /*callback=*/ null);
+            /*determineTests=*/ false);
     if (!doAnalysis) {
       // TODO(bazel-team): What's supposed to happen in this case?
       return null;
diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
index 8bd7de7147..23d5ca181e 100644
--- a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
@@ -127,21 +127,6 @@ public class LoadingPhaseRunnerTest {
   }
 
   @Test
-  public void testSmokeWithCallback() throws Exception {
-    tester.addFile("base/BUILD",
-        "filegroup(name = 'hello', srcs = ['foo.txt'])");
-    final List<Target> targetsNotified = new ArrayList<>();
-    tester.setCallback(new LoadingCallback() {
-      @Override
-      public void notifyTargets(Collection<Target> targets) throws LoadingFailedException {
-        targetsNotified.addAll(targets);
-      }
-    });
-    assertNoErrors(tester.load("//base:hello"));
-    assertThat(targetsNotified).containsExactlyElementsIn(getTargets("//base:hello"));
-  }
-
-  @Test
   public void testNonExistentPackage() throws Exception {
     LoadingResult loadingResult = tester.loadKeepGoing("//base:missing");
     assertThat(loadingResult.hasTargetPatternError()).isTrue();
@@ -391,7 +376,7 @@ public class LoadingPhaseRunnerTest {
     assertThinTargetsEqualToTargets(
         tester.getOriginalTargets(), getTargets("//cc:tests", "//cc:my_test"));
     assertThat(tester.getTestSuiteTargets())
-        .containsExactlyElementsIn(getTargets("//cc:tests"));
+        .containsExactly(Label.parseAbsoluteUnchecked("//cc:tests"));
   }
 
   @Test
@@ -724,7 +709,6 @@ public class LoadingPhaseRunnerTest {
 
     private LoadingOptions options;
     private final StoredEventHandler storedErrors;
-    private LoadingCallback loadingCallback;
 
     private TargetParsingCompleteEvent targetParsingCompleteEvent;
     private LoadingPhaseCompleteEvent loadingPhaseCompleteEvent;
@@ -800,10 +784,6 @@ public class LoadingPhaseRunnerTest {
       this.options = Options.getDefaults(LoadingOptions.class);
     }
 
-    public void setCallback(LoadingCallback loadingCallback) {
-      this.loadingCallback = loadingCallback;
-    }
-
     public void useLoadingOptions(String... options) throws OptionsParsingException {
       OptionsParser parser = OptionsParser.newOptionsParser(LoadingOptions.class);
       parser.parse(ImmutableList.copyOf(options));
@@ -831,22 +811,16 @@ public class LoadingPhaseRunnerTest {
       sync();
       storedErrors.clear();
       LoadingResult result;
-      try {
-        result =
-            skyframeExecutor.loadTargetPatterns(
-                storedErrors,
-                ImmutableList.copyOf(patterns),
-                PathFragment.EMPTY_FRAGMENT,
-                options,
-                keepGoing,
-                determineTests,
-                loadingCallback);
-        this.targetParsingCompleteEvent = findPost(TargetParsingCompleteEvent.class);
-        this.loadingPhaseCompleteEvent = findPost(LoadingPhaseCompleteEvent.class);
-      } catch (LoadingFailedException e) {
-        System.err.println(storedErrors.getEvents());
-        throw e;
-      }
+      result =
+          skyframeExecutor.loadTargetPatterns(
+              storedErrors,
+              ImmutableList.copyOf(patterns),
+              PathFragment.EMPTY_FRAGMENT,
+              options,
+              keepGoing,
+              determineTests);
+      this.targetParsingCompleteEvent = findPost(TargetParsingCompleteEvent.class);
+      this.loadingPhaseCompleteEvent = findPost(LoadingPhaseCompleteEvent.class);
       if (!keepGoing) {
         assertThat(storedErrors.hasErrors()).isFalse();
       }
@@ -869,7 +843,7 @@ public class LoadingPhaseRunnerTest {
         currentPath = currentPath.getParentDirectory();
       }
 
-      FileSystemUtils.createDirectoryAndParents(buildFile.getParentDirectory());
+      buildFile.getParentDirectory().createDirectoryAndParents();
       FileSystemUtils.writeContentAsLatin1(buildFile, Joiner.on('\n').join(content));
     }
 
@@ -925,8 +899,8 @@ public class LoadingPhaseRunnerTest {
       return targetParsingCompleteEvent.getTargets();
     }
 
-    public ImmutableSet<Target> getTestSuiteTargets() {
-      return loadingPhaseCompleteEvent.getFilteredTargets();
+    public ImmutableSet<Label> getTestSuiteTargets() {
+      return loadingPhaseCompleteEvent.getFilteredLabels();
     }
 
     private Iterable<Event> filteredEvents() {