Stream: git-wasmtime

Topic: wasmtime / PR #10566 c-api: Compile a component


view this post on Zulip Wasmtime GitHub notifications bot (Apr 11 2025 at 14:05):

MangoPeachGrape opened PR #10566 from MangoPeachGrape:c-api/component-model/compile to bytecodealliance:main:

First little bit of #9812, "compile a component", with some changes suggested in the original pr:

Based on the efforts of @lanfeust69 and @rockwotj.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 11 2025 at 14:05):

MangoPeachGrape requested wasmtime-core-reviewers for a review on PR #10566.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 11 2025 at 14:05):

MangoPeachGrape requested fitzgen for a review on PR #10566.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 11 2025 at 15:41):

alexcrichton submitted PR review:

Thanks!

view this post on Zulip Wasmtime GitHub notifications bot (Apr 11 2025 at 15:41):

alexcrichton created PR review comment:

Could this be called wasmtime_component_new to look similar to modules?

view this post on Zulip Wasmtime GitHub notifications bot (Apr 11 2025 at 15:41):

alexcrichton created PR review comment:

Could this be changed to <wasmtime/component/component.h> like other wasmtime-specific imports?

view this post on Zulip Wasmtime GitHub notifications bot (Apr 11 2025 at 15:42):

alexcrichton commented on PR #10566:

Also if you're up for it feel free to fill out this header to match the module.h header, e.g. with deserializing and such. Reflection on imports/exports though is going to be a "big deal" so deferring that is probably best.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 11 2025 at 16:18):

MangoPeachGrape updated PR #10566.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 11 2025 at 16:29):

MangoPeachGrape commented on PR #10566:

Also if you're up for it feel free to fill out this header to match the module.h header, e.g. with deserializing and such. Reflection on imports/exports though is going to be a "big deal" so deferring that is probably best.

Added wasmtime_component_serialize() and wasmtime_component_serialize(). Should I do wasmtime_module_deserialize_file() as well, or any other functions?

For the documentation in the header file I did s/module/component/, so it might not be accurate. In general, do you want documentation on everything in the header files in these early commits, or is that something that can be added in PRs down the line?

view this post on Zulip Wasmtime GitHub notifications bot (Apr 11 2025 at 16:32):

MangoPeachGrape edited a comment on PR #10566:

Also if you're up for it feel free to fill out this header to match the module.h header, e.g. with deserializing and such. Reflection on imports/exports though is going to be a "big deal" so deferring that is probably best.

Added wasmtime_component_serialize() and wasmtime_component_deserialize(). Should I do wasmtime_module_deserialize_file() as well, or any other functions?

For the documentation in the header file I did s/module/component/, so it might not be accurate. In general, do you want documentation on everything in the header files in these early commits, or is that something that can be added in PRs down the line?

view this post on Zulip Wasmtime GitHub notifications bot (Apr 11 2025 at 16:48):

alexcrichton commented on PR #10566:

wasmtime_component_deserialize_file sounds good to add yeah, and maybe wasmtime_component_clone as well, but otherwise it looks like other functions aren't as applicable yet so that's probably good for now.

For docs I find it best to add it when the functions come in, but I'll admit during review I generally gloss over docs. I can give them a once-over to confirm everything is right after deserialize_file is added

view this post on Zulip Wasmtime GitHub notifications bot (Apr 11 2025 at 17:53):

MangoPeachGrape updated PR #10566.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 11 2025 at 18:02):

MangoPeachGrape commented on PR #10566:

I'm unsure if all the consts are correct, for example in wasmtime_component_clone(const wasmtime_component_t *component), because wasmtime_module_clone(wasmtime_module_t *m) doesn't have const?

Should the rust file also be split up, similar to the header files?

view this post on Zulip Wasmtime GitHub notifications bot (Apr 11 2025 at 19:55):

alexcrichton commented on PR #10566:

Ah yeah the const there is ok, and it's one where we aren't necessarily the best-principled, but adding const when it maps to &T in Rust doesn't hurt for sure.

For file organization, yeah if you're up for it I think it would be a good idea. We'll be filling this out more in the future so I think it makes sense to separate everything out by file and starting here feels a bit odd as it's so small but it's predicted to get larger and be more justified in the future.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 11 2025 at 20:17):

MangoPeachGrape updated PR #10566.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 11 2025 at 20:21):

MangoPeachGrape commented on PR #10566:

For file organization, yeah if you're up for it I think it would be a good idea. We'll be filling this out more in the future so I think it makes sense to separate everything out by file and starting here feels a bit odd as it's so small but it's predicted to get larger and be more justified in the future.

Done!

What should the scope be for the next PR?

view this post on Zulip Wasmtime GitHub notifications bot (Apr 12 2025 at 21:41):

MangoPeachGrape updated PR #10566.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 14 2025 at 14:22):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 14 2025 at 14:25):

alexcrichton commented on PR #10566:

Personally I think one major missing piece of foundation for the C API is a good testing story. We don't have many tests for the C API and right now they're all classified as examples rather than unit tests. They're also awkward to write because C is pretty awkward to write.

To solve this one thing we've wanted to do was to fold the wasmtime-cpp repository (C++ bindings build on the C API) into this repository. That would make it much easier to write tests and then we could whip up some cmake as well to get tests.

I know writing tests isn't the most glamorous thing but it's a major thing I'd personally like to see before much of the rest of the C API becomes load bearing because I think the conversion of values is going to be significantly trickier than the wasm C API and warrant more thorough testing.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 14 2025 at 15:28):

MangoPeachGrape updated PR #10566.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 14 2025 at 16:34):

MangoPeachGrape commented on PR #10566:

Yes, tests would be great!

All the tests would use GoogleTest, would you still seperate the tests for the C api and the C++ api, or would you have those combined?

Is this something you would want me to take a jab at? I'm not that familiar with the codebase, so I would need some guiding where to put everything.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 14 2025 at 16:40):

alexcrichton commented on PR #10566:

GoogleTest would be fine yeah, and I think it'd be reasonable to test everything together. I'd expected we'd test C++ APIs by deafult (by moving wasmtime.hh to this repository) and then we could write one-off tests in C if necessary.

If you're willing to work on this that'd be great! Most of us in this repo aren't really all that well-versed in C/C++ repository management so if you know of practices/idioms around organization/testing/CI/etc that'd be very helpful. Most of the source would probably want to live in crates/c-api or some sub-folder, and I'm happy to help out with CI integration as well.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 14 2025 at 16:43):

alexcrichton merged PR #10566.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 14 2025 at 21:42):

MangoPeachGrape commented on PR #10566:

@alexcrichton, Some questions that that came up while moving the C++ api into this repository:

view this post on Zulip Wasmtime GitHub notifications bot (Apr 14 2025 at 21:54):

alexcrichton commented on PR #10566:

Yeah let's put examples in examples/{example}.{rs,c,cpp} and in theory we can fill out that matrix over time. For tests I think crates/c-api/tests is a good spot too.

The C++ API build step from an end consumable artifact point of view should just be a header file so I think it's fine to always enable/copy that around. For the tests I think it's also reasonable to require C++ to build tests since that's just for developers which should be easier in general. Given that I'd hope we can skip CMake options to conditionalize the C++ usage and basically just always use it


Last updated: Dec 06 2025 at 07:03 UTC