The wasi-common tests should not be run against the snapshot

[marmistrz]

Currently the tests are run against the snapshot and not the current version of the code which defies their purpose. When invoked as:

RUST_LOG=wasi_common=trace rustup run stable cargo test --features test_programs --package test-programs -- <test_name>

the following log output is produced

 TRACE wasi_common::old::snapshot_0::hostcalls_impl::fs            >      | *nwritten=79

[sunfishcode]

The issue is that the Rust compiler's wasm32-wasi target hasn't switched to the new ABI yet. That'll happen soon!

[jlb6740]

Is this potentially related to CI failures when running cargo test? I've been unable to local reproduce testing errors being reported here #360 where failures keep occurring on compiling wasi-common during cargo test.

I figured it was just my issue but I've noticed similar errors on other recent pull requests.

[kubkon]

@jlb6740 no, the CI failures are related to a regression in rust-lang/rust nightly. This has been fixed in #634 :-)

[kubkon]

Ooops, meant to say it was fixed in #635.

[sunfishcode]

rust-lang/rust#66750 updates the Rust toolchain to use the new ABI.

[alexcrichton]

Looking into this, this is unfortuantely going to be really difficult I think given how things are set up. There's a few things in play here:

• All tests are using the wasi crate at 0.7.0, and they'll need to update to 0.9.0. This is a signficant investment since all the APIs were refactored.
• The standard library itself needs to be updated, but only slightly. So far it looks like env::args() is largely the only main thing used, and while that's buggy currently it should be fixed soon. Otherwise Rust's libstd, on nightly, should be updated.
• Unfortunately though we also need to update the C library at the same time in Rust's libstd. This is because some tests use libc::open, for example, and the file descriptors don't work when passed from one snapshot to another (they both have their own namespace essentially).

To fix all this we basically need to ensure that the each test individually exclusively uses one wasi snapshot version. This means we probably can't use libc and/or libstd in Rust. I think what we probably want to do is to migrate all tests to exclusively using the wasi crate. Some tests could use libstd and/or libc a bit, but they wouldn't be mixed with the wasi crate itself. We could of course have multiple versions of the wasi crate tested, just one-per-test or being careful inside of each test.

[alexcrichton]

I've sent an update of wasi-libc for Rust's libstd at rust-lang/rust#67066, but to get tests working on all of stable/beta/nightly we'll need to be careful where if libc is used in the tests then wasi isn't used, only std is used. So tests can either use libc/std, or they can use wasi, not both. If both are used a risk of a snapshot version mismatch happens, and then the fd namespaces will clash.

[pchickey]

Would it be helpful or desirable for wasi-common to support both snapshot interfaces for the same instance? They would have to map to the same WasiCtx, which is an issue I'm currently working on (#658)

[alexcrichton]

That would certainly make it a bit easier yeah, because then we wouldn't have to worry about what version the tests are using vs what version libstd/libc are using. I wasn't sure if it'd be an easy thing to do though, since some future change may make interoperability not feasible.

I started updating some tests in #675 though and all this really meant was that I had to write this function, so it's not really the end of the world either way. It's probably not worth the effort right now to have file descriptors work across both APIs, and it's best to leave the two snapshots separate as they are.

[pchickey]

Gotcha. I can see it being helpful as the ecosystem evolves, and I'll try to make it work if possible. But its not going to happen in the next couple of days.

[alexcrichton]

Ah yeah I should clarify that as a user it would be pretty convenient to have this sort of spanning snapshots compatibility, but from the perspective of updating the tests don't take that as a motivation for doing so :)

[kubkon]

I guess we can close this issue given that all tests are now migrated to the latest wasi crate (see #675 and #743), so I'm gonna go ahead and close it. Feel free to reopen it though in case I missed something!