search

rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.75k stars 2.42k forks source link

MSRV-dependent dependency version resolution #9930

Open newpavlov opened 3 years ago

newpavlov commented 3 years ago

Based on RFC 2495 Rust now supports the rust-version field, which currently works as:

If the currently selected version of the Rust compiler is older than the stated version, cargo will exit with an error, telling the user what version is require

This behavior is not ideal. If a project depends on crate foo v0.1.0 with MSRV 1.56, then releasing crate foo v0.1.1 with MSRV 1.60 will break build of the project on older toolchains after simple cargo update. Ideally Cargo would select foo v0.1.0 on older tolchains, thus preventing the breakage.

RFC 2495 has described MSRV-dependent version resolution as a potential future extension. This issue is intended for design discussions and tracking implementation progress of this feature.

Third-party support

This has been approved as RFC #3537.

Implementation by stabilization milestone

Changes from RFC

Unresolved questions

Deferred

djc commented 3 years ago

I think this probably makes more sense in the Cargo repository, since version resolution happens in Cargo -- might even make sense to do an RFC for it. @ehuss @Eh2406 what do you think?

I can see the attraction of this feature, but I'm also worried about it -- if this behavior doesn't come with a warning, it's trivially possible that downstream crates no longer get updated to upstream dependencies that fixed security vulnerabilities, for example. While I usually deploy cargo-deny in CI to make sure I'm aware of this, as long as something similar is not integrated in Cargo it would be a little scary that semver-compatible updates no longer flow as freely as they used to.

newpavlov commented 3 years ago

Yes, having a Cargo warning for cases when there are dependency updates with incompatible MSRV would be a good feature.

As for security vulnerabilities, I think it should be solved by advisories like RustSec and by yanking affected versions. Also note that today crates can have minor (for pre-1.0, major otherwise) version updates with security fixes which do not get ported to older releases. So I don't think that the proposed MSRV-dependent version resolution is fundamentally different from the existing status quo in this regard.

Eh2406 commented 3 years ago

Transferred to Cargo.

IMO one of the big problems blocking this is the error messages. The current Resolver has pretty bad messages, it just reports on the last thing that did not work. So In effect the Resolver will just ignore all versions with a different MSRV leaving no record of why newer versions were not considered. This may be on the list of things to consider when/if we use PubGrub. But that is just my opinion.

leo60228 commented 3 years ago

Is this actually a good idea? I feel like rust-version should solely be a safeguard, and not actually affect behavior. IMO pinning a Rust version without pinning dependencies is user error, and it's better for this to be a hard error instead of doing what might be the wrong thing.

gilescope commented 3 years ago

broken code doesn't help anyone. If cargo update / upgrade gives warnings that your rust version is too old to bring in all the latest updates then people will be more likely to run cargo upgrade as they are more likely to get a result that works. Pragmatically some stuff upgraded is better than nothing upgraded.

newpavlov commented 3 years ago

@leo60228

Is this actually a good idea?

It was my main motivation for writing the MSRV RFC. Without it I (and many others) consider bumping MSRV a breaking change, since otherwise we risk breaking people's builds on older toolchains (e.g. think of Rust packaged with Debian). Printing an error is just slightly improves ergonomics, no solves the problem at the root.

djc commented 3 years ago

I think most maintainers have stepped away from the idea of MSRV bumps requiring a semver bump. Even fundamental things like serde have done it. Instead, just keep a conservative enough MSRV for your crate's audience.

asomers commented 3 years ago

I think most maintainers have stepped away from the idea of MSRV bumps requiring a semver bump. Even fundamental things like serde have done it. Instead, just keep a conservative enough MSRV for your crate's audience.

But keeping "a conservative enough MSRV for your crate's audience" is _notpossible if your upstream dependencies don't do the same. If upstream crates raise their MSRV without a major version bump, then you have little choice but to follow suit. As @leo60228 suggested you could pin those crates versions to before their MSRV bump, but that causes a different problem: Cargo refuses to build two semver-compatible versions of a single crate in a single build. That can result in downstream users' builds breaking, even if they don't care about MSRV. So Rust really does need an MSRV-sensitive resolver.

matklad commented 3 years ago

pinning a Rust version without pinning dependencies is user error, and it's better for this to be a hard error instead of doing what might be the wrong thing.

One way to resolve this concern is to make the behavior opt-in. By default, behavior is at is today (modulo a better error message), but you can pass --rust-version flag to commands like update or generate-lockfile to request resolution at a specific version.

djc commented 3 years ago

But keeping "a conservative enough MSRV for your crate's audience" is not_possible if your upstream dependencies don't do the same. If upstream crates raise their MSRV without a major version bump, then you have little choice but to follow suit.

In theory, this is true. In practice, I've found that upstream maintainers have usually been happy to accomodate my requests for changes to allow a more conservative MSRV -- just a matter of (a) being aware when changes happen and (b) engaging with upstream to voice your concerns/discuss trade-offs.

