-
Notifications
You must be signed in to change notification settings - Fork 7
chore: upgrade rust to 1.90.0 #168
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
Conversation
Haven't updated in a while. Should probably semi-automate this (new lints will still require manual intervention). What attracted my attention in [1.90.0](https://blog.rust-lang.org/2025/09/18/Rust-1.90.0) is that `lld` linker is now used by default on Linux. Which also reminds me that we should play with [cross-language LTO sometimes](https://blog.llvm.org/2019/09/closing-gap-cross-language-lto-between.html) - we use rather heavy C dependency, so it could help squeezing some more perf juice out of it. Not even mentioning how cool is that to be able to link C and Rust code like it's the same thing! Need to update LLVM in the container to version 20 first though.
Has been replaced with `LazyLock` and const statics
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.
Pull Request Overview
Upgrade Rust toolchain to 1.90.0 and adopt newly stabilized std features, removing external dependencies and modernizing syntax.
- Replace lazy_static with std::sync::LazyLock and const slices
- Introduce newer standard library conveniences (&raw mut, isize::midpoint, std::io::Error::other) and adjust formatting/error messages
- Remove unused dependency (lazy_static) and tweak lint configuration
Reviewed Changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
rust-toolchain.toml | Bumps toolchain to Rust 1.90.0 |
packages/pangraph/src/utils/lock.rs | Removes must_use on Clone impl |
packages/pangraph/src/utils/float_fmt.rs | Replaces lazy_static with LazyLock |
packages/pangraph/src/utils/error.rs | Simplifies iterator filter logic |
packages/pangraph/src/pangraph/reweave.rs | Cleans stray semicolons after if blocks |
packages/pangraph/src/pangraph/detach_unaligned.rs | Uses inline format string capture |
packages/pangraph/src/io/yaml.rs | Adjusts error message path formatting |
packages/pangraph/src/io/json.rs | Adjusts error message path formatting |
packages/pangraph/src/io/gfa.rs | Adjusts error message path formatting |
packages/pangraph/src/io/fs.rs | Adjusts directory and file error message formatting |
packages/pangraph/src/io/file.rs | Adjusts file open/create error message formatting |
packages/pangraph/src/io/compression.rs | Uses Error::other and removes ErrorKind import |
packages/pangraph/src/commands/root_args.rs | Replaces lazy_static vector with const slice |
packages/pangraph/src/circularize/merge_blocks.rs | Modern inline formatting in logs |
packages/pangraph/src/align/nextclade/align_with_nextclade.rs | Replaces lazy_static with LazyLock in tests |
packages/pangraph/src/align/nextclade/align/seed_alignment.rs | Uses isize::midpoint for safer average |
packages/pangraph/Cargo.toml | Removes lazy_static dependency |
packages/minimap2/src/options.rs | Uses &raw mut for raw pointer creation |
packages/minimap2/src/map.rs | Uses &raw mut for FFI call argument |
Cargo.toml | Removes lazy_static and adds new lint allowance |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
Very nice! Thank you for taking care of the upgrade. Everything looks good to me.
I didn't know about cross-language LTO, that is very cool!
Given that calls to minimap2 might be the most time-consuming part of the process it might be worth looking into, especially if it's a matter of just a few changes...
Haven't updated in a while. Should probably semi-automate this (new lints will still require manual intervention).
What attracted my attention in 1.90.0 is that
lld
linker is now used by default on Linux. Which also reminds me that we should play with cross-language LTO sometimes - we use rather heavy C dependency, so it could help squeezing some more perf juice out of it. Not even mentioning how cool is that to be able to link C and Rust code like it's the same thing! Need to update LLVM in the container to version 20 first though.