bazelbuild / bazelbuild/rules_rust
[Bug/Question] Can we amend rust-analyzer generation crate consolidation to improve support for "cyclic" dependencies
- Dominant language
- Starlark
- Stars
- 843
- Forks
- 651
- Avg merge
- 2d 18h
- Merged PRs (30d)
- 15
Description
## Question
When we do crate consolidation here
https://github.com/bazelbuild/rules_rust/blob/d288ed634f9e149ceb0c81bda6cd93a810cf7e64/tools/rust_analyzer/aquery.rs#L163
on a build graph like so (`A`, `B`, `C` are crates)
```
rust_test(C) -> rust_library(A) -> rust_library(B) -> rust_library(C)
```
`rust_test(C)` and `rust_library(C)` get their dependencies merged, and the result of that is that we can't actually generate `rust-project.json`, as now we have a real dependency cycle.
Here's a reproducer - https://github.com/googleson78/rust-analyzer-bazel-gen-repro
It turns out that having both of them listed in `rust-project.json` is not a big issue and somewhat works (not only in my simple reproducer, but also in the bigger codebase I mention in the context below):
https://github.com/googleson78/rust-analyzer-bazel-gen-repro/blob/main/somewhat-working-rust-project.json
Can we somehow loosen consolidation so that we avoid this situation?
The solution I can think of that sounds the best is to add separate entries for `rust_test(C)` and `rust_library(C)` iff merging them would cause a circular dependency. This might not be that easy to implement however, it would require a lot of computing possibly.
The other extreme is to stop doing crate consolidation entirely. I couldn't find a reason for why it's done in the first place, so I've come to ask - can someone clarify this?
## Context
In the [Dfinity code base](https://github.com/dfinity/ic/tree/master/rs), the dependency situation from above happens, and prevents us from generating a `rust-project.json`
By replacing the implementation of `consolidate_crate_specs` with
```rust
fn consolidate_crate_specs(crate_specs: Vec) -> anyhow::Result> {
Ok(BTreeSet::from_iter(crate_specs))
}
```
generation now works, and the result appears to be at least partially usable - I tried it on a few files (including some of those which form a "cycle") and IDE features appear to work.
## Additional remark
By doing this generation we can even do better than rust-analyzer's builtin Cargo support!
We seemingly somehow support the cyclic dependency if we generate duplicate entries in `rust-project.json` - goto def works in all cases in the reproducer project, even from the "problematic" test in C, for which the builtin cargo based rust-analyzer support fails. See https://github.com/rust-lang/rust-analyzer/issues/13349 for more details.
Contributor guide
Assessment
This issue has not been assessed yet.