Fix error handling in skyframe target pattern parsing

Bazel completely swallowed errors in some cases, e.g., if the pattern is
invalid like bazel build foo//bar:baz.

Note that it previously silently ignored empty targets if --experimental_skyframe_target_pattern_evaluator was passed, and now fails (which is consistent with legacy behavior). This is an intentional change, but may break users who are using the experimental flag and are passing empty strings to Bazel.

PiperOrigin-RevId: 184282856

1 files changed, 18 insertions, 2 deletions

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 75d38b81d3..28803ff1bb 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
@@ -206,8 +206,9 @@ final class TargetPatternPhaseFunction implements SkyFunction {
    */
   private static ResolvedTargets<Target> getTargetsToBuild(
       Environment env, TargetPatternPhaseKey options, PathPackageLocator pkgPath)
-      throws InterruptedException {
+          throws InterruptedException {
     List<TargetPatternKey> patternSkyKeys = new ArrayList<>();
+    ResolvedTargets.Builder<Target> builder = ResolvedTargets.builder();
     for (TargetPatternSkyKeyOrException keyOrException :
         TargetPatternValue.keys(
             options.getTargetPatterns(),
@@ -224,6 +225,16 @@ final class TargetPatternPhaseFunction implements SkyFunction {
         // pattern could be parsed successfully).
         env.getListener().post(
             new ParsingFailedEvent(keyOrException.getOriginalPattern(),  e.getMessage()));
+        try {
+          env.getValueOrThrow(
+              TargetPatternErrorFunction.key(e.getMessage()), TargetParsingException.class);
+        } catch (TargetParsingException ignore) {
+          // We ignore this. Keep going is active.
+        }
+        env.getListener().handle(
+            Event.error(
+                "Skipping '" + keyOrException.getOriginalPattern() + "': " + e.getMessage()));
+        builder.setError();
       }
     }
     Map<SkyKey, ValueOrException<TargetParsingException>> resolvedPatterns =
@@ -232,7 +243,6 @@ final class TargetPatternPhaseFunction implements SkyFunction {
       return null;
     }
 
-    ResolvedTargets.Builder<Target> builder = ResolvedTargets.builder();
     for (TargetPatternKey pattern : patternSkyKeys) {
       TargetPatternValue value;
       try {
@@ -265,6 +275,12 @@ final class TargetPatternPhaseFunction implements SkyFunction {
       } catch (MissingDepException e) {
         return null;
       } catch (TargetParsingException e) {
+        try {
+          env.getValueOrThrow(
+              TargetPatternErrorFunction.key(e.getMessage()), TargetParsingException.class);
+        } catch (TargetParsingException ignore) {
+          // We ignore this. Keep going is active.
+        }
         env.getListener().handle(Event.error(e.getMessage()));
         return ResolvedTargets.failed();
       }