Wasmtime GitHub notifications bot (Feb 05 2025 at 20:30): Wasmtime GitHub notifications bot (Feb 05 2025 at 20:30):SingleAccretion opened PR #10193 from SingleAccretion:DI-Tests to bytecodealliance:main:
Work in progress.
Wasmtime GitHub notifications bot (Feb 05 2025 at 20:32):SingleAccretion updated PR #10193.
Wasmtime GitHub notifications bot (Feb 06 2025 at 19:38):SingleAccretion updated PR #10193.
Wasmtime GitHub notifications bot (Feb 06 2025 at 20:15):SingleAccretion edited PR #10193:
Currently, the test assets (WASM programs) for DWARF debug info testing are checked into the repository in binary form.
This has the large problem of (not) scaling, Git-wise.
To solve this, this change adds a new test-only crate, that will, like the pre-existing
test-programs-artifacts, serve to compile the C/C++ source into its binary form, using a WASI SDK found via an environment variable (WASI_SDK_PATH). Since the DI tests are disabled by default, we can get away with a fairly simple graceful fallback to runtime test failure in case the WASI SDK was not present at build time.
Wasmtime GitHub notifications bot (Feb 06 2025 at 20:17):SingleAccretion edited PR #10193:
Currently, the test assets (WASM programs) for DWARF debug info testing are checked into the repository in binary form.
This has the large problem of (not) scaling, Git-wise.
To solve this, this change adds a new test-only crate, that will, like the pre-existing
test-programs-artifacts, serve to compile the C/C++ source into its binary form, using a WASI SDK found via an environment variable (WASI_SDK_PATH). Since the DI tests are disabled by default, we can get away with a fairly simple graceful fallback to runtime test failure in case the WASI SDK was not present at build time.For now, only one asset,
generic.wasm, has been converted to the new scheme, since it is a bit more challenging to verify with the others whether using a newer clang will produce something that still tests the same thing.
Wasmtime GitHub notifications bot (Feb 06 2025 at 20:18):SingleAccretion edited PR #10193:
Currently, the test assets (WASM programs) for DWARF debug info testing are checked into the repository in binary form.
This has the large problem of (not) scaling, Git-wise.
To solve this, this change adds a new test-only crate, that will, like the pre-existing
test-programs-artifacts, serve to compile the C/C++ source into its binary form, using a WASI SDK found via an environment variable (WASI_SDK_PATH). Since the DI tests are disabled by default, we can get away with a fairly simple graceful fallback to runtime test failure in case the WASI SDK was not present at build time.For now, only one asset,
generic.wasm, has been converted to the new scheme, since it is a bit more challenging to verify with the others whether using a newer clang will produce something that still tests the same thing.Of course, as with any from-source scheme, determinism is an issue - as clang evolves, the output will change. At this point in time, I am not proposing any specific mitigation strategy other than being judicious in how the tests are written.
Wasmtime GitHub notifications bot (Feb 06 2025 at 20:20):SingleAccretion updated PR #10193.
Wasmtime GitHub notifications bot (Feb 07 2025 at 03:47):alexcrichton commented on PR #10193:
This looks awesome, thank you for working on this!
What do you think about folding this directly into the
test-programs-artifactscrate? That build script could build both Rust and C/C++ artifacts perhaps?Of course, as with any from-source scheme, determinism is an issue
One mitigating factor here is that we're going to pin to a particular wasi-sdk in CI. Updating that will be a discrete step and while others may have different versions locally at least for our own CI it should remain deterministic until we update wasi-sdk in which case we'll at least know what was the cause.
Wasmtime GitHub notifications bot (Feb 07 2025 at 14:16):SingleAccretion commented on PR #10193:
What do you think about folding this directly into the test-programs-artifacts crate?
I considered this, but decided against for the following two reasons:
1)test-programs-artifactscan be built with just the standard Rust tools. It is very easy to install the additional WASM pieces if they're missing. In comparison, this C/C++ setup is much more complex - you need to procure the (correct) WASI SDK and need to understand whatWASI_SDK_PATHis. So it's very "different" from that perspective.
2) It has potentially better build time characteristics. While currently the debug tests are under the all-encompassingalltarget, I want to move them to a separate test target, so that they're quicker to build. In such a scheme, the--test=allworkflow won't need to build the C/C++ assets at all.And I couldn't find clear downsides, beside the inherent cost of adding one more crate. But it seems in Rust it is not unusual to have a large number of crates.
Wasmtime GitHub notifications bot (Feb 07 2025 at 14:16):SingleAccretion has marked PR #10193 as ready for review.
Wasmtime GitHub notifications bot (Feb 07 2025 at 14:16):SingleAccretion requested alexcrichton for a review on PR #10193.
Wasmtime GitHub notifications bot (Feb 07 2025 at 14:16):SingleAccretion requested wasmtime-core-reviewers for a review on PR #10193.
Wasmtime GitHub notifications bot (Feb 07 2025 at 14:16):SingleAccretion requested wasmtime-default-reviewers for a review on PR #10193.
Wasmtime GitHub notifications bot (Feb 07 2025 at 14:44):alexcrichton commented on PR #10193:
Those are good points, and you're definitely correct! I would agree that if this were folded in then we wouldn't want to have it required by default due to build requirements, and there's generally no issue with extra crates. That being said though what I'm concerned about here is that there's multiple locations building guest wasm programs. We already have
crates/test-programs/artifacts/build.rsdoing the bulk of the work and there's even alreadycrates/test-programs/src/bin/dwarf_*for various rust programs with debuginfo.What I'd sort of hope for is to have C/C++ not be so special at least at a surface level, ideally we could create
crates/test-programs/src/bin/*.{c,cpp}and that'd automatically get picked up and built. That'd additionally hook into teh rest of the support to, for example, create components for C/C++ modules as well which might be useful in the future.The primary concern of build time I think might be mitigatable by gracefully handling when
WASI_SDK_PATHisn't built and skipping the builds/tests or something like that. For example in addition to a static slice (which would be empty if the compile didn't happen) a static bool could be generated saying whether it was actually compiled. Then the#[ignore]'d tests would assert the boolean before executing.
Wasmtime GitHub notifications bot (Feb 07 2025 at 14:57):SingleAccretion commented on PR #10193:
Sounds reasonable. I'll work on folding it.
Wasmtime GitHub notifications bot (Feb 07 2025 at 14:57):SingleAccretion edited a comment on PR #10193:
Sounds reasonable - I'll work on folding it.
Last updated: Feb 28 2025 at 03:10 UTC