Fix aarch64 macos release builds #6504
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The 'dtolnay/rust-toolchain' action ignores the rust-toolchain.toml file, but the installed 'cargo' respects it. This can create a version mismatch if the MSRV is different from the stable rust version. Any additional targets installed by rustup like aarch64-darwin might not be installed for the correct version. To fix this, we remove the rust-toolchain.toml file before calling 'cargo'.
the-mikedavis
added
C-bug
This step without the '-p' works fine for regular releases but it can fail if the CI is running when this file changes or on a branch matching 'patch/ci-release-*'.
pascalkuthe
reviewed
Mar 31, 2023
Comment on lines
+117
to
125
| # The rust-toolchain action ignores rust-toolchain.toml files. | ||
| # Removing this before building with cargo ensures that the rust-toolchain | ||
| # is considered the same between installation and usage. | ||
| - name: Remove the rust-toolchain.toml file | ||
| run: rm rust-toolchain.toml | ||
|
|
||
| - name: Install ${{ matrix.rust }} toolchain | ||
| uses: dtolnay/rust-toolchain@master | ||
| with: |
There was a problem hiding this comment.
we might want to force this to MSRV (1.65) instead of stable?
There was a problem hiding this comment.
Since we control the runners doing the building here I think it's ok to use a newer Rust. We could definitely pin this at the MSRV though to reduce the chances that a future Rust would give compilation errors for code that is valid when compiled by the MSRV. What do you think @archseer?
There was a problem hiding this comment.
Similar to ripgrep we want to build with the latest nightly to get as many optimizations as we can. The MSRV is just there for backwards compatibility
There was a problem hiding this comment.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #6495
See the dry-run https://github.com/helix-editor/helix/actions/runs/4576449372
This could be solved instead by using
rustup target add ${{ matrix.target }}but that would be papering over the problem: we do install stable-aarch64-apple-darwin but that's for the wrong Rust version.cargolooks to use 1.65 because of therust-toolchain.tomlfile butdtolnay/rust-toolchaininstalls it for stable which is 1.68.2 currently.