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

Remove once_cell as a dependency #2949

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

james7132
Copy link
Contributor

Motivation

Decrease the number of dependencies to improve compilation times.

Solution

Replace existing uses of once_cell::sync::Lazy with std::sync::OnceLock, or use const initialization where possible. This bumps the MSRV for a few crates to 1.70, but should keep the baseline MSRV at 1.63.

@mladedav
Copy link
Contributor

Can you please also look into updating thread-local? tracing-subscriber still transitively depends on once_cell because of that.

Copy link

@notgull notgull left a comment

Choose a reason for hiding this comment

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

Personally I think this is too aggressive of an MSRV bump, especially for a crate that's at the bottom of the Rust ecosystem. Debian Stable is a good MSRV limit, I'd say.

@mladedav
Copy link
Contributor

Debian Stable is a good MSRV limit, I'd say.

For the record, current Debian stable is bookworm which offers 1.63, which is the current MSRV version.

@james7132
Copy link
Contributor Author

james7132 commented Apr 23, 2024

For thread-local, Amanieu/thread_local-rs#76 should handle this without pushing past 1.63.

Note that the MSRV policy in the README states last 3 versions of stable Rust, which would put a cap at 1.74, far past 1.63. It does seem like Debian Sid is at 1.70 now, so it's likely it will be used in the next Debian release, but that need to wait until mid-2025 or later before that's available. I can separate out the changes for tracing-flame, tracing-subscriber, and tracing-log into a separate PR if need be.

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