Panic when patching dependency

[PvdBerg1998]

Problem
I'm patching one of my dependencies (mysql-async) to a branch on github. The crate on this branch requires pin-project:^0.4.0-alpha.10. I'm also using hyper, which requires pin-project:=0.4.0-alpha.7. This in itself causes compilation failure:

versions that meet the requirements ^0.4.0-alpha.10 are: 0.4.0-alpha.10

all possible versions conflict with previously selected packages.

  previously selected package pin-project v0.4.0-alpha.7
    ... which is depended on by hyper v0.13.0-alpha.1`
    ... which is depended on by hyper-tls v0.4.0-alpha.1`
    ... which is depended on by projectname

failed to select a version for `pin-project` which could resolve this conflict

So I tried to patch it to the current master branch like so:

[patch.crates-io]
pin-project = { git = "https://github.com/taiki-e/pin-project", version = "^0.4.0-alpha.10" }

This results in the following panic:

thread 'main' panicked at 'assertion failed: prev.is_none()', src\tools\cargo\src/cargo\core\resolver\context.rs:153:25

https://github.com/rust-lang/cargo/blob/master/src/cargo/core/resolver/context.rs#L153

Steps

1. Have these incompatible dependencies
2. Delete Cargo.lock (not sure if required)
3. Execute cargo update/run/check

Notes

Output of cargo version: cargo 1.39.0-nightly (22f7dd049 2019-08-27).
Compiler: rustc 1.39.0-nightly (c6e9c76c5 2019-09-04) with nightly-x86_64-pc-windows-msvc (default).
Unfortunately I'm unable to share the erroring code nor have I created a minimized example.

[alexcrichton]

Thanks for the report! I think that this may have bene fixed recently on nightly, mind trying again with an updated nightly and see if it fixes the issue?

[PvdBerg1998]

@alexcrichton It's still present on the nightly of today. I worked around it by forking the dependency and patching some code - but this shouldn't be happening ofcourse 😅

[alexcrichton]

Sorry for the delay in getting back to this, but @PvdBerg1998 I'm unfortunately unable to reproduce this. Does this still reproduce for you? If so can you list out the instructions I need to do to reproduce it?

[PvdBerg1998]

No worries @alexcrichton. I'm able to reproduce this by adding the pin-project patch line back to my Cargo.toml. I was able to minimize it to this:

[patch.crates-io]
mysql_async = { git = "https://github.com/PvdBerg1998/mysql_async/", branch = "async-await" }
pin-project = { git = "https://github.com/taiki-e/pin-project", version = "^0.4.0-alpha.10" }

[dependencies]
hyper = "=0.13.0-alpha.1"
mysql_async = "^0.21.0-alpha.1"

Hopefully it is reproducable.

[alexcrichton]

Ah ok I was able to reproduce, thanks! Looks like this bug was relatively recently introduced since only beta/nightly, and I was trying to reproduce with stable!

[alexcrichton]

Ok a local test for this looks like:

#[cargo_test]
fn semver_eq_versions_and_wrenches() {
    let bar = git::repo(&paths::root().join("override"))
        .file("Cargo.toml", &basic_manifest("bar", "0.1.3"))
        .file("src/lib.rs", "")
        .build();

    Package::new("bar", "0.1.1").publish();
    Package::new("bar", "0.1.2").publish();

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

                    [dependencies]
                    bar = "=0.1.1"
                    baz = {{ path = 'baz' }}

                    [patch.crates-io]
                    bar = {{ git = '{}', version = "^0.1.2" }}
                "#,
                bar.url(),
            ),
        )
        .file("src/lib.rs", "")
        .file(
            "baz/Cargo.toml",
            r#"
                [package]
                name = "baz"
                version = "0.2.0"

                [dependencies]
                bar = "^0.1.2"
            "#,
        )
        .file("baz/src/lib.rs", "")
        .build();

    p.cargo("build").run();
}

@Eh2406 you may be interested in this as well, the panicking line is this one which was added in #7118 which sure enough probably wouldn't happen if we didn't have special handling for [patch]. Do you think we could handle this by updating code around here maybe? I find myself always getting lost in the resolver nowadays :(

[Eh2406]

RemainingCandidates::next and flag_activated do the same-ish thing (cc #6967). So we can fix it in ether place. A [patch] ends up in the activations twice one with each source_ids thanks to #7118. Do we need to check both? We definitely need to check the destination source_id, that is the bug #7118 is fixing. Do we need to check the origin?

Assuming we do need to check both:
The fix in flag_activated is to add a check, just like the one that you pointed out, that uses dev's source_id instead of as_activations_key.
The fix in RemainingCandidates::next, is to replace assert!(prev.is_none()); with something like if let Some(other_id) {return (other_id, ConflictReason::Semver).into() and changing the return type to a ActivateResult

I'd lean toward the later, as I don't think this will be common, and it limits the code that is special for [patch].

[pyrrho]

I'd like to take a crack at implementing the fixes outlined by Eh2406. I'm going to be using it as an introductory issue for making changes to Cargo and the Resolver, so progress may be somewhat slow.

[Eh2406]

@pyrrho How is it going? Rereading my comment, I would entirely understand if it is entirely gibberish
to other people. Please reach out!

[pyrrho]

Slower than I would have liked, but I think I have a handle on the components you and Alex were discussing. I expect I'll have something today or tomorrow (even if it is just a couple concrete tests and questions about the semantics of these features).