HeroicKatora commented 3 years ago

How should this interact with rust-toolchain.toml? At the moment it is only selecting a toolchain from rustup. Should it become the default version for update and generate-lockfile as well?

newpavlov commented 2 years ago

@Eh2406

IMO one of the big problems blocking this is the error messages.

Wouldn't it be viable to implement an MVP with bad error messages first and work on improving them later?

So In effect the Resolver will just ignore all versions with a different MSRV leaving no record of why newer versions were not considered.

I think it should be fine to start with a simple warning if at least one dependency version was ignored due to the MSRV filtration.

embediver commented 2 years ago

I came across a related problem, where a CI/CD pipeline failed due to a updated dependency which demanded a higher MSRV than specified for the downstream project.

While I am not sure, that doing a automated MSRV-dependent dependency version resolution is a good idea in general, I would like a feature which notifies you if dependencies can't be build with the specified MSRV. As far as I can tell, it should be no big deal to implement a warning which, when building, notifies you about this. This may not prevent the problem that a upstream crate may change the MSRV in a semver compatible release in the future, but ensures that there isn't already a unwanted mismatch at development time.

Furthermore I am not completely sure how its meant to be specified: Should the developer ensure, that the MSRV of his Crate is at least as high as all upstream dependencies?

RFC 2495 mentions this, but there seems to be no check implemented right now.

rust field value will be checked as well. During crate build cargo will check if all upstream dependencies can be built with the specified MSRV. (i.e. it will check if there is exists solution for given crates and Rust versions constraints) Yanked crates will be ignored in this process.

djc commented 2 years ago

While I am not sure, that doing a automated MSRV-dependent dependency version resolution is a good idea in general, I would like a feature which notifies you if dependencies can't be build with the specified MSRV. As far as I can tell, it should be no big deal to implement a warning which, when building, notifies you about this.

Cargo from Rust 1.56 and newer should yield an MSRV-specific error when trying to build a dependency that has its rust-version set to something lower than the currently executing version of the compiler.

Furthermore I am not completely sure how its meant to be specified: Should the developer ensure, that the MSRV of his Crate is at least as high as all upstream dependencies?

Yes, there is currently no way that the tooling can ensure this.

embediver commented 2 years ago

Cargo from Rust 1.56 and newer should yield an MSRV-specific error when trying to build a dependency that has its rust-version set to something lower than the currently executing version of the compiler.

Just tried to provoke this behaviour with two freshly created crates, and can't really confirm this. As long as the edition and rust-version key match, there seems to be no warning or error at all. Or did I get something wrong here.

When building crate A which depends on crate B, I can set rust-version and edition to whatever I like for both crates and won't get a error (e.g. rust-version 1.42 for crate A and 1.61 for crate B). cargo --version gives me cargo 1.61.0 (a028ae4 2022-04-29).

djc commented 2 years ago

I didn't say you would get an error in that situation. You're building with Cargo 1.61 and both of the crates have 1.61 or older, so there is no error. We currently don't check that depending crates have a newer or equal MSRV compared to the crate they're depending on.

You can refer to the documentation here: https://doc.rust-lang.org/cargo/reference/manifest.html#the-rust-version-field.

As far as I can tell, all of this discussion is off-topic for this issue, which is about influencing the Cargo resolver such that it would select the lower version of a dependency if that is a better match for the current Cargo version.

embediver commented 2 years ago

Ok, thanks for the clarification. And yes its somewhat off-topic, my apologies.

Initially I was just confused that the RFC excerpt I cited isn't implemented in some tooling right now and came up to this Issue.

To contribute at least a bit here: If the Cargo resolver will make some decisions based on the MSRV, the tooling which hints the developer of incompatible MSRVs, is basically included in that. IMO that would be good to have.

tarcieri commented 2 years ago

Perhaps there could be an unstable option similar to cargo update -Z minimal-versions to perform a resolution that considers MSRV?

epage commented 1 year ago

Highlights of discussion so far:

Need for notifying users

if this behavior doesn't come with a warning, it's trivially possible that downstream crates no longer get updated to upstream dependencies that fixed security vulnerabilities, for example. While I usually deploy cargo-deny in CI to make sure I'm aware of this, as long as something similar is not integrated in Cargo it would be a little scary that semver-compatible updates no longer flow as freely as they used to.

Yes, having a Cargo warning for cases when there are dependency updates with incompatible MSRV would be a good feature.

As for security vulnerabilities, I think it should be solved by advisories like RustSec and by yanking affected versions. Also note that today crates can have minor (for pre-1.0, major otherwise) version updates with security fixes which do not get ported to older releases. So I don't think that the proposed MSRV-dependent version resolution is fundamentally different from the existing status quo in this regard.

Error messages

IMO one of the big problems blocking this is the error messages. The current Resolver has pretty bad messages, it just reports on the last thing that did not work. So In effect the Resolver will just ignore all versions with a different MSRV leaving no record of why newer versions were not considered. This may be on the list of things to consider when/if we use PubGrub. But that is just my opinion.

