Skip to content

Use native starlark_doc_extract rule for doc extraction if it is available#166

Merged
tetromino merged 24 commits intomasterfrom
starlark_doc_extract
Jul 25, 2023
Merged

Use native starlark_doc_extract rule for doc extraction if it is available#166
tetromino merged 24 commits intomasterfrom
starlark_doc_extract

Conversation

@tetromino
Copy link
Copy Markdown
Collaborator

@tetromino tetromino commented Jul 18, 2023

  • When available (i.e. in Bazel 7, or in current development Bazel at HEAD), try use the starlark_doc_extract native rule for doc extraction instead of the legacy pre-built extraction jar. This behavior can be disabled by passing use_starlark_doc_extract = False to the stardoc macro.
  • Add templates and markdown rendering functionality for repository rule and module extension info protos (which are output by starlark_doc_extract).
    • Temporary wart: for module extensions, by default we would want the summary blurb to look something like
my_ext = use_extension("@my_local_repo//some:file.bzl", "my_ext")
my_ext.tag(foo, bar)

but alas, we don't have a good way to get the name of the local repo from Starlark when bzlmod is enabled.

  • ... and of course, update tests. Which means in some cases, we have to fork the golden files into current (i.e. starlark_doc_extract-enabled) and legacy flavors.

Fixes #69
Fixes #76
Fixes #81
Fixes #123

@tetromino tetromino requested a review from brandjon as a code owner July 18, 2023 23:39
* Formats the label of the .bzl module being documented for use in documentation, falling back to
* "..." if the label is not available.
*/
// TODO(arostovtsev): we ought to add an option to semi-canonicalize .bzl file labels in the local
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I can look into fixing this as a follow-up based on native.module_name.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately, native.module_name can only be used in the loading phase :/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My idea was to add a module_name parameter to the stardoc macro that uses native.module_name as the default.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

On further reflection, I think the right place to fix this should be in starlark_doc_extract - we want to be able to optionally provide the local repo name for all labels (including e.g. label attribute defaults), in the proto output as well as in markdown.

The starlark macro can then provide the local repo map (obtained from native.module_name or a manual override for cases where the recommended repo name for a module's users is different from the module name) to the starlark_doc_extract target.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When you say "local repo name", which name are you referring to exactly? The apparent name used by the root module?

What would comprise the "local repo map" that you would want to have passed into the rule?

Copy link
Copy Markdown
Collaborator Author

@tetromino tetromino Jul 19, 2023

Choose a reason for hiding this comment

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

By "local repo", I mean the repo of the stardoc (and starlark_doc_extract) target.

Currently, starlark_doc_extract treats that repo as if it were the main repo for the purpose of stringifying labels in proto output. This behavior made sense to me at the time when I implemented it because it meant starlark_doc_extract's output is the same regardless of whether doc targets are being built in their own repo or by a user of the repo, and thus made golden tests sane/predictable.

In retrospect, the correct behavior should have been to allow the name of the local repo to be remapped to a custom one when stringifying labels.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I submitted bazelbuild/bazel#19002, please let me know whether this is what you had in mind.

@tetromino tetromino merged commit f8fab82 into master Jul 25, 2023
@tetromino tetromino deleted the starlark_doc_extract branch July 25, 2023 01:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants