wasi:[email protected]: Add tests for set-size #136
Conversation
wingo
force-pushed
the
filesystem-set-size
branch
2 times, most recently
from
5f1585b to
ae474b6
Compare
wingo
changed the title
wingo
force-pushed
the
filesystem-set-size
branch
from
ae474b6 to
052675b
Compare
| // systems, the `c.cleanup` will have been removed from its dir, | ||
| // whereas on Windows that will happen when the last open descriptor | ||
| // (`c` and `r`) is closed. In any case we can still stat our | ||
| // descriptors, call `set-size` on it, and so on. |
There was a problem hiding this comment.
So.... this is not correct. I mean, in an ideal world there should be a single WASI semantics, but for some areas the cost to impose uniformity between Windows and POSIX is too high (#128 (comment)). Deletion of open files is probably one of these areas.
Apparently there is a FILE_DISPOSITION_POSIX_SEMANTICS thing that you can set on a file to turn on POSIX deletion semantics for that file. However it is new (Windows 10) and requires developer mode to be enabled (apparently), and so is probably not possible to impose.
There is also a FILE_SHARE_DELETE flag that one can pass when opening a file, which is weird (removal is enqueued for when all open descriptors close; any open descriptor without FILE_SHARE_DELETE causes unlink to fail, which can be the case for backup programs; apparently one can create a new file with the same name while the file is pending deletion; etc).
Or, we could just allow unlink to fail if there is an open descriptor.
What should we allow in WASI? :) @sunfishcode @alexcrichton @pchickey
There was a problem hiding this comment.
My recollection, from when we tried to figure this out in the p1 era, is that we couldn't do any better than allowing unlink to fail if there is an open descriptor.
There was a problem hiding this comment.
My understanding is that it's reasonable enough to require runtimes to use FILE_SHARE_DELETE, for example I think that's the default in Rust. For FILE_DISPOSITION_POSIX_SEMANTICS I would agree that if it's either very new or requires developer mode it would be unreasonable to require that.
In the abstract though I'd say it's best to allow either the Windows or the Unix behavior here to avoid causing too much trouble in runtimes, although it's of course a portabilty hazard for guests.
wingo
force-pushed
the
filesystem-set-size
branch
from
052675b to
e10a793
Compare
wingo
force-pushed
the
filesystem-set-size
branch
from
e10a793 to
c166d29
Compare
| } | ||
| Err(ErrorCode::Invalid | ErrorCode::BadDescriptor) => {} | ||
| Err(err) => { | ||
| panic!("unexpected err: {}", err) |
There was a problem hiding this comment.
Windows Wasmtime:
Test filesystem-set-size failed
[exit_code] 0 == 3
STDOUT:
STDERR:
thread '' (1) panicked at src/bin/filesystem-set-size.rs:76:13:
unexpected err: access (error 0)
note: run with RUST_BACKTRACE=1 environment variable to display a backtrace
Error: failed to run main module tests\rust\testsuite\wasm32-wasip3\filesystem-set-size.wasm
3 participants
No description provided.