Wouldn't it be viable to implement an MVP with bad error messages first and work on improving them later?

rust-version as advisory only

Is this actually a good idea? I feel like rust-version should solely be a safeguard, and not actually affect behavior. IMO pinning a Rust version without pinning dependencies is user error, and it's better for this to be a hard error instead of doing what might be the wrong thing.

broken code doesn't help anyone. If cargo update / upgrade gives warnings that your rust version is too old to bring in all the latest updates then people will be more likely to run cargo upgrade as they are more likely to get a result that works. Pragmatically some stuff upgraded is better than nothing upgraded.

But keeping "a conservative enough MSRV for your crate's audience" is not_possible if your upstream dependencies don't do the same. If upstream crates raise their MSRV without a major version bump, then you have little choice but to follow suit. As @leo60228 suggested you could pin those crates versions to before their MSRV bump, but that causes a different problem: Cargo refuses to build two semver-compatible versions of a single crate in a single build. That can result in downstream users' builds breaking, even if they don't care about MSRV. So Rust really does need an MSRV-sensitive resolver.

In theory, this is true. In practice, I've found that upstream maintainers have usually been happy to accomodate my requests for changes to allow a more conservative MSRV -- just a matter of (a) being aware when changes happen and (b) engaging with upstream to voice your concerns/discuss trade-offs.

One way to resolve this concern is to make the behavior opt-in. By default, behavior is at is today (modulo a better error message), but you can pass --rust-version flag to commands like update or generate-lockfile to request resolution at a specific version.

Perhaps there could be an unstable option similar to cargo update -Z minimal-versions to perform a resolution that considers MSRV?

rustup interaction

How should this interact with rust-toolchain.toml? At the moment it is only selecting a toolchain from rustup. Should it become the default version for update and generate-lockfile as well?

Other relevant discussions

epage commented 1 year ago

Blockers for MSRV in the resolver

Exceptions are bolded

The rust-version field existing in the Index so the resolver can reference it

Problem

As an end-user, I might have rust installed but haven't been keeping up with the latest and just want to cargo install without extra steps or might not be familiar with how to upgrade. Even worse if I was using a distribution version, like from Debian.

As a maintainer, it is cumbersome to have an MSRV and for it to be something besides the latest. When upgrading or pulling in a new dependency, a maintainer has to choose

Similarly, as a corporate user, I might be aligned to specific rust versions that have been integrated into our build process or certified for our use.

As a maintainer, I frequently have to support my users through picking the right version of my dependencies

As a maintainer, it can be tiring to deal with people making special requests for MSRV and I want things to "just work" so there is less reason for these requests to be made

Proposed behavior

By default, we resolve with MSRV

Implementation: the resolver would treat rust as just another dependency, one that exists in a different namespace as packages.

For local development tasks

