Skip to content
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

Include Bzlmod globals in builtin.proto #21929

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Apr 8, 2024

This allows consumers of the proto to learn about globals only available in MODULE.bazel under a new ApiContext MODULE.

Also adds the ApiContext.BZL globals for repository rule and module extension definition.

A new smoke test verifies that common symbols are contained in the proto for each API context.

Work towards bazel-contrib/vscode-bazel#1

@fmeum fmeum requested a review from meteorcloudy April 8, 2024 18:54
@github-actions github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Apr 8, 2024
@fmeum
Copy link
Collaborator Author

fmeum commented Apr 8, 2024

FYI @withered-magic

@meteorcloudy Not sure who to assign for this part of the codebase, feel free to reassign.

@fmeum fmeum marked this pull request as draft April 8, 2024 19:18
@fmeum
Copy link
Collaborator Author

fmeum commented Apr 8, 2024

Need to figure out how to add parameters.

Parameters are now available.

@fmeum fmeum marked this pull request as ready for review April 8, 2024 22:16
This allows consumers of the proto to learn about globals only available in `MODULE.bazel` under a new `ApiContext`.

Also adds a smoke test to verify that common symbols are contained in the proto for each API context.
@iancha1992 iancha1992 added the team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. label Apr 8, 2024
@fmeum fmeum changed the title Include module file globals in builtin.proto Include Bzlmod globals in builtin.proto Apr 9, 2024
@fmeum
Copy link
Collaborator Author

fmeum commented Apr 9, 2024

I pushed a new commit to add module_extension and repository_rule, which were also missing.

@meteorcloudy meteorcloudy requested a review from tetromino April 9, 2024 08:58
@meteorcloudy
Copy link
Member

@tetromino are you the right person to review this? It's not too complicated and looks fine to me.

@fmeum
Copy link
Collaborator Author

fmeum commented Apr 17, 2024

@tetromino Friendly ping

@fmeum
Copy link
Collaborator Author

fmeum commented May 11, 2024

@tetromino Gentle ping, following up on #21936 (comment)

@fmeum
Copy link
Collaborator Author

fmeum commented Jun 4, 2024

@tetromino Friendly ping

@vogelsgesang
Copy link

vogelsgesang commented Jun 7, 2024

Hi @tetromino,

Thanks for sharing your thoughts as well as an alternative design earlier this year, and for capturing our feedback on additional requirements for the LSP use cases in #21979 🙂

I still agree with the overall design: aligning the generation of documentation pages and language servers to both rely on Stardoc protos is a very good solution. This makes sure that the published documentation and the in-editor suggestions from the LSP will stay in sync.

I wonder if you can share additional details on your envisioned design, timeline, roadmap by now? Maybe the opensource community could contribute somehow, if we know what you are working towards and which design and work items you are having in mind?

As one of the maintainers of the Bazel VSCode extension, I want to integrate a language server directly into the VSCode extension, such that all users can benefit from auto-completion, signature help etc. However, with the current state of both language servers (bazel-lsp by @cameron-martin and starpls by @withered-magic), I don't feel comfortable shipping them to a larger user base. We would immediately get bug reports that autocompletion only works unreliably or even not at all for bzlmod files. As such, I need your help to make this happen 🙂

I guess that Google has its own, internal language server for Starlark / Bazel and hence I understand that this is not your highest priority. If you could share your roadmap, envisioned design etc. we, the community, could maybe fill in the blanks to pave the path towards a widely available Bazel language server.

Alternatively (e.g., in case the stardoc project is not getting sufficient funding inside Google for whatever reason), I would appreciate if we could reconsider merging this pull request as well as #21135, at least as a stop gap solution until we can work towards the long-term stardoc-based solution

@tetromino
Copy link
Contributor

Hi @vogelsgesang

I wonder if you can share additional details on your envisioned design, timeline, roadmap by now? Maybe the opensource community could contribute somehow, if we know what you are working towards and which design and work items you are having in mind?

Sure.

Step zero was a series of refactorings and cleanups around stardoc_output.proto, which was finished by mid-May.

Step one was adding a way to represent the syntactic role of function params (fixed by bd1c3af) so that a documentation renderer can output *args, *, and **kwargs correctly in all cases.

Steps two and three will be to represent 2 additional bits of data in stardoc_output.proto:

I hope get this done in early July.

@vogelsgesang
Copy link

Thanks for the update and the rough timeline!

Are the remaining steps 2 and 3 all that's left to be done?

Afaik, there is currently no way to dump Bazel's builtin functions, yet. From the Stardoc documentation, my understanding is that it is only possible to run stardoc on Starlark files? But Bazel's builtins are not all migrated to Starlark, and some of them probably never will.

Is there still some additional command (e.g., bazel info builtin-symbols-as-proto, similar to #21936) missing? Or is this also covered by #21979?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review PR is awaiting review from an assigned reviewer team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants