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, 13 insertions, 2 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePkgValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePkgValue.java
index 56ed0e504a..721ebc8f0c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePkgValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePkgValue.java
@@ -16,6 +16,8 @@ package com.google.devtools.build.lib.skyframe;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -32,14 +34,23 @@ import java.util.Objects;
  */
 @Immutable
 @ThreadSafe
-public class RecursivePkgValue implements SkyValue {
+class RecursivePkgValue implements SkyValue {
+  static final RecursivePkgValue EMPTY =
+      new RecursivePkgValue(NestedSetBuilder.<String>emptySet(Order.STABLE_ORDER));
 
   private final NestedSet<String> packages;
 
-  public RecursivePkgValue(NestedSet<String> packages) {
+  private RecursivePkgValue(NestedSet<String> packages) {
     this.packages = packages;
   }
 
+  static RecursivePkgValue create(NestedSetBuilder<String> packages) {
+    if (packages.isEmpty()) {
+      return EMPTY;
+    }
+    return new RecursivePkgValue(packages.build());
+  }
+
   /**
    * Create a transitive package lookup request.
    */