-
Notifications
You must be signed in to change notification settings - Fork 108
CI Updates #426
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
CI Updates #426
Conversation
|
In retrospect, I should have used Cheers |
davidhewitt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! A few further ideas:
| strategy: | ||
| matrix: | ||
| os: [ubuntu-latest, macos-latest] | ||
| os: [ubuntu-latest, ubuntu-24.04-arm, macos-15-intel, macos-latest] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're now building both arm & intel for unix binaries, let's add the same for windows binaries too please. (i.e. windows-11-arm image)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! 👍
.github/workflows/lint.yml
Outdated
| - name: Clippy | ||
| if: ${{ matrix.rust == 'stable' }} | ||
| run: just lint | ||
| # Need to re-add rustfmt clippy in the nightly toolchain for "just" | ||
| run: | | ||
| rustup component add --toolchain nightly-x86_64-unknown-linux-gnu rustfmt clippy | ||
| just lint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it's not working as intended; the clippy job has a matrix.rust == 'stable' guard, nightly toolchain should not be relevant here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, the nightly toolchain addition was a workaround because just lint calls fmt-check first, which requires cargo +nightly fmt. 🤷♂️
Fixed by running clippy commands directly instead of just lint, since formatting is already checked in the nightly matrix job. This removes the confusing nightly dependency from the stable clippy job. Does it make sense?
.github/workflows/ci.yml
Outdated
| - os: windows-latest | ||
| target: x86_64-unknown-linux-gnu | ||
| - os: macos-latest | ||
| rust: beta |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's simplify by removing all beta builds except for beta on ubuntu.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done - removed beta builds from Windows and macOS, keeping beta only on Linux (both x86_64 and arm64). This reduces the CI matrix from 12 to 8 jobs.
| Platform | Jobs |
|---|---|
| Windows (msvc + gnu) | 2 (stable only) |
| macOS (x86_64 + arm64) | 2 (stable only) |
| Linux (x86_64 + arm64) | 4 (stable + beta) |
|
Hi David, thanks for the comments! Agree on everything, and replied above. Here are the workflow runs after the updates: From branch
From branch Btw, I also went ahead and updated actions/cache to v5. |
davidhewitt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, let's give this a whirl and assuming CI passes, will merge 👍
|
Hello @davidhewitt,I hope you're doing good. Is there any chance that you use the new CI to release a new set of artifacts? I'd love to update my CI and fetch the new artifacts. In other words, is there a patch release around the corner? ;) It would be just convenience for me, so no pressure. Again, thanks! |
Hi, thanks a lot for this project!
I use it in my own CI I like the fact you release pre-built binaries, however I was missing some platforms. So here's a PR to modernize the workflows and add 2 platforms. I hope you'll like it!
Note: I think this PR is worth looking at commit by commit, at least at first.
Changes
Release artifacts
aarch64-apple-darwin(macOS ARM64) andaarch64-unknown-linux-musl(Linux ARM64) release binariesmacos-15-intelfor Intel Macs,macos-latestfor ARM64,ubuntu-24.04-armfor Linux ARM64CI improvements
includelist, making it easier to add new platformsaarch64-apple-darwin) and Linux (aarch64-unknown-linux-gnu)egor-tensin/setup-mingw@v2withmsys2/setup-msys2@v2Workflow modernization
set-outputcommands with$GITHUB_OUTPUTactions/checkoutto v4 andSwatinem/rust-cacheto v2actions-rs/toolchain(unmaintained) todtolnay/rust-toolchainacross all workflowsrust-toolchain.tomlremoval in CI workflows to ensure matrix toolchain selection is respectedoverrideinactions-rs/toolchain, needs this removal with the standard dtolnay/rust-toolchainlint.ymlworkflow aroundjustbecause it requires nightly even when testing stable targets in the matrixTesting
Release workflow
Tested the release workflow with the additional targets: check the Release v0.18.0-test created for demo purposes in my fork, on the branch
ci/macos-arm64. It contains the targets I mentioned above.CI workflows
Tested the CI workflows on my fork with one additional commit branch on branch
ci/macos-arm64-exercise- all builds and tests succeeded.