Stream: git-wasmtime

Topic: wasmtime / PR #6886 Host resources wit-bindgen code gener...

view this post on Zulip Wasmtime GitHub notifications bot (Aug 22 2023 at 20:40):

silesmo opened PR #6886 from Nor2-io:resources-basic to bytecodealliance:main:

Implements host export resource generation #6722

After a discussion with @alexcrichton, I have broken out basic resource generation for host resource exports into it's own PR,
A host resource generation will follow, as well as more ergonomic resource code generation that abstracts away the raw resource handles.

Example implementation: https://github.com/Nor2-io/wasmtime-resources-example/blob/main/example-host-export-index/src/main.rs

Example code generation output: https://github.com/Nor2-io/wasmtime-resources-example/blob/main/example-host-export-index/main%20copy%202.rs

Sneak peak of future additions that abstract away resources can be seen in the link below. (It doesn't work with nested resources yet.)
https://github.com/Nor2-io/wasmtime-resources-example/blob/main/example-host-import/src/main.rs
https://github.com/Nor2-io/wasmtime-resources-example/blob/main/example-host-export/src/main.rs

view this post on Zulip Wasmtime GitHub notifications bot (Aug 22 2023 at 20:40):

silesmo requested alexcrichton for a review on PR #6886.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 22 2023 at 20:40):

silesmo requested wasmtime-core-reviewers for a review on PR #6886.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 22 2023 at 20:40):

silesmo requested wasmtime-default-reviewers for a review on PR #6886.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 22 2023 at 21:00):

silesmo updated PR #6886.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 22 2023 at 21:04):

silesmo updated PR #6886.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 22 2023 at 21:05):

silesmo edited PR #6886:

Implements host export resource generation #6722

After a discussion with @alexcrichton, I have broken out basic resource generation for host resource exports into it's own PR,
Host import resource generation will follow, as well as more ergonomic resource code generation that abstracts away the raw resource handles.

Example implementation: https://github.com/Nor2-io/wasmtime-resources-example/blob/main/example-host-export-index/src/main.rs

Example code generation output: https://github.com/Nor2-io/wasmtime-resources-example/blob/main/example-host-export-index/main%20copy%202.rs

Sneak peak of future additions that abstract away resources can be seen in the link below. (It doesn't work with nested resources yet.)
https://github.com/Nor2-io/wasmtime-resources-example/blob/main/example-host-import/src/main.rs
https://github.com/Nor2-io/wasmtime-resources-example/blob/main/example-host-export/src/main.rs

view this post on Zulip Wasmtime GitHub notifications bot (Aug 22 2023 at 21:33):

silesmo updated PR #6886.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 22 2023 at 21:45):

silesmo updated PR #6886.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 22 2023 at 22:09):

silesmo updated PR #6886.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 22 2023 at 22:10):

silesmo updated PR #6886.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 22 2023 at 22:25):

silesmo edited PR #6886:

Implements host export resource generation #6722

After a discussion with @alexcrichton, I have broken out basic resource generation for host resource exports into it's own PR,
Host import resource generation will follow, as well as more ergonomic resource code generation that abstracts away the raw resource handles.

Example implementation: https://github.com/Nor2-io/wasmtime-resources-example/blob/main/example-host-export-index/src/main.rs

Example code generation output: https://github.com/Nor2-io/wasmtime-resources-example/blob/main/example-host-export-index/main%20copy%202.rs

Sneak peak of future additions that abstract away resources can be seen in the link below. (It doesn't work with nested resources yet.)
https://github.com/Nor2-io/wasmtime-resources-example/blob/main/example-host-import/src/main.rs
https://github.com/Nor2-io/wasmtime-resources-example/blob/main/example-host-export/src/main.rs

#62

view this post on Zulip Wasmtime GitHub notifications bot (Aug 22 2023 at 22:26):

silesmo edited PR #6886:

Implements host export resource generation #6722

After a discussion with @alexcrichton, I have broken out basic resource generation for host resource exports into it's own PR,
Host import resource generation will follow, as well as more ergonomic resource code generation that abstracts away the raw resource handles.

Example implementation: https://github.com/Nor2-io/wasmtime-resources-example/blob/main/example-host-export-index/src/main.rs

Example code generation output: https://github.com/Nor2-io/wasmtime-resources-example/blob/main/example-host-export-index/main%20copy%202.rs

Sneak peak of future additions that abstract away resources can be seen in the link below. (It doesn't work with nested resources yet.)
https://github.com/Nor2-io/wasmtime-resources-example/blob/main/example-host-import/src/main.rs
https://github.com/Nor2-io/wasmtime-resources-example/blob/main/example-host-export/src/main.rs

Added temporary exception for cargo deny on multiple versions until memfd has been updated: #62

view this post on Zulip Wasmtime GitHub notifications bot (Aug 22 2023 at 22:32):

alexcrichton submitted PR review:

Thanks again for this! I'm running out of time today but wanted to give some quick feedback on this. There's a few comments below and I have more thoughts on the generated code but I'll try to flesh that out more tomorrow (I'll probably send a PR-to-your-PR in the morning)

In the meantime though this'll need to have some tests as well, both at crates/component-macro/tests/codegen/*.wit and additionally one in tests/all/component_model/*. You'll find some other bindgen-related tests in that directory too.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 22 2023 at 22:32):

alexcrichton submitted PR review:

Thanks again for this! I'm running out of time today but wanted to give some quick feedback on this. There's a few comments below and I have more thoughts on the generated code but I'll try to flesh that out more tomorrow (I'll probably send a PR-to-your-PR in the morning)

In the meantime though this'll need to have some tests as well, both at crates/component-macro/tests/codegen/*.wit and additionally one in tests/all/component_model/*. You'll find some other bindgen-related tests in that directory too.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 22 2023 at 22:32):