For cargo install <foo> (no version specified) (#10903)

For cargo add <foo> (no version specified), versions newer than the selected package's package.rust-version will be ignored (#10653)

--ignore-rust-version would be updated to opt-out of the MSRV resolver

Corner cases

Challenges with doing MSRV-aware resolver with possible solutions

Error reporting

Users running stale, possibly insecure versions

Upgrading to a new rust version causes my lockfile to change

Maintainers who want to different rust versions between MSRV verification and regular development

Alternatives

Make MSRV-aware resolver opt-in

Open Questions

jonhoo commented 1 year ago

I'd like to propose that cargo update should also always warn if an update is withheld due to MSRV. I think there's a decent argument that it should even hard-error unless a flag is passed, but that may be overkill.

epage commented 1 year ago

I'd like to propose that cargo update should also always warn if an update is withheld due to MSRV

What if the table mentioned this? While I didn't specify it, that was one of my expectations with the table to report why a newer version may not be available

I think there's a decent argument that it should even hard-error unless a flag is passed, but that may be overkill.

If the flag always has to be passed in, it loses its meaning and instead only serves as a barrier which just instills a feeling of frustation. For any project lagging behind its dependencies, this will always be needed.

jonhoo commented 1 year ago

I'd like to propose that cargo update should also always warn if an update is withheld due to MSRV

What if the table mentioned this? While I didn't specify it, that was one of my expectations with the table to report why a newer version may not be available

Maybe this is a "me" thing, but I generally do not want verbose output from commands. A table would be far too noisy for me, and I would only want one if I explicitly passed in --verbose. Ideally, in my mind, cargo update would print nothing, with an option to print a list, and another option to print a full table. But this warning should be surfaced no matter which output mode is used.

I think there's a decent argument that it should even hard-error unless a flag is passed, but that may be overkill.

If the flag always has to be passed in, it loses its meaning and instead only serves as a barrier which just instills a feeling of frustation. For any project lagging behind its dependencies, this will always be needed.

I think my proposal here stems from the fact that I worry a lot about folks using old versions of their dependencies, and ending up with insecure, buggy, or under-performing software as a result. To the point that I think having MSRV-aware resolving by default for cargo update is a mistake. I fully acknowledge there's a trade-off here, but I think valuing "Cargo.lock represents rust-version" higher than "you're using the latest possible version of your dependencies" is going to come back to bite us over and over again. If we go that path though, I think at the very least the decision to willingly take older versions should be made very explicit.

newpavlov commented 1 year ago

I think my proposal here stems from the fact that I worry a lot about folks using old versions of their dependencies, and ending up with insecure, buggy, or under-performing software as a result.

Following this logic Cargo should not use Cargo.lock at all and instead always fetch dependency updates on each build job. It's obviously not how we work with dependencies, because updating them has its own set of risks.

rpjohnst commented 1 year ago

The difference is that you can currently expect to get the latest versions compatible with what you wrote in Cargo.toml when running cargo update. With MSRV-dependent resolution, you might start silently getting older versions based solely on the libraries' Cargo.tomls and your current version of Rust.

And "your current version of Rust" is not always a real constraint- sometimes you just haven't run rustup update yet, and would be perfectly willing to do so if you knew it would help.

jonhoo commented 1 year ago

I think my proposal here stems from the fact that I worry a lot about folks using old versions of their dependencies, and ending up with insecure, buggy, or under-performing software as a result.

Following this logic Cargo should not use Cargo.lock at all and instead always fetch dependency updates on each build job. It's obviously not how we work with dependencies, because updating them has its own set of risks.

I don't think that follows. cargo update is an explicit operation I run to update. Cargo does not run it on my behalf.

That said, this is related to one of the concerns I know have been raised in response to the suggestion that folks should start checking in Cargo.lock for libraries — that doing so would mean less automatic testing of new versions of dependencies, and that such an outcome would be detrimental to the health of the ecosystem.

newpavlov commented 1 year ago

@rpjohnst

With MSRV-dependent resolution, you might start silently getting older versions based solely on the libraries' Cargo.tomls and your current version of Rust.

Not silently. With an appropriate warning message on cargo update or on first generation of Cargo.lock.

@jonhoo

I don't think that follows. cargo update is an explicit operation I run to update. Cargo does not run it on my behalf.

By having Cargo.lock in your workspace (regardless whether it's committed to VCS or not) you are likely using "old versions of dependencies", unless you explicitly run cargo update. If we consider it's fine, then what exactly MSRV-dependent resolution changes? If anything, from your point of view, the current status quo should be even worse than the proposed warning system. Today you do not get any built-in warnings if there are outdated dependencies in your tree, you have to use separate tools like cargo outdated.

tarcieri commented 1 year ago

Per the summary above it seems like there's fairly widespread agreement that using an older version to be MSRV compatible should print a warning

illicitonion commented 1 year ago

I think my proposal here stems from the fact that I worry a lot about folks using old versions of their dependencies, and ending up with insecure, buggy, or under-performing software as a result.

In general I agree with this.

To the point that I think having MSRV-aware resolving by default for cargo update is a mistake. I fully acknowledge there's a trade-off here, but I think valuing "Cargo.lock represents rust-version" higher than "you're using the latest possible version of your dependencies" is going to come back to bite us over and over again.

It feels like there's a missing piece here around intent.

If we consider a "how-much-I-care-about-msrv" equivalent to Cargo.toml's existing "maintenance" field, I can imagine the following values:

And if we consider the operations a maintainer may be doing on a crate, I can imagine being in any of the following modes:

When performing operations, particularly cargo update, these two factors combine to give reasonable behaviours.

If you really strongly care about MSRV, you probably want an explicit flag to ever violate MSRV. If you actually have a more flexible policy, maybe a different default makes sense. But it feels like being able to express the intent of why this is your MSRV may affect how cargo should act.

you might start silently getting older versions based solely on the libraries' Cargo.tomls

It feels like transitive dependencies' MSRVs should be capped by the actual package being operated on from the command line is.

If I have a package foo which specifies MSRV 1.56, and I have a dependency which specifies MSRV 1.54, any new package I consider bringing in should be capped by 1.56 not 1.54. I think in the latest proposal, this is actually slightly ambiguous as to which crates in the dep graph factor into MSRV calculation.

jonhoo commented 1 year ago

By having Cargo.lock in your workspace (regardless whether it's committed to VCS or not) you are likely using "old versions of dependencies", unless you explicitly run cargo update. If we consider it's fine, then what exactly MSRV-dependent resolution changes? If anything, from your point of view, the current status quo should be even worse than the proposed warning system. Today you do not get any built-in warnings if there are outdated dependencies in your tree, you have to use separate tools like cargo outdated.

The big difference is that running cargo update is an indication of intent. When I run that command I'm saying "I want to update now", and it's unfortunate that after running that command I'm not actually up to date.

newpavlov commented 1 year ago

it's unfortunate that after running that command I'm not actually up to date.

As mentioned in my previous comment, it's quite likely that you will not be "up to date" even today. One of you dependencies (maybe buried deep in your dependency tree) could have a breaking release and you will have absolutely no idea about it without running a tool like cargo outdated. It could make you software "insecure, buggy, or under-performing". And even before that, the fact that you are not on the latest toolchain already makes the talk about being "up to date" moot.

jonhoo commented 1 year ago

It's true that major version updates require more work, but that's not an argument that we should make the situation (significantly) worse by extending the same problem to minor version updates as well.

I agree it's a problem when someone's Rust version is behind, but a) it's a smaller problem, and b) the proposal here won't surface that any more than the current state of affairs — quite to the contrary.

8573 commented 1 year ago

With MSRV-dependent resolution, you might start silently getting older versions based solely on the libraries' Cargo.tomls and your current version of Rust.

Is this true? I had thought that "MSRV-dependent resolution" would depend on the rust-version declared in one's Cargo.toml, not on the Rust version one happens to be using to build.

djc commented 1 year ago

I'd like to propose that cargo update should also always warn if an update is withheld due to MSRV

What if the table mentioned this? While I didn't specify it, that was one of my expectations with the table to report why a newer version may not be available

Maybe this is a "me" thing, but I generally do not want verbose output from commands. A table would be far too noisy for me, and I would only want one if I explicitly passed in --verbose. Ideally, in my mind, cargo update would print nothing, with an option to print a list, and another option to print a full table. But this warning should be surfaced no matter which output mode is used.

+1. A table (assuming your proposal would be similar to the table which appears for cargo upgrade today) scales really badly to larger workspaces (as I've explained before for cargo upgrade). Instead, output should focus only on deviant/attention-worthy cases.

I think there's a decent argument that it should even hard-error unless a flag is passed, but that may be overkill.

If the flag always has to be passed in, it loses its meaning and instead only serves as a barrier which just instills a feeling of frustation. For any project lagging behind its dependencies, this will always be needed.

I think my proposal here stems from the fact that I worry a lot about folks using old versions of their dependencies, and ending up with insecure, buggy, or under-performing software as a result. To the point that I think having MSRV-aware resolving by default for cargo update is a mistake. I fully acknowledge there's a trade-off here, but I think valuing "Cargo.lock represents rust-version" higher than "you're using the latest possible version of your dependencies" is going to come back to bite us over and over again. If we go that path though, I think at the very least the decision to willingly take older versions should be made very explicit.

I wonder if there's room for nuance based on update (semver-compatible only) and upgrade (potentially including semver-incompatible) semantics. Arguably update means "give me the latest version of everything as long as it is compatible with my current dependency specifications", and update today doesn't warn about the availability of newer, semver-incompatible versions. In this sense, cargo update already doesn't mean "you're using the latest possible version of your dependencies".

epage commented 1 year ago

I'd like to propose that cargo update should also always warn if an update is withheld due to MSRV

What if the table mentioned this? While I didn't specify it, that was one of my expectations with the table to report why a newer version may not be available

Maybe this is a "me" thing, but I generally do not want verbose output from commands. A table would be far too noisy for me, and I would only want one if I explicitly passed in --verbose. Ideally, in my mind, cargo update would print nothing, with an option to print a list, and another option to print a full table. But this warning should be surfaced no matter which output mode is used.

+1. A table (assuming your proposal would be similar to the table which appears for cargo upgrade today) scales really badly to larger workspaces (as I've explained before for cargo upgrade). Instead, output should focus only on deviant/attention-worthy cases.

We can limit the table to the "deviant" cases by default and show more with --verbose, making it effectively a structured version of the warning

My theory is

epage commented 1 year ago

I think my proposal here stems from the fact that I worry a lot about folks using old versions of their dependencies, and ending up with insecure, buggy, or under-performing software as a result. To the point that I think having MSRV-aware resolving by default for cargo update is a mistake. I fully acknowledge there's a trade-off here, but I think valuing "Cargo.lock represents rust-version" higher than "you're using the latest possible version of your dependencies" is going to come back to bite us over and over again. If we go that path though, I think at the very least the decision to willingly take older versions should be made very explicit.

I can understand the concerns over stagnation, security, and "do what I told you" behavior but I also feel we need to meet people where they are at. A workflow that is doomed to fail on first try and requires a special flag to get working, even if you are told about that flag, is a frustrating experience. Then this compounds further when you are maintaining an MSRV so everything you do, you need to make sure you don't accidentally get too new versions without passing in a special flag. Within that workflow, it is reasonable to run cargo update and only get what will work for my project. It doesn't update dependencies that break semver. It also doesn't upgrade dependencies that something has = on.

jonhoo commented 1 year ago

Let me try to be a bit more explicit here about my beliefs, assumptions, and wants.

I think an outcome where the Rust ecosystem is generally less up-to-date over time is significantly worse than it being painful to maintain MSRV. I also think it affects significantly more people.

I think making cargo update respect rust-version by default is going to lead to that outcome.

I think there are (many) more application authors than library authors, and thus that most users do not need to care about MSRV or rust-version as far as their own crate goes. And thus it's fine to impose some cost for MSRV checking since it affects fewer users (and per the previous point I think the downsides to the majority of users is significant).

I think cargo update should warn if it ever doesn't update to the latest version to highlight that the caller may want to take another action to unblock the update.

Consequently, I think it's a mistake that it does not warn today in the presence of = or < specifiers if they prevent an update.

I also think it's a mistake that it does not warn today in the presence of newer major versions. I think it's something that's going to come back to bite the entire Rust ecosystem, as it leads to stagnation without visibility, which in turn leads to everything slipping further and further behind (dependabot is not sufficient to solve this problem).

I think it may even be reasonable to fail if a cargo update cannot update to the newest available version of something (but still update Cargo.lock as best it can) to clearly indicate to users that they are not up to date.

On the topic of verbosity, I think the standard output of a command should be concise, and only point out deviations. That means no tables and only "anything that didn't do the expected thing" such as if an update didn't happen. I think a table view is totally reasonable, but should be opt-in, not opt-out. I also think that the table view should only include "uncommon" rows (e.g., those were an update was partial) by default. I hold the first of these views weakly, and the second strongly.

remram44 commented 1 year ago

I don't understand, wouldn't it be easy for application author to not specify an MSRV at all? In this situation, do they run into any problems?

adamreichold commented 1 year ago

I don't understand, wouldn't it be easy for application author to not specify an MSRV at all? In this situation, do they run into any problems?

Exactly this. If I do specify rust-version in my Cargo.toml, this basically is the commitment of following that MSRV and I would prefer that it is be considered if it exists. When I write an application, I usually do not have that property in my Cargo.toml and there is nothing there for Cargo to consider in the first place. Hence, I think the presence or absence of the rust-version key in the current crate's manifest is a pretty good indicator of intent in both situations.

Nemo157 commented 1 year ago

Hence, I think the presence or absence of the rust-version key in the current crate's manifest is a pretty good indicator of intent in both situations.

These two comments seem to be at odds, my understanding of @epage's implication is that eventually every crate should have a rust-version set so that downstream MSRV-following users can use that when picking their dependencies, even if those upstream crates aren't themselves following an MSRV policy and just increasing rust-version on every release. (Unless the intent is that you create a new branch for every release and set rust-version just for that published commit, or that eventually cargo package automatically sets it; but IMO the first is too much effort and the latter hasn't been proposed as an actual change yet).

Really an MSRV-following application author is the one that doesn't need to publish their rust-version, they only need to know it transiently during cargo update. It's all the libraries they use that need to have that metadata published for them to then consume during version resolution.

EDIT: To add a concrete scenario:

As a conscientious maintainer that wants to support MSRV restricted downstreams (but do not maintain a fixed MSRV myself) I have rust-version = "1.57" set in my Cargo.toml because that was the latest version at the time of my last release. I now am developing a new feature that needs a new dependency so I cargo add foobar. Despite there being a foobar = "2.0.0" available to add, that has rust-version = "1.59", so instead cargo decides to add foobar = "1.10.2". Unless I then run cargo outdated or something I won't realise that there's a newer version available and that I need to increase my rust-version to allow access to it.

(There's a similar scenario where cargo update does not allow testing against the latest releases without manual intervention too, but that at least doesn't affect downstreams (other than them running untested configurations) like a major version difference does).

adamreichold commented 1 year ago

even if those upstream crates aren't themselves following an MSRV policy and just increasing rust-version on every release.

While it is not a very conservative one, it still is an MSRV policy that the solver could use as an additional constraint. That said, personally, I would not add a rust-version property to a library that does not really have an MSRV policy, because that property needs to tested for and considered when making code changes.

Really an MSRV-following application author is the one that doesn't need to publish their rust-version, they only need to know it transiently during cargo update. It's all the libraries they use that need to have that metadata published for them to then consume during version resolution.

They don't need to publish it. But they don't need to publish their Cargo manifest at all. (I don't think cargo install is a good long-term solution for distributing applications.) Still, the presence of the key in their Cargo manifest can signal the intent of following that MSRV to the dependency solver which should then consider it.

I as a conscientious maintainer that wants to support MSRV restricted downstreams have rust-version = "1.57" set in my Cargo.toml because that was the latest version at the time of my last release. I now am developing a new feature that needs a new dependency so I cargo add foobar. Despite there being a foobar = "2.0.0" available to add, that has rust-version = "1.59", so instead cargo decides to add foobar = "1.10.2". Unless I then run cargo outdated or something I won't realise that there's a newer version available and that I need to increase my rust-version to allow access to it.

My first guess would be that the manual step of running cargo add foobar should clearly spell out the reasoning behind its choice of version 1.10.2, so that I am aware that my MSRV prevents me from using version 2.0.0. I can then make a conscious decision to bump my MSRV if I deem the difference between these version a sufficient reason for doing so.

As a general remark, I suspect that it will not be possible to resolve the difference of approach between following the latest versions available and constraining one's dependency to a given "MSRV closure". I do think there is room for both approaches though and hence I think the main usability issue to decide is how I as a maintainer can opt into the "slow world". I think the initial status quo of not having rust-version set, could default to the "fast world", because as written above keeping an MSRV entails more than just handling the dependency closure even though that is the most chore-like part of it.

djc commented 1 year ago

Note that an edition imposes a rust-version constraint as well: for a crate that has edition = "2021", rust-version is set to 1.56. So from that perspective, if you use cargo init today your crate will have a rust-version even though it is not explicitly mentioned in the manifest. From this perspective, it seems likely that we will trend towards all crates having a rust-version set, but it does make it harder to derive the author's intent about whether they want their crate's dependencies to match a particular MSRV. RFC 2495 being a minimal implementation, the only goal its implementation serves is to help consumers decide whether the crate is fit for compilation using a particular Rust version. If you want to communicate intent about which MSRV a transitive dependency graph should support, the implementation has to change (or we should additional metadata to the manifest).

I think there are (many) more application authors than library authors, and thus that most users do not need to care about MSRV or rust-version as far as their own crate goes. And thus it's fine to impose some cost for MSRV checking since it affects fewer users (and per the previous point I think the downsides to the majority of users is significant).

This makes a lot of sense to me.

I think cargo update should warn if it ever doesn't update to the latest version to highlight that the caller may want to take another action to unblock the update.

Consequently, I think it's a mistake that it does not warn today in the presence of = or < specifiers if they prevent an update.

I also think it's a mistake that it does not warn today in the presence of newer major versions. I think it's something that's going to come back to bite the entire Rust ecosystem, as it leads to stagnation without visibility, which in turn leads to everything slipping further and further behind (dependabot is not sufficient to solve this problem).

Today, cargo update already prints a list of actions it took (added dependencies, upgrades and downgrades -- though downgrades are shown as upgrades in today's stable Rust). Conceptually it would make sense to me to add lines of output to it that mention if a newer version is available but unable to be used because of version constraints in the compilation target. As far as I understand, getting this information out of the resolver is considered to be hard by Cargo team members, but maybe it's possible to work around the resolver for this limited use case.

I think it may even be reasonable to fail if a cargo update cannot update to the newest available version of something (but still update Cargo.lock as best it can) to clearly indicate to users that they are not up to date.

I'm not sure I consider this to be reasonable. So here you are referring to changing the process' return value in that scenario (and it's output)? I'm not sure I agree that this would be reasonable.

On the topic of verbosity, I think the standard output of a command should be concise, and only point out deviations. That means no tables and only "anything that didn't do the expected thing" such as if an update didn't happen. I think a table view is totally reasonable, but should be opt-in, not opt-out. I also think that the table view should only include "uncommon" rows (e.g., those were an update was partial) by default. I hold the first of these views weakly, and the second strongly.

Do you consider cargo update's current output a "table view"? IMO its current output is useful, and I don't think it should be more concise by default.

Screenshot 2023-04-12 at 13 40 37

(Downgrades are coming in 1.70.)

adamreichold commented 1 year ago

Note that an edition imposes a rust-version constraint as well: for a crate that has edition = "2021", rust-version is set to 1.56. So from that perspective, if you use cargo init today your crate will have a rust-version even though it is not explicitly mentioned in the manifest. From this perspective, it seems likely that we will trend towards all crates having a rust-version set, but it does make it harder to derive the author's intent about whether they want their crate's dependencies to match a particular MSRV.

Isn't it purely an implementation issue to make Cargo's internal data model differentiate between an implied MSRV due to an edition being set and a rust-version actually specified in the manifest file thereby modelling the intent represented by the crate author explicit setting a rust-version?

tarcieri commented 1 year ago

Alternatively to inferring it from rust-version, perhaps there could be an explicit setting in Cargo.toml or .cargo/config.toml?

Something like resolver-msrv = "1.56" perhaps?

I'm also curious whether it would make more sense for it to be something that happens at the granularity of a workspace, rather than considering the individual packages within the workspace.

epage commented 1 year ago

@jonhoo and I got to talk synchronously about this

Some quick things we can get out of the way before the details

So moving on to non-cargo-install cases...

Some related threads

People's Rust version might be behind because

We also want a culture that moves forward rather than a culture of stagnation, which is affected by how tools affect our behavior

Problems with stagnation include:

Tied into the two main workflows is the idea of what your target rust-version is (whats used for development, official binary builds) vs your verified minimal rust-version

Possible solutions we discussed (which are similar to my other comment, see it for some more trade offs)

Regarding security: being on an old version doesn't inherently make you less secure. For unreported vulnerabilities, the main risk is there are known unreported vulnerabilities. Otherwise, we could also improve the security angle by integration cargo-audit behavior into cargo (#7678, #10016)

Regarding "ensuring my dependencies do not break my dependents"

I know there is more but I've forgotten it being 24-hours later...

jonhoo commented 1 year ago

I mostly agree with the summary above. I want to clarify my thoughts on one point though, which is where I think the disagreement about where we should land in the trade-off space of this decision comes to:

Regarding security: being on an old version doesn't inherently make you less secure. For unreported vulnerabilities, the main risk is there are known unreported vulnerabilities. Otherwise, we could also improve the security angle by integration cargo-audit behavior into cargo

I think this is false. In general, newer versions have fewer bugs than older versions, whether they are known or not, and whether they are security-related or not. By running older versions of a dependency, you're more likely to have either active or latent security risks, performance issues, or even just outright incorrect behavior. Whether that matters depends on where you're building. If you're building in a development context, I totally agree that most of these are fine, and using older versions is A-OK. But if you're building for any kind of production, in my opinion, you should strongly favor using the newest stable versions of all your dependencies that your environment allows (whether rustc or crates). It follows from this that if your current environment prevents you from using the newest stable versions, you should be alerted to this fact and given an opportunity to update the environment rather than have the build "just work" by falling back to older versions.

Unfortunately, there isn't a way in Cargo to cleanly distinguish between "during development" and "for production". cargo install is the closest we have, but it's both used for development and not always used for production. Which then leaves us in a position where we need to choose: does Cargo seamlessly use old versions, or push you towards updating. I think the argument is in favor of the latter, but I completely acknowledge that there's a trade-off involved where there isn't (and won't ever be) universal consensus.

Ultimately, I took from my chat with @epage that where we disagree is on where we should come down on the following trade-off:

  1. A user that checks out, builds, modifies, and submits changes to a crate should encounter zero Rust version related friction in doing so, unless their version is older than package.rust-version or they explicitly try to write new code that requires a newer Rust version.
  2. When I build a crate for production, assuming there isn't a Cargo.lock or I have run cargo update, I get the newest version of my dependencies that my rustc allows.

The logic for how we get to these being deeply at odds with one another is a little convoluted, but I believe they are. That said, if someone has a solution that doesn't force these two to be traded off against each other, please do speak up!

If that is indeed the trade-off we have to make (or the other options are to make worse trade-offs), my instinct is to favor the second one. It makes me very sad to not be able to make the contributor experience for those with older rustc entirely friction-free, but I think the cost of 2 is too great to trade that one off. I know @epage comes down on the other side of that, and would prefer path 1.

Ultimately, I'm not a decision-making authority here. I think the Cargo team will have to collectively decide which of these use-cases is the one they want to favor over the other, and either way they'll be trading something away.

epage commented 1 year ago

I think this is false. In general, newer versions have fewer bugs than older versions, whether they are known or not, and whether they are security-related or not.

I am unsure if we can make such a statement. So long as new development is happening, new bugs are being introduced, including bug fixes (see #11999 for a recent example). We are helped by tests but that mostly helps us avoid repeating the same mistakes over again. We're human; we are creating in finding new mistakes to make.

If that is indeed the trade-off we have to make (or the other options are to make worse trade-offs), my instinct is to favor the second one. It makes me very sad to not be able to make the contributor experience for those with older rustc entirely friction-free, but I think the cost of 2 is too great to trade that one off. I know @epage comes down on the other side of that, and would prefer path 1.

One of the reasons I lean the other way is my assumption that

For context, how I'm managing my personal projects

adamreichold commented 1 year ago

Opt-in CLI flag

Coming from a "maintaining a library targeting Debian's Rust toolchain" background, I think an opt-in flag to produce an MSRV-compatible Cargo.lock file (based on package.rust-version) would be a minimum viable solution that could significantly simplify our work by replacing kludges like

https://github.com/PyO3/pyo3/blob/2f8bf513b506e8b093794ad695b2d6628e6ff8d2/noxfile.py#L408-L508

without significantly impacting the status quo of version resolution.

(I think this reasoning applies to -Zdirect-minimal-versions as well, i.e. producing a compatible lock file upon request is enough to simply and automatically make CI jobs work would be a big improvement without impacting any defaults. Usually, our contributors have more issues with our CI than just dependency resolution, e.g. we test additional Python versions and variants as well as additional architectures. We do try to mitigate this by making it feasible to run the same build automation locally and often maintainers push build fixes onto contributor PR to avoid putting this extra work onto the contributor's shoulders. Manually juggling MSRV-compatible versions is currently a relatively common occurrence of such fix-ups.)

epage commented 1 year ago

In theory we could initially implement the flag so people are not blocked on further discussions but

With that said, if someone is just interested in testing in CI or for one-off cases, then a nightly-only flag should be sufficient to unblock people while we at least get a better idea of what stable subset of the functionality we can expose as an MVP

jonhoo commented 1 year ago

So long as new development is happening, new bugs are being introduced, including bug fixes

While it's true that new bugs arise over time as well, the rate of bugs being fixed should be higher than the rate at which bugs are introduced for any given project, especially when it comes to already-existing features (as opposed to new feature development).