Wasmtime GitHub notifications bot (Jan 08 2024 at 05:33): Wasmtime GitHub notifications bot (Jan 08 2024 at 05:33):rockwotj opened PR #7759 from rockwotj:component-example to bytecodealliance:main.
Wasmtime GitHub notifications bot (Jan 08 2024 at 05:34):rockwotj updated PR #7759.
Wasmtime GitHub notifications bot (Jan 08 2024 at 05:35):rockwotj edited PR #7759:
As part of my journey on learning how the component model works, I realized there is no example in wasmtime, so I figured it'd be useful to add a simple example of guest and host calls.
Wasmtime GitHub notifications bot (Jan 08 2024 at 05:37):rockwotj updated PR #7759.
Wasmtime GitHub notifications bot (Jan 08 2024 at 05:49):rockwotj has marked PR #7759 as ready for review.
Wasmtime GitHub notifications bot (Jan 08 2024 at 05:49):rockwotj requested alexcrichton for a review on PR #7759.
Wasmtime GitHub notifications bot (Jan 08 2024 at 05:49):rockwotj requested wasmtime-core-reviewers for a review on PR #7759.
Wasmtime GitHub notifications bot (Jan 08 2024 at 05:49):rockwotj requested wasmtime-default-reviewers for a review on PR #7759.
Wasmtime GitHub notifications bot (Jan 08 2024 at 15:46):alexcrichton submitted PR review:
Thanks for this!
I'm somewhat of two minds on this. On one hand this example showcases a lot of downsides to the unsupported nature of the component model in languages. For example embeddings may not always embed
wit-componentitself and turn core wasm into components on-the-fly. Additionally this probably won't work for many "real world" use cases in the sense that WASI isn't handled here. That would require tweaks to both the build process and the embedder. Finaly thewasm32-unknown-unknowntarget, rarely used off the web, is used here (for simplicity of course, which makes sense)On the other hand this showcases a concrete working example which is something folks can base work on. In that sense it's great to have! Given the number of moving parts in the component model being able to start somewhere is definitely valuable.
I'd probably lean towards landing this, although perhaps with a few more comments with respect to WASI and usage. Could you add a comment indicating that
wasm32-unknown-unknownis used for simplicity here and additionally indicate that the usage ofwit-componentin embeddings is done for the purposes of this example and otherwise may not be expected in all host embeddings of Wasmtime?
Wasmtime GitHub notifications bot (Jan 08 2024 at 15:46):alexcrichton submitted PR review:
Thanks for this!
I'm somewhat of two minds on this. On one hand this example showcases a lot of downsides to the unsupported nature of the component model in languages. For example embeddings may not always embed
wit-componentitself and turn core wasm into components on-the-fly. Additionally this probably won't work for many "real world" use cases in the sense that WASI isn't handled here. That would require tweaks to both the build process and the embedder. Finaly thewasm32-unknown-unknowntarget, rarely used off the web, is used here (for simplicity of course, which makes sense)On the other hand this showcases a concrete working example which is something folks can base work on. In that sense it's great to have! Given the number of moving parts in the component model being able to start somewhere is definitely valuable.
I'd probably lean towards landing this, although perhaps with a few more comments with respect to WASI and usage. Could you add a comment indicating that
wasm32-unknown-unknownis used for simplicity here and additionally indicate that the usage ofwit-componentin embeddings is done for the purposes of this example and otherwise may not be expected in all host embeddings of Wasmtime?
Wasmtime GitHub notifications bot (Jan 08 2024 at 15:46):alexcrichton created PR review comment:
For this the
default-features = falseisn't probably the best example to give since default features should be ok for 99% of embeddings. The reason it's disabled in this repository is for the unique use case of the adapter.
Wasmtime GitHub notifications bot (Jan 08 2024 at 15:46):alexcrichton created PR review comment:
I think this
usecan be omitted?Also, can this file have a comment pointing to the
wit-bindgenrepository for more information about the generated bindings?
Wasmtime GitHub notifications bot (Jan 09 2024 at 18:48):rockwotj submitted PR review.
Wasmtime GitHub notifications bot (Jan 09 2024 at 18:48):rockwotj created PR review comment:
Done.
Wasmtime GitHub notifications bot (Jan 09 2024 at 18:48):rockwotj updated PR #7759.
Wasmtime GitHub notifications bot (Jan 09 2024 at 18:48):rockwotj submitted PR review.
Wasmtime GitHub notifications bot (Jan 09 2024 at 18:48):rockwotj created PR review comment:
Fixed by using the default.
Wasmtime GitHub notifications bot (Jan 09 2024 at 18:52):rockwotj commented on PR #7759:
Thanks for the review! I ended up submitting this because I wanted to:
1) Have a place where there was a single simple as possible example of the component model that was in code. Everywhere else I could find were documentation of the steps of how to do things and were not specific to wasmtime (so didn't include how to create custom worlds using wasmtime's binding macro).
2) Learn more about the generated bindings viacargo expandso I know what to expose via the C-APII thought the first use case was something others wanted to do, so I ended up creating this. I whole heartedly agree with your comments and have updated the PR.
Wasmtime GitHub notifications bot (Jan 09 2024 at 18:52):rockwotj requested alexcrichton for a review on PR #7759.
Wasmtime GitHub notifications bot (Jan 09 2024 at 18:53):rockwotj updated PR #7759.
Wasmtime GitHub notifications bot (Jan 09 2024 at 21:55):alexcrichton submitted PR review:
Sounds good!
Wasmtime GitHub notifications bot (Jan 09 2024 at 22:23):alexcrichton merged PR #7759.
Last updated: Nov 22 2024 at 16:03 UTC