Properly handle exceptions that RecursivePkgFunction can encounter, instead of silently swallowing them.

The old behavior was simply incorrect on --keep_going builds because it meant any directory with an uncaught error caused all of its ancestors to not have any values. It wasn't noticed because SkyframeTargetPatternEvaluator was overly permissive in the errors it expects.

We also use a singleton for the empty RecursivePkgValue which might have a negligible (beneficial) memory impact.

--
MOS_MIGRATED_REVID=95037551

1 files changed, 5 insertions, 0 deletions

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 d166126038..e504f88001 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
@@ -180,6 +180,11 @@ final class SkyframeTargetPatternEvaluator implements TargetPatternEvaluator {
           continue;
         }
         if (error.getException() != null) {
+          // This exception may not be a TargetParsingException because in a nokeep_going build, the
+          // target pattern parser may swallow a NoSuchPackageException but the framework will
+          // bubble it up anyway.
+          Preconditions.checkArgument(!keepGoing
+              || error.getException() instanceof TargetParsingException, error);
           errorMessage = error.getException().getMessage();
         } else if (!Iterables.isEmpty(error.getCycleInfo())) {
           errorMessage = "cycles detected during target parsing";