Skip to content

feat: generate doc_extract targets #571

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alexeagle
Copy link
Contributor

@alexeagle alexeagle commented May 12, 2025

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

Copy link
Collaborator

@comius comius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@alexeagle
Copy link
Contributor Author

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:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

selects don't work in a macro

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

@comius
Copy link
Collaborator

comius commented May 28, 2025

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?

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

@alexeagle
Copy link
Contributor Author

What's the intent?

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.

@alexeagle
Copy link
Contributor Author

Another intent here is merely to ensure that rulesets have correct bzl_library targets.

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 proto_library having no descriptor actions and therefore no validation of the deps until some lang_proto_library rule depends on it.

@rickeylev
Copy link

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.

@alexeagle
Copy link
Contributor Author

starlark_doc_extract DOES take the bzl_library as an input. It's okay that you need one per source file, just by iterating them, i.e.

+def bzl_library(name, srcs = [], deps = [], **kwargs):    
+    _bzl_library(
+        name = name,
+        srcs = srcs,
+        deps = deps,
+        **kwargs
+    )
+    
+    if hasattr(native, "starlark_doc_extract"):
+        targets = []
+        for i, src in enumerate(srcs):
+            extract_target = "{}.doc_extract{}".format(name, i if i > 0 else "")
+            native.starlark_doc_extract(
+                name = extract_target,
+                src = src,
+                deps = [name],
+            )
+            targets.append(extract_target)
+        native.filegroup(
+            name = "{}.docs-as-proto".format(name),
+            srcs = targets,
+        )

What I'm looking for in this PR is just a more ergonomic way to run the action behind starlark_doc_extract WITHOUT actually forcing developers to put things in their BUILD files or ship a macro. So that would be an aspect that adds validation actions onto the existing bzl_library graph. The outputs of those validation actions could be only in the _validation OutputGroup, and also could be in another named output group for tooling that wants to collect the protos.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bzl_library does not validate the inputs are correct
3 participants