Use Labels, rather than PathFragments, to represent Skylark loads internally. The load location for a Skylark Aspect is specified via a PathFragment, for consistency with current non-Aspect Skylark loads.

This should be a semantics-preserving change for users. In a subsequent CL, I'll change the Skylark syntax to allow load statements to use labels as well as paths, with the goal of eventually deprecating the latter.

Also:

- Removed the hack for handling relative loads in the prelude file.

- Refactored some redundant functionality in PackageFunction and SkylarkImportLookupFunction for handling loads.

- Removed the ability to put the BUILD file for the package containing a Skylark file under a different package root than the Skylark file itself. This functionality isn't currently used and is inconsistent with Blaze's handling of the package path elsewhere.

- Added BUILD files to a number of tests that load Skylark files; this is consistent with the requirement that all Skylark files need to be part of some package.

- Changed the constants used to set the location of the prelude file from paths to labels.

--
MOS_MIGRATED_REVID=107741568

1 files changed, 65 insertions, 26 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupValue.java
index 99e4f188ac..4581cb2521 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupValue.java
@@ -14,48 +14,87 @@
 
 package com.google.devtools.build.lib.skyframe;
 
-import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.common.base.Preconditions;
+import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.syntax.BuildFileAST;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 
-import javax.annotation.Nullable;
-
 /**
- * A value that represents an AST file lookup result.
+ * A value that represents an AST file lookup result. There are two subclasses: one for the
+ * case where the file is found, and another for the case where the file is missing (but there
+ * are no other errors).
  */
-public class ASTFileLookupValue implements SkyValue {
+abstract class ASTFileLookupValue implements SkyValue {
+  public abstract boolean lookupSuccessful();
+  public abstract BuildFileAST getAST();
+  public abstract String getErrorMsg();
+  
+  /** If the file is found, this class encapsulates the parsed AST. */
+  private static class ASTLookupWithFile extends ASTFileLookupValue {
+    private final BuildFileAST ast;
 
-  private static final ASTFileLookupValue NO_FILE = new ASTFileLookupValue(null);
+    private ASTLookupWithFile(BuildFileAST ast) {
+      Preconditions.checkNotNull(ast);
+      this.ast = ast;
+    }
 
-  @Nullable private final BuildFileAST ast;
+    @Override
+    public boolean lookupSuccessful() {
+      return true;
+    }
 
-  private ASTFileLookupValue(@Nullable BuildFileAST ast) {
-    this.ast = ast;
-  }
+    @Override
+    public BuildFileAST getAST() {
+      return this.ast;
+    }
 
-  public static ASTFileLookupValue noFile() {
-    return NO_FILE;
+    @Override
+    public String getErrorMsg() {
+      throw new IllegalStateException(
+          "attempted to retrieve unsuccessful lookup reason for successful lookup");
+    }
   }
+ 
+  /** If the file isn't found, this class encapsulates a message with the reason. */
+  private static class ASTLookupNoFile extends ASTFileLookupValue {
+    private final String errorMsg;
 
-  public static ASTFileLookupValue withFile(BuildFileAST ast) {
-    return new ASTFileLookupValue(ast);
-  }
+    private ASTLookupNoFile(String errorMsg) {
+      this.errorMsg = Preconditions.checkNotNull(errorMsg);
+    }
 
-  /**
-   * Returns the original AST file.
-   */
-  @Nullable public BuildFileAST getAST() {
-    return ast;
+    @Override
+    public boolean lookupSuccessful() {
+      return false;
+    }
+
+    @Override
+    public BuildFileAST getAST() {
+      throw new IllegalStateException("attempted to retrieve AST from an unsuccessful lookup");
+    }
+
+    @Override
+    public String getErrorMsg() {
+      return this.errorMsg;
+    }
   }
 
-  static SkyKey key(PackageIdentifier astFileIdentifier) {
-    return new SkyKey(SkyFunctions.AST_FILE_LOOKUP, astFileIdentifier);
+  static ASTFileLookupValue forBadPackage(Label fileLabel, String reason) {
+    return new ASTLookupNoFile(
+        String.format("Unable to load package for '%s': %s", fileLabel, reason));
+  }
+  
+  static ASTFileLookupValue forBadFile(Label fileLabel) {
+    return new ASTLookupNoFile(
+        String.format("Unable to load file '%s': file doesn't exist or isn't a file", fileLabel));
+  }
+  
+  public static ASTFileLookupValue withFile(BuildFileAST ast) {
+    return new ASTLookupWithFile(ast);
   }
 
-  static final class ASTLookupInputException extends Exception {
-    ASTLookupInputException(String msg) {
-      super(msg);
-    }
+  static SkyKey key(Label astFileLabel) {
+    return new SkyKey(SkyFunctions.AST_FILE_LOOKUP, astFileLabel);
   }
 }