Move ParsingFailureEvent reporting to TargetPatternFunction

We're parsing the target pattern before we create the SkyKey, so all callers of
the key creation also need to report this event.

PiperOrigin-RevId: 162326973

4 files changed, 19 insertions, 6 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunction.java
index 2ae25ccd07..422462e723 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunction.java
@@ -48,6 +48,11 @@ public class PrepareDepsOfPatternsFunction implements SkyFunction {
       try {
         skyKeyBuilder.add(skyKeyOrException.getSkyKey());
       } catch (TargetParsingException e) {
+        // We post an event here rather than in handleTargetParsingException because the
+        // TargetPatternFunction already posts an event unless the pattern cannot be parsed, in
+        // which case the caller (i.e., us) needs to post an event.
+        eventHandler.post(
+            new ParsingFailedEvent(skyKeyOrException.getOriginalPattern(), e.getMessage()));
         handleTargetParsingException(eventHandler, skyKeyOrException.getOriginalPattern(), e);
       }
     }
@@ -112,7 +117,6 @@ public class PrepareDepsOfPatternsFunction implements SkyFunction {
       ExtendedEventHandler eventHandler, String rawPattern, TargetParsingException e) {
     String errorMessage = e.getMessage();
     eventHandler.handle(Event.error("Skipping '" + rawPattern + "': " + errorMessage));
-    eventHandler.post(new ParsingFailedEvent(rawPattern, errorMessage));
   }
 
   @Nullable
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeTargetPatternEvaluator.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeTargetPatternEvaluator.java
index dccc1eff74..a0ccc8894c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeTargetPatternEvaluator.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeTargetPatternEvaluator.java
@@ -110,12 +110,16 @@ final class SkyframeTargetPatternEvaluator implements TargetPatternEvaluator {
       try {
         builder.add(skyKeyOrException.getSkyKey());
       } catch (TargetParsingException e) {
+        // We report a parsing failed exception to the event bus here in case the pattern did not
+        // successfully parse (which happens before the SkyKey is created). Otherwise the
+        // TargetPatternFunction posts the event.
+        eventHandler.post(
+            new ParsingFailedEvent(skyKeyOrException.getOriginalPattern(),  e.getMessage()));
         if (!keepGoing) {
           throw e;
         }
         String pattern = skyKeyOrException.getOriginalPattern();
         eventHandler.handle(Event.error("Skipping '" + pattern + "': " + e.getMessage()));
-        eventHandler.post(new ParsingFailedEvent(pattern, e.getMessage()));
       }
     }
     ImmutableList<SkyKey> skyKeys = builder.build();
@@ -185,8 +189,6 @@ final class SkyframeTargetPatternEvaluator implements TargetPatternEvaluator {
           eventHandler.post(PatternExpandingError.skipped(rawPattern, errorMessage));
         }
         finalTargetSetEvaluator.setError();
-
-        eventHandler.post(new ParsingFailedEvent(rawPattern,  errorMessage));
       }
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java
index 3ef3b1184b..62b537c146 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java
@@ -23,6 +23,7 @@ import com.google.devtools.build.lib.cmdline.TargetPattern;
 import com.google.devtools.build.lib.collect.CompactHashSet;
 import com.google.devtools.build.lib.concurrent.MultisetSemaphore;
 import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.pkgcache.ParsingFailedEvent;
 import com.google.devtools.build.lib.skyframe.EnvironmentBackedRecursivePackageProvider.MissingDepException;
 import com.google.devtools.build.lib.util.BatchCallback;
 import com.google.devtools.build.lib.util.Preconditions;
@@ -71,6 +72,7 @@ public class TargetPatternFunction implements SkyFunction {
       parsedPattern.eval(resolver, excludedSubdirectories, callback, RuntimeException.class);
       resolvedTargets = ResolvedTargets.<Target>builder().addAll(results).build();
     } catch (TargetParsingException e) {
+      env.getListener().post(new ParsingFailedEvent(patternKey.getPattern(),  e.getMessage()));
       throw new TargetPatternFunctionException(e);
     } catch (MissingDepException e) {
       // The EnvironmentBackedRecursivePackageProvider constructed above might throw
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 444fd4258a..967883c698 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
@@ -26,6 +26,7 @@ import com.google.devtools.build.lib.packages.TargetUtils;
 import com.google.devtools.build.lib.pkgcache.CompileOneDependencyTransformer;
 import com.google.devtools.build.lib.pkgcache.FilteringPolicies;
 import com.google.devtools.build.lib.pkgcache.LoadingPhaseRunner;
+import com.google.devtools.build.lib.pkgcache.ParsingFailedEvent;
 import com.google.devtools.build.lib.pkgcache.TargetProvider;
 import com.google.devtools.build.lib.pkgcache.TestFilter;
 import com.google.devtools.build.lib.skyframe.EnvironmentBackedRecursivePackageProvider.MissingDepException;
@@ -198,7 +199,12 @@ final class TargetPatternPhaseFunction implements SkyFunction {
       try {
         patternSkyKeys.add(keyOrException.getSkyKey());
       } catch (TargetParsingException e) {
-        // Skip.
+        // We generally skip patterns that don't parse. We report a parsing failed exception to the
+        // event bus here, but not in determineTests below, which goes through the same list. Note
+        // that the TargetPatternFunction otherwise reports these events (but only if the target
+        // pattern could be parsed successfully).
+        env.getListener().post(
+            new ParsingFailedEvent(keyOrException.getOriginalPattern(),  e.getMessage()));
       }
     }
     Map<SkyKey, ValueOrException<TargetParsingException>> resolvedPatterns =
@@ -214,7 +220,6 @@ final class TargetPatternPhaseFunction implements SkyFunction {
       try {
         value = (TargetPatternValue) resolvedPatterns.get(key).get();
       } catch (TargetParsingException e) {
-        // TODO(ulfjack): Report to EventBus.
         String rawPattern = pattern.getPattern();
         String errorMessage = e.getMessage();
         env.getListener().handle(Event.error("Skipping '" + rawPattern + "': " + errorMessage));