workflows: check MSRV in CI.

[kivikakk]

• Run CI against the MSRV.
  • Ensure the naming means we don't have to adjust the branch protection rules when we adjust the MSRV.
• Update the MSRV used in CI when it's updated in Cargo.toml. There's no clear time to do such an update; CI can instead check this for us.

[github-actions]

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
./bench.sh ./comrak-1e8fa42	318.5 ± 2.2	314.8	323.9	2.85 ± 0.04
./bench.sh ./comrak-main	318.4 ± 1.5	316.1	323.0	2.85 ± 0.03
./bench.sh ./pulldown-cmark	111.7 ± 1.2	110.2	115.0	1.00
./bench.sh ./cmark-gfm	119.6 ± 4.1	116.7	135.0	1.07 ± 0.04
./bench.sh ./markdown-it	480.6 ± 2.0	475.7	484.3	4.30 ± 0.05

Run on Thu May 9 15:27:35 UTC 2024

[kivikakk]

Hooboy. This is frustrating enough that I want to pull the CLI binary out as its own crate — which makes sense, given I want different MSRV disciplines between them.

[digitalmoksha]

frustrating enough that I want to pull the CLI binary out as its own crate

I understand - I'm at 1.73 and while starting a new branch I had to up level because of the CLI.

However it would be a bit of a bummer, as it's nice to be able to debug directly using the RustRover IDE - it runs the CLI and I can just step through code.

Also, I think your compliance specs are run using the CLI.

Can we not pin the CLI crates to the version that we're supporting? Is some critical functionality/bug fixes getting lost?

[kivikakk]

Also, I think your compliance specs are run using the CLI.

Yep, my current WIP builds the library on nightly/beta/stable+MSRV and runs unit tests with it, then builds the binary on nighty/beta/stable and runs specs with it.

Can we not pin the CLI crates to the version that we're supporting? Is some critical functionality/bug fixes getting lost?

There's just a bunch of crates that are starting to get rather old — they left 1.62 behind a while ago — and so as a matter of course I don't really love leaving them back. This mostly applies to the CLI ones. The recently added in-place is hard to support too, though I can just as easily tear that out again, it's minor.

[kivikakk]

I think it's easier just to set a standard MSRV across both, so yeah, I'm going to do the work to get us back to 1.62.1 👍

[digitalmoksha]

If there are say security or performance fixes in a crate, when a user builds the library they should be able to specify a more recent version of that crate, right? My Rust dependency knowledge is still young.

[kivikakk]

when a user builds the library they should be able to specify a more recent version of that crate, right? My Rust dependency knowledge is still young.

Mine too, lol 😅

I'm not quite sure — probably? If it gets to that point, I'm sure someone will let us know!

[kivikakk]

lol okay. I was thinking cargo update would know not to update to versions out of its MSRV. It doesn't, yet, but it wouldn't help us anyway until we bump our MSRV to one with that Cargo anyway, I guess. Yeesh.

[kivikakk]

I don't think there's any non-insane way to make cargo update work while respecting MSRV for us. Even if I pin our top-level packages (some e.g.s: regex at = 1.9.6, clap at = 4.0.32), their dependencies still happily update to things beyond our MSRV (e.g. clap 4.0.32 depends on clap_lex@^0.3.0; clap_lex 0.3.3 requires Rust 1.64.0).

Dependency updates that Dependabot opens and CI passes on (as of this PR) are fine, and we can respond to e.g. cargo audit or requests if something has to change, but unless there's any other automated means out there ... ¯\_(ツ)_/¯ I've typed cargo update -p blah@x.y.z --precise a.b.c too many times already today.

[kivikakk]

Happy to take feedback from anyone interested; will merge in a day or two otherwise!

[digitalmoksha]

This looks great @kivikakk. I tested a build on 1.73 and it worked like a charm - it didn't require me to bump the rust version.

[kivikakk]

Awesome, really glad! 😊 Thanks for testing it!

[digitalmoksha]

@kivikakk so propfuzz.rs is no longer needed to do fuzzing?

[kivikakk]

That used https://github.com/facebookarchive/propfuzz, which (as the URL implies!) isn't actively worked on these days and never made it past an initial test release. (IIRC propfuzz predated cargo-fuzz, but yeah, we're in a much better position with the latter now.)

[gjtorikian]

Bleh, looks like the latest dependabot updates didn't merged because the requires MSRV tests failed. Maybe we should just chuck that automation—the longest time interval for the check is a month, which isn't going to really change anything in terms of support for MSRV (I would assume that would be on the order of months, if not years).

[digitalmoksha]

Ack, I think you might be right. I don't suppose dependabot can be made to have awareness of what our MSRV is and only recommend those updates?

What about just commenting it out for now, instead of ripping it out? Or do you think it's cleaner just to pull it.

[kivikakk]

What about just commenting it out for now, instead of ripping it out? Or do you think it's cleaner just to pull it.

Always delete! It'll be there in revision history when we need it again.

the longest time interval for the check is a month, which isn't going to really change anything in terms of support for MSRV (I would assume that would be on the order of months, if not years).

I'm inclined to agree. :/ Will remove for now. Thanks for all your work on that! I feel like we'll be restoring it in no time, the way the days seem to pass lately.

[gjtorikian]

I don't suppose dependabot can be made to have awareness of what our MSRV is and only recommend those updates?

Afraid not. The same issue @kivikakk linked to in rust-core is what's holding up dependabot 🙃 dependabot/dependabot-core#5423