Allow load of external label in the WORKSPACE file.

Issue #824 Step 5.

The only thing left to resolve issue #824 are correct handling of
the following cases:
  - Forbidding overloading of a repository outside of the first part
    of the workspace file (or it will leads to incorrectness issue
    and the only use-case is the redefinition of the bazel_tools
    repository).
  - Better error reporting when trying to refer to a non-existent
    repository from the workspace file, for now it will print
    a SkyFrame cycle exception.

--
MOS_MIGRATED_REVID=114316464

2 files changed, 37 insertions, 8 deletions

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 82d0b12780..86890628c7 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
@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.skyframe;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -22,7 +23,6 @@ import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.ConstantRuleVisibility;
 import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
-import com.google.devtools.build.lib.skyframe.SkylarkImportLookupFunction.SkylarkImportFailedException;
 import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.skyframe.ErrorInfo;
@@ -216,7 +216,7 @@ public class SkylarkImportLookupFunctionTest extends BuildViewTestCase {
   }
 
   @Test
-  public void testLoadFromExternalRepoInWorkspaceFileDisallowed() throws Exception {
+  public void testLoadFromExternalRepoInWorkspaceFileAllowed() throws Exception {
     scratch.deleteFile("tools/build_rules/prelude_blaze");
     scratch.overwriteFile("WORKSPACE",
         "local_repository(",
@@ -234,11 +234,6 @@ public class SkylarkImportLookupFunctionTest extends BuildViewTestCase {
         SkyframeExecutorTestUtils.evaluate(
             getSkyframeExecutor(), skylarkImportLookupKey, /*keepGoing=*/ false, reporter);
 
-    assertTrue(result.hasError());
-    ErrorInfo errorInfo = result.getError(skylarkImportLookupKey);
-    String errorMessage = errorInfo.getException().getMessage();
-    String expectedMsgTemplate = SkylarkImportFailedException.NO_EXT_WORKSPACE_LOAD_MSG_TEMPLATE;
-    assertEquals(String.format(expectedMsgTemplate, "@a_remote_repo//remote_pkg:ext.bzl"),
-        errorMessage);
+    assertFalse(result.hasError());
   }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
index a75c93ff57..7f9e9e1713 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
@@ -721,4 +721,38 @@ public class SkylarkRuleContextTest extends SkylarkTestCase {
     Label depLabel = (Label) evalRuleContextCode(context, "ruleContext.attr.internal_dep.label");
     assertThat(depLabel).isEqualTo(Label.parseAbsolute("@r//:dep"));
   }
+
+  @Test
+  public void testExternalWorkspaceLoad() throws Exception {
+    scratch.file(
+        "/r1/BUILD",
+        "filegroup(name = 'test',",
+        " srcs = ['test.txt'],",
+        " visibility = ['//visibility:public'],",
+        ")");
+    scratch.file("/r1/WORKSPACE");
+    scratch.file("/r2/BUILD", "exports_files(['test.bzl'])");
+    scratch.file(
+        "/r2/test.bzl",
+        "def macro(name, path):",
+        "  native.local_repository(name = name, path = path)"
+    );
+    scratch.file(
+        "/r2/other_test.bzl",
+        "def other_macro(name, path):",
+        "  print(name + ': ' + path)"
+    );
+    scratch.file("BUILD");
+    scratch.overwriteFile(
+        "WORKSPACE",
+        "local_repository(name='r2', path='/r2')",
+        "load('@r2//:test.bzl', 'macro')",
+        "macro('r1', '/r1')",
+        "NEXT_NAME = 'r3'",
+        "load('@r2//:other_test.bzl', 'other_macro')",  // We can still refer to r2 in other chunks.
+        "macro(NEXT_NAME, '/r2')"  // and we can still use macro outside of its chunk.
+    );
+    invalidatePackages();
+    assertThat(getConfiguredTarget("@r1//:test")).isNotNull();
+  }
 }