Land preview2-prototyping's Wasi Preview 2 implementation into wasmtime #6391
Conversation
This gets us another step closer to running the Wasmtime tests.
Add a `result` return type to `command` so that it can indicate success or failure. The idea here is that this isn't a full `i32` return value because the meaning of return values isn't portable across platforms. Also, Typed Main is a better long-term answer for users that want rich error return values from commands.
…lliance#45) * implement `wasi-filesystem::readdir` and related functions This adds a `directory_list` test and provides the required host implementation. I've also added a file length check to the `file_read` test, just to cover a bit more of the API. Signed-off-by: Joel Dice <joel.dice@fermyon.com> * fix memory corruption in `fd_readdir` polyfill We were copying `name.len() * 256` bytes instead of just `name.len()` bytes, which was overwriting other parts of `State` and causing untold havoc. Signed-off-by: Joel Dice <joel.dice@fermyon.com> * check type of entry in `Table::delete` Signed-off-by: Joel Dice <joel.dice@fermyon.com> Signed-off-by: Joel Dice <joel.dice@fermyon.com>
…ce#47) Signed-off-by: Joel Dice <joel.dice@fermyon.com> Signed-off-by: Joel Dice <joel.dice@fermyon.com>
Many users won't need these, because they'll just grab one default clock handle and reuse it for the rest of the program. But it should still be possible to close a clock handle. Also, add a simple test for the clock APIs.
* Use `use` imports consistently. * Rename `flags_from_descriptor_flags` to `descriptor_flags_from_flags`. * Rename `black_box` to `obscure`. * Make some comments be doc comments. * Use a hyphen for compound adjectives. * Delete an unused variable. * Update the name of `change-file-permissions-at`. * Use generated typedefs instead of hard-coding u64. * Use core::hint::black_box now that it's stabilized.
1. It needed a `config.async_support(true)` to actually work at all. 2. Add `inherit_stdin` and `inherit_stdout` calls when building WasiCtx to make stdio useful. 3. Set stdout descriptor to 1. Signed-off-by: Joel Dice <joel.dice@fermyon.com> Signed-off-by: Joel Dice <joel.dice@fermyon.com>
* Implement file append functionality. - In the preview1-to-preview2 polyfill, using `append_via_stream`. - In the host implementation, using a new system-interface `FileIoExt::append` function. * Add a basic testcase.
Replace `AsRawHandleOrSocket` with `AsBorrowedHandleOrSocket`, to make the Windows code work more like the Unix code. And add CI tests for macOS too.
This deletes the old unmaintained Windows `poll_oneoff` implementation and replaces it with one based on `WSAPoll`, which rustix is able to present via a compatible API with Unix `poll`, so it can share the same implementation.
Use `num_ready_bytes()` on objects which don't contain pollable handles.
When a `ReadPipe` is reading from a memory buffer, there is no host file descriptor to poll with, but there doesn't need to be because the contents are immediately available. Teach the `poll_oneoff` function to use system-interface's `ReadReady` trait, which returns the number of bytes read to be read immediately, to implement polling for such input sources. This also fixes the polyfill to avoid calling `bytes_readable` on non-socket handles, which is needed to make the test pass.
On Windows, there doesn't appear to be a way to sync a directory, to ensure that the directory entry for a file is sync'd. So for now, just silently succeed. I've opened WebAssembly/wasi-filesystem#79 to track this at the spec level.
* Import the Wasmtime WASI tests. Copy the Wasmtime WASI tests in crates/test-programs/wasi-tests as of 7b75ba9 into the preview2 prototype at test-programs/wasi-tests. This adds TODO lines to tests/runtime.rs for tests which aren't passing yet. * Open directories properly on Windows.
* implement some host filesystem operations: create_directory_at stat remove_directory_at symlink_at unlink_file_at * host filesystem tests: some new ones pass, others use EXPECT_FAIL constant by using `if EXPECT_FAIL` instead of `if true` its now easier to find which failing tests are actually passing. The following are now expected to pass (though it may not be the fault of this branch fixing them): dangling_fd file_pread_pwrite file_unbuffered_write isatty path_exists symlink_create * dangling_fd fails on windows * test-programs macros: deduplicate tests I invoked the test_log::test and tokio::test proc macros incorrectly, so it created 2 identically named versions of each test
…iance#67) * On Windows, ignore access-denied errors from `sync_all`. On Windows `sync_all` does `FlushFileBuffers` which fails with an access-denied error if the file not opened for writing. Ignore this error, for compatibility with POSIX-style APIs. Fixes bytecodealliance#66. * Ignore `ERROR_ACCESS_DENIED` from `sync_data` too. * Temporarily disable directory syncing for now.
There was a problem hiding this comment.
I've glanced over crates/test-programs and crates/wasi/src/preview2 but I assume the idea is we want to merge these as work-in-progress and then iterate on them in-tree, right? I see plenty of fixme-type comments and the like. I was particularly curious about the comment in crates/test-programs/command-tests/src/bin/file_dir_sync.rs about not being able to open directories as files on Windows.
In particular, it looks like the preview2 implementation doesn't interfere at all with using the existing preview1 implementation. If I've missed any caveats to that I'd like to hear about them, but assuming it's true then I think this is fine to merge, since we aren't making any promises about stability of the component model or preview2 yet.
I've reviewed the rest (supply-chain, ci/run-tests.sh, and various Cargo.toml changes) reasonably carefully.
|
That's correct: The new implementation is totally separate from the old one. There are a number of known deficiencies with the new implementation but we decided its time to move to this and iterate here. The new implementation won't replace the old one until we have resolved all of those problems, and it passes all of the same tests. The comment in |
|
I think that comment in file_dir_sync.rs is outdated. I think we can uncomment those two lines. Windows doesn't have a way to do a sync on a directory handle. Originally, WASI's impl would fail on Windows because we'd try to call the Windows API for files and it would fail when passed a directory handle. Now, we handle this so that it silently does nothing, which is the direction I think WASI as a whole is heading. The semantics for |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
this is causing a link error because pulldown_cmark is available both in the rust_wasi_markdown_parser deps directory and, now, in the root of the project as well.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
pchickey
force-pushed
the
pch/merge_preview2_impl
branch
from
20d4467 to
ff4ad05
Compare
Third task in #6370
This PR lands preview2-prototyping's Wasi Preview 2 implementation inside
wasmtime_wasi::preview2.on bytecodealliance/preview2-prototyping@5be99d6
And then merging wasmtime's
pch/test_programs@87fedb59d6dcf6081285bf390ccf8b5194eaff2c in with --allow-unrelated-histories.