Stream: git-wasmtime

Topic: wasmtime / Issue #2010 WASI: make O_CREAT|O_TRUNC actuall...

view this post on Zulip Wasmtime GitHub notifications bot (Jul 10 2020 at 19:41):

github-actions[bot] commented on Issue #2010:

Subscribe to Label Action

cc @kubkon

<details>
This issue or pull request has been labeled: "wasi"

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

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

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Jul 10 2020 at 19:45):

whitequark commented on Issue #2010:

Would it be possible to add your test case (ported to a test-programs test maybe) to ensure we don't regress this fix?

Sorry, I'm not familiar with this part of wasmtime's testing infrastructure, and the test case is pretty dirty (as-is it uses libc++...). If you have a step-by-step guide for doing this I could do it though!

view this post on Zulip Wasmtime GitHub notifications bot (Jul 10 2020 at 19:52):

peterhuene commented on Issue #2010:

No worries! There's a bunch of tests in crates/test-programs/wasi-tests that use the wasi crate to directly test against the Wasmtime's WASI API surface.

I think a new test case in there that creates a file with some text, closes it, and then opens it with OFLAGS_CREAT AND OFLAGS_TRUNC, then reads it to confirm it is truncated would do the trick.

I'd be happy to write one if you would like.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 10 2020 at 19:52):

peterhuene edited a comment on Issue #2010:

No worries! There's a bunch of tests in crates/test-programs/wasi-tests that use the wasi crate to directly test against the Wasmtime's WASI API surface.

I think a new test case in there that creates a file with some text, closes it, and then opens it with OFLAGS_CREAT | OFLAGS_TRUNC, then reads it to confirm it is truncated would do the trick.

I'd be happy to write one if you would like.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 10 2020 at 19:54):

whitequark commented on Issue #2010:

I'd be happy to write one if you would like.

Please do, that would help a lot.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 10 2020 at 20:26):

kubkon commented on Issue #2010:

No worries! There's a bunch of tests in crates/test-programs/wasi-tests that use the wasi crate to directly test against the Wasmtime's WASI API surface.

I think a new test case in there that creates a file with some text, closes it, and then opens it with OFLAGS_CREAT | OFLAGS_TRUNC, then reads it to confirm it is truncated would do the trick.

I'd be happy to write one if you would like.

Would it make sense to modify truncation_rights.rs instead of/in addition to adding a new one? It seems this one should already test what has been discussed here, but due to the fact the file is always empty and never reopened, this bug was missed on Windows.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 10 2020 at 20:37):

peterhuene commented on Issue #2010:

I think that truncation_rights is specifically centered around the WASI right checking whereas the new test would be simply to check that OFLAGS_TRUNC was respected properly for path_open.

Personally I'd prefer two smaller tests to one complicated test.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 10 2020 at 20:38):

peterhuene edited a comment on Issue #2010:

I think that truncation_rights is specifically centered around the WASI rights checking whereas the new test would be simply to check that OFLAGS_TRUNC was respected properly for path_open.

Personally I'd prefer two smaller tests to one complicated test.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 10 2020 at 20:54):

peterhuene commented on Issue #2010:

@kubkon I've pushed up a test case for file truncation. Would you mind reviewing it? I've confirmed the test fails without @whitequark's fix and passes with the fix on Windows.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 10 2020 at 21:20):

peterhuene edited a comment on Issue #2010:

No worries! There's a bunch of tests in crates/test-programs/wasi-tests that use the wasi crate to directly test against Wasmtime's WASI API surface.

I think a new test case in there that creates a file with some text, closes it, and then opens it with OFLAGS_CREAT | OFLAGS_TRUNC, then reads it to confirm it is truncated would do the trick.

I'd be happy to write one if you would like.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 10 2020 at 21:30):

peterhuene commented on Issue #2010:

Third time's a charm on those force pushes (I was lazy and just cargo test --feature test_programs the test programs crate directly, which doesn't error on unused imports when building).

view this post on Zulip Wasmtime GitHub notifications bot (Jul 10 2020 at 21:35):

peterhuene edited a comment on Issue #2010:

Third time's a charm on those force pushes (I was lazy and just cargo test --feature test_programs the test programs crate directly, which doesn't error on unused imports when building like in CI).

view this post on Zulip Wasmtime GitHub notifications bot (Jul 12 2020 at 00:11):

peterhuene commented on Issue #2010:

@whitequark thanks for fixing this!

Last updated: Oct 23 2024 at 20:03 UTC