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

feature request: bindings subcommand #362

Closed
dakom opened this issue Dec 13, 2024 · 5 comments · Fixed by #366
Closed

feature request: bindings subcommand #362

dakom opened this issue Dec 13, 2024 · 5 comments · Fixed by #366

Comments

@dakom
Copy link

dakom commented Dec 13, 2024

When working on a component locally, I often go through this flow:

  1. Make changes to the wit
  2. Generate the new bindings
  3. Make changes to the component

Currently, afaik, the way to generate the new bindings is to run a full cargo component build. This has a few drawbacks:

  1. It inevitably has errors, because I don't make the rest of the code changes until I have the new bindings to build against
  2. It's a bit unintuitive, I genuinely don't want to build the component, feels a bit like using the non-optimal command for its side-effect
  3. It's doing more than necessary (I think), a bindings-only subcommand would likely be much faster

I think it would be a tooling improvement to have an explicit subcommand that just generates the bindings, without building the full component

@sunfishcode
Copy link
Member

A somewhat lighter-weight alternative that works today is cargo component check. It still prints the errors, but it doesn't do as much work so it is faster. That said, I agree, a way to generate just the bindings is a good idea.

@vados-cosmonic
Copy link

This is a slightly bigger change, but what @sunfishcode what do you think about re-introducing a macro to generate the bindings, or a build.rs?

Reading over the previous context it seems like we removed it in order to increase build speed, but it really seems like what we want is to hook into the build process and ensure bindings are built (and if they can't be, to error early/quickly) -- I think in most of the ecosystem outside of cargo-component this is done with macros that possibly output to local files for easy review (otherwise people lean on cargo-expand)...

I agree with both of you here and want to point out that the experience of cargo component new and being greeted with tons of "red squigglies" isn't great. My immediate first thought was:

oh, I should add information about the binding generation command to component-docs so that people can at least know to run that before the open src/lib.rs

And then found this issue because it doesn't exist (yet!). That made me start to think again about the whole starting experience here.

sunfishcode added a commit to sunfishcode/cargo-component that referenced this issue Jan 3, 2025
Add a `bindings` subcommand, which just generates the bindings.rs file.

And make the `new` subcommand auto-generate the bindings.rs file too, so
that the tree is fully set up after a `new`.

Fixes bytecodealliance#362.
@sunfishcode
Copy link
Member

I've now submitted #366 which adds a bindings subcommand which just regenerates the bindings.

@vados-cosmonic I also added some code to auto-generate the bindings as part of cargo component new, which should improve the startup experience.

I'm not opposed to larger changes, but I'm curious if these simple fixes are enough.

@vados-cosmonic
Copy link

Hey @sunfishcode IMO that's perfect, thanks -- the bindings subcommand was more than enough, and the new integration sounds like it'll be a very ergonomic change.

The README and related documentation should probably be changed to match as well, but I'm in favor of making that a separate, focused issue (so we can capture the parts to change, and maybe even do a bit of a docs overhaul for this crate).

@sunfishcode
Copy link
Member

The new bindings subcommand and cargo component new improvements are now released in cargo-component 0.20.

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 a pull request may close this issue.

3 participants