search

hwchen / keyring-rs

Cross-platform library and utility to manage passwords
Apache License 2.0
450 stars 49 forks source link

Replace lazy_static with OnceLock #160

Open russellbanks opened 6 months ago

russellbanks commented 6 months ago

This pull request removes lazy_static in favour of OnceLock. OnceCell is technically a crate. However, it has been partially merged into the standard library.

lazy_static hasn't had a release in 4 years and now has a section on its ReadMe that shows how to achieve the exact same thing with the standard library.

brotskydotcom commented 6 months ago

I don't want to move the minimum build platform for this crate to 1.70 (June 2023), which is required by these changes. There are a lot of clients on second-tier platforms (such as the BSDs), and I have seen the built-in rust on those platforms be a year or more old. I released v2 in Feb 2023 (on Rust 1.68), and I'd rather not break backward compatibility by moving the library to require 1.70. (Yes, the example uses clap 4.22, which requires Rust 1.70, but folks who use the library don't need clap.)

I'm happy to revisit this when the 2024 edition ships, and I expect to release a v3 somewhere around then that will integrate this change. Marking this PR as waiting until then.

russellbanks commented 6 months ago

Until then, we could use the actual OnceCell crate? It has an MSRV of 1.60. We'd still benefit from it being maintained and it achieves the same thing as lazy_static without using a macro.

russellbanks commented 6 months ago

I've replaced this with Lazy from OnceCell which more closely matches lazy_static. Once LazyCell (standard library equivalent of Lazy) gets stabilised and/or we move to a higher MSRV, we can replace this with OnceLock or LazyCell.

brotskydotcom commented 6 months ago

How about, instead of just switching crates from lazy_static to OnceCell, we instead leave lazy_static in place for older compilers, add the rustversion crate, and use the stable language feature you were originally suggesting on MSRV of 1.70 or higher? Since almost everyone is on 1.70 anyway, I'd rather make a change to the stable language feature now instead of just picking up a different crate dependency that affects everyone (and that i would have to carefully test on 1.68 to be sure there's no change).