fix(resolver): Treat unset MSRV as compatible
What does this PR try to resolve?
Have the resolver treat no-MSRV as rust-version = "*", like cargo add does for version-requirement selection
How should we test and review this PR?
We last tweaked this logic in #13066.
However, we noticed this was inconsistent with cargo add in automatically selecting version requirements.
It looks like this is a revert of #13066, taking us back to the behavior in #12950. In #12950 there was a concern about the proliferation of no-MSRV and whether we should de-prioritize those to make the chance of success more likely.
There are no right answes here, only which wrong answer is ok enough.
- Do we treat lack of rust version as
rust-version = "*"as some people expect or do we try to be smart? - If a user adds or removes
rust-version, how should that affect the priority?
One piece of new information is that the RFC for this has us trying to fill the no-MSRV gap with
rust-version = some-value-representing-the-current-toolchain>.
See also https://github.com/rust-lang/cargo/issues/9930#issuecomment-1977471059
r? @Eh2406
Additional information
I wish we had taken better notes about what problems #13066 fixed. I remember feeling strongly about it at the time.
Let me try and reconstructed through brute force. There are three categories of compatibility Incompatible (I), None (N) and, Compatible (C). Which leads to six possible release orders.
- If
Cis the most recent then we should obviously pick that one. The highest version release has a compatible MSRV, there is no reason to look at an older one. (Oldest to newestINCandNIC). - If
Iis the most recent then we should look an the next older. The most recent release has told us it won't work, so we should try something else. This is also a normal situation, the package has an MSRV policy which has updated past our needs. (Oldest to newestNCIandCNI). - If
Nis the most recent then the library has removed its MSRV policy. Do we try it because the library has made no statement, or do we go back to an older version where the library promised to work? If we grab an old version, then removing a MSRV policy is a decision that cargo (the tool) will actively fight you on. (Oldest to newestICNandCIN).
The think about it differently:
| Older | Newer | main | PR |
|---|---|---|---|
N |
C |
C |
C |
I |
C |
C |
C |
C |
I |
C |
C |
N |
I |
N |
N |
I |
N |
N |
N |
C |
N |
C |
N |
Generated with https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=5c0b44e43e2cac5b4f598c24e6eff132
~So clearly I've done something wrong because they both always agree. Feel free to correct my table (and my testing examples) when you spot the problem.~ I found the bug. I was inputting from oldest to newest, and then expecting the best choice to be in position 0. A simple summaries.reverse() before the sort better matches the fallback behavior in the real code.
I wish we had taken better notes about what problems https://github.com/rust-lang/cargo/pull/13066 fixed. I remember feeling strongly about it at the time.
One change from when we had that conversation is that we had a lot less clear of a picture of how to deal with not enough data to make good choices. Since then, the RFC evolved to include rust-version = "<current-toolchain>" which helped close some of that gap (on top of any second order effects from making it easier to deal with MSRVs).
So clearly I've done something wrong because they both always agree. Feel free to correct my table (and my testing examples) when you spot the problem.
I'll have to come back later to dig into this. I think the core of the problem is when there are at least 3 versions involved, rather than just 2.
After dinner the problem became obvious, and a better design for the script also seems worth doing. The table has been updated.
To summarize, when upgrading from an MSRV compatible to an unset MSRV, we are changing the behavior from "prefer MSRV compatible" to "prefer unset". The nuance comes in when dealing with what dependents can rely on working for version with an unset MSRV.
Initially
- if unset MSRV has an effective MSRV that matches the old versions, then users may prefer that
- if unset MSRV has an effective MSRV that is still below the user's, then user may prefer that
- if the unset MSRV has an effective MSRV that is above the user's, then the user would prefer the compatible version
However, over time, the user will upgrade their rust version far enough that they won't want a years-old version but one of those mysterious versions. It could be latest, who knows.
- Picking the latest might work
- Picking the latest will work but won't make the user happy
- However, we may not want to optimize for the case of "moving away from an MSRV"
- However, this could send a signal to the user that there is a mismatch in expectations. The question is how much we want to be policing this. I lean towards "we shouldn't".
Another angle to this is if the dependency adds back in an MSRV.
- If the new MSRV is like the old MSRV and we pick "unset" then you were likely never supported in the first place and I don't care as much
- If the new MSRV higher than the old MSRV, this is somewhat like when the MSRV was removed. It depends on circumstances but will eventually not matter.
From this analysis, I'm finding that I don't care enough about the differences between the two cases which leads me to preferring consistency with cargo add (and i prefer that behavior because it would be a lot messier in that circumstance to support the resolver's current behavior)
Seams convincing. @bors r+
:pushpin: Commit 89ead6f60eaf733d232eb18db0e31da04ba8b88d has been approved by Eh2406
It is now in the queue for this repository.
:hourglass: Testing commit 89ead6f60eaf733d232eb18db0e31da04ba8b88d with merge 82dca28700173a784fcf1240849075023cf22aba...
:sunny: Test successful - checks-actions Approved by: Eh2406 Pushing 82dca28700173a784fcf1240849075023cf22aba to master...
I'm glad for the conclusion, but if you view this from the perspective of the crate author I think this wouldn't have been very hard to figure out:
If a crate author publishes a new semver compatible version with an unset MSRV, do they want their users to switch to it? Or do they want people to keep using the older release forever?