abrown opened PR #7815 from abrown:pku-ci
to bytecodealliance:main
:
After discussing several options for testing MPK in CI, this stopgap
measure at least provides some coverage. Other options include
maintaining a separate MPK-enabled CI runner (configuration is not
transparent, hard to maintain) or running the MPK-enabled tests in a
system-mode QEMU VM (tricky to get right, also hard to maintain). For
now, those of us at the Cranelift meeting agreed this at least gets some
CI testing for the MPK bits, which shouldn't be changing too often.
Since not all GitHub runners have MPK available, we only set the
WASMTIME_TEST_FORCE_MPK
environment variable when it is. This change
also emits a warning to the GitHub logs in either case for easier
troubleshooting (e.g., to attempt to provide context if MPK logic breaks
and is only found in a later CI run).prtest:full
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
abrown updated PR #7815.
abrown updated PR #7815.
alexcrichton commented on PR #7815:
Could this add a dedicated builder which is the only one enabling MPK? That one could perhaps run just a small subset of the test suite as well. I'm also a bit worried about the inverse where we always run with MPK by accident somehow and never see issues when it's turned off.
Otherwise, could the warning be tweaked a bit? Ideally what I'd imagine is that on failure a warning would be printed, but if everything succeeded no warning necessary as otherwise it can clutter up the view (e.g. this link has a number of non-actionable warnings at the bottom)
abrown updated PR #7815.
abrown has marked PR #7815 as ready for review.
abrown requested alexcrichton for a review on PR #7815.
abrown requested wasmtime-core-reviewers for a review on PR #7815.
abrown requested wasmtime-default-reviewers for a review on PR #7815.
Take a look at this run; this change will now only cause a single warning when MPK is not present.
alexcrichton submitted PR review.
alexcrichton merged PR #7815.
Last updated: Nov 22 2024 at 17:03 UTC