[patch] causes cargo run to succeed once, then fail, with no intervening changes

[benesch]

A somewhat pathological [patch] block can cause cargo run to succeed once, then fail on subsequent runs with no intervening changes to the crate, for a rather mind-bending experience.

Repro instructions:

1. Clone https://github.com/benesch/rust-play:

$ git clone https://github.com/benesch/rust-play

2. Run cargo run, and watch it succeed:

$ cd rust-play
$ cargo run
    Updating git repository `https://github.com/benesch/timely-dataflow.git`...
   Compiling proc-macro2 v0.4.30
   Compiling ...
    Finished dev [unoptimized + debuginfo] target(s) in 1m 04s
     Running `target/debug/rust-play`
((0, (0, 0)), 0, 1)
((0, (0, 1)), 0, 1)
((1, (0, 2)), 0, 1)
((1, (0, 3)), 0, 1)
((2, (1, 4)), 0, 1)
((2, (1, 5)), 0, 1)
((3, (1, 6)), 0, 1)
((3, (1, 7)), 0, 1)
((4, (2, 8)), 0, 1)
((4, (2, 9)), 0, 1)

3. Run cargo run and watch it fail:

$ cargo run
    Updating git repository `https://github.com/TimelyDataflow/timely-dataflow.git`
   Compiling timely_logging v0.10.0 (https://github.com/TimelyDataflow/timely-dataflow.git#b63bea65)
   Compiling timely_bytes v0.10.0 (https://github.com/TimelyDataflow/timely-dataflow.git#b63bea65)
   Compiling timely_communication v0.10.0 (https://github.com/TimelyDataflow/timely-dataflow.git#b63bea65)
   Compiling timely v0.10.0 (https://github.com/TimelyDataflow/timely-dataflow.git#b63bea65)
   Compiling differential-dataflow v0.10.0 (https://github.com/TimelyDataflow/differential-dataflow.git#c78b860a)
   Compiling rust-play v0.1.0 (/Users/benesch/Desktop/rust-play)
error[E0412]: cannot find type `TestPatch` in module `timely`
  --> src/main.rs:38:22
   |
38 | const _TEST: timely::TestPatch = timely::TestPatch;
   |                      ^^^^^^^^^ not found in `timely`

error[E0425]: cannot find value `TestPatch` in module `timely`
  --> src/main.rs:38:42
   |
38 | const _TEST: timely::TestPatch = timely::TestPatch;
   |                                          ^^^^^^^^^ not found in `timely`

error: aborting due to 2 previous errors

Some errors have detailed explanations: E0412, E0425.
For more information about an error, try `rustc --explain E0412`.
error: Could not compile `rust-play`.

To learn more, run the command again with --verbose.

The reason it fails the second time around is because the first run leaves this change to Cargo.lock lying around. Somehow the state that the first patch resolution writes causes the next patch resolution to fail.

Part of the problem here is that the rust-play crate depends on https://github.com/TimelyDataflow/timely-dataflow.git (note the ".git" suffix) directly, and https://github.com/TimelyDataflow/timely-dataflow (no ".git" suffix) via differential dataflow. I realize this might seem contrived here, but this actually happened accidentally in a real project, and it was extraordinarily confusing to debug.

I guess I don't understand whether it's intended that Cargo can determine when two different URLs actually point to the same crate. In rust-play without the [patch] block, i.e., with this diff applied

diff --git a/Cargo.toml b/Cargo.toml
index 732babc..0396948 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -8,5 +8,5 @@ edition = "2018"
 differential-dataflow = { git = "https://github.com/TimelyDataflow/differential-dataflow.git" }
 timely = { git = "https://github.com/TimelyDataflow/timely-dataflow.git" }
 
-[patch."https://github.com/TimelyDataflow/timely-dataflow"]
-timely = { git = "https://github.com/benesch/timely-dataflow.git", branch = "sample" }
+# [patch."https://github.com/TimelyDataflow/timely-dataflow"]
+# timely = { git = "https://github.com/benesch/timely-dataflow.git", branch = "sample" }
diff --git a/src/main.rs b/src/main.rs
index cec240f..712d8bc 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -35,4 +35,4 @@ fn main() {
 }
 
 // Comment this out to compile without the timely patch in Cargo.toml.
-const _TEST: timely::TestPatch = timely::TestPatch;
+// const _TEST: timely::TestPatch = timely::TestPatch;

Cargo somehow figures out that "timely-dataflow.git" and "timely" are actually the same crate, and uses one of them everywhere. The program wouldn't compile otherwise, since the types don't match in a way that causes compilation to fail, because differential exposes its copy of timely in its public API.

If it's intended that Cargo can handle having multiple URLs for the same crate, then it seems like there's just a bug in [patch] blocks where this same-crate-different-URL thing isn't getting sorted out. If Cargo isn't supposed to handle having multiple URLs for the same crate... then the situation is more complicated, and I'm not sure what the right fix is.

Anyway, I'm out of my depth here, but let me know if I can help at all!

I initially discovered this on macOS with Cargo 1.36, but it seems to reproduce readily with Cargo 1.37.

...actually, I just tried this on master, and it has a totally different presentation. Rather than failing to compile on the second try, it crashes outright:

$ RUST_BACKTRACE=1 ~/Sites/cargo/target/debug/cargo run
    Updating git repository `https://github.com/TimelyDataflow/timely-dataflow.git`
thread 'main' panicked at 'assertion failed: prev.is_none()', src/cargo/core/resolver/context.rs:153:25
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /Users/vsts/.cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.29/src/backtrace/libunwind.rs:88
   1: backtrace::backtrace::trace_unsynchronized
             at /Users/vsts/.cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.29/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:47
   3: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:36
   4: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:200
   5: std::panicking::default_hook
             at src/libstd/panicking.rs:214
   6: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:477
   7: std::panicking::begin_panic
             at /rustc/eae3437dfe991621e8afdc82734f4a172d7ddf9b/src/libstd/panicking.rs:411
   8: cargo::core::resolver::context::Context::flag_activated
             at src/cargo/core/resolver/context.rs:153
   9: cargo::core::resolver::activate
             at src/cargo/core/resolver/mod.rs:655
  10: cargo::core::resolver::activate_deps_loop
             at src/cargo/core/resolver/mod.rs:382
  11: cargo::core::resolver::resolve
             at src/cargo/core/resolver/mod.rs:137
  12: cargo::ops::resolve::resolve_with_previous
             at src/cargo/ops/resolve.rs:338
  13: cargo::ops::resolve::resolve_with_registry
             at src/cargo/ops/resolve.rs:119
  14: cargo::ops::resolve::resolve_ws_with_opts
             at src/cargo/ops/resolve.rs:72
  15: cargo::ops::cargo_compile::compile_ws
             at src/cargo/ops/cargo_compile.rs:303
  16: cargo::ops::cargo_compile::compile_with_exec
             at src/cargo/ops/cargo_compile.rs:251
  17: cargo::ops::cargo_compile::compile
             at src/cargo/ops/cargo_compile.rs:240
  18: cargo::ops::cargo_run::run
             at src/cargo/ops/cargo_run.rs:73
  19: cargo::commands::run::exec
             at src/bin/cargo/commands/run.rs:75
  20: cargo::cli::execute_subcommand
             at src/bin/cargo/cli.rs:179
  21: cargo::cli::main
             at src/bin/cargo/cli.rs:94
  22: cargo::main
             at src/bin/cargo/main.rs:41
  23: std::rt::lang_start::{{closure}}
             at /rustc/eae3437dfe991621e8afdc82734f4a172d7ddf9b/src/libstd/rt.rs:64
  24: std::rt::lang_start_internal::{{closure}}
             at src/libstd/rt.rs:49
  25: std::panicking::try::do_call
             at src/libstd/panicking.rs:296
  26: __rust_maybe_catch_panic
             at src/libpanic_unwind/lib.rs:82
  27: std::panicking::try
             at src/libstd/panicking.rs:275
  28: std::panic::catch_unwind
             at src/libstd/panic.rs:394
  29: std::rt::lang_start_internal
             at src/libstd/rt.rs:48
  30: std::rt::lang_start
             at /rustc/eae3437dfe991621e8afdc82734f4a172d7ddf9b/src/libstd/rt.rs:64
  31: cargo::init_git_transports

The panic seems pretty obviously introduced in #7118, though I'm not familiar enough with the code to say whether #7118 moved closer or further to solving this overarching issue.

[alexcrichton]

Thanks for the report, and this definitely sounds confusing! I'm having a tough time minimizing this though without creating a number of github repos. Would you be able to help out in narrowing this down to just a few crates to explore what Cargo's doing here?

[benesch]

Thanks for looking into this! So I'm afraid I can't reproduce the specific example much more. Every time I try to recreate it, it fails to reproduce exactly.

But! I can give you a similar error. I created three crates, base, intermediate, and app. base has no dependencies; intermediate depends on base; and app depends on both intermediate and base. Note that intermediate uses no ".git" suffix to depend on base, while app does use a ".git" suffix to depend on base. If you try to build app, cargo build will succeed without complaint on the first build but generate a lock file change that causes all futures cargo builds to complain about an unused patch:

$ git clone https://github.com/benesch/cargo-7282-app.git
$ cd cargo-7282-app
$ cargo build
# Should succeed.
$ cargo build
# Should complain about an unused patch.

Note that I didn't do anything crazy to get into this state. The Cargo.lock that's committed into the app crate was generated by running cargo build after adding the patch block. The progression is something like:

• no patch
• run cargo build; works
• add patch
• run cargo build; works and generates Cargo.lock that correctly uses patch
• run cargo build; works but generates Cargo.lock that claims patch is unused
• run cargo build; complains about unused patch
• run cargo build; complains about unused patch
• ...

[alexcrichton]

Ok apologies for the excessive delay in getting back to this, but the fix should be posted at #7368. Thanks again for helping me reduce this, that made this much easier to debug, verify a fix, and write a test case!

[benesch]

apologies for the excessive delay in getting back to this

Excessive delay?! The fact that you got to this in a few weeks is super impressive, in my opinion. Thanks, @alexcrichton!