Make Stardoc repository mapping aware

By taking in .bzl files as runfiles rather than inputs, Stardoc can use the existing Java runfiles library to resolve apparent repository names in load labels into canonical repository names.
bazelbuild · Nov 16, 2022 · 96d65bc · 96d65bc
1 parent d2aaf0c
commit 96d65bc
Show file tree

Hide file tree

Showing 12 changed files with 170 additions and 244 deletions.
diff --git a/distdir_deps.bzl b/distdir_deps.bzl
@@ -274,16 +274,16 @@ DIST_DEPS = {
         ],
     },
     "io_bazel_skydoc": {
-        "archive": "1ef781ced3b1443dca3ed05dec1989eca1a4e1cd.tar.gz",
-        "sha256": "5a725b777976b77aa122b707d1b6f0f39b6020f66cd427bb111a585599c857b1",
+        "archive": "d09735f49f046fdebf2fc36312dc9f021f89ce27.tar.gz",
+        "sha256": "be9452206851d50af4831b4eae15290351f0d2f8300dc9002501fc00bf6dfeba",
         "urls": [
-            "https://mirror.bazel.build/github.com/bazelbuild/stardoc/archive/1ef781ced3b1443dca3ed05dec1989eca1a4e1cd.tar.gz",
-            "https://github.com/bazelbuild/stardoc/archive/1ef781ced3b1443dca3ed05dec1989eca1a4e1cd.tar.gz",
+            "https://mirror.bazel.build/github.com/bazelbuild/stardoc/archive/d09735f49f046fdebf2fc36312dc9f021f89ce27.tar.gz",
+            "https://github.com/bazelbuild/stardoc/archive/d09735f49f046fdebf2fc36312dc9f021f89ce27.tar.gz",
         ],
         "used_in": [
             "additional_distfiles",
         ],
-        "strip_prefix": "stardoc-1ef781ced3b1443dca3ed05dec1989eca1a4e1cd",
+        "strip_prefix": "stardoc-d09735f49f046fdebf2fc36312dc9f021f89ce27",
     },
     "rules_license": {
         "archive": "rules_license-0.0.3.tar.gz",

diff --git a/src/main/java/com/google/devtools/build/skydoc/BUILD b/src/main/java/com/google/devtools/build/skydoc/BUILD
@@ -23,15 +23,6 @@ filegroup(
 
 java_binary(
     name = "skydoc",
-    jvm_flags = [
-        # quiet warnings from com.google.protobuf.UnsafeUtil,
-        # see: https://github.com/google/protobuf/issues/3781
-        # and: https://github.com/bazelbuild/bazel/issues/5599
-        "--add-opens=java.base/java.nio=ALL-UNNAMED",
-        "--add-opens=java.base/java.lang=ALL-UNNAMED",
-        # ... but only on JDK >= 9
-        "-XX:+IgnoreUnrecognizedVMOptions",
-    ],
     main_class = "com.google.devtools.build.skydoc.SkydocMain",
     visibility = ["//visibility:public"],
     runtime_deps = [
@@ -56,5 +47,7 @@ java_library(
         "//src/main/java/net/starlark/java/lib/json",
         "//src/main/java/net/starlark/java/syntax",
         "//third_party:guava",
+        "//tools/java/runfiles",
+        "//tools/java/runfiles:runfiles_for_stardoc",
     ],
 )
diff --git a/src/main/java/com/google/devtools/build/skydoc/FilesystemFileAccessor.java b/src/main/java/com/google/devtools/build/skydoc/FilesystemFileAccessor.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
@@ -30,6 +30,8 @@
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
+import com.google.devtools.build.runfiles.Runfiles;
+import com.google.devtools.build.runfiles.RunfilesForStardoc;
 import com.google.devtools.build.skydoc.fakebuildapi.FakeApi;
 import com.google.devtools.build.skydoc.fakebuildapi.FakeDeepStructure;
 import com.google.devtools.build.skydoc.fakebuildapi.FakeProviderApi;
@@ -46,6 +48,7 @@
 import java.io.BufferedOutputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.nio.file.Files;
 import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -103,21 +106,13 @@ public class SkydocMain {
   private final EventHandler eventHandler = new SystemOutEventHandler();
   private final LinkedHashSet<Path> pending = new LinkedHashSet<>();
   private final Map<Path, Module> loaded = new HashMap<>();
-  private final StarlarkFileAccessor fileAccessor;
-  private final List<String> depRoots;
   private final String workspaceName;
+  private final Runfiles.Preloaded runfiles;
 
   public SkydocMain(
-      StarlarkFileAccessor fileAccessor, String workspaceName, List<String> depRoots) {
-    this.fileAccessor = fileAccessor;
+      String workspaceName, Runfiles.Preloaded runfiles) {
     this.workspaceName = workspaceName;
-    if (depRoots.isEmpty()) {
-      // For backwards compatibility, if no dep_roots are specified, use the current
-      // directory as the only root.
-      this.depRoots = ImmutableList.of(".");
-    } else {
-      this.depRoots = depRoots;
-    }
+    this.runfiles = runfiles;
   }
 
   public static void main(String[] args)
@@ -134,7 +129,6 @@ public static void main(String[] args)
 
     String targetFileLabelString;
     String outputPath;
-    ImmutableList<String> depRoots;
 
     if (Strings.isNullOrEmpty(skydocOptions.targetFileLabel)
         || Strings.isNullOrEmpty(skydocOptions.outputFilePath)) {
@@ -143,7 +137,6 @@ public static void main(String[] args)
 
     targetFileLabelString = skydocOptions.targetFileLabel;
     outputPath = skydocOptions.outputFilePath;
-    depRoots = ImmutableList.copyOf(skydocOptions.depRoots);
 
     Label targetFileLabel = Label.parseAbsolute(targetFileLabelString, ImmutableMap.of());
 
@@ -155,7 +148,7 @@ public static void main(String[] args)
     Module module = null;
     try {
       module =
-          new SkydocMain(new FilesystemFileAccessor(), skydocOptions.workspaceName, depRoots)
+          new SkydocMain(skydocOptions.workspaceName, Runfiles.preload())
               .eval(
                   semanticsOptions.toStarlarkSemantics(),
                   targetFileLabel,
@@ -289,7 +282,8 @@ public Module eval(
 
     List<AspectInfoWrapper> aspectInfoList = new ArrayList<>();
 
-    Module module = recursiveEval(semantics, label, ruleInfoList, providerInfoList, aspectInfoList);
+    Module module = recursiveEval(semantics, label, label.getRepository().getName(), ruleInfoList,
+        providerInfoList, aspectInfoList);
 
     Map<StarlarkCallable, RuleInfoWrapper> ruleFunctions =
         ruleInfoList.stream()
@@ -375,20 +369,24 @@ private static void putStructFields(
    * dependencies using a fake build API and collects information about all rule definitions made in
    * those files.
    *
-   * @param label the label of the Starlark file to evaluate
-   * @param ruleInfoList a collection of all rule definitions made so far (using rule()); this
-   *     method will add to this list as it evaluates additional files
+   * @param label                  the label of the Starlark file to evaluate
+   * @param parentSourceRepository the canonical name of the Bazel repository that loads label
+   * @param ruleInfoList           a collection of all rule definitions made so far (using rule());
+   *                               this method will add to this list as it evaluates additional
+   *                               files
    * @throws InterruptedException if evaluation is interrupted
    */
   private Module recursiveEval(
       StarlarkSemantics semantics,
       Label label,
+      String parentSourceRepository,
       List<RuleInfoWrapper> ruleInfoList,
       List<ProviderInfoWrapper> providerInfoList,
       List<AspectInfoWrapper> aspectInfoList)
-      throws InterruptedException, IOException, LabelSyntaxException, StarlarkEvaluationException,
-          EvalException {
-    Path path = pathOfLabel(label, semantics);
+      throws InterruptedException, IOException, LabelSyntaxException, StarlarkEvaluationException {
+    Path path = pathOfLabel(label, parentSourceRepository);
+    String sourceRepository = RunfilesForStardoc.getCanonicalRepository(
+        runfiles.withSourceRepository(parentSourceRepository), label.getRepository().getName());
 
     if (pending.contains(path)) {
       throw new StarlarkEvaluationException("cycle with " + path);
@@ -422,7 +420,7 @@ private Module recursiveEval(
         };
 
     // parse & compile (and get doc)
-    ParserInput input = getInputSource(path.toString());
+    ParserInput input = ParserInput.fromLatin1(Files.readAllBytes(path), path.toString());
     Program prog;
     try {
       StarlarkFile file = StarlarkFile.parse(input, FileOptions.DEFAULT);
@@ -435,16 +433,17 @@ private Module recursiveEval(
     // process loads
     Map<String, Module> imports = new HashMap<>();
     for (String load : prog.getLoads()) {
-      Label relativeLabel = label.getRelativeWithRemapping(load, ImmutableMap.of());
+      Label apparentLabel = label.getRelativeWithRemapping(load, ImmutableMap.of());
       try {
         Module loadedModule =
-            recursiveEval(semantics, relativeLabel, ruleInfoList, providerInfoList, aspectInfoList);
+            recursiveEval(semantics, apparentLabel, sourceRepository, ruleInfoList,
+                providerInfoList, aspectInfoList);
         imports.put(load, loadedModule);
       } catch (NoSuchFileException noSuchFileException) {
         throw new StarlarkEvaluationException(
             String.format(
-                "File %s imported '%s', yet %s was not found, even at roots %s.",
-                path, load, pathOfLabel(relativeLabel, semantics), depRoots),
+                "File %s imported '%s', yet %s was not found.",
+                path, load, pathOfLabel(apparentLabel, sourceRepository)),
             noSuchFileException);
       }
     }
@@ -474,25 +473,11 @@ path, load, pathOfLabel(relativeLabel, semantics), depRoots),
     return module;
   }
 
-  private Path pathOfLabel(Label label, StarlarkSemantics semantics) throws EvalException {
-    String workspacePrefix = "";
-    if (!label.getWorkspaceRootForStarlarkOnly(semantics).isEmpty()
-        && !label.getWorkspaceName().equals(workspaceName)) {
-      workspacePrefix = label.getWorkspaceRootForStarlarkOnly(semantics) + "/";
-    }
-
-    return Paths.get(workspacePrefix + label.toPathFragment());
-  }
-
-  private ParserInput getInputSource(String bzlWorkspacePath) throws IOException {
-    for (String rootPath : depRoots) {
-      if (fileAccessor.fileExists(rootPath + "/" + bzlWorkspacePath)) {
-        return fileAccessor.inputSource(rootPath + "/" + bzlWorkspacePath);
-      }
-    }
-
-    // All depRoots attempted and no valid file was found.
-    throw new NoSuchFileException(bzlWorkspacePath);
+  private Path pathOfLabel(Label label, String sourceRepository) throws IOException {
+    String targetRepositoryApparentName =
+        label.getRepository().isMain() ? workspaceName : label.getRepository().getName();
+    String rlocationPath = targetRepositoryApparentName + "/" + label.toPathFragment().toString();
+    return Paths.get(runfiles.withSourceRepository(sourceRepository).rlocation(rlocationPath));
   }
 
   private static void addMorePredeclared(ImmutableMap.Builder<String, Object> env) {

diff --git a/src/main/java/com/google/devtools/build/skydoc/SkydocOptions.java b/src/main/java/com/google/devtools/build/skydoc/SkydocOptions.java
@@ -59,13 +59,4 @@ public class SkydocOptions extends OptionsBase {
               + " is empty, then documentation for all exported rule definitions will be"
               + " generated.")
   public List<String> symbolNames;
-
-  @Option(
-      name = "dep_roots",
-      allowMultiple = true,
-      defaultValue = "null",
-      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
-      effectTags = OptionEffectTag.UNKNOWN,
-      help = "File path roots to search when resolving transitive bzl dependencies")
-  public List<String> depRoots;
 }
diff --git a/src/main/java/com/google/devtools/build/skydoc/StarlarkFileAccessor.java b/src/main/java/com/google/devtools/build/skydoc/StarlarkFileAccessor.java
diff --git a/src/test/java/com/google/devtools/build/skydoc/BUILD b/src/test/java/com/google/devtools/build/skydoc/BUILD
@@ -23,18 +23,16 @@ java_test(
     deps = [
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/packages/semantics",
-        "//src/main/java/com/google/devtools/build/lib/vfs",
-        "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
         "//src/main/java/com/google/devtools/build/skydoc:skydoc_lib",
         "//src/main/java/com/google/devtools/build/skydoc/fakebuildapi",
         "//src/main/java/com/google/devtools/build/skydoc/rendering",
         "//src/main/java/com/google/devtools/build/skydoc/rendering/proto:stardoc_output_java_proto",
         "//src/main/java/net/starlark/java/eval",
         "//src/main/java/net/starlark/java/syntax",
-        "//src/test/java/com/google/devtools/build/lib/analysis/util",
         "//third_party:guava",
         "//third_party:junit4",
         "//third_party:truth",
+        "//tools/java/runfiles",
     ],
 )