googlefonts
MSRV
What is the desired MSRV for this family of crates?
Right now, it is 1.76 to get a build. This is because tests within int-set import std::hash::DefaultHasher which is a new location for it. Prior to 1.76, it was available as std::collections::hash_map::DefaultHasher.
If that's addressed, then the MSRV is 1.75 because code within int-set uses the features described here: https://github.com/rust-lang/rust/issues/91611
I think it's fine to say 1.76 for now, and I would not place much value on trying to maintain a low MSRV for the foreseeable future; that is, if something in a new rust release is useful I'm happy for us to adopt it.
Pragmatically it's the lowest of google3 and Chrome. I'm not sure that is published in a form we could readily republish or consume in CI (which would be nice to do at some point).
On Tue, Jul 23, 2024, 1:30 PM Colin Rofls @.***> wrote:
I think it's fine to say 1.76 for now, and I would not place much value on trying to maintain a low MSRV for the foreseeable future; that is, if something in a new rust release is useful I'm happy for us to adopt it.
— Reply to this email directly, view it on GitHub https://github.com/googlefonts/fontations/issues/1019#issuecomment-2245704796, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABRKXAGRMKGEAK5OTSC5TBLZN2AKPAVCNFSM6AAAAABLKXVDDCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENBVG4YDINZZGY . You are receiving this because you are subscribed to this thread.Message ID: @.***>
The currently used 1.75 is a bit dated (December 2023). There has been quiet a few improvements since then - most are fairly small but together adding up. For example, c-literals c"foo" in v1.77 - not worth by itself, but cleaner for any FFI. If not a hard requirement, would it perhaps make sense to keep things "about a year old", i.e. 1.81 or so? Or better yet, just switch to the 2024 edition (1.85)
AFAIK there is no harm to a low MSRV if we don't need any of the new features?
As we now set a rust-version I believe this can be closed? - please reopen if I am mistaken.
@rsheeter there are two issues:
- for dependencies, your "real" current MSRV is actually 1.82 because
backtrace0.3.75 requires it:
$ cargo msrv find --all-features --ignore-lockfile --min 1.75
Result:
Considered (min … max): Rust 1.75 … Rust 1.89.0
Search method: bisect
MSRV: 1.82.0
Target: x86_64-unknown-linux-gnu
The 1.81 failed due to backtrace:
$ cargo tree -i backtrace
backtrace v0.3.75
├── backtrace-ext v0.2.1
│ └── miette v5.10.0
│ └── font-codegen v0.0.0 (.../fontations/font-codegen)
└── miette v5.10.0 (*)
- for regular code, it's not a "need" (required), but "want" (nice to have). For example, I just did a PR to harfrust which must have the same MSRV: https://github.com/harfbuzz/harfrust/pull/119
-libc::setlocale(libc::LC_ALL, b"\0" as *const _ as *const i8);
+libc::setlocale(libc::LC_ALL, c"".as_ptr());
clearly much more readable, but not a hard "must have". Later, we had to revert this because it breaks MSRV.
Codegen is not something a user would depend on. It's fair to say that makes workspace level version misleading.
I agree the c-literal is much nicer but ... worth increasing MSRV? That i'm less sure of, as you say it seems like a nice to have.
codegen just happened to determine the latest msrv. Without it, MSRV is still 1.82 because fauntlet uses iter_repeat_n feature. Without both, MSRV is 1.77 because of klippa. So in short -- the irony is that you are already on 1.77, which supports cstr :)
cargo msrv find --all-features --ignore-lockfile --min 1.75 -- cargo check --workspace --exclude font-codegen --exclude fauntlet
In short, I suspect MSRV is declared but not truly verified. In most crates I mantain, I have this workflow for MSRV validation.
Another thing I noticed - you rely on the old clippy - which is not a good pattern - clippy respects MSRV, so it won't suggest anything if the crates' msrv is older than the suggested fix. But at the same time you don't get the benefit of the last two+ years of clippy improvements that would catch a lot of newer issues.
Lastly - i brought up c-literals not as the required feature, but as an example of many features that devs cannot use - each one of them is too small in itself, but together they may represent significant inefficiency.
Good catch re klippa, filed https://github.com/googlefonts/fontations/issues/1594.
I believe we did do work to check msrv in CI (afk so not checking rn), but likely only for the crates that are actively used by major consumers (font-types, read-fonts, skrifa). klippa is on it's way but not quite there yet. If that's a figment of my imagination then we'll add it :)
Ty for the reference re MSRV validation.