Correctly reports error in the workspace file

The previous implementation was hiding errors from before a load
statement as a result of the workspace split.

--
MOS_MIGRATED_REVID=117933781

2 files changed, 27 insertions, 1 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
index 2e0843ca33..b7edfeee04 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
@@ -21,6 +21,7 @@ import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
+import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.packages.AggregatingAttributeMapper;
 import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
@@ -330,9 +331,9 @@ public abstract class RepositoryFunction {
       if (value == null) {
         return null;
       }
-      // TODO(dmarting): stop at cycle and report a more intelligible error than cycle reporting.
       Package externalPackage = value.getPackage();
       if (externalPackage.containsErrors()) {
+        Event.replayEventsOn(env.getListener(), externalPackage.getEvents());
         throw new RepositoryFunctionException(
             new BuildFileContainsErrorsException(
                 Label.EXTERNAL_PACKAGE_IDENTIFIER, "Could not load //external package"),
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java
index a7ee7213bb..8c4bd05fc6 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java
@@ -26,6 +26,7 @@ import com.google.devtools.build.lib.analysis.config.ConfigurationFactory;
 import com.google.devtools.build.lib.analysis.util.AnalysisMock;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
 import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
+import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.util.MockCcSupport;
 import com.google.devtools.build.lib.packages.util.MockToolsConfig;
 import com.google.devtools.build.lib.rules.repository.LocalRepositoryFunction;
@@ -300,4 +301,28 @@ public class SkylarkRepositoryIntegrationTest extends BuildViewTestCase {
     assertContainsEvent("Maybe repository 'foo' was defined later in your WORKSPACE file?");
     assertContainsEvent("Failed to load Skylark extension '@foo//:def.bzl'.");
   }
+
+  @Test
+  public void testLoadDoesNotHideWorkspaceError() throws Exception {
+    reporter.removeHandler(failFastHandler);
+    scratch.file("/repo2/data.txt", "data");
+    scratch.file("/repo2/BUILD", "exports_files_(['data.txt'])");
+    scratch.file("/repo2/def.bzl", "def macro():", "  print('bleh')");
+    scratch.file("/repo2/WORKSPACE");
+    scratch.overwriteFile(
+        rootDirectory.getRelative("WORKSPACE").getPathString(),
+        "local_repository(name='bleh')",
+        "local_repository(name='foo', path='/repo2')",
+        "load('@foo//:def.bzl', 'repo')",
+        "repo(name='foobar')");
+    try {
+      invalidatePackages();
+      getTarget("@foo//:data.txt");
+      fail();
+    } catch (NoSuchPackageException e) {
+      // This is expected
+      assertThat(e.getMessage()).contains("Could not load //external package");
+    }
+    assertContainsEvent("missing value for mandatory attribute 'path' in 'local_repository' rule");
+  }
 }