-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Only add dev dependencies to the dependency list of dev targets #14166
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
Conversation
Fixes rust-lang#9574 Fixes rust-lang#11410 `rust-analyzer` already counts all of the different targets of a crate as different crates, so it was pretty easy to just exclude dev dependencies from the dependency lists of the targets other than test, dev and bench when ingesting `cargo metadata`'s output. A similar thing is already done for build dependencies. It's possible there might be something I'm missing here, since this is quite a long-standing issue which I expected to be much more complicated to solve; but, this seems to work. The circular dependency errors that `rust-analyzer` used to throw on `wasm-bindgen` went away after switching to this patched version.
I don't think this works, because we analyze library targets with their tests where the test code can rely on dev-dependencies. A quick check on the impact of the analysis stats from this also seems to show this:
This PR:
Note the increase of unknown types ( |
I closed the linked issues (and some others) in favor of #14167, to have a more concrete tracking issue about this. In general in one of the related issues the proper fix was described which is now noted down in that issue. |
I also had to change the logic for whether they get put into test mode or not: * Bin crates now only get put into test mode if they're a part of the current workspace, because otherwise dev dependencies don't work * this didn't actually change anything because bin crates outside the current workspace are already ignored but idk it's better to make it explicit? * The version of a lib crate with test mode enabled is now separate from the normal version of the lib crate, since it's allowed to depend on the normal version as a dev dependency. (as suggested in rust-lang#2414 (comment)) * The test-mode version of the lib crate is also only created if it's a part of the current workspace.
I tried to follow the suggestion in #14167 to treat lib crates compiled for testing separately from their normal versions, but it doesn't work very well, as rust-analyzer doesn't seem to be designed to have the same file be part of multiple crates right now. Sections with I don't think I know enough about |
Thanks for having looked into it though, it's still appreciated :) |
Fixes #9574
Fixes #11410
rust-analyzer
already counts all of the different targets of a crate as different crates, so it was pretty easy to just exclude dev dependencies from the dependency lists of targets other than test, dev and bench when ingestingcargo metadata
's output. A similar thing is already done for build dependencies.It's possible there might be something I'm missing here, since this is quite a long-standing issue which I expected to be much more complicated to solve; but, this seems to work. The circular dependency errors that
rust-analyzer
used to throw onwasm-bindgen
went away after switching to this patched version.