[ci] Use native arm runner instead of cross compiling #509
Conversation
mcbarton
changed the title
|
@alexcrichton can you review this PR, and if happy approve the ci to run? I'm not entirely sure I've named the PR well. Basically as far as I can tell from the ci your running a job which cross compiles for the arm architecture from the an x86 one (in this case Ubuntu). This feels unnecessary with Github recently making their Ubuntu arm runners generally available. |
|
CI I think should be running now (there's a failure to debug). I think it's reasonable to do a native build, yeah. Although I also don't personally think it's much of a problem to do a cross-build either, it's nice to exercise the infrastructure to ensure it's working. Basically I have no preference either way myself. |
|
@alexcrichton @sbc100 I thought it was the rust_target in the ci matrix that was causing this error https://github.com/WebAssembly/wasi-sdk/actions/runs/13142634365/job/36673180959?pr=509#step:10:741 . I was wrong, and unsure where to look to debug this. I've set the PR to be editable by maintainers if you have any ideas. |
|
Not sure what is happening with the ninja error at the moment. Trying reinstalling ninja to fix error like I did with rust without success. Will try and think of another solution. |
mcbarton
force-pushed
the
main
branch
3 times, most recently
from
9e8f478 to
d5f3fd6
Compare
|
@sbc100 This PR is ready for another review. I tried removing the --file and --tags from the docker build/run commands, but hit an issue (Removed both together so don't know if I can run one without the other). It feels that there must be an elegant alternative to the if statement I added in the Dockerfile, but just couldn't seem to see it. |
.github/workflows/main.yml
Outdated
| if: matrix.os == 'ubuntu-24.04-arm' | ||
|
|
||
| - uses: actions-rust-lang/setup-rust-toolchain@v1 | ||
| if: matrix.os == 'ubuntu-24.04-arm' |
There was a problem hiding this comment.
My only remaining question is why this is needed? Where was the rust toolchain previously coming from before the use of actions-rust-lang/setup-rust-toolchain here?
There was a problem hiding this comment.
I'm note entirely sure where in the repo the previous rust install was coming from, but any attempt by the ci at using it resulted in an error.
There was a problem hiding this comment.
It looks like maybe if you retore the rust_target then the rustup command below will run:
if [ "${{ matrix.rust_target }}" != "" ]; then
rustup target add ${{ matrix.rust_target }}
cmake_args="$cmake_args -DRUST_TARGET=${{ matrix.rust_target }}"
fi
Perhaps removing the rust_target was why this extra run was needed?
There was a problem hiding this comment.
If that doesn't work perhaps @alexcrichton has some clue?
There was a problem hiding this comment.
I know this doesn't work, since its something I've already tried.
There was a problem hiding this comment.
Do you have a CI log for when the changes here weren't present?
Where was the rust toolchain previously coming from before the use of actions-rust-lang/setup-rust-toolchain here?
GHA runners have rustup/rustc/cargo installed by default which is where the toolchain came from.
In the abstract though there are known crashes in rustc on the new GHA runners for arm64. I'm not sure if that would affect this.
There was a problem hiding this comment.
I reverted the change expecting to see the same error I saw previously, and the workflow ran without issue. Not sure what change I made since the error which fixed it.
There was a problem hiding this comment.
The crashes in question are random and seem to be an interaction between these CPUs and a kernel version that starts somewhere between Ubuntu 22 and Ubuntu 24. Use Ubuntu 22 if you want stable CI on these runners.
There was a problem hiding this comment.
Sounds reasonable, can we switch to Ubuntu 22 @mcbarton?
|
I'm not sure why, but why the latest commit downgrading from ubuntu 24 to 22? I agree with @sbc100 that prior to that this looks reasonable to me |
|
I also updated the issue description of the pinned rust-lang/rust issue with similar text |
|
Not sure whether I should revert back to Ubuntu 24.04 arm. Until I get confirmation either way i'll leave this PR as is with Ubuntu 22.04 arm. |
sbc100
changed the title
|
Leaving the arm runner on 22.04 makes sense. I think this change is good to land now, but I'll wait for @alexcrichton to confirm |
|
Oh sorry I missed the reply from @saethlin in my emails, apologies! |
With Ubuntu arm Github runners now available for general availability (see Github blogpost here https://github.blog/changelog/2025-01-16-linux-arm64-hosted-runners-now-available-for-free-in-public-repositories-public-preview/ ) there is no longer a need to use the Ubuntu x86 runner and cross compile in the ci. This means you no longer need to skip building the sysroot on the ci for this platform.