Stream: git-wasmtime

Topic: wasmtime / PR #8823 Inherit Linux semantics for `fd_pwrit...

Wasmtime GitHub notifications bot (Jun 17 2024 at 19:39):

alexcrichton opened PR #8823 from alexcrichton:match-linux-for-fd-pwrite to bytecodealliance:main:

This commit updates the implementation of fd_pwrite in WASI to match Linux semantics for an under-specified corner of WASI. Specifically if fd_pwrite is used the offset specified is ignored if the file is opened in append mode and the bytes are instead appended.

This commit additionally refactors fd_write and fd_pwrite to have basically the same code with only a minor branch internally when the final write is being performed to help deduplicate more logic.

Closes #8817

<!--
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 (Jun 17 2024 at 19:39):

alexcrichton requested wasmtime-core-reviewers for a review on PR #8823.

Wasmtime GitHub notifications bot (Jun 17 2024 at 19:39):

alexcrichton requested fitzgen for a review on PR #8823.

Wasmtime GitHub notifications bot (Jun 17 2024 at 19:52):

fitzgen requested pchickey for a review on PR #8823.

Wasmtime GitHub notifications bot (Jun 17 2024 at 19:52):

fitzgen commented on PR #8823:

Redirecting to @pchickey

Wasmtime GitHub notifications bot (Jun 17 2024 at 20:10):

alexcrichton updated PR #8823.

Wasmtime GitHub notifications bot (Jun 17 2024 at 20:10):

alexcrichton has enabled auto merge for PR #8823.

Wasmtime GitHub notifications bot (Jun 17 2024 at 20:10):

alexcrichton has disabled auto merge for PR #8823.

Wasmtime GitHub notifications bot (Jun 17 2024 at 20:12):

pchickey submitted PR review.

Wasmtime GitHub notifications bot (Jun 17 2024 at 20:18):

alexcrichton has enabled auto merge for PR #8823.

Wasmtime GitHub notifications bot (Jun 17 2024 at 20:48):

alexcrichton updated PR #8823.

Wasmtime GitHub notifications bot (Jun 17 2024 at 21:07):

alexcrichton updated PR #8823.

Wasmtime GitHub notifications bot (Jun 17 2024 at 21:08):

alexcrichton commented on PR #8823:

CI turned up an interesting phenomena which makes sense in retrospect. With wasi-common the behavior is subject to whatever the platform OS provides which makes Linux the odd-one-out. Papering over OS differences is easy enough in wasmtime-wasi but is not trivial in wasi-common.

@pchickey how do you feel about sticking with these semantics and just ignoring the whole test in wasi-common? Alternatives could include not landing this in Wasmtime and taking it up in an issue with WASI and/or adding more stuff to wasi-common.

Wasmtime GitHub notifications bot (Jun 17 2024 at 21:19):

pchickey commented on PR #8823:

I think it is fine to just leave this bug in wasi-common and ignore the test - I don't think there is much value in bringing bug fixes for corner cases like this, if users need the fix the real question is why they havent upgraded to wasmtime-wasi.

Wasmtime GitHub notifications bot (Jun 17 2024 at 21:39):

alexcrichton merged PR #8823.

Last updated: Nov 22 2024 at 17:03 UTC