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

Backport to release-18: three WASI PRs #7928

Merged
merged 3 commits into from
Feb 13, 2024
Merged

Conversation

pchickey
Copy link
Contributor

@pchickey pchickey commented Feb 13, 2024

Backport 3 PRs from main to release-18.0.0:

#7876 Preview 2 filesystem open-mode
#7926 Return correct fs errors from proxy adapter
#7881 Make wasi-common self-contained, deprecate exports from wasmtime-wasi

Pat Hickey and others added 2 commits February 13, 2024 10:39
…sions (#7876)

* fd_filestat_set test: assert that a file descriptor behaves the same...

whether opened readonly or readwrite.

This test now reproduces the behavior reported in #7829

Co-authored-by: Trevor Elliott <[email protected]>

* preview1_path_link test: refactor; create and open file separately

we create and open the file with separate descriptors because the
creation descriptor will always imply writing, whereas the rest do not.

there were a number of auxillary functions in here that were obscuring
what was going on, and making errors harder to read, so i changed some
assertions to use macro-rules so the panic message had the relevant line
number, and i inlined the creation / opening functions.

* preview2 filesystem: track open mode instead of reporting permissions it n DescriptorFlags

FilePerms/DirPerms is a way of having wasmtime-wasi, instead of the operating
system, mediate permissions on a preview2 Descriptor. This was conflated
with the open mode, which should determine whether DescriptorFlags
contains READ or WRITE or MUTATE_DIRECTORY.

We no longer mask FilePerms with the DescriptorFlags (oflags) in open_at -
instead, track what open mode is requested, and see if the file and
directory permissions permit it.

Additionally, File and Dir now track their open_mode (represented using
OpenMode, which just like FilePerms is just a read and write bit),
and report that in DescriptorFlags. Previously, we reported the
FilePerms or DirPerms in the DescriptorFlags, which was totally wrong.

Co-authored-by: Trevor Elliott <[email protected]>

* different error on windows i guess?

prtest:full

* apparently set-times of read-only is rejected on windows. just skip testing that

* path open read write: another alternate error code for windows

* wasi-common actually gives a badf, so accept either

* this case too

---------

Co-authored-by: Trevor Elliott <[email protected]>
This commit fixes an issue with the proxy adapter where if the proxy
program attempted to look at prestat items, which wasi-libc always does
on startup, it would invoke `fd_prestat_get` and receive `ERRNO_NOTSUP`
in response (the standard response when using the
`cfg_filesystem_available!` macro). This error code is unexpected by
wasi-libc and causes wasi-libc to abort. The PR here is to instead
return `ERRNO_BADF` which is indeed expected by wasi-libc and is
recognized as meaning "that prestat isn't available".
@pchickey pchickey requested review from a team as code owners February 13, 2024 18:46
@pchickey pchickey requested review from alexcrichton and removed request for a team February 13, 2024 18:46
@pchickey
Copy link
Contributor Author

pchickey commented Feb 13, 2024

I need to back out the extra changes to Cargo.lock that cherry-picking 7881 pulled in. (Edit: done.)

@github-actions github-actions bot added wasi Issues pertaining to WASI wasmtime:c-api Issues pertaining to the C API. labels Feb 13, 2024
Copy link

Subscribe to Label Action

cc @peterhuene

This issue or pull request has been labeled: "wasi", "wasmtime:c-api"

Thus the following users have been cc'd because of the following labels:

  • peterhuene: wasmtime:c-api

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

…#7881)

* WIP: try to make wasi-common self contained.

* rebase: cargo.lock

* remove all dependencies between wasi-common and wasmtime-wasi

* use wasi-common directly throughout tests, benches, examples, cli run

* wasi-threads: use wasi-common's maybe_exit_on_error in spawned thread

not a very modular design, but at this point wasi-common and
wasi-threads are forever wed

* fix wasmtime's docs

* re-introduce wasmtime-wasi's exports of wasi-common definitions behind deprecated

* factor out determining i32 process exit code

and remove libc dep because rustix provides the same constant

* commands/run: inline the logic about aborting on trap

since this is the sole place in the codebase its used

* Add high-level summary to wasi-common's top-level doc comment.

* c-api: fix use of wasi_cap_std_sync => wasi_common::sync, wasmtime_wasi => wasi_common

* fix tokio example

* think better of combining downcast and masking into one method

* fix references to wasmtime_wasi in docs

prtest:full

* benches: use wasi-common

* cfg-if around use of rustix::process because that doesnt exist on windows

* wasi-common: include tests, caught by verify-publish

* fix another bench

* exit requires wasmtime dep. caught by verify-publish.
@pchickey pchickey force-pushed the pch/backport_18_wasi branch from 588f033 to c869304 Compare February 13, 2024 21:53
@pchickey pchickey merged commit aee1e1d into release-18.0.0 Feb 13, 2024
44 checks passed
@pchickey pchickey deleted the pch/backport_18_wasi branch February 13, 2024 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasi Issues pertaining to WASI wasmtime:c-api Issues pertaining to the C API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants