-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Use wasi crate for Core API #63676
Use wasi crate for Core API #63676
Conversation
r? @Kimundi (rust_highfive has picked a reviewer for you, use r? to override) |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@sunfishcode |
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.
Thanks for the PR, the changes largely look good to me, except:
Ideally WASI target should be completely libc-free in my opinion.
This is not the purpose of the wasi target, that's the purpose of the wasm32-unknown-unknown target. The purpose of the wasi target is for Rust/C to interoperate well, so we shouldn't be removing integration with libc
where it breaks that interoperation (like changes here to environment, abort
, etc)
WASI High-Level Goals does not state anything about it. I believe it's quite important to be able to compile pure Rust applications for WASI target without bringing Using |
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.
Many of the changes here look good; I like using the safe and more-idiomatic interfaces in the wasi
crate in general. But I'm concerned about the parts which impact C/Rust interop.
It's certainly useful to be able to support C and Rust interop. Quite a few popular Rust crates are *-sys wrappers around C/C++ libraries, for example.
Also, while you could theoretically re-implement crt1.c, libpreopen, etc., in Rust and eliminate libc
entirely, this is code which will very likely be evolving as WASI evolves, so with my WASI maintenance hat on, I'm concerned that this would increase the overhead of making changes right now.
I guess we will need a separate target for libc-free WASI then... :/ Something like |
I think I've reverted all potentially C-incompatible places. What do you think about adding a new libc-less WASI target in a separate PR? |
This looks good to me, r=me with the wasi crate changes merged and published. I do not think there is any practical interest currently in having a wasi target without the libc at https://github.com/cranestation/wasi-libc. |
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 this all looks great to me, thanks for pushing on this @newpavlov!
@sunfishcode would you be ok adding github:rust-lang:libs
to the owner list of the wasi
crate on crates.io? We currently tend to like to make sure there's not one point of failure for publishing to crates.io in case anything weird happens. AFAIK nothing weird has happened for years yet, we just want to make sure we're safe if need be!
I'm also going to gently ping @rust-lang/libs on this for awareness. This is adding a new dependency to the standard library, the |
Ok I'm going to go ahead and r+ this and we can sort out the logistics asynchronously @bors: r+ |
📌 Commit 0662fcf has been approved by |
Upgrade rand to 0.7 Also upgrades `getrandom` to avoid bug encountered by rust-lang#61393 which bumps libc to `0.2.62`. `wasi` crate was going to be added by rust-lang#63676 anyway.
Use wasi crate for Core API Blocked by: bytecodealliance/wasi-rs#5 Blocks: rust-lang/libc#1461 cc @sunfishcode @alexcrichton
Rollup of 5 pull requests Successful merges: - #63676 (Use wasi crate for Core API) - #64094 (Improve searching in rustdoc and add tests) - #64111 (or-patterns: Uniformly use `PatKind::Or` in AST & Fix/Cleanup resolve) - #64156 (Assume non-git LLVM is fresh if the stamp file exists) - #64175 (Fix invalid span generation when it should be div) Failed merges: - #63806 (Upgrade rand to 0.7) r? @ghost
Use wasi crate for Core API Blocked by: bytecodealliance/wasi-rs#5 Blocks: rust-lang/libc#1461 cc @sunfishcode @alexcrichton
⌛ Testing commit 0662fcf with merge cc8c5a93376093421d83ead1e7ce12326a429272... |
Use wasi crate for Core API Blocked by: bytecodealliance/wasi-rs#5 Blocks: rust-lang/libc#1461 cc @sunfishcode @alexcrichton
@bors retry rolled up. |
Rollup of 10 pull requests Successful merges: - #63676 (Use wasi crate for Core API) - #64094 (Improve searching in rustdoc and add tests) - #64111 (or-patterns: Uniformly use `PatKind::Or` in AST & Fix/Cleanup resolve) - #64156 (Assume non-git LLVM is fresh if the stamp file exists) - #64161 (Point at variant on pattern field count mismatch) - #64174 (Add missing code examples on Iterator trait) - #64175 (Fix invalid span generation when it should be div) - #64186 (std: Improve downstream codegen in `Command::env`) - #64190 (fill metadata in rustc_lexer's Cargo.toml) - #64198 (Add Fuchsia to actually_monotonic) Failed merges: r? @ghost
Remove WASI Core API Closes #1434 This change does not break the backwards compatibility promise since WASI Core API is unstable right now. If applications or libraries want to use Core API directly they should use [`wasi`](https://crates.io/crates/wasi) instead of `libc`. Blocked by: rust-lang/rust#63676 cc @sunfishcode
Blocked by: bytecodealliance/wasi-rs#5
Blocks: rust-lang/libc#1461
cc @sunfishcode @alexcrichton