commit b749ab1f8549cdad0a574c356a57fe7181b07851
author cparsons <cparsons@google.com> Thu Aug 16 12:45:02 2018 -0700
committer Copybara-Service <copybara-piper@google.com> Thu Aug 16 12:46:22 2018 -0700
tree b01d5d4e475d3863f730aeb8ddc04f78c2f5d138
parent e7d38d0d866f3ee778c6bb227939cd24b3c85fa4

(Skydoc) Handle cross-repository dependencies

RELNOTES: None.
PiperOrigin-RevId: 209029649

src/main/java/com/google/devtools/build/skydoc/SkydocMain.java

diff --git a/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java b/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java
index 2a0bf3d..53a87d5 100644
--- a/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java
+++ b/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java
@@ -230,7 +230,8 @@
   private Environment recursiveEval(
       Label label, List<RuleInfo> ruleInfoList)
       throws InterruptedException, IOException, LabelSyntaxException {
-    Path path = Paths.get(label.toPathFragment().toString());
+    Path path = pathOfLabel(label);
+
     if (pending.contains(path)) {
       throw new IllegalStateException("cycle with " + path);
     } else if (loaded.containsKey(path)) {
@@ -244,7 +245,6 @@
     Map<String, Extension> imports = new HashMap<>();
     for (SkylarkImport anImport : buildFileAST.getImports()) {
       Label relativeLabel = label.getRelative(anImport.getImportString());
-      Path importPath = Paths.get(relativeLabel.toPathFragment().toString());
 
       try {
         Environment importEnv = recursiveEval(relativeLabel, ruleInfoList);
@@ -252,7 +252,7 @@
       } catch (NoSuchFileException noSuchFileException) {
         throw new IllegalStateException(
             String.format("File %s imported '%s', yet %s was not found.",
-                path, anImport.getImportString(), importPath));
+                path, anImport.getImportString(), pathOfLabel(relativeLabel)));
       }
     }
 
@@ -264,6 +264,14 @@
     return env;
   }
 
+  private Path pathOfLabel(Label label) {
+    String workspacePrefix = label.getWorkspaceRoot().isEmpty()
+        ? ""
+        : label.getWorkspaceRoot() + "/";
+
+    return Paths.get(workspacePrefix + label.toPathFragment());
+  }
+
   /**
    * Evaluates the AST from a single skylark file, given the already-resolved imports.
    */

src/test/java/com/google/devtools/build/skydoc/SkydocTest.java

diff --git a/src/test/java/com/google/devtools/build/skydoc/SkydocTest.java b/src/test/java/com/google/devtools/build/skydoc/SkydocTest.java
index f9f4d62..91faae8 100644
--- a/src/test/java/com/google/devtools/build/skydoc/SkydocTest.java
+++ b/src/test/java/com/google/devtools/build/skydoc/SkydocTest.java
@@ -205,6 +205,58 @@
   }
 
   @Test
+  public void testRulesAcrossRepository() throws Exception {
+    scratch.file(
+        "/external/dep_repo/lib/rule_impl.bzl",
+        "def rule_impl(ctx):",
+        "  return struct()");
+
+    scratch.file(
+        "/deps/foo/docstring.bzl",
+        "doc_string = 'Dep rule'");
+
+    scratch.file(
+        "/deps/foo/dep_rule.bzl",
+        "load('@dep_repo//lib:rule_impl.bzl', 'rule_impl')",
+        "load(':docstring.bzl', 'doc_string')",
+        "",
+        "_hidden_rule = rule(",
+        "    doc = doc_string,",
+        "    implementation = rule_impl,",
+        ")",
+        "",
+        "dep_rule = rule(",
+        "    doc = doc_string,",
+        "    implementation = rule_impl,",
+        ")");
+
+    scratch.file(
+        "/test/main.bzl",
+        "load('@dep_repo//lib:rule_impl.bzl', 'rule_impl')",
+        "load('//deps/foo:dep_rule.bzl', 'dep_rule')",
+        "",
+        "main_rule = rule(",
+        "    doc = 'Main rule',",
+        "    implementation = rule_impl,",
+        ")");
+
+    ImmutableMap.Builder<String, RuleInfo> ruleInfoMapBuilder = ImmutableMap.builder();
+
+    skydocMain.eval(
+        Label.parseAbsoluteUnchecked("//test:main.bzl"),
+        ruleInfoMapBuilder,
+        ImmutableList.builder());
+
+    Map<String, RuleInfo> ruleInfoMap = ruleInfoMapBuilder.build();
+
+    // dep_rule is available here, even though it was not defined in main.bzl, because it is
+    // imported in main.bzl. Thus, it's a top-level symbol in main.bzl.
+    assertThat(ruleInfoMap.keySet()).containsExactly("main_rule", "dep_rule");
+    assertThat(ruleInfoMap.get("main_rule").getDocString()).isEqualTo("Main rule");
+    assertThat(ruleInfoMap.get("dep_rule").getDocString()).isEqualTo("Dep rule");
+  }
+
+  @Test
   public void testSkydocCrashesOnCycle() throws Exception {
     scratch.file(
         "/dep/dep.bzl",