Multiple git dependencies with different refs confuses cargo

[ehuss]

Problem

If there are multiple git dependencies on the same repository with different refs, but those refs contain the same package version, then cargo gets confused. There is no package spec that can be passed to -p to unambiguously refer to them. Thus commands like cargo update -p my-git-dep can't work.

Steps

Example using cargo's testsuite:

#[cargo_test]
fn multiple_git_same_version() {
    // Test what happens if different packages refer to the same git repo with
    // different refs, and the package version is the same.
    let (xyz_project, xyz_repo) = git::new_repo("xyz", |project| {
        project
            .file("Cargo.toml", &basic_lib_manifest("xyz"))
            .file("src/lib.rs", "fn example() {}")
    });
    let rev1 = xyz_repo.revparse_single("HEAD").unwrap().id();
    xyz_project.change_file("src/lib.rs", "pub fn example() {}");
    git::add(&xyz_repo);
    let rev2 = git::commit(&xyz_repo);
    // Both rev1 and rev2 point to version 0.1.0.

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

                    [dependencies]
                    bar = {{ path = "bar" }}
                    xyz = {{ git = "{}", rev = "{}" }}

                "#,
                xyz_project.url(),
                rev1
            ),
        )
        .file("src/lib.rs", "")
        .file(
            "bar/Cargo.toml",
            &format!(
                r#"
                    [package]
                    name = "bar"
                    version = "0.1.0"

                    [dependencies]
                    xyz = {{ git = "{}", rev = "{}" }}
                "#,
                xyz_project.url(),
                rev2
            ),
        )
        .file("bar/src/lib.rs", "")
        .build();

    p.cargo("check").run();
    p.cargo("tree")
        .with_stdout(&format!(
            "\
foo v0.1.0 ([..]/foo)
├── bar v0.1.0 ([..]/foo/bar)
│   └── xyz v0.5.0 (file://[..]/xyz?rev={}#{})
└── xyz v0.5.0 (file://[..]/xyz?rev={}#{})
",
            rev2,
            &rev2.to_string()[..8],
            rev1,
            &rev1.to_string()[..8]
        ))
        .run();
    // FIXME: This fails since xyz is ambiguous, but the
    // possible pkgids are also ambiguous.
    p.cargo("pkgid xyz")
        .with_status(101)
        .with_stderr(
            "\
error: There are multiple `xyz` packages in your project, and the specification `xyz` is ambiguous.
Please re-run this command with `-p <spec>` where `<spec>` is one of the following:
  file://[..]/xyz#0.5.0
  file://[..]/xyz#0.5.0
",
        )
        .run();
    // TODO, what should the `-p` value be here?
    //p.cargo("update -p")
}

Possible Solution(s)

I'm not sure if this should be an allowed configuration, since @alexcrichton has mentioned a few times like here that is an invalid Cargo.lock configuration. However, I think it probably should be supported, since otherwise it can make it hard to get all Cargo.toml files in sync.

I could image cargo could allow pkgid specs of the form https://.../foo.git?rev=499e8c4bb3b75ae6c4dcc91a8bdd81947c577147 or something like that.

Notes

There are some related issues like #8101 and #8293, though I'm not sure if those are duplicates.

Version

cargo 1.59.0-nightly (fcef61230 2021-12-17)
release: 1.59.0-nightly
commit-hash: fcef61230c3b6213b6b0d233a36ba4ebd1649ec3
commit-date: 2021-12-17
host: x86_64-apple-darwin
libgit2: 1.3.0 (sys:0.13.23 vendored)
libcurl: 7.77.0 (sys:0.4.51+curl-7.80.0 system ssl:(SecureTransport) LibreSSL/2.8.3)
os: Mac OS 12.0.1 [64-bit]

[alexcrichton]

I think this is a bug in pkgid specs, but otherwise this is valid and we should support this in Cargo. I think the lock files should work out alright in the sense that there are two nodes since the #... bits are distinct.

I think your suggestion about putting the rev in the url of the pkgid spec makes sense. (AFAIK this has been a bug since the beginning of pkgid specs, but pkgid specs never really took off in a big fashion either)

[jyn514]

Note that this can happen even if you have the same rev - cargo thinks that source = "git+ssh://git@redacted/service-common.git?branch=master#0f5d38a082ec97316ee58002e370d4c030e79e7a" and source = "git+ssh://git@redacted/service-common.git?rev=0f5d38a082ec97316ee58002e370d4c030e79e7a#0f5d38a082ec97316ee58002e370d4c030e79e7a are different in the lockfile. The only difference is whether you use rev = "..." or branch = "..." in Cargo.toml.

[jyn514]

Another consequence of this is that cargo will think there's a linker conflict between the two versions, even if they point to the same git hash.

# outer/Cargo.toml
[dependencies]
inner = { path = "./inner" }
boring-sys = { git = "https://github.com/cloudflare/boring", branch = "master" }

# inner/Cargo.toml
[dependencies]
boring-sys = { git = "https://github.com/cloudflare/boring", rev = "5f327aba86aab67af58f0db44e61d5b71b6eb84e" }

error: failed to select a version for `boring-sys`.
    ... required by package `inner v0.1.0 (/home/jnelson/rust-community/cargo-native-git-weirdness/inner)`
    ... which satisfies path dependency `inner` (locked to 0.1.0) of package `cargo-native-git-weirdness v0.1.0 (/home/jnelson/rust-community/cargo-native-git-weirdness)`
versions that meet the requirements `*` are: 2.0.0

the package `boring-sys` links to the native library `boringssl`, but it conflicts with a previous package which links to `boringssl` as well:
package `boring-sys v2.0.0 (https://github.com/cloudflare/boring?branch=master#5f327aba)`
    ... which satisfies git dependency `boring-sys` (locked to 2.0.0) of package `cargo-native-git-weirdness v0.1.0 (/home/jnelson/rust-community/cargo-native-git-weirdness)`
Only one package in the dependency graph may specify the same links value. This helps ensure that only one copy of a native library is linked in the final binary. Try to adjust your dependencies so that only one package uses the links ='boring-sys' value. For more information, see https://doc.rust-lang.org/cargo/reference/resolver.html#links.

failed to select a version for `boring-sys` which could resolve this conflict

[nagisa]

This can get as bad as making it impossible to update either of the conflicting dependency:

$ cargo update -p https://github.com/repo#package:3.0.0
error: There are multiple `package` packages in your project, and the specification `https://github.com/repo#package:3.0.0` is ambiguous.
Please re-run this command with `-p <spec>` where `<spec>` is one of the following:
  https://github.com/repo#package:3.0.0
  https://github.com/repo#package:3.0.0

and produces an unactionable message.

[jyn514]

This also prevents running cargo vendor.

$ cargo vendor
error: failed to sync

Caused by:
  found duplicate version of package `redacted v0.0.0` vendored from two sources:

  	source 1: ssh://git@redacted/redacted.git?branch=master#7f2b2621
  	source 2: ssh://git@redacted/redacted.git?rev=b1b97b2671202fad44bd882593c9750f1de986a6#b1b97b26

[jyn514]

A further issue: cargo considers rev = '44fde65' and rev = '44fde65b0e5' to be different versions, even if they resolve to exactly the same full hash. I think cargo needs to serialize the lockfile differently to only have the full hash and not the shortened version.