Improve query error msg when a package has a broken Skylark load

The error message logged during query (and build) when a package has a
broken Skylark load statement was not specific. Previously, it said
"package contains errors:" and then the package name.

Also, this error message was not emitted when using SkyQueryEnvironment
and evaluating a query containing a "TargetsBelowDirectory" pattern
(such as //foo/...) when a package below the specified directory had
such an error.

The approach taken by this CL is to include any package loading error
message in the SkyValue produced by
CollectPackagesUnderDirectoryFunction, and report them during
evaluation of a TargetsBelowDirectory pattern.

RELNOTES: Evaluation of commands on TargetsBelowDirectory patterns
(e.g. //foo/...) matching packages that fail to load now report more
detailed error messages in keep_going mode.

--
PiperOrigin-RevId: 149802362
MOS_MIGRATED_REVID=149802362

1 files changed, 22 insertions, 9 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveDirectoryTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveDirectoryTraversalFunction.java
index a7491d5fa1..fcdccbd04a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveDirectoryTraversalFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveDirectoryTraversalFunction.java
@@ -35,14 +35,12 @@ import com.google.devtools.build.skyframe.ValueOrException;
 import java.util.Map;
 
 /**
- * RecursiveDirectoryTraversalFunction traverses the subdirectories of a directory, looking for
- * and loading packages, and builds up a value from these packages in a manner customized by
- * classes that derive from it.
+ * RecursiveDirectoryTraversalFunction traverses the subdirectories of a directory, looking for and
+ * loading packages, and builds up a value from the packages and package loading errors in a manner
+ * customized by classes that derive from it.
  */
-abstract class RecursiveDirectoryTraversalFunction
-    <TVisitor extends RecursiveDirectoryTraversalFunction.Visitor, TReturn> {
-  private static final String SENTINEL_FILE_NAME_FOR_NOT_TRAVERSING_SYMLINKS =
-      "DONT_FOLLOW_SYMLINKS_WHEN_TRAVERSING_THIS_DIRECTORY_VIA_A_RECURSIVE_TARGET_PATTERN";
+abstract class RecursiveDirectoryTraversalFunction<
+    TVisitor extends RecursiveDirectoryTraversalFunction.Visitor, TReturn> {
 
   private final ProcessPackageDirectory processPackageDirectory;
 
@@ -105,6 +103,18 @@ abstract class RecursiveDirectoryTraversalFunction
      * afterwards.
      */
     void visitPackageValue(Package pkg, Environment env) throws InterruptedException;
+
+    /**
+     * Called iff the directory contains a BUILD file but *not* a package, which can happen under
+     * the following circumstances:
+     *
+     * <ol>
+     *   <li>The BUILD file contains a Skylark load statement that is in error
+     *   <li>TODO(mschaller), not yet implemented: The BUILD file is a symlink that points into a
+     *       cycle
+     * </ol>
+     */
+    void visitPackageError(NoSuchPackageException e, Environment env) throws InterruptedException;
   }
 
   /**
@@ -159,8 +169,11 @@ abstract class RecursiveDirectoryTraversalFunction
       } catch (NoSuchPackageException e) {
         // The package had errors, but don't fail-fast as there might be subpackages below the
         // current directory.
-        env.getListener()
-            .handle(Event.error("package contains errors: " + rootRelativePath.getPathString()));
+        env.getListener().handle(Event.error(e.getMessage()));
+        visitor.visitPackageError(e, env);
+        if (env.valuesMissing()) {
+          return null;
+        }
       }
       if (pkg != null) {
         visitor.visitPackageValue(pkg, env);