bazel-distribution icon indicating copy to clipboard operation
bazel-distribution copied to clipboard

Issue with `skydoc`/`stardoc` dependency and `nodejs_rules` versions

Open adammitchelldev opened this issue 4 years ago • 1 comments

We forked skydoc (now called stardoc) a while ago due to an issue where Bazel can't see various dependencies that are load(..)ed whilst interpreting the .bzl files (originally raised here). This is because it effectively hacks the way that the files are read to read from predefined roots that it kind of "guesses" will work (it's an acknowledged issue in the stardoc source), and this breaks when loading files outside of the current WORKSPACE.

However, this causes us to depend on a very old version of skydoc, which in turn depends on an old version of skylib, which in turn depends on an old version of rules_sass, which depends on an old version of nodejs_rules, which means that our npm_deploy rule can only work the old nodejs_rules (since updating the nodejs_rules breaks skydoc). This limits our ability to depend on the latest nodejs_rules downstream, such as in client-java, which means we can't upgrade to the latest version of npm @bazel/jasmine. The nodejs_rules changes have been very major and no longer backwards compatible and it should be considered a significant tech debt if we can't upgrade.

We started trying to solve the issue and found a workaround as listed in the original issue: include the dependent source files as deps to stardoc via another bzl_library rule, however, this only works when the files themselves are public (bazel visibility wise). We fixed this with workarounds for everything except pip_import, which generates a bazel workspace with a requirements.bzl that it does not export as public. Unfortunately, this means our workaround does not fully fix everything and we cannot build docs for the pip rules.

This leaves us with a few solutions:

Do nothing now and hope the issue can be fixed upstream

We don't immediately need to update rules_nodejs but this isn't actually a solution. There is no "clear party to blame" here that we can submit an easy fix to, the "real fix" is that the whole thing should work differently, but it may be possible to convince one of the involved repos to allow configuration for a workaround.

Use a patch to complete the workaround

The easiest workaround that we could get is in rules_python, which needs a single line change to make the generated BUILD file export the requirements.bzl file. We could also implement this as an http_archive patch option (which is not an uncommon solution in other bazel repos) and that would work around the whole issue for now, but is not necessarily sustainable. We could submit a PR to make this change (and optionally) as there may be value in it for the rules_python repo anyway. However, they do not appear to move very quickly.

Merge the workaround without a patch and accept that we can't build docs for the pip rules

We can still build docs for everything else, but without some kind of workaround from some party somewhere, be that bazel, stardoc or rules_python, our own workaround is not complete.

We could also add a custom-formatted rule to the end of the README file after generation exclusively for pip rules.

Update our forks of bazel and skydoc

We continue to use the same hacks but at least all of the versions are updated, however this likely means we would need to revisit the same situation later. Unfortunately, our solutions are complete hacks and can't be merged upstream (we scan the filesystem for a file we know exists, README and then hack it in as a source route in the Stardoc tool initialisation.)

Completely remove stardoc, at least for now

Definitely the easiest and cleanest solution but we'd lose all documentation.

Write our own docs parser that doesn't use bazel directly

This is by far the hardest solution and may also end up being very unmaintainable.

adammitchelldev avatar Aug 19 '20 17:08 adammitchelldev

This issue is now blocking us from upgrading rules_pkg, which is not great.

The fastest solution might be the use of an http_archive patch to expose some targets that are currently private in rules_pkg. We should investigate this route in the near future. cc @lukas-slezevicius

alexjpwalker avatar Sep 20 '22 09:09 alexjpwalker

rules_nodejs has been deprecated for a couple of years now. Sooner or later, we'll need to unentangle this.

alexjpwalker avatar Jan 11 '23 18:01 alexjpwalker

This is resolved by updating to the latest versions of our dependencies, which as a rule facilitate the generation of stardoc documentation by either exposing bzl_library themselves or a filegroup which makes all files publicly visible.

flyingsilverfin avatar Jul 12 '23 15:07 flyingsilverfin