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.

Sign up for GitHub

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

Jump to bottom

Programmatically advertize MSRV #513

Closed
wants to merge 1 commit into from
Closed

Programmatically advertize MSRV #513

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ authors = ["Ryan Leckey <leckey.ryan@gmail.com>"]
categories = ["config"]
license = "MIT OR Apache-2.0"
edition = "2018"
rust-version = "1.75"
Copy link
Collaborator

@polarathene polarathene Feb 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I have encouraged to add this in the past, and at least prior to recent activity the other day the MSRV was actually much lower IIRC, although now a dependency update was merged that definitely bumped it notably higher 🤷‍♂️

This could be lowered before the next release, but I don't have time to go through the process again of minimizing the MSRV.

Since this is an area @epage is quite familiar with, should rust-version represent the actual MSRV? (which I believe we had verified for 0.13 at least via CI with a msrv focused lockfile)

Or should it represent the MSRV that you could get from say cargo-msrv and/or related cargo commands (not sure of the status of the msrv one that I recall required nightly), without any lock file assistance?

I recall encountering a variety of scenarios where rust-version was either inaccurate, or the lack of it (along with some other toolchain version specific parsing gotchas) complicated installing MSRV compatible dep versions.

For example (from memory):

  • A tokio crate added a change but the rust-version wasn't bumped accordingly.
  • A popular crate hashbrown? Chose a rust-version that was only valid if you opt-in to a feature for one of their dependencies which didn't have rust-version configured.
  • edition (despite what I recall official docs saying) enforces a min rust-version implicitly, so rust-version is effectively a granular offset from that?
  • Some crates set the patch version in rust-version rather than just major.minor, even if that's not necessary.
  • rust-version is only accurate without a lockfile for a published crate provided the dep chain respects that with their own releases/dependencies. Notably when rust-version isn't used throughout the dep chain?

The maintainer of this repo tends to have the habit of updating deps and raising the MSRV when that no longer installs deps successfully. That's historically also been when dev dependencies only needed for CI testing introduce the bump.

That doesn't really match my impression/expectation of what the rust-version here should be, but I understand why that's their approach out of convenience / effort required to maintain it.

Regardless, if this change is applied, edition may as well be bumped to 2021 too, likewise to 2024 when that's out and this gets bumped again (even though it may be unnecessary). Which I assume may negatively impact those with older toolchains, especially when the crate could be supported on an older toolchain release just fine (last year IIRC a contributor was insistent on keeping 1.65 support?).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An MSRV should be verified. If a Cargo.lock is known to contain MSRV compatible dependencies, then existing CI jobs with latest rustc should be sufficient for cargo test with just a cargo +<msrv> check job (can be automated with cargo hack).

This verification usually means that the lockfile and all parts of the projects are usable with the MSRV. There are more advanced / specialized cases where people do otherwise.

The opinion of at least some of us on the cargo team is that "support" in MSRV is a matter of policy and not syntax, so it doesn't have to be what cargo msrv reports but its a a matter of what you are willing to test and workaround.

As for dependencies, so long as there are resolvable dependencies that pass your tests, your MSRV is satisfied and you do not need to bump MSRV just because a new dependency version is out that uses it, unless you are choosing to update your dependency.

There are more advanced cases where a feature may raise an MSRV but those should be exception cases. A user, or an MSRV-aware resolver (which no one disputes should happen; they only dispute defaults) will only have package.rust-version to go off of so that should generally be used to represent all interactions with the project.

(last year IIRC a contributor was insistent on keeping 1.65 support?).

For 1.0+ crates, I'm thinking of reserving MSRV bumps for minor versions and offering a certain level of support for previous minor versions, accepting cherry-picks of fixes to every prior supported version. I'm somewhat tempted to say that my MSRV policy applies to that version and that mains package.rust-version will be as low as that version but may be updated to newer versions when a maintainer-defined reason requires. This would allow me to give faster turnaround for features dependent on new toolchains while offering support for people on older versions.

Copy link
Collaborator

