Skip to content
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

Merged
merged 95 commits into from
Oct 10, 2022
Merged

Conversation

fschutt
Copy link
Contributor

@fschutt fschutt commented Sep 7, 2022

Fixes #2972.

Supersedes #3150.

@fschutt
Copy link
Contributor Author

fschutt commented Sep 15, 2022

@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 #[repr(u16)]

Comment on lines 776 to 805
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,
}
Copy link
Member

@syrusakbary syrusakbary Sep 15, 2022

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 ?

@@ -2,6 +2,12 @@
use crate::{WasiEnv, WasiError, WasiState, WasiThread};
use wasmer::{FunctionEnvMut, Memory, Memory32, MemorySize, StoreMut, WasmPtr, WasmSlice};
use wasmer_wasi_types::*;
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Member

@syrusakbary syrusakbary Sep 15, 2022

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

Copy link
Contributor Author

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).

Copy link
Member

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

@fschutt
Copy link
Contributor Author

fschutt commented Sep 26, 2022

Currently we can't merge this because of bytecodealliance/wit-bindgen#323

@syrusakbary There are only a few types that are repr(u16), I'll just backport them, so that we can merge this PR.

EDIT: This is fixed now.

@syrusakbary syrusakbary merged commit 19e7e7a into master Oct 10, 2022
@bors bors bot deleted the wasi-types-generation-2 branch October 10, 2022 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Wasmer-WASI types with wit-bindgen bindings
4 participants