-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add wasmtime-c-api-impl to the list of crates to publish #7837
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
Add wasmtime-c-api-impl to the list of crates to publish #7837
Conversation
Wouldn't this line here need to be removed? I could be totally wrong wasmtime/crates/c-api/Cargo.toml Line 10 in 94b3e84
|
Thanks! I don't have a great suggestion for how to fix the namig problem, and it's probably not worth holding this up for fixing that. In addition to @amaanq's suggestion would you be up for helping spruce up the documentation a bit? For example I think it'd be good to have some basic crate documentation explaining what things are (as nothing has rustdoc internally otherwise) and ideally we'd also have a |
09d1218
to
e7f70e1
Compare
203eef4
to
e69274e
Compare
Ok, I updated the Cargo metadata, enabling publishing and documentation. In order to make it convenient for a downstream crate to include wasmtime's headers, I also added a small I added some content to the I also added a bit to the module-level doc comment for Let me know if there were other forms of documentation that make sense for this. |
Subscribe to Label Actioncc @peterhuene
This issue or pull request has been labeled: "wasmtime:c-api"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
257069d
to
f6ab8fe
Compare
ping @alexcrichton @peterhuene, this would be immensely helpful for tree-sitter to have the C API published to crates.io 😅 since this is a blocker for us to publish the latest release on crates.io |
I just realized I could test this with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks! I suspect that the publish.rs
may need reordering though as well, yeah. If you add prtest:full
to a commit message in this PR it'll run the full CI if you'd like (before it goes to the merge queue)
Note that most of the Wasmtime team has been at the BA plumbers summit this week and is traveling home today.
b736f58
to
e3e72a2
Compare
Ah ok, I'll do that. Thanks for responding while you're all traveling. |
1a7e0e7
to
ec0e122
Compare
Ah apologies now that I've gotten back to this it now has a merge conflict. If you don't mind rebasing again I'll add it to the merge queue. Otherwise though I can do the rebase/merge later this week. |
ec0e122
to
b174248
Compare
Thanks so much @alexcrichton. Just curious - Now that this is landed, would it be easy to publish to crates.io the |
Doing this for the current release would require a backport and we typically try not to backport features and instead only backport bugfixes. Not impossible but if you're ok I think this would be best to wait. That being said there's three active branches at the moment. |
Got it. I would love to backport to 18.0 if possible. Is that something I can help with, via a PR against that branch? |
Indeed that's all that's required, and please feel free! |
Ok, PR up: #7872 |
* Add wasmtime-c-api-impl to the list of crates to publish * Enable rustdoc and publishing for c-api crate * Provide paths to c-api headers as cargo links metadata * Add a README section about using wasm-c-api in a rust crate * In C API doc comment, mention use case for crates w/ C bindings * Enable publishing for wasmtime-c-api-macros (prtest:full) * Move c-api crates later in the publishing sequence (prtest:full)
Prior discussion: #7735
Having the C API available on crates.io is useful for crates that use C code that use Wasmtime via the C API, allowing those crates themselves to be published to crates.io.
I'm not sure if there are drawbacks to adding this; I just thought I'd open this PR as a concrete suggestion, in case it's uncontroversial.
Currently, the package name is
wasmtime-c-api-impl
(sincewasmtime-c-api
is used by the cdylib). That seems fine to me, but I'm also open to some name change.