Conversation
comius
left a comment
There was a problem hiding this comment.
A PR like this would create a lot of additional targets. There's a cost connected with each additional target. Such feature would need to be disabled at Google.
A lot of automatically created targets are potentially unused. Usually to generate documentation of one rules_* repo, you need a couple of starlar_doc_extract rules, while the number of bzl_libraries is much larger.
|
What do you think of the alternative: making it possible to write an aspect that invokes starlark_doc_extract actions? If someone worked on such a PR to bazelbuild/bazel do you think it might be accepted? |
Allows bzl_library targets to produce documentation without needing to load stardoc or add additional targets. Note, it would be better to do this with an aspect that visits the bzl_library graph, but the starlark_doc_extract rule is in Bazel java core, and so we cannot spawn actions using its Java code. It needs a main() entry point. Use a config_setting to make this opt-in, so we don't generate so many targets all the time.
| Label("//:extract_docs_flag"): hasattr(native, "starlark_doc_extract"), | ||
| "//conditions:default": False, | ||
| }) | ||
| if enable_doc_extract: |
There was a problem hiding this comment.
selects don't work in a macro
There was a problem hiding this comment.
Yeah oops, evaluation is deferred until after the loading phase, at which point both branches were taken and the targets were created anyway. I didn't manage to test this yesterday since it was hard to back-port to 1.7.1.
(Aside: could we get a release of this module? It has been nearly a year)
I don't fully understand how to make such an aspect possible without incurring tech-debt. What's the intent? Automatically extracting all the docs? I think we have some support in query to do that. cc @tetromino |
Yes I'm experimenting with a pipeline to generate docs, similar to readthedocs. @rickeylev and I discussed in a rules authors SIG meeting. https://github.com/alexeagle/doc.bzl is the prototype. |
|
Another intent here is merely to ensure that rulesets have correct As I'm visiting the 720 modules on BCR I find that many are missing deps, as one example aspect-build/rules_swc#305 You might say those repos ought to include more testing to avoid such a defect, but this is extra burden on those maintainers. It's akin to |
|
Another approach would be to let starlark_doc_extract take a bzl_library as input? Or multiple files as inputs. If memory serves, it isn't possible today because of the combination of bzl_library not returning a single output, and starlark_doc_extract requiring a single_file_only input. In rules_python, I worked around this by having a custom rule take a bzl_library as input and produce a single output to then feed into starlark_doc_extract, and just forcing the convention of every bzl_library having a single srcs. |
|
What I'm looking for in this PR is just a more ergonomic way to run the action behind |
Allows bzl_library targets to produce documentation without needing to load stardoc or add additional targets. Note, it would be better to do this with an aspect that visits the bzl_library graph, but the starlark_doc_extract rule is in Bazel java core, and so we cannot spawn actions using its Java code. It needs a main() entry point.
Fixes #568