-
Notifications
You must be signed in to change notification settings - Fork 13k
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
wasi: Implement more of the standard library #59619
Conversation
This commit switches the wasi target to loading CLI arguments via the syscalls provided by wasi rather than through the argc/argv passed to the main function. While serving the same purpose it's hoped that using syscalls will make us a bit more portable (less reliance from libstd on an external C library) as well as avoiding the need for a lock!
I've since learned that the mapping between libc fds and wasi fds are expected to be one-to-one, so we can use the raw syscalls for writing to stdout/stderr and reading from stdin! This should help ensure that we don't depend on a C library too unnecessarily.
This routes the `error_string` API to `strerror` in libc which should have more human readable descriptions.
r? @bluss (rust_highfive has picked a reviewer for you, use r? to override) |
r? @fitzgen |
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.
LGTM!
I am curious about the preopened map approach. If I understand correctly, the WASI API (like Capsicum and CloudABI) doesn't support absolute paths, only dirfd-relative paths, but the std::fs API has many absolute-path APIs, but no dirfd-relative APIs. To work around this, this PR maintains a hashmap in the runtime which maps directory path -> dirfd for a set of "preopened files", and then absolute paths are converted to dirfd-relative by prefix matching. This is pretty clever, and I understand why it is needed for the WASI support to be useful. However, I find it a bit surprising and unusual that Rust should do such things behind the scenes. The "harder" alternative is to design APIs for dirfd-relative operations (wrapping For practical reasons, I think the approach in this PR is a good idea, but maybe this should be discussed later before stabilization. |
@bluetech good questions! Sounds like you've got a good handle on how things are implemented, so I'll just explain the rationale! I think that it's not feasible for the wasi target to not provide Now you're right though in that this isn't true to wasi! Currently this PR adds a suite of extension traits in Long-term I think there's a story to be made for stabilizing a cross-platform version of these APIs. For example I think we could add In the near term it might be interesting to prototype these APIs on crates.io, and envision Hopefully that clears things up! |
@bors: r=fitzgen |
📌 Commit 38fb7a7 has been approved by |
@@ -34,18 +19,31 @@ pub struct Args { | |||
|
|||
/// Returns the command line arguments | |||
pub fn args() -> Args { | |||
maybe_args().unwrap_or_else(|_| { |
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.
What's the reasoning behind getting the args from the host every time args()
is called? Wouldn't it make sense to get it once at program startup and store 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.
Oh sure yeah, the thinking is that this reduces our dependency on a C library somewhat, but it also removes the need for global state and locks. This way if you don't actually use the arguments we never reserve space or allocate memory for them.
In general I don't think acquiring the arguments isn't too performance critical, so I wanted to make sure that the binary and memory impact would be as small as possible. This also matches what we do on OSX I believe, for example.
wasi: Implement more of the standard library This commit fills out more of the `wasm32-unknown-wasi` target's standard library, notably the `std::fs` module and all of its internals. A few tweaks were made along the way to non-`fs` modules, but the last commit contains the bulk of the work which is to wire up all APIs to their equivalent on WASI targets instead of unconditionally returning "unsupported". After this some basic filesystem operations and such should all be working in WASI!
Failed in #59653 (comment), @bors r- |
This commit fills out the `std::fs` module and implementation for WASI. Not all APIs are implemented, such as permissions-related ones and `canonicalize`, but all others APIs have been implemented and very lightly tested so far. We'll eventually want to run a more exhaustive test suite! For now the highlights of this commit are: * The `std::fs::File` type is now backed by `WasiFd`, a raw WASI file descriptor. * All APIs in `std::fs` (except permissions/canonicalize) have implementations for the WASI target. * A suite of unstable extension traits were added to `std::os::wasi::fs`. These traits expose the raw filesystem functionality of WASI, namely `*at` syscalls (opening a file relative to an already opened one, for example). Additionally metadata only available on wasi is exposed through these traits. Perhaps one of the most notable parts is the implementation of path-taking APIs. WASI actually has no fundamental API that just takes a path, but rather everything is relative to a previously opened file descriptor. To allow existing APIs to work (that only take a path) WASI has a few syscalls to learn about "pre opened" file descriptors by the runtime. We use these to build a map of existing directory names to file descriptors, and then when using a path we try to anchor it at an already-opened file. This support is very rudimentary though and is intended to be shared with C since it's likely to be so tricky. For now though the C library doesn't expose quite an API for us to use, so we implement it for now and will swap it out as soon as one is available.
@bors: r=fitzgen |
📌 Commit 61b487c has been approved by |
wasi: Implement more of the standard library This commit fills out more of the `wasm32-unknown-wasi` target's standard library, notably the `std::fs` module and all of its internals. A few tweaks were made along the way to non-`fs` modules, but the last commit contains the bulk of the work which is to wire up all APIs to their equivalent on WASI targets instead of unconditionally returning "unsupported". After this some basic filesystem operations and such should all be working in WASI!
Rollup of 5 pull requests Successful merges: - #59076 (Include trailing comma in multiline Debug representation) - #59619 (wasi: Implement more of the standard library) - #59639 (Never return uninhabited values at all) - #59643 (std: Upgrade `compiler_builtins` to fix wasi linkage) - #59664 (Updated the documentation of spin_loop and spin_loop_hint) Failed merges: r? @ghost
wasi: Implement more of the standard library This commit fills out more of the `wasm32-unknown-wasi` target's standard library, notably the `std::fs` module and all of its internals. A few tweaks were made along the way to non-`fs` modules, but the last commit contains the bulk of the work which is to wire up all APIs to their equivalent on WASI targets instead of unconditionally returning "unsupported". After this some basic filesystem operations and such should all be working in WASI!
@bors p=2 Rollup fairness. |
wasi: Implement more of the standard library This commit fills out more of the `wasm32-unknown-wasi` target's standard library, notably the `std::fs` module and all of its internals. A few tweaks were made along the way to non-`fs` modules, but the last commit contains the bulk of the work which is to wire up all APIs to their equivalent on WASI targets instead of unconditionally returning "unsupported". After this some basic filesystem operations and such should all be working in WASI!
☀️ Test successful - checks-travis, status-appveyor |
This commit fills out more of the
wasm32-unknown-wasi
target's standard library, notably thestd::fs
module and all of its internals. A few tweaks were made along the way to non-fs
modules, but the last commit contains the bulk of the work which is to wire up all APIs to their equivalent on WASI targets instead of unconditionally returning "unsupported". After this some basic filesystem operations and such should all be working in WASI!