bazelbuild / stardoc

Stardoc: Starlark Documentation Generator
Apache License 2.0
108 stars 45 forks source link

stardoc should gracefully handle borked dependencies #128

Closed jonathan-enf closed 1 year ago

jonathan-enf commented 2 years ago

Currently, stardoc recurses through all loaded .bzl files to produce correct documentation. This requires a correct bzl_library (with correct dependencies and visibility) not only for the .bzl file being documented, but for the complete transitive dependency set.

This makes stardoc fragile. In my use case, one package added a dependency on rule_pkg.bzl, which had visibility set incorrectly (already fixed in https://github.com/bazelbuild/rules_pkg/commit/76f5651ed90209ffb9eff40fef3cee19a1f1e8a3 but not yet released), which broke our automated stardoc-based generation flow.

In the wild and wooly world of the internets, this is just going to happen from time to time. It would be great if stardoc could more gracefully handle broken dependencies: just raise a warning that some loaded block couldn't be found, but then valiantly soldier on using the information that is available. For the vast majority of cases, a missing dependency won't matter.

Thanks!

jonathan-enf commented 2 years ago

Just pinging this issue. I felt certain this issue would be a dupe of an existing issue, but couldn't find such an issue when I searched the issues list. Is this a reasonable request?

tetromino commented 2 years ago

This is a reasonable request. It cannot be accomplished with the current architecture, but we will try to fix it as part of the rewrite in #69.

jonathan-enf commented 2 years ago

In case this is useful for anyone else, I created a very limited documentation generator that solves this issue for me and released it under GPL license. https://github.com/enfabrica/enkit/tree/master/tools/bzldoc

It works by parsing bzl files itself with the python AST -- but doesn't do anything fancy like import rules from other packages or transform data. I suspect that eventually bazel will implement a feature enabling it to dump fully processed rule metadata itself -- when that happens, I'll adapt bzldoc to use that metadata instead of parsing bzl itself.

tetromino commented 1 year ago

I tried, but alas, it turned out to be too difficult given realistic time constraints. There are systems out there which rely on bazel query to produce the set of targets on which any given target - such as the .md file emitted by stardoc - depends. And bazel query can only query the target graph, not the loading graph; allowing it to query the loading graph without significantly degrading performance on large repos would be technically quite challenging (as in: quite probably several months of work). Which means we still have to express stardoc's deps in the target graph, not just in the loading graph. Which means we have to continue using bzl_libary or a moral equivalent.

So given a borked dependency, the best we can do is fail with a somewhat clearer and more actionable error message. See https://github.com/bazelbuild/bazel/commit/889bb08755742acaaa1d540f41c90464a84771b5

Perhaps we can revisit this in a year or two - but for now, I'm closing the issue.