Stack overflow if dev dependency has links

[dtolnay]

Minimal repro:

# Cargo.toml

[package]
name = "repro"
version = "0.0.0"
links = "repro"

[dev-dependencies]
dep = { path = "dep" }

// src/lib.rs (empty)

// build.rs

fn main() {}

# dep/Cargo.toml

[package]
name = "dep"
version = "0.0.0"
links = "dep"

[dependencies]
repro = { path = ".." }

// dep/src/lib.rs (empty)

// dep/build.rs

fn main() {}

---

$ cargo check --tests

thread 'main' has overflowed its stack
fatal runtime error: stack overflow
Aborted (core dumped)

This crashes quite early on in Cargo. A target directory isn't even created.

This is a regression introduced in whatever Cargo shipped with Rust 1.28.0. Earlier toolchains build this successfully.

$ cargo +1.27.0 check --tests
   Compiling repro v0.0.0 (file:///git/repro)
   Compiling dep v0.0.0 (file:///git/repro/dep)
    Finished dev [unoptimized + debuginfo] target(s) in 0.19s

[Eh2406]

Fascinating! I am very much looking forward to digging in on this. I'm wondering how this has gone unnoticed for so long.

[alexcrichton]

Wow we are really bad about stack overflowing with build scripts. We seem to never be able to get this quite right... With a blast from the past I've found that the stack overflow is in this function, but I don't believe that function is at fault. The problem is that we have a cycle in the dependency graph which says that the execution of repro's build script depends on the execution of dep's build script, and vice versa.

The correct dependency graph would have dep's build script depend on repro's, but that's it. Build scripts shouldn't depend on the build scripts off dev-dependencies, only normal dependencies. Turns out the bug is basically here where we also need to filter for dev-dependencies.

That function traces its history back to #5711 (spawned from #5708, more stack overflows). That function's logic didn't start in that commit though and traces back further, but bisection locally doesn't seem to be helping. In any case this has clearly been in Cargo for a long time, and I think it makes sense that this situation doesn't arise that much in practice because links libraries are pretty far apart, much less depending on each other.

In any case definitely still something that we should fix!

[alexcrichton]

Ah ok it looks like the original regression came in #5651 after a second bisection run. In any case I think I have a fix, will post soon.