Exit with an error message, rather than crashing, when an attempt is made to load a Skylark file not contained in a package.

--
MOS_MIGRATED_REVID=107811126

2 files changed, 22 insertions, 6 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
index e60e0ec0cd..3b18e09fcb 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
@@ -107,7 +107,7 @@ public class SkylarkImportLookupFunction implements SkyFunction {
           SkylarkImportFailedException,
           InterruptedException {
     PathFragment filePath = fileLabel.toPathFragment();
-    
+
     // Load the AST corresponding to this file.
     ASTFileLookupValue astLookupValue;
     try {
@@ -134,7 +134,7 @@ public class SkylarkImportLookupFunction implements SkyFunction {
     Map<PathFragment, Extension> importMap = Maps.newHashMapWithExpectedSize(loadStmts.size());
     ImmutableList.Builder<SkylarkFileDependency> fileDependencies = ImmutableList.builder();
     ImmutableMap<PathFragment, Label> importPathMap;
-    
+
     // Find the labels corresponding to the load statements.
     importPathMap = findLabelsForLoadStatements(loadStmts, fileLabel, env);
     if (importPathMap == null) {
@@ -240,7 +240,7 @@ public class SkylarkImportLookupFunction implements SkyFunction {
 
     // Import PathFragments are absolute, so there is a 1-1 mapping from corresponding Labels.
     ImmutableMap.Builder<PathFragment, Label> outputMap = new ImmutableMap.Builder<>();
- 
+
     // The SkyKey here represents the directory containing an import PathFragment, hence there
     // can in general be multiple imports per lookup.
     Multimap<SkyKey, PathFragment> lookupMap = LinkedHashMultimap.create();
@@ -250,7 +250,7 @@ public class SkylarkImportLookupFunction implements SkyFunction {
           PackageIdentifier.createInDefaultRepo(relativeImportPath.getParentDirectory());
       lookupMap.put(ContainingPackageLookupValue.key(pkgToLookUp), importPath);
     }
- 
+
     // Attempt to find a package for every directory containing an import.
     Map<SkyKey,
         ValueOrException2<BuildFileNotFoundException,
@@ -269,7 +269,6 @@ public class SkylarkImportLookupFunction implements SkyFunction {
                    InconsistentFilesystemException>> entry : lookupResults.entrySet()) {
         ContainingPackageLookupValue lookupValue =
             (ContainingPackageLookupValue) entry.getValue().get();
-        PackageIdentifier pkgIdForImport = lookupValue.getContainingPackageName();
         if (!lookupValue.hasContainingPackage()) {
           // Although multiple imports may be in the same package-less directory, we only
           // report an error for the first one.
@@ -277,6 +276,7 @@ public class SkylarkImportLookupFunction implements SkyFunction {
           PathFragment importFile = lookupKey.getPackageFragment();
           throw SkylarkImportFailedException.noBuildFile(importFile);
         }
+        PackageIdentifier pkgIdForImport = lookupValue.getContainingPackageName();
         PathFragment containingPkgPath = pkgIdForImport.getPackageFragment();
         for (PathFragment importPath : lookupMap.get(entry.getKey())) {
          PathFragment relativeImportPath = importPath.toRelative();
@@ -290,7 +290,7 @@ public class SkylarkImportLookupFunction implements SkyFunction {
            // the error message to refer to a Label even though the filename was specified via a
            // simple path.
            throw new SkylarkImportFailedException(e);
-         }         
+         }
         }
       }
     } catch (BuildFileNotFoundException e) {
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java
index 546caf3088..0dcea66a43 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java
@@ -138,6 +138,22 @@ public class SkylarkImportLookupFunctionTest extends BuildViewTestCase {
         + "BUILD file not found on package path", errorMessage);
   }
 
+  public void testSkylarkImportLookupNoBuildFileForLoad() throws Exception {
+    scratch.file("pkg2/BUILD");
+    scratch.file("pkg1/ext.bzl", "a = 1");
+    scratch.file("pkg2/ext.bzl", "load('/pkg1/ext', 'a')");
+    SkyKey skylarkImportLookupKey =
+        SkylarkImportLookupValue.key(Label.parseAbsoluteUnchecked("//pkg:ext.bzl"));
+    EvaluationResult<SkylarkImportLookupValue> result =
+        SkyframeExecutorTestUtils.evaluate(
+            getSkyframeExecutor(), skylarkImportLookupKey, /*keepGoing=*/ false, reporter);
+    assertTrue(result.hasError());
+    ErrorInfo errorInfo = result.getError(skylarkImportLookupKey);
+    String errorMessage = errorInfo.getException().getMessage();
+    assertEquals("Extension file not found. Unable to load package for '//pkg:ext.bzl': "
+        + "BUILD file not found on package path", errorMessage);
+  }
+
   public void testSkylarkAbsoluteImportFilenameWithControlChars() throws Exception {
     scratch.file("pkg/BUILD", "");
     scratch.file("pkg/ext.bzl", "load('/pkg/oops\u0000', 'a')");