Wasmtime GitHub notifications bot (Aug 22 2023 at 20:40): 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
Wasmtime GitHub notifications bot (Aug 22 2023 at 20:40):silesmo requested alexcrichton for a review on PR #6886.
Wasmtime GitHub notifications bot (Aug 22 2023 at 20:40):silesmo requested wasmtime-core-reviewers for a review on PR #6886.
Wasmtime GitHub notifications bot (Aug 22 2023 at 20:40):silesmo requested wasmtime-default-reviewers for a review on PR #6886.
Wasmtime GitHub notifications bot (Aug 22 2023 at 21:00):silesmo updated PR #6886.
Wasmtime GitHub notifications bot (Aug 22 2023 at 21:04):silesmo updated PR #6886.
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
Wasmtime GitHub notifications bot (Aug 22 2023 at 21:33):silesmo updated PR #6886.
Wasmtime GitHub notifications bot (Aug 22 2023 at 21:45):silesmo updated PR #6886.
Wasmtime GitHub notifications bot (Aug 22 2023 at 22:09):silesmo updated PR #6886.
Wasmtime GitHub notifications bot (Aug 22 2023 at 22:10):silesmo updated PR #6886.
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
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.rsAdded temporary exception for cargo deny on multiple versions until memfd has been updated: #62
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/*.witand additionally one intests/all/component_model/*. You'll find some other bindgen-related tests in that directory too.
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/*.witand additionally one intests/all/component_model/*. You'll find some other bindgen-related tests in that directory too.
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?
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?
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?
Wasmtime GitHub notifications bot (Aug 22 2023 at 22:32):alexcrichton created PR review comment:
I think this (and the other
wasi-httpchanges) could be separated out from this?
Wasmtime GitHub notifications bot (Aug 22 2023 at 22:33):alexcrichton assigned PR #6886 to alexcrichton.
Wasmtime GitHub notifications bot (Aug 23 2023 at 06:38):silesmo submitted PR review.
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:
Wasmtime GitHub notifications bot (Aug 23 2023 at 06:38):silesmo edited PR review comment.
Wasmtime GitHub notifications bot (Aug 23 2023 at 06:41):silesmo submitted PR review.
Wasmtime GitHub notifications bot (Aug 23 2023 at 06:41):silesmo created PR review comment:
Will do!
Wasmtime GitHub notifications bot (Aug 23 2023 at 06:41):silesmo submitted PR review.
Wasmtime GitHub notifications bot (Aug 23 2023 at 06:41):silesmo created PR review comment:
Yes will revert.
Wasmtime GitHub notifications bot (Aug 23 2023 at 06:43):silesmo submitted PR review.
Wasmtime GitHub notifications bot (Aug 23 2023 at 06:43):silesmo created PR review comment:
I will take a look.
Wasmtime GitHub notifications bot (Aug 23 2023 at 07:15):silesmo updated PR #6886 (assigned to alexcrichton).
Wasmtime GitHub notifications bot (Aug 23 2023 at 07:57):silesmo updated PR #6886 (assigned to alexcrichton).
Wasmtime GitHub notifications bot (Aug 23 2023 at 08:02):silesmo updated PR #6886 (assigned to alexcrichton).
Wasmtime GitHub notifications bot (Aug 23 2023 at 08:19):silesmo updated PR #6886 (assigned to alexcrichton).
Wasmtime GitHub notifications bot (Aug 23 2023 at 08:21):silesmo submitted PR review.
Wasmtime GitHub notifications bot (Aug 23 2023 at 08:21):silesmo created PR review comment:
added doc comment
Wasmtime GitHub notifications bot (Aug 23 2023 at 08:21):silesmo submitted PR review.
Wasmtime GitHub notifications bot (Aug 23 2023 at 08:21):silesmo created PR review comment:
done
Wasmtime GitHub notifications bot (Aug 23 2023 at 08:22):silesmo submitted PR review.
Wasmtime GitHub notifications bot (Aug 23 2023 at 08:22):silesmo created PR review comment:
Moved into it's own PR: #6886
Wasmtime GitHub notifications bot (Aug 23 2023 at 08:30):silesmo updated PR #6886 (assigned to alexcrichton).
Wasmtime GitHub notifications bot (Aug 23 2023 at 08:34):silesmo updated PR #6886 (assigned to alexcrichton).
Wasmtime GitHub notifications bot (Aug 23 2023 at 09:00):silesmo updated PR #6886 (assigned to alexcrichton).
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.
Wasmtime GitHub notifications bot (Aug 23 2023 at 09:03):silesmo submitted PR review.
Wasmtime GitHub notifications bot (Aug 23 2023 at 20:05):silesmo updated PR #6886 (assigned to alexcrichton).
Wasmtime GitHub notifications bot (Aug 24 2023 at 13:21):silesmo updated PR #6886 (assigned to alexcrichton).
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:
Wasmtime GitHub notifications bot (Aug 24 2023 at 21:37):alexcrichton requested pchickey for a review on PR #6886.
Wasmtime GitHub notifications bot (Aug 28 2023 at 18:39):alexcrichton has enabled auto merge for PR #6886.
Wasmtime GitHub notifications bot (Aug 30 2023 at 02:50):alexcrichton has disabled auto merge for PR #6886.
Wasmtime GitHub notifications bot (Aug 30 2023 at 02:50):silesmo updated PR #6886 (assigned to alexcrichton).
Wasmtime GitHub notifications bot (Aug 30 2023 at 02:57):alexcrichton has enabled auto merge for PR #6886.
Wasmtime GitHub notifications bot (Aug 30 2023 at 03:58):alexcrichton merged PR #6886.
Last updated: Oct 23 2024 at 20:03 UTC