Fix callback handling in the Skyframe-based target pattern eval.

Add a unit test - there is test coverage for this in RunCommandTest, but this
runs much faster.

--
MOS_MIGRATED_REVID=114302062

2 files changed, 29 insertions, 1 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 291730adba..12c38a8cba 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
@@ -1789,6 +1789,9 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
       eventBus.post(new TargetParsingCompleteEvent(patternParsingValue.getTargets(),
           patternParsingValue.getFilteredTargets(), patternParsingValue.getTestFilteredTargets(),
           time));
+      if (callback != null) {
+        callback.notifyTargets(patternParsingValue.getTargets());
+      }
       eventBus.post(new LoadingPhaseCompleteEvent(
           /*was expandedTargetsToLoad*/patternParsingValue.getTargets(),
           // TODO(ulfjack): Should be: Sets.difference(originalTargetsToLoad, expandedTargetsToLoad)
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 d96507ae18..e56117e454 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
@@ -72,6 +72,7 @@ import org.junit.runners.JUnit4;
 import java.io.IOException;
 import java.util.AbstractMap;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -123,6 +124,25 @@ 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);
+      }
+
+      @Override
+      public void notifyVisitedPackages(Set<PackageIdentifier> visitedPackages) {
+      }
+    });
+    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();
@@ -562,6 +582,7 @@ public class LoadingPhaseRunnerTest {
 
     private LoadingOptions options;
     private final StoredEventHandler storedErrors;
+    private LoadingCallback loadingCallback;
 
     private Set<Target> filteredTargets;
     private Set<Target> testFilteredTargets;
@@ -604,6 +625,10 @@ 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));
@@ -637,7 +662,7 @@ public class LoadingPhaseRunnerTest {
         eventBus.register(listener);
         result = loadingPhaseRunner.execute(storedErrors, eventBus,
             ImmutableList.copyOf(patterns), options, ImmutableListMultimap.<String, Label>of(),
-            keepGoing, /*enableLoading=*/true, determineTests, /*callback=*/null);
+            keepGoing, /*enableLoading=*/true, determineTests, loadingCallback);
         this.filteredTargets = listener.filteredTargets;
         this.testFilteredTargets = listener.testFilteredTargets;
       } catch (LoadingFailedException e) {