Stop throwing an exception if a Package was successfully created but contains errors. Instead, require callers to process the package and throw if they need to.

This allows us to avoid embedding a Package in an exception, which is icky. This also allows us to remove Package#containsTemporaryErrors.

Most callers' changes are fairly straightforward. The exception is EnvironmentBackedRecursivePackageProvider, which cannot throw an exception of its own in case of a package with errors (because it doesn't do that in keep_going mode), but whose request for a package with errors *should* shut down the build in case of nokeep_going mode. To do this in Skyframe, we have a new PackageErrorFunction which is to be called only in this situation, and will unconditionally throw. EnvironmentBackedRecursivePackageProvider can then catch this exception and continue on as usual, except that the exception will shut down the thread pool in a nokeep_going build.

--
MOS_MIGRATED_REVID=103247761

1 files changed, 7 insertions, 11 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetMarkerFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetMarkerFunction.java
index 75d4ad80bd..23186442d9 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetMarkerFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetMarkerFunction.java
@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.skyframe;
 
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
 import com.google.devtools.build.lib.packages.BuildFileNotFoundException;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.NoSuchTargetException;
@@ -79,7 +80,6 @@ public final class TargetMarkerFunction implements SkyFunction {
     }
 
     SkyKey pkgSkyKey = PackageValue.key(label.getPackageIdentifier());
-    NoSuchPackageException nspe = null;
     Package pkg;
     try {
       PackageValue value = (PackageValue)
@@ -89,14 +89,8 @@ public final class TargetMarkerFunction implements SkyFunction {
       }
       pkg = value.getPackage();
     } catch (NoSuchPackageException e) {
-      // For consistency with pre-Skyframe Blaze, we can return a valid Target from a Package
-      // containing errors.
-      pkg = e.getPackage();
-      if (pkg == null) {
-        // Re-throw this exception with our key because root causes should be targets, not packages.
-        throw new TargetMarkerFunctionException(e);
-      }
-      nspe = e;
+      // Re-throw this exception with our key because root causes should be targets, not packages.
+      throw new TargetMarkerFunctionException(e);
     }
 
     Target target;
@@ -106,12 +100,14 @@ public final class TargetMarkerFunction implements SkyFunction {
       throw new TargetMarkerFunctionException(e);
     }
 
-    if (nspe != null) {
+    if (pkg.containsErrors()) {
       // There is a target, but its package is in error. We rethrow so that the root cause is the
       // target, not the package. Note that targets are only in error when their package is
       // "in error" (because a package is in error if there was an error evaluating the package, or
       // if one of its targets was in error).
-      throw new TargetMarkerFunctionException(new NoSuchTargetException(target, nspe));
+      throw new TargetMarkerFunctionException(
+          new NoSuchTargetException(
+              target, new BuildFileContainsErrorsException(label.getPackageIdentifier())));
     }
     return TargetMarkerValue.TARGET_MARKER_INSTANCE;
   }