rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.6k stars 2.39k forks source link

Vendoring a git dependency with `+` in branch name is broken #14584

Closed thecaralice closed 3 hours ago

thecaralice commented 3 hours ago

Problem

In the example given below, this is what cargo check says:

error: failed to get `tokio-listener` as a dependency of package `givc v0.0.1 (/private/tmp/ghaf-givc)`

Caused by:
  failed to load source for dependency `tokio-listener`

Caused by:
  Unable to update https://github.com/avnik/tokio-listener?branch=avnik/vsock+tonic

Caused by:
  the source git+https://github.com/avnik/tokio-listener?branch=avnik/vsock+tonic requires a lock file to be present first before it can be
  used against vendored source code

  remove the source replacement configuration, generate a lock file, and then
  restore the source replacement configuration to continue the build

Expected behavior: no issue with vendoring this dependency. This was also reported in ipetkov/crane#549 but seems to actually be a bug in cargo.

Steps

git clone https://github.com/avnik/ghaf-givc.git -b avnik/listeners+vsock+tonic
cd ghaf-givc
mkdir .cargo/
cargo vendor > .cargo/config.toml
cargo check # observe the error
rm -rf .cargo/
cargo update # observe the suspicious message
cargo update # -//-

Possible Solution(s)

Percent-encode URLs more strictly (see notes)

Notes

This is what cargo update says in the example given:

    Updating git repository `https://github.com/rust-vsock/tokio-vsock`
    Updating crates.io index
    Updating git repository `https://github.com/avnik/tokio-listener`
     Locking 1 package to latest compatible version
    Removing tokio-listener v0.4.3 (https://github.com/avnik/tokio-listener?branch=avnik/vsock tonic#eba7a9a0)
      Adding tokio-listener v0.4.3 (https://github.com/avnik/tokio-listener?branch=avnik/vsock+tonic#eba7a9a0)
note: pass `--verbose` to see 31 unchanged dependencies behind latest

Looks like cargo treats + in the URL as a space symbol encoded following RFC 1866, notices that the actual branch name has a plus and not a space, but does not encode the + symbol as recommended by RFC 3986.

Version

cargo 1.82.0-nightly (257b72b8a 2024-07-30) release: 1.82.0-nightly commit-hash: 257b72b8adfb1f2aa9916cefca67285c21666276 commit-date: 2024-07-30 host: aarch64-apple-darwin libgit2: 1.8.1 (sys:0.19.0 vendored) libcurl: 8.9.1 (sys:0.4.73+curl-8.8.0 system ssl:OpenSSL/3.0.14) ssl: OpenSSL 3.3.1 4 Jun 2024 os: Mac OS 14.4.1 [64-bit]

weihanglo commented 3 hours ago

Looks like the same bug in https://github.com/rust-lang/cargo/issues/11085, and is already fixed via https://github.com/rust-lang/cargo/pull/12280 and now is part of lockfile v4 https://github.com/rust-lang/cargo/pull/12852. Could you try changing version = 3 to version = 4 in Cargo.lock, and runcargo vendor` again and see if it works?

thecaralice commented 3 hours ago

Yep, changing the version to 4 and running cargo update changed ?branch=avnik/vsock+tonic fragment in the lockfile to ?branch=avnik%2Fvsock%2Btonic and this fixes everything. Closing as already fixed.