-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Stack overflow if dev dependency has links #8966
Comments
Fascinating! I am very much looking forward to digging in on this. I'm wondering how this has gone unnoticed for so long. |
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 In any case definitely still something that we should fix! |
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. |
Fix the unit dependency graph with dev-dependency `links` This commit fixes #8966 by updating the unit generation logic to avoid generating an erroneous circular dependency between the execution of two build scripts. This has been present for Cargo in a long time since #5651 (an accidental regression), but the situation appears rare enough that we didn't get to it until now! Closes #8966
Minimal repro:
// src/lib.rs (empty)
// dep/src/lib.rs (empty)
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.
The text was updated successfully, but these errors were encountered: