-
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
Build foo's dev-dependencies when running cargo test -p foo
#2621
Conversation
Fixes rust-lang#860. Transitive dev dependencies are now always resolved and included in the lock file, but are built only when testing a crate that depends directly on them.
r? @wycats (rust_highfive has picked a reviewer for you, use r? to override) |
Unfortunately this may not be so simple to fix right now. The fundamental problem currently is that dev-dependencies are not listed in lock files, so every time you run |
That is fixed by this PR. |
@@ -469,8 +470,15 @@ fn activate_deps_loop<'a>(mut cx: Context<'a>, | |||
} | |||
}; | |||
|
|||
let dev_deps = match *top_method { | |||
Method::Everything => true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Everything" now includes dev-dependencies. This affects lock file generation.
Aha! That unfortunately goes against one of the main ideas behind Cargo, which is that the |
As mentioned in the commit message, this doesn't make the lock file oscillate. All dev dependencies are always included in the lock file. |
Yeah there's two reasons that this is a bit sketchy unfortunately:
Fundamentally, however, fixing this bug will break lock files, which is something we've avoided doing up to this point. That's partly why I wanted to accelerate workspaces as it'll fix this bug nicely. |
Ah, yes, this is true.
Yes. Lock file generation uses Method::Everything, which bypasses those conditions and includes transitive dev-dependencies unconditionally. (I have tested this.)
Got it. Feel free to close this PR. Maybe after workspaces are implemented, |
Ok, in that case I'm gonna close this for now (as it's slated to be fixed with workspaces), but thanks regardless! I do think it's a good idea to reject |
#3125 is an updated version of this patch, which takes advantage of workspaces. |
Fixes #860. Transitive dev dependencies are now always resolved and included in the lock file, but are built only when testing a crate that dev-depends directly on them.
This still needs a regression test for #860, but I wanted feedback on the patch before I write the test.