Add a module reporting back where an external repository is defined

A common complaint about WORKSPACES files is that, due to their imperative
semantics, it is hard to find out where a repository is actually defined.
While we do not want to spam the overall output with all the definitions of
external repositories, report those that are involved in errors. As errors
include circular definitions, at the location of the error, the actual repository
rule is not always available, so we have to pass the information out-of-band via
the event bus.

Improves on #7871.

Change-Id: Ib6dbc22c850ab33f6149114dc5b682bc9ddbe1b6
PiperOrigin-RevId: 242634739
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java b/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java
index f2829af..b02a1d6 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java
@@ -43,6 +43,7 @@
           com.google.devtools.build.lib.bazel.BazelWorkspaceStatusModule.class,
           com.google.devtools.build.lib.bazel.BazelDiffAwarenessModule.class,
           com.google.devtools.build.lib.bazel.BazelRepositoryModule.class,
+          com.google.devtools.build.lib.bazel.repository.skylark.SkylarkRepositoryDebugModule.class,
           com.google.devtools.build.lib.bazel.debug.WorkspaceRuleModule.class,
           com.google.devtools.build.lib.bazel.coverage.BazelCoverageReportModule.class,
           com.google.devtools.build.lib.skylarkdebug.module.SkylarkDebuggerModule.class,
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryDebugModule.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryDebugModule.java
new file mode 100644
index 0000000..cd4eae7
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryDebugModule.java
@@ -0,0 +1,70 @@
+// Copyright 2019 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.bazel.repository.skylark;
+
+import com.google.common.eventbus.Subscribe;
+import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.Reporter;
+import com.google.devtools.build.lib.repository.RequestRepositoryInformationEvent;
+import com.google.devtools.build.lib.runtime.BlazeModule;
+import com.google.devtools.build.lib.runtime.CommandEnvironment;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Module reporting back the place an external repository was defined, if requested by some error
+ * involving that repository. This also covers cases where the definition of the repository is not
+ * directly available, e.g., during detection of a dependency cycle.
+ */
+public final class SkylarkRepositoryDebugModule extends BlazeModule {
+  Map<String, String> repositoryDefinitions;
+  Reporter reporter;
+  Set<String> reported;
+
+  @Override
+  public void beforeCommand(CommandEnvironment env) {
+    repositoryDefinitions = new HashMap<>();
+    reported = new HashSet<>();
+    reporter = env.getReporter();
+    env.getEventBus().register(this);
+  }
+
+  @Override
+  public void afterCommand() {
+    repositoryDefinitions = null;
+    reporter = null;
+    reported = null;
+  }
+
+  @Subscribe
+  public synchronized void definitionLocation(SkylarkRepositoryDefinitionLocationEvent event) {
+    repositoryDefinitions.put(event.getName(), event.getDefinitionInformation());
+  }
+
+  @Subscribe
+  public void requestDefinition(RequestRepositoryInformationEvent event) {
+    String toReport = null;
+    synchronized (this) {
+      if (!reported.contains(event.getName())
+          && repositoryDefinitions.containsKey(event.getName())) {
+        toReport = repositoryDefinitions.get(event.getName());
+      }
+    }
+    if (toReport != null) {
+      reporter.handle(Event.info(toReport));
+    }
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryDefinitionLocationEvent.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryDefinitionLocationEvent.java
new file mode 100644
index 0000000..267bcaa
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryDefinitionLocationEvent.java
@@ -0,0 +1,36 @@
+// Copyright 2019 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.bazel.repository.skylark;
+
+import com.google.devtools.build.lib.events.ExtendedEventHandler.ProgressLike;
+
+/** Event reporting about the place where a Starlark repository rule was defined. */
+public class SkylarkRepositoryDefinitionLocationEvent implements ProgressLike {
+
+  private final String name;
+  private final String definitionInformation;
+
+  public SkylarkRepositoryDefinitionLocationEvent(String name, String definitionInformation) {
+    this.name = name;
+    this.definitionInformation = definitionInformation;
+  }
+
+  public String getName() {
+    return name;
+  }
+
+  public String getDefinitionInformation() {
+    return definitionInformation;
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java
index c503bae..5da88d6 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java
@@ -70,6 +70,12 @@
       Map<String, String> markerData,
       SkyKey key)
       throws RepositoryFunctionException, InterruptedException {
+    if (rule.getDefinitionInformation() != null) {
+      env.getListener()
+          .post(
+              new SkylarkRepositoryDefinitionLocationEvent(
+                  rule.getName(), rule.getDefinitionInformation()));
+    }
     BaseFunction function = rule.getRuleClassObject().getConfiguredTargetFunction();
     if (declareEnvironmentDependencies(markerData, env, getEnviron(rule)) == null) {
       return null;
diff --git a/src/main/java/com/google/devtools/build/lib/repository/RequestRepositoryInformationEvent.java b/src/main/java/com/google/devtools/build/lib/repository/RequestRepositoryInformationEvent.java
new file mode 100644
index 0000000..ef7eec7
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/repository/RequestRepositoryInformationEvent.java
@@ -0,0 +1,32 @@
+// Copyright 2019 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.repository;
+
+import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
+
+/**
+ * Event requesting that additional information, in particular the place of definition, be shown for
+ * a given external repository.
+ */
+public class RequestRepositoryInformationEvent implements Postable {
+  private final String name;
+
+  public RequestRepositoryInformationEvent(String name) {
+    this.name = name;
+  }
+
+  public String getName() {
+    return name;
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkModuleCycleReporter.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkModuleCycleReporter.java
index df84173..37c8958 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkModuleCycleReporter.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkModuleCycleReporter.java
@@ -24,6 +24,7 @@
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.packages.WorkspaceFileValue;
+import com.google.devtools.build.lib.repository.RequestRepositoryInformationEvent;
 import com.google.devtools.build.skyframe.CycleInfo;
 import com.google.devtools.build.skyframe.CyclesReporter;
 import com.google.devtools.build.skyframe.SkyKey;
@@ -139,6 +140,15 @@
           };
       AbstractLabelCycleReporter.printCycle(ImmutableList.copyOf(repos), cycleMessage, printer);
       eventHandler.handle(Event.error(null, cycleMessage.toString()));
+      // To help debugging, request that the information be printed about where the respective
+      // repositories were defined.
+      for (SkyKey repo : repos) {
+        if (repo instanceof RepositoryValue.Key) {
+          eventHandler.post(
+              new RequestRepositoryInformationEvent(
+                  ((RepositoryValue.Key) repo).argument().strippedName()));
+        }
+      }
       return true;
     } else if (Iterables.any(cycle, IS_WORKSPACE_FILE)
         || IS_REPOSITORY_DIRECTORY.apply(lastPathElement)
diff --git a/src/test/shell/bazel/external_integration_test.sh b/src/test/shell/bazel/external_integration_test.sh
index 9f64e81..dceeda6 100755
--- a/src/test/shell/bazel/external_integration_test.sh
+++ b/src/test/shell/bazel/external_integration_test.sh
@@ -2073,7 +2073,8 @@
 
 function test_circular_definition_reported() {
   # Verify that bazel reports a useful error message upon
-  # detecting a circular definition of a repository
+  # detecting a circular definition of a repository.
+  # Also verify that the call stack of the definition is shown.
 
   WRKDIR=$(mktemp -d "${TEST_TMPDIR}/testXXXXXX")
   cd "${WRKDIR}"
@@ -2102,20 +2103,32 @@
 
   mkdir main
   cd main
-  cat > WORKSPACE <<EOF
+  cat > foo.bzl <<'EOF'
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 
-http_archive(
-  name = "a",
-  url = "file://${WRKDIR}/ext.tar",
-  build_file = "@b//:a.BUILD",
-)
+def foo():
+  http_archive(
+    name = "a",
+    url = "file://${WRKDIR}/ext.tar",
+    build_file = "@b//:a.BUILD",
+  )
+EOF
+  cat > bar.bzl <<'EOF'
+load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 
-http_archive(
-  name = "b",
-  url = "file://${WRKDIR}/ext.tar",
-  build_file = "@a//:b.BUILD",
-)
+def bar():
+  http_archive(
+    name = "b",
+    url = "file://${WRKDIR}/ext.tar",
+    build_file = "@a//:b.BUILD",
+  )
+EOF
+  cat > WORKSPACE <<EOF
+load("//:foo.bzl", "foo")
+load("//:bar.bzl", "bar")
+
+foo()
+bar()
 
 load("@a//:notabuildfile.bzl", "x")
 EOF
@@ -2128,6 +2141,14 @@
   expect_log '[Cc]ircular definition.*repositor'
   expect_log '@a'
   expect_log '@b'
+
+  # We expect to find the call stack for the definition of the repositories
+  # a and b
+  expect_log "WORKSPACE:4:1"
+  expect_log "foo.bzl:4:3"
+
+  expect_log "WORKSPACE:5:1"
+  expect_log "bar.bzl:4:3"
 }
 
 run_suite "external tests"