multiple path dependencies with the same name confuses the resolver

[emilio]

I recently published a new rust-bindgen version (0.69.4), and that caused some Firefox CI jobs to fail (https://bugzilla.mozilla.org/show_bug.cgi?id=1878575), because some CI checks that run cargo metadata ended up touching the lockfile and updating bindgen.

The relevant bits that seem to be needed to reproduce this is a local crate for an older version of the crate.

To reproduce:

• git clone https://github.com/mozilla/gecko-dev
• cd gecko-dev
• git reset --hard 9d8d867dac5d35156a51ef21fe7b46f573aaabef
• cargo metadata

Somehow, the lockfile changes to bindgen 0.69.4 even though the lockfile specifies 0.69.2

[emilio]

(Apologies for not having something more reduced, can try to do so if needed, though might take a bit)

[ehuss]

Thanks for the report! I believe I have a partial understanding of the problem. The following is a test for cargo's testsuite that demonstrates the issue:

#[cargo_test]
fn patched_reexport_stays_locked() {
    // Patch example where you emulate a semver-incompatible patch via a re-export.
    // Testing an issue where the lockfile does not stay locked after a new version is published.
    Package::new("bar", "1.0.0").publish();
    Package::new("bar", "2.0.0").publish();
    Package::new("bar", "3.0.0").publish();

    let p = project()
        .file(
            "Cargo.toml",
            r#"
                [workspace]


                [package]
                name = "foo"

                [dependencies]
                bar1 = {package="bar", version="1.0.0"}
                bar2 = {package="bar", version="2.0.0"}

                [patch.crates-io]
                bar1 = { package = "bar", path = "bar-1-as-3" }
                bar2 = { package = "bar", path = "bar-2-as-3" }
            "#,
        )
        .file("src/lib.rs", "")
        .file(
            "bar-1-as-3/Cargo.toml",
            r#"
                [package]
                name = "bar"
                version = "1.0.999"

                [dependencies]
                bar = "3.0.0"
            "#,
        )
        .file("bar-1-as-3/src/lib.rs", "")
        .file(
            "bar-2-as-3/Cargo.toml",
            r#"
                [package]
                name = "bar"
                version = "2.0.999"

                [dependencies]
                bar = "3.0.0"
            "#,
        )
        .file("bar-2-as-3/src/lib.rs", "")
        .build();

    p.cargo("tree")
        .with_stdout(
            "\
foo v0.0.0 ([ROOT]/foo)
├── bar v1.0.999 ([ROOT]/foo/bar-1-as-3)
│   └── bar v3.0.0
└── bar v2.0.999 ([ROOT]/foo/bar-2-as-3)
    └── bar v3.0.0
",
        )
        .run();

    fs::copy(
        p.root().join("Cargo.lock"),
        p.root().join("Cargo.lock.orig"),
    )
    .unwrap();

    Package::new("bar", "3.0.1").publish();
    p.cargo("tree")
        .with_stdout(
            "\
foo v0.0.0 ([ROOT]/foo)
├── bar v1.0.999 ([ROOT]/foo/bar-1-as-3)
│   └── bar v3.0.0
└── bar v2.0.999 ([ROOT]/foo/bar-2-as-3)
    └── bar v3.0.0
",
        )
        .run();

    assert_eq!(p.read_file("Cargo.lock"), p.read_file("Cargo.lock.orig"));
}

The problem I believe is here. When loading the original Cargo.lock, that bit of code expects package names to be unique in the set of path dependencies. However, in this scenario there are two packages both with the same name. I think the solution is to also check the version, though there might be some subtle issues there.

[linyihai]

@rustbot claim

[linyihai]

Hi, I copy the test into the cargo repo and get this output

   Compiling cargo v0.79.0 (/root/workspace/cargo)
    Finished test [unoptimized + debuginfo] target(s) in 40.17s
     Running tests/testsuite/main.rs (target/debug/deps/testsuite-a446100106b28075)

running 1 test
running `/root/workspace/cargo/target/debug/cargo tree`
running `/root/workspace/cargo/target/debug/cargo tree`
thread 'tree::patched_reexport_stays_locked' panicked at tests/testsuite/tree.rs:2287:10:

test failed running `/root/workspace/cargo/target/debug/cargo tree`
error: stdout did not match:
1   1     foo v0.0.0 (/root/workspace/cargo/target/tmp/cit/t0/foo)
2   2     ├── bar v1.0.999 (/root/workspace/cargo/target/tmp/cit/t0/foo/bar-1-as-3)
3        -│   └── bar v3.0.0
    3    +│   └── bar v3.0.1
4   4     └── bar v2.0.999 (/root/workspace/cargo/target/tmp/cit/t0/foo/bar-2-as-3)
5        -    └── bar v3.0.0
    5    +    └── bar v3.0.1

Hmm, the test case failed to run, and I think the final code fix is to get the use case to pass