alexcrichton created PR review comment:

I think there's a number of unrelated updates in this lock file, could those be backed out in favor of just those that are necessary?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 22 2023 at 22:32):

alexcrichton created PR review comment:

Could the formatting-related changes in this file be backed out?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 22 2023 at 22:32):

alexcrichton created PR review comment:

Mind updating the docs as well to indicating what the return value of this closure means?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 22 2023 at 22:32):

alexcrichton created PR review comment:

I think this (and the other wasi-http changes) could be separated out from this?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 22 2023 at 22:33):

alexcrichton assigned PR #6886 to alexcrichton.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 06:38):

silesmo submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 06:38):

silesmo created PR review comment:

Yes, If I add an exception for cargo deny I guess? I just made the updates to not have duplicated dependencies to pass Cargo deny. Is that the way to go and then a PR that makes the change and removes the exception or? I haven't used cargo deny in a work flow before so I appreciate guidance on this :smile:

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 06:38):

silesmo edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 06:41):

silesmo submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 06:41):

silesmo created PR review comment:

Will do!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 06:41):

silesmo submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 06:41):

silesmo created PR review comment:

Yes will revert.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 06:43):

silesmo submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 06:43):

silesmo created PR review comment:

I will take a look.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 07:15):

silesmo updated PR #6886 (assigned to alexcrichton).

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 07:57):

silesmo updated PR #6886 (assigned to alexcrichton).

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 08:02):

silesmo updated PR #6886 (assigned to alexcrichton).

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 08:19):

silesmo updated PR #6886 (assigned to alexcrichton).

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 08:21):

silesmo submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 08:21):

silesmo created PR review comment:

added doc comment

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 08:21):

silesmo submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 08:21):

silesmo created PR review comment:

done

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 08:22):

silesmo submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 08:22):

silesmo created PR review comment:

Moved into it's own PR: #6886

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 08:30):

silesmo updated PR #6886 (assigned to alexcrichton).

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 08:34):

silesmo updated PR #6886 (assigned to alexcrichton).

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 09:00):

silesmo updated PR #6886 (assigned to alexcrichton).

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 09:03):

silesmo created PR review comment:

removed the updates, so now the cargo-deny errors are back.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 09:03):

silesmo submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2023 at 20:05):

silesmo updated PR #6886 (assigned to alexcrichton).

view this post on Zulip Wasmtime GitHub notifications bot (Aug 24 2023 at 13:21):

silesmo updated PR #6886 (assigned to alexcrichton).

view this post on Zulip Wasmtime GitHub notifications bot (Aug 24 2023 at 14:26):

alexcrichton submitted PR review:

Thanks again @silesmo for all your work on this!

Since this now has bits and pieces from me too I'm going to ask @pchickey to do a once-over on this to confirm it's all reasonable, but otherwise I think this is good to go :+1:

view this post on Zulip Wasmtime GitHub notifications bot (Aug 24 2023 at 21:37):

alexcrichton requested pchickey for a review on PR #6886.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 28 2023 at 18:39):

alexcrichton has enabled auto merge for PR #6886.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 30 2023 at 02:50):

alexcrichton has disabled auto merge for PR #6886.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 30 2023 at 02:50):

silesmo updated PR #6886 (assigned to alexcrichton).

view this post on Zulip Wasmtime GitHub notifications bot (Aug 30 2023 at 02:57):

alexcrichton has enabled auto merge for PR #6886.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 30 2023 at 03:58):

alexcrichton merged PR #6886.

Last updated: Nov 22 2024 at 17:03 UTC