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

feat: add wasi-0.3.0 draft #164

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

rvolosatovs
Copy link

@rvolosatovs rvolosatovs commented Jan 14, 2025

Refs https://github.com/orgs/bytecodealliance/projects/16/views/1?pane=issue&itemId=88153596
Refs WebAssembly/wasi-sockets#111
Refs #156 (change sets seem to be complimentary)

Followed the example of wasi-http and wasi-clocks duplicating the package in a subdirectory

This vendors wasi-clocks from WebAssembly/wasi-clocks#77

  • Remove would-block error code
  • Replace wasi:io/error.error usage by error-context
  • Replace wasi:io/streams.input-stream return values by stream<u8> in return position
  • Replace wasi:io/streams.output-stream return values by stream<u8> in parameter position - Guests should be able to rely on stream.new to construct streams
  • Merge read{,via-stream} into a singular read. Both functions take an offset as a parameter and since they now return stream<u8>, callers can limit the amount of bytes read by using the stream<u8> directly, therefore functionality of both is identical
  • Merge write{,via-stream} into a singular write. Both functions take an offset and stream<u8> as a parameter.

It is assumed that error-context returned by writes to the stream and reads from the stream are sufficient for error handling.

See also

Followed the example of `wasi-http` and `wasi-clocks` duplicating the
package in a subdirectory

- Remove `would-block` error code
- Replace `wasi:io/error.error` usage by `error-context`
- Replace `wasi:io/streams.input-stream` return values by `stream<u8>`
  in return position
- Replace `wasi:io/streams.output-stream` return values by `stream<u8>`
  in parameter position
     - Guests should be able to rely on `stream.new` to construct
       streams
- Merge `read{,via-stream}` into a singular `read`. Both functions take
  an offset as a parameter and since they now return `stream<u8>`,
  callers can limit the amount of bytes read by using the `stream<u8>`
  directly, therefore functionality of both is identical
- Merge `write{,via-stream}` into a singular `write`. Both functions take
  an offset and `stream<u8>` as a parameter.

It is assumed that `error-context` returned by writes to the stream and
reads from the stream are sufficient for error handling.

Signed-off-by: Roman Volosatovs <[email protected]>
@rvolosatovs rvolosatovs marked this pull request as ready for review January 14, 2025 12:25
@rvolosatovs
Copy link
Author

Same as in WebAssembly/wasi-sockets#111 (comment), removed the error-context introspection in a separate commit (which can be dropped if desired)

This seems to be better aligned with latest specification on error context

https://github.com/WebAssembly/component-model/blob/cbdd15d9033446558571824af52a78022aaa3f58/design/mvp/Explainer.md#error-context-type

> A consequence of this, however, is that components *must not* depend on the
> contents of `error-context` values for behavioral correctness. In particular,
> case analysis of the contents of an `error-context` should not determine
> *error recovery*; explicit `result` or `variant` types must be used in the
> function return type instead (e.g.,
> `(func (result (tuple (stream u8) (future $my-error)))`).

Signed-off-by: Roman Volosatovs <[email protected]>
wit-0.3.0-draft/types.wit Outdated Show resolved Hide resolved
rvolosatovs added a commit to rvolosatovs/wasi-sockets that referenced this pull request Jan 14, 2025
rvolosatovs added a commit to rvolosatovs/wasi-filesystem that referenced this pull request Jan 14, 2025
rvolosatovs added a commit to rvolosatovs/wasi-sockets that referenced this pull request Jan 14, 2025
@ricochet ricochet requested a review from sunfishcode January 14, 2025 18:35
@sunfishcode
Copy link
Member

sunfishcode commented Jan 15, 2025

Thanks for putting this together! Overall this looks good.

One additional thing we should look into is translating directory-entry-stream into stream<directory-entry>.

Similar to this discussion in wasi-sockets, what would you think about renaming read/write/append back to read-via-stream/write-via-stream/append-via-stream in order to make room for a future read/write that uses newer language features when they become available?

    read: func(
        /// The offset within the file at which to start reading.
        offset: filesize,
    ) -> result<tuple<stream<u8>, future<option<error-code>>>, error-code>;

Could we simplify this interface by eliminating the outer result and error-code here? The way this function works, any implementation is likely going to need to maintain its own cursor, so this function itself doesn't need to do any I/O. It currently does fail if the stream is not readable, but perhaps we could instead have implementations return a closed stream and report it in the future<option<error-code>> instead.

@rvolosatovs rvolosatovs force-pushed the feat/0.3.0-draft branch 2 times, most recently from 07e2d44 to eed891a Compare January 17, 2025 09:21
Signed-off-by: Roman Volosatovs <[email protected]>
This ensures that e.g.:

```
wit-bindgen markdown wit-0.3.0-draft -w imports --html-in-md
```

works with wit-bindgen 0.37

refs:
WebAssembly/wasi-sockets@3abda6e

refs:
WebAssembly/wasi-sockets#111 (comment)

Signed-off-by: Roman Volosatovs <[email protected]>
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.

3 participants