Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add m1 to build matrix and release #3983

Merged
merged 2 commits into from
Apr 7, 2022

Conversation

ricochet
Copy link
Contributor

Adds m1 target as discussed in #3982.

NOTE: This doesn't adjust the install.sh script.

Locally, I ran the wasmtime tests on a 2020 macOS Monterey M1:

test result: ok. 641 passed; 0 failed; 20 ignored; 0 measured; 0 filtered out; finished in 13.92s

@alexcrichton
Copy link
Member

Thanks for this! I don't think that CI can run the tests since I believe it's x86_64 hardware, but we should probably be able to add a builder so long as it doesn't take unduly long in CI. I think though that the build-tarballs.sh script needs to be updated for the new target

@cfallin
Copy link
Member

cfallin commented Mar 31, 2022

This seems fine to me as long as we have some testing on the releases -- unfortunately GitHub Actions still does not support macOS/aarch64 (see actions/runner-images#2187) so we can't run our tests in CI. (This has been the blocker for adding official M1 releases so far, though we've had unofficial support in-tree thanks to @bnjbvr.) Last I heard, Embark Studios folks were running their own internal CI on M1 systems (@bnjbvr can you confirm this is still active?) -- if we have that, and if #3955 goes in so we have two weeks between branching a release and publishing it, then we could rely on Embark to notify us if there is some breakage with a given release. Not optimal, but it's something. Thoughts?

@ricochet
Copy link
Contributor Author

ricochet commented Apr 1, 2022

If we have a release milestone, we can do manual testing on a m1 and sign-off for releases. Not ideal, but workable. I would gladly volunteer until an automated solution is put in place.

A different option would be to use a self-hosted runner. Now that AWS and other providers have aarch64-darwin VM's, we could kickoff a workflow for verifying the m1 binary. This issue seems a little closer to completion than the a hosted VM but that's just a guess actions/runner#805.

@alexcrichton
Copy link
Member

I think even with a custom runner our hands are sort of tied because GitHub's own official recommendation is that for public repositories you shouldn't use self-hosted runners for security reasons. If you're ok manually verifying that builds are ok that may be the best way to go. With the release process from #3955 there will be a 2-week window between when a release branch is created and when it's actually published which should provide a good opportunity to test on non-CI-tested-platforms.

If you'd like it could also be set up to notify you. We could apply a tag to the PR and then using our labeling bot you'd get auto-cc'd on any PRs related to a major version bump

@bnjbvr
Copy link
Member

bnjbvr commented Apr 4, 2022

Confirming that we still do have some nightly CI job running the test suite (using the checked-in scripts) every day, using the latest commit on main at the time of running the CI job. Results are public, but only we at Embark Studios can start jobs etc.

@ricochet
Copy link
Contributor Author

ricochet commented Apr 5, 2022

That's fantastic. Then this might be ready to merge? I added a reference to embark's CI so that it will be part of the initial PR for releases.

@alexcrichton
Copy link
Member

Oh I think CI is still failing on this, the build-tarballs.sh script needs an update to use the right path to the artifacts (right now it assumes target/release/* since macOS previously didn't cross-compile but now it's sometimes in target/aarch64-apple-darwin/release/*)

This will create a pre-compiled binary for m1 macs and adds
a link to review embark studios CI for verification.
@alexcrichton
Copy link
Member

I think the matrix entry for the test build may have snuck back in? Otherwise though the script changes look good to me, thanks!

To double-check, can you confirm the release on this page works locally for you? This should be the direct link there

@ricochet
Copy link
Contributor Author

ricochet commented Apr 6, 2022

I think the matrix entry for the test build may have snuck back in? Otherwise though the script changes look good to me, thanks!

Ack!

To double-check, can you confirm the release on this page works locally for you? This should be the direct link there

LGTM tried the example and ran a compile on one of my wasm modules.

./wasmtime --version
wasmtime 0.37.0

Tests will not succeed for macos arm until GitHub provides a an m1 hosted runner.
@alexcrichton alexcrichton merged commit 76f7cde into bytecodealliance:main Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants