Stream: git-wasmtime

Topic: wasmtime / PR #6791 Sync wasi-filesystem with wit definit...

Wasmtime GitHub notifications bot (Aug 01 2023 at 01:21):

pchickey opened PR #6791 from bytecodealliance:pch/sync_wasi_filesystem_wit to bytecodealliance:main:

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Aug 01 2023 at 01:23):

pchickey edited PR #6791:

See also:
https://github.com/WebAssembly/wasi-filesystem/pull/125
https://github.com/WebAssembly/wasi-filesystem/pull/126

Wasmtime GitHub notifications bot (Aug 01 2023 at 19:21):

pchickey updated PR #6791.

Wasmtime GitHub notifications bot (Aug 01 2023 at 22:02):

pchickey updated PR #6791.

Wasmtime GitHub notifications bot (Aug 01 2023 at 23:18):

pchickey updated PR #6791.

Wasmtime GitHub notifications bot (Aug 02 2023 at 02:12):

pchickey updated PR #6791.

Wasmtime GitHub notifications bot (Aug 02 2023 at 02:14):

pchickey updated PR #6791.

Wasmtime GitHub notifications bot (Aug 02 2023 at 02:16):

pchickey has marked PR #6791 as ready for review.

Wasmtime GitHub notifications bot (Aug 02 2023 at 02:16):

pchickey requested itsrainy for a review on PR #6791.

Wasmtime GitHub notifications bot (Aug 02 2023 at 02:16):

pchickey requested wasmtime-core-reviewers for a review on PR #6791.

Wasmtime GitHub notifications bot (Aug 02 2023 at 02:24):

pchickey edited PR #6791:

This PR pulls in the latest wit files from the wasi-filesystem repo, with the addition of just one pending upstream change to remove the nonblocking descriptor-flag.

The meat of this change is:

• the wasi:filesystem/filesystem interface has been renamed wasi:filesystem/types
• the interface removes device and inode from the descriptor-stat struct, and provides new metadata-hash family functions to replace this concept
  * these new methods are implemented in preview2 host, using std::hash::Hasher
  * the preview 1 adapters still need to fill in a device and inode in some places. I elected to fill in 1 for the device (because the stdio stream "files" use 0) and use the lower 64 bits of the metadata hash to fill in the inode.
  * the preview 1 host adapter needed to use an async fn to calculate each entry in fd_readdir, so I had to move some calculation out of the iterator map and into a for loop, done eagerly for each entry in the directory. Ideally this would be lazy for only entries in the directory that are being serialized as part of the adapter, or cache it between entries, but I didn't consider this worthwhile to invest in right now
• the new wasi:filesystem/preopens interface replaces the local wasi:cli-base/preopens. (There is no such thing as an upstream cli-base package, but morally it is in wasi-cli)

See also:
https://github.com/WebAssembly/wasi-filesystem/pull/125: remove nonblocking descriptor flag. still open.
https://github.com/WebAssembly/wasi-filesystem/pull/126: fix parameters to metadata functions. merged.

Wasmtime GitHub notifications bot (Aug 02 2023 at 02:24):

pchickey requested elliottt for a review on PR #6791.

Wasmtime GitHub notifications bot (Aug 02 2023 at 02:27):

pchickey edited PR #6791:

This PR pulls in the latest wit files from the wasi-filesystem repo, with the addition of just one pending upstream change to remove the nonblocking descriptor-flag.

The changes look like:

• the wasi:filesystem/filesystem interface has been renamed wasi:filesystem/types. This resulted in lots of renaming across the repo. For the main filesystem impl I use the new types:: module prefix but in most other places I followed use ...::filesystem::types as filesystem; to make it clearer to read.
• the interface removes device and inode from the descriptor-stat struct, and provides new metadata-hash family functions to replace this concept.
  * these new methods are implemented in preview2 host, using std::hash::Hasher to hash the device and inode of the file.
  * the preview 1 adapters still need to fill in a device and inode in some places (fd_filestat_get and fd_readdir). I elected to fill in 1 for the device (because the stdio stream "files" use 0) and use the lower 64 bits of the metadata hash to fill in the inode.
  * the preview 1 host adapter needed to use an async method to calculate each inode in fd_readdir, so I had to move some calculation out of the iterator map and into a for loop, done eagerly for each entry in the directory. Ideally this would be lazy for only entries in the directory that are being serialized as part of the adapter, or cache it between entries, but I didn't consider this worthwhile to invest in right now.
• the new wasi:filesystem/preopens interface replaces the local wasi:cli-base/preopens. (There is no such thing as an upstream cli-base package, but morally it is in wasi-cli)

See also:
https://github.com/WebAssembly/wasi-filesystem/pull/125: remove nonblocking descriptor flag. still open.
https://github.com/WebAssembly/wasi-filesystem/pull/126: fix parameters to metadata functions. merged.

Wasmtime GitHub notifications bot (Aug 02 2023 at 02:27):

pchickey requested sunfishcode for a review on PR #6791.

Wasmtime GitHub notifications bot (Aug 03 2023 at 18:40):

elliottt submitted PR review:

I think this looks great! Just had a question about the hasher choice before merging.

Wasmtime GitHub notifications bot (Aug 03 2023 at 18:40):

elliottt submitted PR review:

I think this looks great! Just had a question about the hasher choice before merging.

Wasmtime GitHub notifications bot (Aug 03 2023 at 18:40):

elliottt created PR review comment:

These will need to be updated after #6795 merges.

Wasmtime GitHub notifications bot (Aug 03 2023 at 18:40):

elliottt created PR review comment:

Why not use std::collections::hash_map::DefaultHasher here instead?

Wasmtime GitHub notifications bot (Aug 03 2023 at 19:28):

pchickey created PR review comment:

I missed it, but that is a better option.

Wasmtime GitHub notifications bot (Aug 03 2023 at 19:28):

pchickey submitted PR review.

Wasmtime GitHub notifications bot (Aug 03 2023 at 19:28):

pchickey updated PR #6791.

Wasmtime GitHub notifications bot (Aug 03 2023 at 20:10):

elliottt submitted PR review:

:rocket:

Wasmtime GitHub notifications bot (Aug 03 2023 at 21:20):

pchickey updated PR #6791.

Wasmtime GitHub notifications bot (Aug 03 2023 at 21:21):

pchickey has enabled auto merge for PR #6791.

Wasmtime GitHub notifications bot (Aug 04 2023 at 21:32):

pchickey updated PR #6791.

Wasmtime GitHub notifications bot (Aug 04 2023 at 23:42):

pchickey updated PR #6791.

Wasmtime GitHub notifications bot (Aug 04 2023 at 23:43):

pchickey updated PR #6791.

Wasmtime GitHub notifications bot (Aug 07 2023 at 22:14):

pchickey updated PR #6791.

Wasmtime GitHub notifications bot (Aug 08 2023 at 22:06):

elliottt updated PR #6791.

Wasmtime GitHub notifications bot (Aug 08 2023 at 22:06):

elliottt has enabled auto merge for PR #6791.

Wasmtime GitHub notifications bot (Aug 08 2023 at 22:26):

elliottt has disabled auto merge for PR #6791.

Wasmtime GitHub notifications bot (Aug 08 2023 at 22:26):

elliottt has enabled auto merge for PR #6791.

Wasmtime GitHub notifications bot (Aug 08 2023 at 23:36):

elliottt merged PR #6791.

Last updated: Nov 22 2024 at 16:03 UTC