Skip to content
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

Update rust toolchain to 1.84 #3945

Merged
merged 14 commits into from
Jan 25, 2025

Conversation

har7an
Copy link
Contributor

@har7an har7an commented Jan 19, 2025

Updates the Rust toolchain from 1.75.0 (which is deprecated) to 1.84.0 (which is the latest stable version at the time of this writing).

This update covers quite a few toolchain version changes and necessitates a few changes in our code. See below (or the commits in this PR) for additional information.

I'll keep this open until I'm certain the change doesn't break the application. Most notably I currently get an error in the snapshot tests and I haven't ran a simulated release yet.

Once all of these pass, I'll merge this change.

Update the wasm32-wasi target to wasm32-wasip1

The wasm32-wasi target has triggered compiler-warnings since 1.81.0 and has been fully removed in January 2025. It has been renamed to wasm32-wasip1 so that's what we're also using now.

The target has been renamed appropriately in all locations where it was previously used, this includes (afaict):

  • Github CI stuff (where it installs the toolchain)
  • All plugin configurations
  • The toolchain config (in rust-toolchain.toml)
  • A few other internal files and functions, mostly for e2e tests and similar

Other breaking changes

  • The PanicInfo type has been renamed to PanicHookInfo (260be7f)
  • Two clippy lint errors have been introduced that actually uncovered a bug in our code (a9aa1c3, 5a2ca0e, 50d570d)

Other non-breaking changes

  • Don't request the rust-analysis toolchain component any longer (b2c86dd)
  • Remove the deprecated Makefile.toml (29d8750)

which really should have been deleted as part of zellij-org#2012. This hasn't been
updated for more than 2 years now and I don't expect anyone to still use
this. Our build process is now managed by `cargo xtask`.
from 1.75.0 which has been deprecated for a while now. Along with this
change, the `wasm32-wasi` target is no longer available (see subsequent
commit for additional info).
as required by the Rust project. The `wasm32-wasi` target name has been
retired and will likely be reused at a later time, although to express
an entirely different target (i.e. implementation of the WASI standard).

For additional information, see:

  - https://blog.rust-lang.org/2024/04/09/updates-to-rusts-wasi-targets.html
  - https://blog.rust-lang.org/2024/09/05/Rust-1.81.0.html#wasi-01-target-naming-changed
from the `rust-toolchain.toml` definition. This was added way back in
2021 via 8688569, and while I'm not sure what it expressed back then,
nowadays it refers to [Metadata for RLS][1], which apparently was an
early language server implementation and has long since been replaced by
*rust-analyzer*.

We don't want to propose or enforce the use of a specific toolchain and
in any case, setting this up properly is the job of a developers
IDE/Editor.

[1]:
https://github.com/rust-lang/rustup/blob/1f06e3b31d444f3649dd51225a9d38362f7313e0/doc/user-guide/src/concepts/components.md#previous-components
from `std::panic::PanicInfo` to `std::panic::PanicHookInfo`, which was
introduced in Rust 1.81.0. For additional information, see:

- https://releases.rs/docs/1.81.0/#compatibility-notes
- rust-lang/rust#115974
in match arm patterns, since the expression being matched against has
been modified using `to_ascii_lowercase`. Hence, we cannot have upper
case ASCII chars in the expressions (these arms were previously no-ops).
in `input/layout` since the `PartialEq` trait is also implemented
manually. Previously the `Hash` impl wasn't consistent with the `Eq`
impl, which can have weird effects when using these types in e.g.
`HashMap`s or similar types. For additional information, see:

  - https://rust-lang.github.io/rust-clippy/master/index.html#derived_hash_with_manual_eq
  - https://doc.rust-lang.org/stable/std/hash/trait.Hash.html#hash-and-eq
in `pane_size` since the `PartialEq` trait is also implemented manually.
Previously the `Hash` impl wasn't consistent with the `Eq` impl, which
can have weird effects when using these types in e.g. `HashMap`s or
similar types. For additional information, see:

  - https://rust-lang.github.io/rust-clippy/master/index.html#derived_hash_with_manual_eq
  - https://doc.rust-lang.org/stable/std/hash/trait.Hash.html#hash-and-eq
with their same names. Latest rust toolchains reject this code.
for the Rust toolchain. The previously used action has been archived
over a year ago. The new one should also support reading our
`rust-toolchain.toml`, so we no longer have to keep track of the
toolchain in multiple places.
to make them better visually parsable.
since as far as I can tell, this isn't used any more.
and only request an action without running other code.
@har7an
Copy link
Contributor Author

har7an commented Jan 20, 2025

So the CI passes alright and cargo x make passes as well locally on my PC. Next I'll verify the simulated release works and the built binary is executable.

@tlinford I hope it's okay if I ping you: Iirc you've got a Mac lying around somewhere. Could you try to run the built binary off this branch on Mac just to make sure I'm not upsetting apple users with this change?

@tlinford
Copy link
Contributor

So the CI passes alright and cargo x make passes as well locally on my PC. Next I'll verify the simulated release works and the built binary is executable.

@tlinford I hope it's okay if I ping you: Iirc you've got a Mac lying around somewhere. Could you try to run the built binary off this branch on Mac just to make sure I'm not upsetting apple users with this change?

Yes, no worries. Gave it a quick smoke test, compiled and ran fine.

@har7an
Copy link
Contributor Author

har7an commented Jan 25, 2025

Thanks for the feedback @tlinford! In the meantime I got the simulated releases to work with Rust 1.84 and the binaries I built/released from this branch compile and run fine (as far as I could determine).

The new rust version seems to also introduce a bunch of build-time warnings into xtask and zellij itself but as far as I can tell it's still operating as normal. In order to get this out the door so everyone can get on with their stuff, I'll merge this PR and likely open a follow-up to go through these warnings and eliminate them.

@imsnif This PR touches CI stuff (also for the release pipeline) so I'll stick around during the next release.

@har7an har7an merged commit 10df29e into zellij-org:main Jan 25, 2025
6 checks passed
@har7an har7an deleted the chore/update-rust-toolchain branch January 25, 2025 17:43
@har7an
Copy link
Contributor Author

har7an commented Jan 25, 2025

@bjorn3 I saw your comment over in bytecodealliance/wasmtime#10074, so I thought I'd let you know the toolchain has been updated now. :)

@bjorn3
Copy link
Contributor

bjorn3 commented Jan 25, 2025

Thanks. Opened #3955.

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