-
Notifications
You must be signed in to change notification settings - Fork 821
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
Auto-generate wasi-types from .wit files #3177
Conversation
Now all wit-bindgen libraries are MIT licensed so that cargo deny doesn't complain about invalid licensing
@syrusakbary okay, tests are finally passing, but I wouldn't merge it just yet because I'd need to review whether the old types are the same as the new ones esp. for ones marked |
record tty { | ||
cols: u32, | ||
rows: u32, | ||
width: u32, | ||
height: u32, | ||
stdin-tty: bool, | ||
stdout-tty: bool, | ||
stderr-tty: bool, | ||
echo: bool, | ||
line-buffered: bool, | ||
} | ||
|
||
enum bus-data-format { | ||
raw, | ||
bincode, | ||
message-pack, | ||
json, | ||
yaml, | ||
xml, | ||
rkyv, | ||
} | ||
|
||
enum bus-event-type { | ||
noop, | ||
exit, | ||
call, | ||
result, | ||
fault, | ||
close, | ||
} |
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.
I believe this should be part of wasix.wit instead of the general wasi.wit? Which types are part of WASIX only @john-sharratt ?
lib/wasi/src/syscalls/wasix32.rs
Outdated
@@ -2,6 +2,12 @@ | |||
use crate::{WasiEnv, WasiError, WasiState, WasiThread}; | |||
use wasmer::{FunctionEnvMut, Memory, Memory32, MemorySize, StoreMut, WasmPtr, WasmSlice}; | |||
use wasmer_wasi_types::*; |
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.
Do we still need wasmer_wasi_types
? Why?
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.
Not all types can be ported to WIT, especially not types that use generics.
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.
Kind of make sense...
I see the file: https://github.com/wasmerio/wasmer/blob/wasi-types-generation-2/lib/wasi-types/src/lib.rs
I think it might make sense to move that into the wasmer-wasi
repo.
Although I see some types that might be feasible to completely move to wit?
subscription::EventType
net::*
(most of the net types?)
I also think the bus type names would make sense to have different types if it's 32 or 64 bits (so it doesn't need to be generic).
cc @john-sharratt
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.
@syrusakbary The subscription
module is portable (I missed that somehow), but net uses arrays, which WIT does not support (that's why I couldn't port it).
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.
Ok. Let's port the subscription module. And for the rest of things that can't be ported let's just have a types.rs
files inside wasmer-wasi
instead of a new crate
Currently we can't merge this because of bytecodealliance/wit-bindgen#323 @syrusakbary There are only a few types that are EDIT: This is fixed now. |
Fixes #2972.
Supersedes #3150.