Skip to content

Conversation

@dhardy
Copy link
Member

@dhardy dhardy commented Oct 20, 2025

  • Added a CHANGELOG.md entry

Summary

Bump MSRV and Edition for rand_core

@dhardy dhardy requested a review from newpavlov October 20, 2025 09:17
Copy link
Member

@newpavlov newpavlov left a comment

Choose a reason for hiding this comment

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

Do we still need a separate Cargo.lock.msrv with MSRV-aware resolver?

I think we could commit MSRV-compatible Cargo.lock (as much as I dislike it, it's the default resolver behavior) and re-generate Cargo.lock for non-MSRV jobs.

@dhardy
Copy link
Member Author

dhardy commented Oct 20, 2025

Our automated tooling should:

  1. Verify that the project is compatible with the MSRV using some specific set of dependencies. I don't think the particular versions matter much here; we just need to know that some compatible configuration exists.
  2. Check whether the project works with the latest Rust version and set of dependencies which match Cargo.toml requirements
  3. (Less critical) notify us of new breaking releases of dependencies

In theory Cargo.lock.msrv + specific CI test provides (1), other CI tests provides (2) and Dependabot config provides (3), though the latter only happens monthly. I don't see a particular reason to change this; I also don't particularly want to frequently update Cargo.lock as you do in getrandom.

@dhardy dhardy merged commit e99a339 into master Oct 20, 2025
15 checks passed
@dhardy dhardy deleted the push-qvmyotnmmqyo branch October 20, 2025 10:52
@newpavlov
Copy link
Member

My point is that unless you manually pass --ignore-rust-version Cargo.lock will be equivalent to Cargo.lock.msrv.

My suggestion is to remove Cargo.lock.msrv and simply use Cargo.lock instead of it and for (2) modify our non-MSRV CI jobs to use --ignore-rust-version. It would mean that breaking changes in upstream dependencies may break our CI, but it may not be a bug, but a feature, since it would resolve (3).

@dhardy
Copy link
Member Author

dhardy commented Oct 20, 2025

By "breaking releases" I meant bumps to the major/minor version, so no it would not resolve (3). For (2) we'd need an explicit cargo update.

I don't think we'd want --ignore-rust-version. Theoretically it shouldn't do anything on jobs using the latest rustc, while it could break MSRV builds (if a cargo update also happens there). So... why?

I guess the primary difference is what happens when someone checks out the repo and runs a Cargo command.

@newpavlov
Copy link
Member

newpavlov commented Oct 20, 2025

Theoretically it shouldn't do anything on jobs using the latest rustc, while it could break MSRV builds.

Nope. You have a wrong understanding of how MSRV-aware resolver works by default. If we have a patch release in an upstream dependency which requires Rust 1.90, the resolver would select an older version compatible with rust-version declared in our crate even on the latest stable. This is why it issues "Locking N packages to latest Rust 1.x compatible versions" message to notify you that some dependencies were not updated because of the MSRV requirement.

In the RFC discussion I strongly argued that it should use the currently used toolchain version, but, unfortunately, it was decided otherwise...

@dhardy
Copy link
Member Author

dhardy commented Oct 20, 2025

This still doesn't give us an incentive to commit Cargo.lock I think, but it does mean we shouldn't need Cargo.lock.msrv and that we should use --ignore-rust-version in at least the nightly CI job.

@newpavlov
Copy link
Member

newpavlov commented Oct 20, 2025

If you do not want to update Cargo.lock regularly, then one option is to not commit Cargo.lock at all and generate it from scratch as part of CI jobs (with or without --ignore-rust-version depending on whether it's an MSRV job or not).

Personally, I like to commit Cargo.lock since it allows to catch issues introduced in upstream dependencies, but I understand if you would prefer a slightly different trade off.

@dhardy
Copy link
Member Author

dhardy commented Oct 20, 2025

Yes, I'd prefer not to have so many dependabot PRs updating deps. We'll get build failures either way.

In the RFC discussion I strongly argued that it should use the currently used toolchain version, but, unfortunately, it was decided otherwise...

This actually feels pretty stupid: users could easily end up missing important fixes. Maybe then binaries should not specify rust-version?

@newpavlov
Copy link
Member

We'll get build failures either way.

The difference is that we would get build errors in PRs which update lock files, not in random PRs with unrelated changes.

This actually feels pretty stupid: users could easily end up missing important fixes.

Well, I agree. But it's not me whom you should be telling this, but @epage.

@epage
Copy link

epage commented Oct 20, 2025

If you commit your Cargo.lock but keep it MSRV compatible (having a "latest deps" job that does a cargo update), that helps with

  • any deps that don't declare their MSRV but bump their effective MSRV
  • root causing "it worked on my machine" issues
  • root causing performance issues
  • git bisecting

This actually feels pretty stupid: users could easily end up missing important fixes.

Feel free to checkout the RFC. We analyzed archetypes of use cases, compared their impacts on users, and then prioritized them for when they come into conflict with each other when selecting a solution. This led to "using latest deps despite MSRV" as one of the lowest priority workflow.

See also https://doc.rust-lang.org/nightly/cargo/reference/rust-version.html

@dhardy
Copy link
Member Author

dhardy commented Oct 21, 2025

@epage I apologise if I caused offence. I likely have a biased point of view, in particular since rand has for a while been testing against:

  • The latest nightly, beta and stable rustc with the latest dependencies
  • Nightly rustc with minimal-versions of dependencies
  • The MSRV with a fixed set of dependency versions

I appreciate that MSRV-aware version resolution simplifies testing of the MSRV, and also that for projects which commit Cargo.lock it likely makes sense to update this in a conservative manner.

I am surprised about the MSRV-aware dependency resolution behaviour for projects which do not commit Cargo.lock (it makes less sense to use older versions in this case), though I also wouldn't expect behaviour to depend on whether Cargo.lock has been checked into version control. This comes back to the updated recommendations of whether libraries should check Cargo.lock into version control — which clearly is useful to some libraries, but I'm not convinced is the best option here.

(And just committing Cargo.lock would not be enough for us — we'd still want to test minimal-versions and latest-versions too. Really, it would just be a less-stale variation of what we already to with Cargo.lock.msrv.)


The difference is that we would get build errors in PRs which update lock files, not in random PRs with unrelated changes.

Yes, and especially for less-frequently-updated repos like https://github.com/rust-random/rngs/ it would make sense to get such updates sooner. My preference however would be to run CI jobs on a schedule — I should set this up!

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.

4 participants