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.

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

Command to update Cargo.lock to minimal versions #4100

Closed
matklad opened this issue May 25, 2017 · 13 comments · Fixed by #5200
Closed

Command to update Cargo.lock to minimal versions #4100

matklad opened this issue May 25, 2017 · 13 comments · Fixed by #5200
Labels
A-dependency-resolution Area: dependency resolution and the resolver A-new-subcommand Area: new subcommand C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`

Comments

@matklad
Copy link
Member

matklad commented May 25, 2017

From today's meeting.

Problem:

You write a library A, which depends on B, so you put B = 1.0 in A's Cargo.toml. The you run Cargo build, and Cargo greedily pulls B 1.1 into the lockfile. Then you accidentally start depending on features introduced in 1.1, but you don't change Cargo.toml. Your test locally pass, and CI passes as well, and you publish a crate whose Cargo.toml is a lie.

Solution:

Add cargo update --minimal, which generates lockfile picking the minimum possible version of all crates (it's not possible, of course, because there's no total order on dependency graphs, but some heuristics might work well in practice). Then in CI environment you generate the minimal lockfile to make sure you don't accidentally depend on newer than Cargo.toml features.

@matklad
Copy link
Member Author

matklad commented May 26, 2017

Note that we can envision some kind of static solution here, for example, by tagging library functionality with #since attribute. However, actually testing things is still needed, because you may depend on a critical bugfix from the dependency.

@Eh2406
Copy link
Contributor

Eh2406 commented Jan 29, 2018

This I think would just involve changing

b.summary.version().cmp(a.summary.version())
to depend on an argument; Then bubble that argument up till it is user facing.

@Eh2406
Copy link
Contributor

Eh2406 commented Feb 21, 2018

I am still learning cargo internals, but I would be willing to help someone who wanted to take this on!

@bluetech
Copy link
Contributor

Then you accidentally start depending on features introduced in 1.1, but you don't change Cargo.toml

Perhaps there's some way to tackle this problem head-on? Probably not, but it's interesting to think about. Two possible ways I can think of:

@alexcrichton
Copy link
Member

@Eh2406 I'd imagine that with yours and @aidanhs's work on the resolver recently it may be relatively easy to encode this as a constraint and have it finish in a hopefully-not-too-long amount of time too!

@Eh2406
Copy link
Contributor

Eh2406 commented Mar 5, 2018

The relevant line of code has moved to:

b.summary.version().cmp(a.summary.version())

I think we would add a arg to:
struct RegistryQueryer<'a> {

@Dylan-DPC-zz
Copy link

Can give this a try :)

@Eh2406
Copy link
Contributor

Eh2406 commented Mar 5, 2018

@Dylan-DPC That is wonderful!

The idea is that the sort on line L664 determines the order that cargo tries versions of packages. Currently from newest to oldest and we want to reverse that order.
Steps as I see it:

  • Add a -Z flag, like I did in https://github.com/rust-lang/cargo/pull/4990/files
  • Get that flag on L364 something like config.cli_unstable().minimal_versions and pass it to RegistryQueryer
  • Use it in the sort function, if minimal_versions {a.summary.version().cmp(b.summary.version()) } else the existing code
  • Add it to some tests. I'd fine them by (temporarily) hard coding minimal_versions to true and looking at wich test in cargo\tests\testsuite\resolve.rs fail.

How can I be helpful!?

@Dylan-DPC-zz
Copy link

Thanks @Eh2406 will take a look whenever I'm free and see how far I get :)

@Eh2406
Copy link
Contributor

Eh2406 commented Mar 5, 2018

If that turns out to be challenging, feel free to ask!

@Dylan-DPC-zz
Copy link

@Eh2406

Get that flag on L364 something like config.cli_unstable().minimal_versions and pass it to RegistryQueryer

L364 of which file?

@Eh2406
Copy link
Contributor

Eh2406 commented Mar 11, 2018

Sorry I wasn't clear
cargo/src/cargo/core/resolver/mod.rs

bors added a commit that referenced this issue Mar 23, 2018
feat(resolver): Add CLI option to resolve minimal version dependencies

Fixes #4100

Test cases are still missing. We need to come up with a plan what cases we want to cover.

Thanks a lot to @Eh2406 for very helpful instructions to kick this off.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dependency-resolution Area: dependency resolution and the resolver A-new-subcommand Area: new subcommand C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants