bump msrv to 1.63

[pascalkuthe]

Firefox 109 was released with a MSRV of 1.63.
In accordance with the MSRV policy this PR bumps the MSRV to 1.63.

Fixes #1557
Fixes #5432

[pascalkuthe]

There is only one related TODO in the codebase (in view.rs) but that code will be replaced #5420 anyway so I didn't fix the TODO to avoid merge conflicts.

[the-mikedavis]

I think this file doesn't end up being used since we switched to dtolnay/rust-toolchain. This can probably be removed now

[the-mikedavis]

Ah actually it looks like we don't use stable rust for the stable CI check: msrv and stable checks are running the exact same thing. In the check job, we could use matrix: [stable, 1.63] and then:

- uses: dtolnay/rust-toolchain@master
  with:
    toolchain: ${{ matrix.toolchain }}

to replace the helix-editor/toolchain action.

[pascalkuthe]

I am not sure we actually want to have a stable build.
Whenever a new rust version releases you get new clippy lints which means that PR piplines could just randomly start failing. This is especially problematic as clippy often inrdocues new lints which can only be fixed with new language features (example then_some).

It would also be annoying development wise because the rust_toolchain file ist pinned to the MSRV so we only know about clippy lints once the CI fails. I think it honestly makes more sense to only run CI on MSRV.

Rust breaking backwars incomptatbility is rare enough that we don't need to worry about it and I would rather fix new clippy lints when the MSRV is bumped like in this PR so I don't see much value in running a stable CI. This would also speedup CI times which is not super important but a nice bonus

[the-mikedavis]

Ah good point, I agree. I can't find the history for it but if I remember correctly, we pinned to 1.61 because we had a bunch of CI failures from a new rust version release being used automatically that ❌'d a bunch of open PR's CI aa87adf. I would be in favor of just removing the "stable" branch from CI

[CptPotato]

One thing to note is that I'm unable to build helix on windows with a rust toolchain lower than 1.64.

This is due to the depenency on the windows crate:

error: failed to compile `helix-term v0.6.0 ([...]\helix\helix-term)`, intermediate artifacts can be found at `[...]\helix\target`

Caused by:
  package `windows v0.43.0` cannot be built because it requires rustc 1.64 or newer, while the currently active rustc version is 1.61.0

[pascalkuthe]

One thing to note is that I'm unable to build helix on windows with a rust toolchain lower than 1.64.

This is due to the depenency on the windows crate:

error: failed to compile `helix-term v0.6.0 ([...]\helix\helix-term)`, intermediate artifacts can be found at `[...]\helix\target`

Caused by:
  package `windows v0.43.0` cannot be built because it requires rustc 1.64 or newer, while the currently active rustc version is 1.61.0

That is unfortunate. Probably just another MSRV bump on a minor release. Using cargo install --locked will fix the problem (already part of the documentation). We stick with the MSRV of firefox so that helix can be build on all distros (except debian). As all distros usually want to package the newest firefox fore security reasons. This is documented in our MSRV policy.

Firefox will raise it's MSRV to 1.65 in mid feburary so this Problem should also not exist for long.

[pascalkuthe]

I have changed the CI so it only runs on MSRV (the same behaviour as before but we now only have one check). However github still expects the old checks so the CI can not succed for this PR. I am not sure where to change that (or maybe it changes automatically once this PR is merged into master?).

[the-mikedavis]

We'll need @archseer to adjust that: there are branch protection rules in the settings that will need to be updated.

This will be a sort of breaking change for the CI runs for each PR: each PR will need to be rebased on this to satisfy the new check requirements. I wonder if we can side-step that by renaming the new "Check" job as "Check (msrv)"? Then we could just remove the "Check (stable)" from the required passing checks and no PRs would need to be rebased. I'm not sure if it's based on the naming like that though

[pascalkuthe]

This will be a sort of breaking change for the CI runs for each PR: each PR will need to be rebased on this to satisfy the new check requirements. I wonder if we can side-step that by renaming the new "Check" job as "Check (msrv)"? Then we could just remove the "Check (stable)" from the required passing checks and no PRs would need to be rebased. I'm not sure if it's based on the naming like that though

That is something I didn't think about. I am not sure how to solve this.
I would except the checks to be ID based but if we can work around the problem with names that would nb nice.
I tried understanding the GH docs but they are not exactly written well.

It would be neat to remove the duplicate CI run but it's not big deal and probably not worth breaking PRs over.
If there is no way around that I will revert to simply running the MSRV CI twice.

What do you think @archseer?

[the-mikedavis]

Hah, it works! https://github.com/the-mikedavis/ghactionstest/pull/2

I guess it must be naming-based internally. I suppose that makes sense since they're reading the workflows out of the yaml so it would be hard to have a recognizable workflow ID

[pascalkuthe]

Hah, it works! the-mikedavis/ghactionstest#2

I guess it must be naming-based internally. I suppose that makes sense since they're reading the workflows out of the yaml so it would be hard to have a recognizable workflow ID

Thanks for testing this! I have updated the PR accordingly.
I somehow taught it would be a huge pain to setup but I just realized I could have performed the same test you did by simply pushing the same changes I just did and test if the CI passes. Sorry about that and thanks for checking :)

@archseer still needs to remove the Check (stable) requirement (which is already fine because currently Check (msrv) is identical anyway) but once that is done, then this should be good to go.