Bzlmod: stardoc is broken by repo mappings

[meteorcloudy]

When Bzlmod is enabled, building stardoc for a bzl_library which has external dependencies will fail.

This is discovered in bazelbuild/bazel-central-registry#18, and the build failure can be found here.

The root cause is that when Bzlmod is enabled, the canonical repo name for bazel_skylib is bazel_skylib.1.0.3, which now contains the version number. In the normal bazel build, we have repo mappings to make sure BUILD/bzl files can still access the targets under the repository with @bazel_skylib. However, when stardoc is parsing a label, it's not doing any repo mapping to locate the canonical repo name. Which led to:

Stardoc documentation generation failed: File external/io_bazel_stardoc.0.5.0/stardoc/stardoc.bzl imported '@bazel_skylib//:bzl_library.bzl', yet external/bazel_skylib/bzl_library.bzl was not found, even at roots [., external/__main__].

Note that this is also an issue if you use the repo mapping feature in WORKSPACE file, but it's not widely used so no one has hit this problem before. Since we are embracing the repo mapping feature in Bzlmod to avoid conflicts and provide strict deps, we have to solve this problem.

Related #13316

[keith]

Ah, I also filed bazelbuild/stardoc#117 about this

[Wyverald]

Is there a runfiles library for Java? I think @fmeum also had a rules_runfiles or something. (Sorry I've never really looked into runfiles) That's probably the best way to resolve this.

[fmeum]

Is there a runfiles library for Java? I think @fmeum also had a rules_runfiles or something. (Sorry I've never really looked into runfiles) That's probably the best way to resolve this.

The runfiles library at @bazel_tools//tools/java/runfiles (and my ruleset based on it) solves the somewhat orthogonal problem of getting the runtime path on disk of a runfile given its logical path under external, i.e. internal_repo_name/path/to/pkg/file. But if I understand the current issue correctly, the problem is that Stardoc is not repo mappings aware?

[Wyverald]

Yeah Stardoc needs to convert a label into a runfile path (runtime path of a runfile?) and that's not so simple with repo mappings.

[Wyverald]

Update: We're considering solving this in a similar fashion to #15029, by outputting a file containing all repo mappings and having that file available to binaries at runtime. It's still to be decided where this file should live.

[aiuto]

cc: @aiuto

[comius]

Yeah Stardoc needs to convert a label into a runfile path (runtime path of a runfile?) and that's not so simple with repo mappings.

It's slightly worse. It needs to convert it to execpath. There are no runfiles (because render is packed separately than input .bzl files), and no repo_mapping because of this.

[fmeum]

@comius I formated an alternative idea in https://github.com/bazelbuild/proposals/blob/main/designs/2022-07-21-locating-runfiles-with-bzlmod.md#4-make-stardoc-compatible-with-repository-mappings, do you think that could work? @tetromino is working on a rewrite of some parts of stardoc that would allow it to request bzl files from Skyframe (if I understood him correctly), which should also solve this issue.

[comius]

Yes, that works. I was thinking of a slight alternative where macro would add a custom java_binary StardocMain with data runfiles. Maybe slightly less hacky, because you don't need a no-op executable.

Second alternative option is to add repository mapping information to bzl_library and dump it into a file in stardoc rule. I'm guessing you can collect the needed triples in bzl_library into a depset. And then you could use args trick to dump to a file neatly. But that would need a second implementation of reading repo mapping in StartdocMain similar to runfiles.

[fmeum]

The custom StardocMain binary sounds better, yes.

I am not so sure you can actually collect the apparent names because the rule implementation typically only sees the canonical names. But even if you could, reusing the existing repository mapping logic even if by a small hack seems more reliable to me.

@tetromino Can you estimate when you would get to work on the improved bzl loading for Stardoc? Would the changes require a new Bazel version or could they be used independentl

[fmeum]

@comius Saw the merge, thanks for that! Maybe we should keep this issue open until there is a BCR release of stardoc with the changes?

[tetromino]

Fixed by bazelbuild/bazel-central-registry#635