cargo generate-lockfile doesn't give a working lockfile in some situations

[Nemo157]

Problem
For some crates running generate-lockfile does not result in a working lockfile, whereas running fetch does.

Steps

$ cargo download -x alcro==0.1.4
INFO: cargo-download v0.1.2
INFO: Crate `alcro==0.1.4` downloaded successfully
INFO: Crate content extracted to ./alcro-0.1.4/
$ cd alcro-0.1.4/
$ cp Cargo.lock Cargo.lock.orig
$ cargo fetch --locked
error: the lock file /tmp/tmp.Uh4w4Ih9yF/alcro-0.1.4/Cargo.lock needs to be updated but --locked was passed to prevent this
If you want to try to generate the lock file without accessing the network, use the --offline flag.
$ cargo generate-lockfile
    Updating crates.io index
       Fetch [                                                       ]   0.00%
$ diff Cargo.lock Cargo.lock.orig
244c244
<  "proc-macro2 1.0.18",
---
>  "proc-macro2 1.0.17",
# [...] other just version changes
$  cargo fetch --locked
error: the lock file /tmp/tmp.Uh4w4Ih9yF/alcro-0.1.4/Cargo.lock needs to be updated but --locked was passed to prevent this
If you want to try to generate the lock file without accessing the network, use the --offline flag.
$ cargo fetch
$ diff Cargo.lock Cargo.lock.orig
10c10
<  "futures",
---
>  "futures 0.1.29",
# [...] lots more structural changes
$ cargo fetch --locked
# works

[ehuss]

Thanks for the report!

It doesn't matter which command is run. Building a lock file with any command, and then running any command with --locked will fail.

This seems similar to #7753. alcro specifies two different cfg dependencies on futures:

[target."cfg(unix)".dev-dependencies.futures]
version = "^0"
[target."cfg(windows)".dev-dependencies.futures]
version = "^0.1"

I think that is a use case that Cargo never really considers, and it doesn't seem to work properly. As mentioned in #7753 (comment), it would be very difficult to determine that these two expressions are mutually exclusive. I would lean towards forbidding that as a duplicate dependency for now. @Eh2406 wdyt?

[Nemo157]

Note that the final cargo fetch --locked worked after having run cargo fetch, that did a much bigger rewrite of the Cargo.lock whereas the cargo generate-lockfile call only appeared to update the version numbers.

[Eh2406]

I believe that it is currently treated as being mutually true. I get a lockfile that just has version = "0.1.29", that satisfies both requirements. That being said I was unable to reproduce the error.

[Nemo157]

I originally noticed this on the latest nightly since that's what docs.rs uses, the repro I posted above was when using cargo 1.45.0-nightly (cb06cb269 2020-05-08) locally.

[ehuss]

@Eh2406 it is probably because you are on windows (and the cfg(windows) has a different selection). Here is an isolated test that should trigger on all platforms:

#[cargo_test]
fn mixed_targets_same_dependency() {
    Package::new("bar", "0.1.0").publish();
    Package::new("bar", "0.2.0").publish();
    Package::new("zoo", "1.0.0")
        .dep("bar", "0.1")
        .publish();

    let p = project()
        .file("Cargo.toml",
            r#"
                [package]
                name = "foo"
                version = "0.1.0"

                [dependencies]
                zoo = "1.0"
                bar = "^0"

                [target.'cfg(does_not_exist)'.dependencies]
                bar = "^0.1"
            "#)
        .file("src/lib.rs", "")
        .build();

    p.cargo("generate-lockfile").run();
    p.cargo("check --locked").run();
}

I haven't checked, but I suspect this is relatively common, so rejecting it outright might not be the right thing.

[alexcrichton]

I'm a bit surprised by this behavior, I thought we fixed this some time ago! I don't think we necessarily want a special case to reject this in the manifest, it seems like it would be pretty difficult to detect overlapping ranges. The resolver, however, should be trying to solve both version constraints and realize there's one solution. Currently it seems like it's picking the first for generate-lockfile and the second for a target-specific resolution, which is not great.

[ehuss]

I took a little look at this. It seems to be unique to this situation where one version req is a superset of another. If you change ^0 to ^0.2, then it works fine.

What happens is that when it is locking the summary, both bar deps get locked to =0.1.0. This happens here, where it is iterating over the locked entries. It picks the first entry it finds that matches, and for ^0 it matches 0.1.0 because it appears in the list before 0.2.0.

I don't know how to fix that. Perhaps it should pick the dep with the greatest matching version? Iterate in reverse order?

[alexcrichton]

Hm this seems like it's even more evidence that the current implementation of locking is not really up to par. We should know that if a crate has two versions in a lock file exactly which one you should be locked to for each dep.

This sounds like we're basically not keeping track of that information, only what's available to lock. I don't think iteration order can solve this, because you should be able to construct situations where there's as many versions in play as you need, and the lock should always be able to pick the one the lock file indicates.

In any case this seems like a more fundamental issue in how lock files are handled. This doesn't really ever come up much in practice because 99% of version requirements are "semver compatible" which means there's no overlap across major versions.