Wasmtime GitHub notifications bot (Dec 08 2025 at 10:46): Wasmtime GitHub notifications bot (Dec 08 2025 at 10:46):wingo opened PR #12136 from wingo:windows-open-dir-with-no-read to bytecodealliance:main:
On Windows, it was possible to return a directory descriptor if READ wasn't in the permissions. Fixes wasmtime for
https://github.com/WebAssembly/wasi-testsuite/issues/176.
Wasmtime GitHub notifications bot (Dec 08 2025 at 10:46):wingo requested wasmtime-wasi-reviewers for a review on PR #12136.
Wasmtime GitHub notifications bot (Dec 08 2025 at 10:51):In all honesty I haven't tested on Windows, it's just that this patch should fix the issue. To repro, check out the
prod/testsuite-basebranch of https://github.com/WebAssembly/wasi-testsuite and run:wasmtime -Wcomponent-model-async -Sp3,http --dir 'tests\rust\testsuite\wasm32-wasip3\fs-tests.dir::fs-tests.dir' 'tests\rust\testsuite\wasm32-wasip3\filesystem-flags-and-type.wasm'
Wasmtime GitHub notifications bot (Dec 08 2025 at 10:52):wingo updated PR #12136.
Wasmtime GitHub notifications bot (Dec 08 2025 at 10:52):Rebased to fix the rustfmt error
Wasmtime GitHub notifications bot (Dec 08 2025 at 10:52):wingo edited PR #12136.
Wasmtime GitHub notifications bot (Dec 08 2025 at 11:12):rvolosatovs submitted PR review.
Wasmtime GitHub notifications bot (Dec 08 2025 at 11:12):rvolosatovs created PR review comment:
doesn't this work?
OpenResult::Dir(dir) if !flags.contains(DescriptorFlags::READ) => Err(ErrorCode::IsDirectory),
Wasmtime GitHub notifications bot (Dec 08 2025 at 11:29):wingo submitted PR review.
Wasmtime GitHub notifications bot (Dec 08 2025 at 11:29):wingo created PR review comment:
Line is too long that way,
cargo fmtwill want to fold it. The problem was that match clauses with a guard don't require a comma (I did not know this :) and sorustfmtwants the comma gone, which the patch I just pushed does.
Wasmtime GitHub notifications bot (Dec 08 2025 at 11:30):wingo updated PR #12136.
Wasmtime GitHub notifications bot (Dec 08 2025 at 11:38):rvolosatovs has enabled auto merge for PR #12136.
Wasmtime GitHub notifications bot (Dec 08 2025 at 13:15):wingo updated PR #12136.
Wasmtime GitHub notifications bot (Dec 08 2025 at 13:17):My goodness, there are many CI checks in this project :) Patch fixes unused-variable warning/error when compiling for Windows.
Wasmtime GitHub notifications bot (Dec 08 2025 at 13:21):rvolosatovs has enabled auto merge for PR #12136.
Wasmtime GitHub notifications bot (Dec 08 2025 at 13:23):rvolosatovs commented on PR #12136:
My goodness, there are many CI checks in this project :) Patch fixes unused-variable warning/error when compiling for Windows.
You can add
prtest:fullto commit message description to trigger a full CI run for cases like these
Wasmtime GitHub notifications bot (Dec 08 2025 at 14:36):alexcrichton commented on PR #12136:
Test-wise this is something that'll be caught when updating to the latest wasi-testsuite, right? If so, I think it's ok to skip a test here. Mind leaving a comment though on this match arm explaining that this is sync'ing windows semantics to unix semantics?
Wasmtime GitHub notifications bot (Dec 09 2025 at 08:12):wingo updated PR #12136.
Wasmtime GitHub notifications bot (Dec 09 2025 at 08:13):wingo edited PR #12136.
Wasmtime GitHub notifications bot (Dec 09 2025 at 08:13):wingo edited PR #12136:
On Windows, it was possible to return a directory descriptor even if WRITE was in the permissions. Fixes wasmtime for
https://github.com/WebAssembly/wasi-testsuite/issues/176.
Wasmtime GitHub notifications bot (Dec 09 2025 at 08:14):I had misremembered the nature of this error, my apologies. The problem is not the lack of READ, it's the presence of WRITE; see the
openspecification. Thank you for your patience.
Wasmtime GitHub notifications bot (Dec 09 2025 at 08:26):wingo updated PR #12136.
Wasmtime GitHub notifications bot (Dec 09 2025 at 08:26):wingo requested alexcrichton for a review on PR #12136.
Wasmtime GitHub notifications bot (Dec 09 2025 at 08:26):wingo requested wasmtime-core-reviewers for a review on PR #12136.
Wasmtime GitHub notifications bot (Dec 09 2025 at 10:15):wingo updated PR #12136.
Wasmtime GitHub notifications bot (Dec 09 2025 at 10:24):wingo updated PR #12136.
Wasmtime GitHub notifications bot (Dec 09 2025 at 11:22):Lordie me, this PR has been an education for me, thanks all for patience. Ready now (again).
Wasmtime GitHub notifications bot (Dec 09 2025 at 15:34):alexcrichton commented on PR #12136:
Anything in particular we could make easier on our end? Or more of a "lots of stuff to discover" kind of education?
Wasmtime GitHub notifications bot (Dec 09 2025 at 16:02):alexcrichton merged PR #12136.
Wasmtime GitHub notifications bot (Dec 10 2025 at 09:30):Anything in particular we could make easier on our end? Or more of a "lots of stuff to discover" kind of education?
Oh, I think things are pretty good on the wasmtime side, just that there were a number of considerations that I didn't fully grasp (the history of wasi versions and their implementations, the layers of error codes (std::io vs ErrorCode etc), the specificities of how CI works here...).
So things are good. Marginal improvements can be made tho:
- I didn't know about prtest:full, many thanks to @rvolosatovs; perhaps worth mentioning this when a merge fails, so as to avoid multiple merge failures
- Would be nice to be able to manually re-run tests that failed, or choose specific jobs, as in chromium's review system (image attached)
- It could make sense that if a PR includes a line that removes or adds cfg(), that we select the full build matrix
- One has to click through a few things to get to build errors (the list of checks, scroll to bad job, click, scroll down in the logs enough so that they are all loaded, search for a term); perhaps some summarization is possible
But like I say things are good, and thank you and Roman for your help & patience <3
<img width="753" height="1496" alt="image" src="https://github.com/user-attachments/assets/3efde466-7243-473b-8073-36cc729fd2f6" />
Wasmtime GitHub notifications bot (Dec 10 2025 at 16:17):alexcrichton commented on PR #12136:
Heh all very good points! (and thanks for writing them out)
To a large degree we're subject to the whims of github/github actions here insofar as that's only but so configurable. Otherwise addressing much of what you lay out (which I agree would all be great to have...) would require a bot/webhooks/etc with Wasmtime-specific integration. We haven't quite crossed such a threshold yet so we don't have such a bot ready to go (e.g. I've seen similar bots in the Rust/LLVM projects), and I at least don't personally know how to set up such a bot easily-ish alas :(
Last updated: Dec 13 2025 at 19:03 UTC