@polarathene polarathene Feb 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

unless you are choosing to update your dependency.

Yes, that's what happened recently by the project maintainer.

More information: #494 (comment) (NOTE: MSRV was bumped recently, before the rebase to pass CI and merge)

There are more advanced cases where a feature may raise an MSRV but those should be exception cases.

In the above case:

  • rust-ini made a release with a dependency ordered-multimap 0.7. If you used this dependency at the time all was well.
  • Later ordered-multimap 0.7.1 was published with a change implicitly raising MSRV.
  • Since rust-version was not used, this would be resolved for that same rust-ini release and fail on the same toolchain version that worked before.
  • This wouldn't be helped by rust-version being present for rust-ini, since it would have occurred after release and the change was out of their control.

Just an example of what I've seen in the wild. I expect it to be less of an issue if rust-version becomes more widely adopted 👍

(last year IIRC a contributor was insistent on keeping 1.65 support?).
For 1.0+ crates, I'm thinking of reserving MSRV bumps for minor versions and offering a certain level of support for previous minor versions, accepting cherry-picks of fixes to every prior supported version.

I don't quite follow what you're saying here 😅

I was referring to config-rs release branch 0.13:

  • Which was capable of an MSRV of 1.56 IIRC.
  • There was some concern with implicit dependencies and the CI, where the active project maintainer usually raises the MSRV policy for it to pass.
  • That was too high for a contributor, who proved the MSRV could still remain lower, satisfying their need for compatibility with Rust 1.65 (where they also request that rust-version not be set higher)

Recently a new release was requested and that was promptly resolved with 0.14, which raised the MSRV considerably (there were several bumps made prior, all unreleased though).

I observed 0.13 was still receiving point releases (before 0.14 at least, while master was still developed on separately). Not sure how rust-version should behave differently there with 0.x vs 1.x with what you've said? (support for previous minor versions, cherry-pick fixes to prior versions)

I have also witnessed other crates rollback / relax their rust-version in newer releases.

My understanding of rust-version is it can be helpful when you need to resolve deps compatibility for earlier toolchains (within the 3 year window of an edition), provided it's used throughout the dep chain and is accurate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ultimately an MSRV is up to the maintainer and not just what cargo msrv can do. "Support" is an active process and not syntactic.

However, it sounds like MSRV bumps are happening because of new dependency releases. This is futile. A package and its transitive dependencies are independent. You can go a year without a release of config-rs but new dependencies come out. A person could be stuck on an old config-rs due to MSRV but new dependencies come out. A package should not be maintaining an MSRV on behalf of its dependencies.

Not sure how rust-version should behave differently there with 0.x vs 1.x with what you've said? (support for previous minor versions, cherry-pick fixes to prior versions)

The main distinction with 1.x.y vs 0.y.z is that 1.x.y allows room for patches to previous versions within a stable release.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can go a year without a release of config-rs but new dependencies come out. A person could be stuck on an old config-rs due to MSRV but new dependencies come out. A package should not be maintaining an MSRV on behalf of its dependencies.

Right, but that is the scenario where rust-version breaks when any dependency doesn't adopt rust-version but resolves to a release that is no longer compatible with that rust-version correct?

For the rust-version feature to work reliably, one must know that all deps have declared that accurately with their releases?

The main distinction with 1.x.y vs 0.y.z is that 1.x.y allows room for patches to previous versions within a stable release.

Right, but if you're referring to this with the crate version, what is the relevance to the rust-version (eg: 1.56.1)? My original question was about rust-version most of the time is probably fine with x.y, and rarely should need a patch version declared?

I could see something like dependabot updating that, given how it does with Cargo.toml for crate deps presently (patch versions bumped when that shouldn't be necessary).

I'm not sure if there is currently or will be some guidance page for crate authors with setting rust-version? As with the various examples of it going wrong mentioned above, I'm sure it can be confusing or questionable in value that it affects adoption :(

I think rust-version sounds great, at least when it works reliably.


[badges]
maintenance = { status = "actively-developed" }
Expand Down