Stream: git-wasmtime

Topic: wasmtime / PR #7815 mpk: enable MPK if available in CI


view this post on Zulip Wasmtime GitHub notifications bot (Jan 24 2024 at 22:01):

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:

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

view this post on Zulip Wasmtime GitHub notifications bot (Jan 24 2024 at 22:05):

abrown updated PR #7815.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 24 2024 at 22:30):

abrown updated PR #7815.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 24 2024 at 22:31):

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)

view this post on Zulip Wasmtime GitHub notifications bot (Feb 03 2024 at 00:17):

abrown updated PR #7815.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 03 2024 at 00:31):

abrown has marked PR #7815 as ready for review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 03 2024 at 00:31):

abrown requested alexcrichton for a review on PR #7815.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 03 2024 at 00:31):

abrown requested wasmtime-core-reviewers for a review on PR #7815.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 03 2024 at 00:31):

abrown requested wasmtime-default-reviewers for a review on PR #7815.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 03 2024 at 00:53):

abrown commented on PR #7815:

Take a look at this run; this change will now only cause a single warning when MPK is not present.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 03 2024 at 18:33):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 03 2024 at 19:18):

alexcrichton merged PR #7815.


Last updated: Oct 23 2024 at 20:03 UTC