From 0773430188885e075121ebf720c82bb05a39db21 Mon Sep 17 00:00:00 2001
From: Damien Martin-Guillerez <dmarting@google.com>
Date: Wed, 23 Mar 2016 15:48:50 +0000
Subject: 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
---
 .../lib/rules/repository/RepositoryFunction.java   |  3 ++-
 .../skylark/SkylarkRepositoryIntegrationTest.java  | 25 ++++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

(limited to 'src')

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");
+  }
 }
-- 
cgit v1.2.3