Skip to content

CI: pin rust to 1.61.0 and check MSRV#2529

Merged
archseer merged 4 commits intohelix-editor:masterfrom
the-mikedavis:md-pin-rust-in-ci
May 23, 2022
Merged

CI: pin rust to 1.61.0 and check MSRV#2529
archseer merged 4 commits intohelix-editor:masterfrom
the-mikedavis:md-pin-rust-in-ci

Conversation

@the-mikedavis
Copy link
Copy Markdown
Member

Pinning to a particular version brings some extra burden that we have to remember to update rust, but I think it's worth it to make CI more reproducible around new rust releases. The CI uses new rust versions almost immediately after release which means that CI may start failing faster than we can fix the lints, as in #2514. Ideally we update rust and fix clippy lints all in one go.

Connects #2528

@the-mikedavis
Copy link
Copy Markdown
Member Author

the-mikedavis commented May 22, 2022

The CI rules will need to be updated to expect "Build / Test Suite (runner)" checks rather than "Build / Test Suite (runner, rust-channel)" and to replace the "Build / Check" expectation with "Build / Check (stable)" and "Build / Check (msrv)"


- name: Install stable toolchain
uses: actions-rs/toolchain@v1
uses: helix-editor/rust-toolchain@v1
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need our own fork?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We merged in actions-rs/toolchain#209 - without it we would need to use the deprecated rust-toolchain file rather than rust-toolchain.toml.

Looks like upstream might be unmaintained at the moment as well but we'll leave #2528 open to switch back if 209 gets merged upstream.

uses: helix-editor/rust-toolchain@v1
with:
profile: minimal
toolchain: stable
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not run with both stable and the fixed version? That we can make sure helix code can still run on stable in case someone use it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah we could have smoke tests on stable running on a cron or something. I just don't want it to fail CI for PRs when rust does a release.

@the-mikedavis the-mikedavis changed the title inherit rust toolchain channel from rust-toolchain.toml, pin rust to 1.61.0 CI: pin rust to 1.61.0 and check MSRV May 23, 2022
@the-mikedavis
Copy link
Copy Markdown
Member Author

I updated this branch to include a matrix for checking on the stable version and the MSRV and added the fix that brings us back down from 1.60.0 to 1.57.0.

@the-mikedavis the-mikedavis force-pushed the md-pin-rust-in-ci branch 2 times, most recently from 77fc474 to 1505423 Compare May 23, 2022 02:31
We've forked actions-rs/toolchain and merged
actions-rs/toolchain#209
so we can take advantage of full support of `rust-toolchain.toml`.
Without that PR, the action fails because the `rustup` version
built into the runners by default is too old. helix-editor#2528 covers switching
back to the upstream when it includes those changes.
1.61.0 in particular introduced new clippy lints that unexpectedly
failed CI until addressed. The lints are a bit tough to fix since
the toolchain action starts using new rust versions almost immediately
after release, so if you aren't using rustup, you may have a hard
time reproducing the lint results until your package manager updates
rust.

This brings an extra burden that we have to remember to make a
commit/PR to update rust in CI.
It's very easy to use new rust features without realizing it since
the CI and local development workflows may use the latest rust version.
We try to keep some backwards compatibility with rust versions to make
packaging easier for some OS-level package-managers like Void Linux's.
See helix-editor#1881.

This change runs the "Check" step for the pinned version of rust in
the rust-toolchain.toml file as well as the MSRV version in a matrix.
In order to bump the MSRV, we need to edit

    .github/workflows/msrv-rust-toolchain.toml

This commit sets the MSRV as 1.60.0 but a later child commit will
reduce the MSRV back to 1.57.0.

Closes helix-editor#2482.
This line uses the Display trait for io::ErrorKind which was
stabilized in Rust 1.60.0. We can set MSRV all the way back to
1.57.0 by replacing it with a pretty-print.

Closes helix-editor#2460.
@archseer archseer merged commit 89c0998 into helix-editor:master May 23, 2022
@the-mikedavis the-mikedavis deleted the md-pin-rust-in-ci branch May 23, 2022 16:06
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.

3 participants