-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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.
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
MSRV resolver has surprising behaviour in workspaces with mixed MSRV #14414
Comments
btw Cargo is a multi-MSRV workspace but we only have two and one of those is "N" so we avoid most of this problem. |
So another way to put this is that when you are in a mixed-MSRV workspace, you get proper handling of your lowest MSRV but for all other MSRVs you get one of
Depending on the dependency and how "wide" of a version requirement you have. For older dependencies, the answer is "just pick a higher version req" but then that puts you in the second situation unless you make your MSRV "N" |
As this puts this use caae in the "No MSRV-aware resolver" situation, this is no worse off than we are today except for a communication / expectations issue. The MSRV-aware resolver not being path aware is a fundamental limitation of the "fallback" strategy. Quoting from #13873 (comment)
Neither of these facts mean we shouldn't consider ways of improving things. |
So far, the only idea I have is that we collect all MSRVs in the workspace and sort the version candidates into MSRV buckets. If a version req disallows versions in the lowest MSRV bucket, consider the next MSRV bucket on up. So long as your version requirements are MSRV compatible, you'll get an MSRV-aware resolve. You just might get older versions than you absolutely need for MSRVs sake but you did say you are compatible with it, so I'm not too worried without mitigating context being brought up. I just checked |
Before I forget, it would be very easy for dependency unification to cause a package with a high MSRV to make it so you can't verify a package with a low MSRV, so the mixed-MSRV workflow is already a bit "here be dragons" (which is why we need to be careful with #10608). |
A third possible workaround would be to split out "stabilised crates" from your workspace into their own separate git repos. That is slightly non-trivial though (you need to use some arcane git commands if you want to keep the history, you need to set up CI anew, generate new and separate publishing secrets, possibly add contributors again if there are other contributors, etc etc). Doable, but there is a definite barrier to entry. Plus if you go back to a active development phase later on again you won't have the benefits. Monorepos are in many ways a lot nicer to work with. Frankly it is a bit more work than I have the energy to deal with right now, but I might consider it. Bonus: It would also simplify MSRV CI testing (that is a bit complex in a multi-MSRV repo),
This sounds similar to my graph suggestion (I guess it depends on what your internal representation is). If it is a full blown graph you could traverse the graph from the workspace crates, and mark each crate with the oldest MSRV it needs to support. Then you have your answer there. Though a complication is that an older/newer version of a dependency might have different dependencies transitive dependencies I guess.
I do put all dependencies (and features that aren't tied to feature flags of my own crates) in the workspace currently. This seems to help a bit with feature unification (without going to a full blown workspace hack, which I have been considering, though the publishing story for that seems complex). I think it is fine if you get a (somewhat reasonable) error message for these tricky situations though. Unexpected behaviour is worse, but a "Can't resolve tokio due to conflicting MSRV requirements from crate_a and crate_b" is perfectly fine. |
You don't have to go so far, you can have multiple workspaces in one repo. Still not great. Without worrying about transient state like "stabilization level" (unless you coupled MSRV to that), you could break up your workspace by MSRV. I've been tempted to do that for cargo.
If I'm understanding, you are suggesting we traverse the transitive dependency graph, marking packages with the MSRV that they need to uphold. The problem is we are in the process of building that graph as we go and there can be multiple paths to a single dependency and by the time we walk down the second path to it, it is already selected. We'd need a solution that allows us to then reject that dependency and try again, like when you resolve for |
Oh right, you need each graph node to be a set of possible versions (with associated MSRV versions). Or do search with backtracking (which likely won't be performant, it seldom is). And yes indirect dependencies (that may contain subgraphs that are also reachable via other paths, this is a DAG not a tree after all) makes this really annoying. |
I would find it really useful if there was a flag to tell cargo what version to use as MSRV when resolving packages. That way, I can at least use that in cases that are too complicated for the tool to handle automatically. I also have a case where I want to resolve using a different version than the MSRV of any crate in the workspace. Specifically, I'd want to resolve using whichever rustc was the latest release when I last used the branch. |
Say we move forward with my earlier proposal, I assume these use cases would still exist? Could you describe what the use case if? Is this something that would need to block any MSRV-aware resolver from being released and, if so, why?
Could you describe the use case and the motivation behind it? Most likely, we'll want to split the conversation about manually specifying the rust-version to resolve to as a separate issue and it will be important to understand the problems that are being solved with it to know how to prioritize it. My inclination at this moment would be to not adjust the status from "future possibility". |
Manually specifying the MSRV to use for resolution could be a viable workaround for me, since the crates that become stable for me tend to have fairly few and slow moving dependencies. It could however be risky, should one of those dependencies bump MSRV if I don't notice it. In my case CI would notice that, but that isn't a given for everyone. |
Likely, it would also be the least generally applicable solution. It works for your mixed MSRV use case because of how its being maintained. Its unclear how much we can generalize that to other mixed MSRV use cases. |
No, no, there's no need to block anything. The solution of having several buckets sounds sensible enough. I suggested manually specifying the version because it seems like there are a lot of different ways you could set up your workspace, and it seems difficult to support all of them with a single system. Having an escape hatch in cases like this is often a good idea. One use-case is this: I generally want to check that my project works with both with the MSRV rustc and the latest rustc. When checking with the latest rustc I also want to use the latest deps. However, if I go back to an old branch where CI doesn't pass with the newest compiler (e.g., due to new warnings), then I may want to generate a lockfile using the newest rustc that compiles the project warning-free. I can do this by manually telling the tool to resolve according to whichever version that is. But again, the intent behind the suggestion is as an escape hatch for all of the things we aren't able to think of. I don't necessarily think it's the best solution to any specific case. |
Interesting use case. Thanks for sharing! |
To lay out the situation more explicitly. Say you have a workspace:
The registry has at least
Today, we resolve everything without MSRV and get
With the MSRV-aware resolver, we'll resolve with MSRV 1.70 and get:
With my proposal in this issue, you'd resolve for 1.70 and 1.75 and get:
|
When discussing this, the question came up of whether this was worth fixing. The MSRV-aware resolver has corner cases. How much should we patch over and hide those corner cases vs teaching users how it works and giving them the resources they need to fix issues. One of those potential tools could be to slip in an as-yet designed (but discussed) To analyze this, we need to first break down the corner cases users will be hitting
Say we change the Say we do nothing. We've improved a lot of reporting but users will need to manually run play whack-a-mole with Say we provide
With the multi-MSRV bucketing solution, we would greatly improve the quality of life for people in the resolving-mixed-MSRV case. The happy path would be MSRV compatible dependencies out of the box, which is a goal of the RFC. We'll inform them when they could manually intercede to get newer versions. MSRV-only-in-CI users would get the right behavior in more cases, reducing the need to rely on |
The solution of using each crate's MSRV to affect only the things needed by that crate would be obviously correct, but the problem with that would be the complexity of implementation. (Which is why that solution might not be viable, at least with the current resolver.) The proposal to do MSRV bucketing and prioritization still seems to me like it'll produce unwanted surprises, where in a mixed-MSRV workspace, it's almost like the old MSRV in one crate affects others, but worse, because it sometimes affects others but gets bumped forward if it fails to resolve. So a crate with a brand new MSRV will still get old dependencies as long as they resolve. (Assuming I'm understanding the proposed solution correctly.) I definitely agree that the current state is not ideal, but it is at least really predictable. It seems like a On the simpler side, even just forcing the specification of a workspace-wide MSRV for dependency resolution would be the easiest way to get an easily predictable outcome when doing a single workspace-wide dependency resolution. |
What would work for me is if it tried to use a newer MSRV and reported an error that some crates in the workspace use too old an MSRV for that to be viable. My policy is aggressive (and I don't do LTS branches or fixes for older releases on general) so if that happens I could simply bump the MSRV of those crates. This is what currently happens once CI runs (cargo hack style, verifying the MSRV for each crate). So the other option would be to be able to opt out of the new resolver and use the traditional one (and catch the need to bump in CI). This wouldn't work for the new resolver though (as I understand it). The really big issue is silent downgrading of crates with no obvious warning that this is because of MSRV (there is a lot of noise about not using the latest version due to indirect dependencies depending on older semver). |
My current reporting plan is, in precedence order
|
…lver results In discussing rust-lang#14414, the general problem of the resolver picking a version older than a package needs for its MSRV (or lack of one) because of the MSRV of other packages came up. This tries to patch over that problem by telling users that a dependency might be able to be newer than the resolver selected. The message is fairly generic and might be misread to be about any MSRV update which an MSRV `fallback` strategy allows, which would make the count off. The reason it is so generic is we don't know with precision why it was held back - Direct dependents may have a non-semver upper bound on the version as we aren't trying to unify the version requirements across direct dependents at this time - A dependency could have removed a feature without making a breaking change - This seems like it should instead be an error but thats a conversation for another day - ~~The user enabled `-Zminimal-versions`~~ - This is now detected and the message skipped Note: separate from this, we may also print the status suffix for this case if the package was not selected for update (e.g. passing `--workspace`).
We do this by resolving for a package version that is compatible with the most number of MSRVs within a workspace. If a version requirement is just right, every package will get the highest MSRV-compatible dependency. If its too high, packages will get MSRV-incompatible dependency versions. This will happen regardless of what we do due to the nature of `"fallback"`. If its too low, packages with higher MSRVs will get older-than-necessary dependency versions. This is similar to the "some with and without MSRV" workspaces. When locking dependencies, we do report to users when newer MSRV/SemVer compatible dependencies are available to help guide them to upgrading if desired. Fixes rust-lang#14414
We do this by resolving for a package version that is compatible with the most number of MSRVs within a workspace. If a version requirement is just right, every package will get the highest MSRV-compatible dependency. If its too high, packages will get MSRV-incompatible dependency versions. This will happen regardless of what we do due to the nature of `"fallback"`. If its too low, packages with higher MSRVs will get older-than-necessary dependency versions. This is similar to the "some with and without MSRV" workspaces. When locking dependencies, we do report to users when newer MSRV/SemVer compatible dependencies are available to help guide them to upgrading if desired. Fixes rust-lang#14414
We do this by resolving for a package version that is compatible with the most number of MSRVs within a workspace. If a version requirement is just right, every package will get the highest MSRV-compatible dependency. If its too high, packages will get MSRV-incompatible dependency versions. This will happen regardless of what we do due to the nature of `"fallback"`. If its too low, packages with higher MSRVs will get older-than-necessary dependency versions. This is similar to the "some with and without MSRV" workspaces. When locking dependencies, we do report to users when newer MSRV/SemVer compatible dependencies are available to help guide them to upgrading if desired. Fixes rust-lang#14414
…lver results In discussing rust-lang#14414, the general problem of the resolver picking a version older than a package needs for its MSRV (or lack of one) because of the MSRV of other packages came up. This tries to patch over that problem by telling users that a dependency might be able to be newer than the resolver selected. The message is fairly generic and might be misread to be about any MSRV update which an MSRV `fallback` strategy allows, which would make the count off. The reason it is so generic is we don't know with precision why it was held back - Direct dependents may have a non-semver upper bound on the version as we aren't trying to unify the version requirements across direct dependents at this time - A dependency could have removed a feature without making a breaking change - This seems like it should instead be an error but thats a conversation for another day - ~~The user enabled `-Zminimal-versions`~~ - This is now detected and the message skipped Note: separate from this, we may also print the status suffix for this case if the package was not selected for update (e.g. passing `--workspace`).
We do this by resolving for a package version that is compatible with the most number of MSRVs within a workspace. If a version requirement is just right, every package will get the highest MSRV-compatible dependency. If its too high, packages will get MSRV-incompatible dependency versions. This will happen regardless of what we do due to the nature of `"fallback"`. If its too low, packages with higher MSRVs will get older-than-necessary dependency versions. This is similar to the "some with and without MSRV" workspaces. When locking dependencies, we do report to users when newer MSRV/SemVer compatible dependencies are available to help guide them to upgrading if desired. Fixes rust-lang#14414
We do this by resolving for a package version that is compatible with the most number of MSRVs within a workspace. If a version requirement is just right, every package will get the highest MSRV-compatible dependency. If its too high, packages will get MSRV-incompatible dependency versions. This will happen regardless of what we do due to the nature of `"fallback"`. If its too low, packages with higher MSRVs will get older-than-necessary dependency versions. This is similar to the "some with and without MSRV" workspaces. When locking dependencies, we do report to users when newer MSRV/SemVer compatible dependencies are available to help guide them to upgrading if desired. Fixes rust-lang#14414
fix(resolve): Improve multi-MSRV workspaces ### What does this PR try to resolve? We do this by resolving for a package version that is compatible with the most number of MSRVs within a workspace. If a version requirement is just right, every package will get the highest MSRV-compatible dependency. If its too high, packages will get MSRV-incompatible dependency versions. This will happen regardless of what we do due to the nature of `"fallback"`. If its too low, packages with higher MSRVs will get older-than-necessary dependency versions. This is similar to the "some with and without MSRV" workspaces. When locking dependencies, we do report to users when newer MSRV/SemVer compatible dependencies are available to help guide them to upgrading if desired. Fixes #14414 ### How should we test and review this PR? Is this the right behavior? - This feature is unstable and letting people try it is one way to determine that - A concern was raised within the Cargo team about new users with workspace member MSRVs all set to latest having someone ask them to lower an MSRV and them dealing with staler-than-required dependencies - At this point, there seems to be agreement on #14414 being a problem, the resolver magically fixing this is unlikely to happen for the foreseeable future, and this does fix it with the potential for user confusion. From my understanding of those conversations, they are mostly in the area of UX, like with #14543. Rather than blocking on that discussion, this moves forward with the implementation. ### Additional information
…lver results In discussing rust-lang#14414, the general problem of the resolver picking a version older than a package needs for its MSRV (or lack of one) because of the MSRV of other packages came up. This tries to patch over that problem by telling users that a dependency might be able to be newer than the resolver selected. The message is fairly generic and might be misread to be about any MSRV update which an MSRV `fallback` strategy allows, which would make the count off. The reason it is so generic is we don't know with precision why it was held back - Direct dependents may have a non-semver upper bound on the version as we aren't trying to unify the version requirements across direct dependents at this time - A dependency could have removed a feature without making a breaking change - This seems like it should instead be an error but thats a conversation for another day - ~~The user enabled `-Zminimal-versions`~~ - This is now detected and the message skipped Note: separate from this, we may also print the status suffix for this case if the package was not selected for update (e.g. passing `--workspace`).
…lver results In discussing rust-lang#14414, the general problem of the resolver picking a version older than a package needs for its MSRV (or lack of one) because of the MSRV of other packages came up. This tries to patch over that problem by telling users that a dependency might be able to be newer than the resolver selected. The message is fairly generic and might be misread to be about any MSRV update which an MSRV `fallback` strategy allows, which would make the count off. The reason it is so generic is we don't know with precision why it was held back - Direct dependents may have a non-semver upper bound on the version as we aren't trying to unify the version requirements across direct dependents at this time - A dependency could have removed a feature without making a breaking change - This seems like it should instead be an error but thats a conversation for another day - ~~The user enabled `-Zminimal-versions`~~ - This is now detected and the message skipped Note: separate from this, we may also print the status suffix for this case if the package was not selected for update (e.g. passing `--workspace`).
…lver results In discussing rust-lang#14414, the general problem of the resolver picking a version older than a package needs for its MSRV (or lack of one) because of the MSRV of other packages came up. This tries to patch over that problem by telling users that a dependency might be able to be newer than the resolver selected. The message is fairly generic and might be misread to be about any MSRV update which an MSRV `fallback` strategy allows, which would make the count off. The reason it is so generic is we don't know with precision why it was held back - Direct dependents may have a non-semver upper bound on the version as we aren't trying to unify the version requirements across direct dependents at this time - A dependency could have removed a feature without making a breaking change - This seems like it should instead be an error but thats a conversation for another day - ~~The user enabled `-Zminimal-versions`~~ - This is now detected and the message skipped Note: separate from this, we may also print the status suffix for this case if the package was not selected for update (e.g. passing `--workspace`).
Problem
As state by @epage in #13873 (comment) , a new issue was preferred for this. The issue is that the new MSRV aware resolver leads to surprising and suboptimal behaviour in a workspace with mixed MSRV:
The issue is that (for whatever reason) you have a workspace where one (or more) of you crates is on a much older MSRV than the rest. Then the MSRV aware resolver will attempt to create a lockfile for all of the workspace based on that oldest version even if the dependency in question isn't even used by that old-MSRV crate. In certain cases this can cause there to be no possible solutions to the dependency resolution (as detailed by the comment linked in the third bullet point above).
There are valid reasons to have mixed MSRV, such as the different crates having different MSRV policy. or just that some crates naturally have solver development and don't need to be bumped to a newer MSRV as often as other crates.
I will here describe my use case where I run into this (I cannot speak for the cases where others run into this as what @Darksonn described):
I have a workspace I use to develop a command line program. Having it all the related crates (both the main binary and supporting libraries) all in one workspace makes development much easier, especially during the early phases when everything is evolving rapidly. Some of these crates are generally useful to others as well (file format parsers in my case).
I also have an agressive MSRV policy: only N-2 is guaranteed (but I won't bump MSRV gratuitously just for the sake of it, so actual MSRV can be much further back).
Eventually some of these crates will start to stabilise, especially the "leaf node" supporting crates with simple and clearly defined scope. There is only so many things you need in a library that parses a file format. That means it won't get breaking changes nor get published to crates.io as often. And also that there will be no reason to bump MSRV.
Now the problem here is that this leaf node dependency with older MSRV will now drag down the versions of dependencies that get used for my main crate of the workspace (the "flagship" binary crate).
Steps
CARGO_RESOLVER_INCOMPATIBLE_RUST_VERSIONS=fallback cargo +nightly -Zmsrv-policy generate-lockfile
Possible Solution(s)
Possible solutions (and why they are suboptimal):
The better option would seem to me that cargo would consider the entire dependency graph when writing the lock file and look at what dependencies are reachable from what crates, and compute the minimum needed MSRV for each dependency.
Notes
No response
Version
The text was updated successfully, but these errors were encountered: