Stream: git-wasmtime

Topic: wasmtime / issue #6396 Directory listings don't include p...

Wasmtime GitHub notifications bot (May 17 2023 at 09:06):

SteveSandersonMS opened issue #6396:

Wasmtime's behavior for directory listings (via the WASI fd_readdir API?) is surprising and is inconsistent with Wasmer.

If there is a preopen for /blah, do you expect a directory listing for / to include /blah? Most people would do, because that's how all normal filesystems work. However:

• This is not the case with wasmtime. Each directory listing returns only the files that literally exist on the host within that directory, ignoring any other preopens that may be mapped into the guest directory.
• But it is the case with wasmer - it correctly includes other preopens that have been mapped to the guest directory.

Repro code: https://gist.github.com/SteveSandersonMS/ff5f5cb91524bbcde24a168841e66f10

Existing application code could be broken in strange ways if typical filesystem invariants are not maintained (e.g., "a directory's parent always contains that directory").

Should this be considered a WASI spec defect, since it does not seem to define any rules around this? Or would people agree the desired semantics are obvious (i.e., should behave like normal filesystems) and that it should be counted as a Wasmtime bug?

Wasmtime GitHub notifications bot (May 17 2023 at 09:27):

bjorn3 commented on issue #6396:

I think if anything should do this remapping it should be wasi-libc, not the host wasi impl. If I close the preopen at /blah I did expect it to become impossible to access anything under this preopen that I haven't already opened. Being able to do open("/blah/foo") instead of having to do openat(<<blah>>, "foo") is a mechanism inside of wasi-libc for compatibility with existing code. A program designed for wasi that only uses openat shouldn't see anything of this back compat mechanism IMHO. Also what if there is no preopen at /? There is nothing to add the extra dir entries too, so only wasi-libc can provide the illusion like there is a blah dir in /.

Wasmtime GitHub notifications bot (May 17 2023 at 09:34):

SteveSandersonMS commented on issue #6396:

Being able to do open("/blah/foo") instead of having to do openat(<<blah>>, "foo") is a mechanism inside of wasi-libc for compatibility with existing code.

That makes sense. But if the goal is compatibility, it also makes sense to complete this picture and simulate the existence of ancestor directories containing the preopens (whether that's done by wasmtime or wasi-libc).

Wasmtime GitHub notifications bot (May 17 2023 at 09:35):

SteveSandersonMS edited a comment on issue #6396:

Being able to do open("/blah/foo") instead of having to do openat(<<blah>>, "foo") is a mechanism inside of wasi-libc for compatibility with existing code.

That makes sense. But if the goal is compatibility, it also makes sense to complete this picture and simulate the existence of ancestor directories containing the preopens (whether that's done by wasmtime or wasi-libc).

Wasmtime GitHub notifications bot (May 17 2023 at 10:14):

bjorn3 commented on issue #6396:

Yeah, I think it should be done in wasi-libc.

Wasmtime GitHub notifications bot (May 17 2023 at 10:59):

SteveSandersonMS commented on issue #6396:

Thanks @bjorn3. I've filed https://github.com/WebAssembly/wasi-libc/issues/414

Wasmtime GitHub notifications bot (May 17 2023 at 23:58):

abrown commented on issue #6396:

Should we close this issue then?

Wasmtime GitHub notifications bot (May 18 2023 at 00:19):

pchickey commented on issue #6396:

I agree that this needs to be solved in wasi-libc, rather than in the host, so I am in favor of closing this. If for whatever reason the wasi-libc decision is that this needs to be implemented in hosts, then we'll want to update the spec text and tests to note that and we'll open a new issue to implement the spec correctly.

Wasmtime GitHub notifications bot (May 18 2023 at 00:19):

pchickey edited a comment on issue #6396:

I agree that this ought to be to be solved in wasi-libc, rather than in the host, so I am in favor of closing this. If for whatever reason the wasi-libc decision is that this needs to be implemented in hosts, then we'll want to update the spec text and tests to note that and we'll open a new issue to implement the spec correctly.

Wasmtime GitHub notifications bot (May 18 2023 at 00:32):

sunfishcode commented on issue #6396:

I agree; let's discuss this in the wasi-libc issue, and see where it goes from there.

Wasmtime GitHub notifications bot (May 18 2023 at 00:32):

sunfishcode closed issue #6396:

Wasmtime's behavior for directory listings (via the WASI fd_readdir API?) is surprising and is inconsistent with Wasmer.

If there is a preopen for /blah, do you expect a directory listing for / to include /blah? Most people would do, because that's how all normal filesystems work. However:

• This is not the case with wasmtime. Each directory listing returns only the files that literally exist on the host within that directory, ignoring any other preopens that may be mapped into the guest directory.
• But it is the case with wasmer - it correctly includes other preopens that have been mapped to the guest directory.

Repro code: https://gist.github.com/SteveSandersonMS/ff5f5cb91524bbcde24a168841e66f10

Existing application code could be broken in strange ways if typical filesystem invariants are not maintained (e.g., "a directory's parent always contains that directory").

Should this be considered a WASI spec defect, since it does not seem to define any rules around this? Or would people agree the desired semantics are obvious (i.e., should behave like normal filesystems) and that it should be counted as a Wasmtime bug?

Wasmtime GitHub notifications bot (May 18 2023 at 08:05):

SteveSandersonMS commented on issue #6396:

Sounds good. Thanks everyone!

Last updated: Nov 22 2024 at 16:03 UTC