Skip to content
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

[Question] What does rust-analyzer expect in rust-project.json for "cyclic" dev-dependencies? #13349

Open
googleson78 opened this issue Oct 5, 2022 · 2 comments
Labels
A-rust-project rust-project.json related issues C-support Category: support questions

Comments

@googleson78
Copy link

googleson78 commented Oct 5, 2022

Question

When we have a "cyclic" dependency of the form of X having itself as a dev-dependency (possibly transitively, e.g.
X -dev> A -> B -> X - see the reproducer below), what kind of data should I put in my rust-project.json so that it functions correctly? I know that rust-analyzer is able to handle this situation, because with a cargo based build, it works just fine - even the things inside X which really do depend on itself (or in my case depend on A).

EDIT: It seems I had a slip in my perception - it does not function correctly, i.e. in X (from the example above) I can't find out information regarding A (e.g. if I attempt to call a function from A I can't go to its definition or look at type information for it).

Context

The context is that right now, the rust-project.json generator in rules_rust can't handle a dependency of the form

rust_test(C) -> rust_library(A) -> rust_library(B) -> rust_library(C)

I'm trying to fix this, but I'm not sure what it should output in this case so that rust-analyzer can function correctly.

Reproducer

Here's a reproducer for the situation with both a Cargo and a Bazel based build.
In it, the dependency situation is as previously described,

C -dev> A -> B -> C
@googleson78 googleson78 changed the title [Question] What does rust-analyzer expect in rust-project.json for cyclic dependencies? [Question] What does rust-analyzer expect in rust-project.json for "cyclic" dev-dependencies? Oct 5, 2022
@flodiebold
Copy link
Member

RA expects there to be no cyclic dependencies. This doesn't work with Cargo projects either. The 'correct' way of modeling this would be to have two separate crates in the graph for the two configurations of C (test and non-test), but we don't do this for Cargo projects either so I don't know how well it would work.

@googleson78
Copy link
Author

googleson78 commented Oct 6, 2022

I tried your suggestion by adding another crate in the graph (suffixed with _test) and that seems to work!

What's interesting is that it does even better than the "native" cargo solution in that I can now get information even inside the, which should theoretically have a circular dep 🤔

This makes me a bit doubtful that solution works in general, as my example is quite simple, and I could just be getting lucky. But I think it's definitely worth investigating, as if it does work in general, it might be an improvement for rust-analyzer overall.

Here's the the rust-project.json for reference.

Additionally, I only left the test tag from cfg in the _test crate, but it does not appear to matter, as even if I add it back to the original c crate, it doesn't break anything.

googleson78 added a commit to dfinity-lab/rules_rust that referenced this issue Oct 7, 2022
This is a "temporary" workaround because during consolidation, we may
merge crates in such a way as to cause a cyclic dependency.

For more details, see
rust-lang/rust-analyzer#13349
bazelbuild#1589
googleson78 added a commit to dfinity-lab/rules_rust that referenced this issue Oct 28, 2022
This is a "temporary" workaround because during consolidation, we may
merge crates in such a way as to cause a cyclic dependency.

For more details, see
rust-lang/rust-analyzer#13349
bazelbuild#1589
googleson78 added a commit to dfinity-lab/rules_rust that referenced this issue Oct 28, 2022
This is a "temporary" workaround because during consolidation, we may
merge crates in such a way as to cause a cyclic dependency.

For more details, see
rust-lang/rust-analyzer#13349
bazelbuild#1589
Ali-Piccioni pushed a commit to dfinity-lab/rules_rust that referenced this issue Dec 23, 2022
This is a "temporary" workaround because during consolidation, we may
merge crates in such a way as to cause a cyclic dependency.

For more details, see
rust-lang/rust-analyzer#13349
bazelbuild#1589
Ali-Piccioni pushed a commit to Ali-Piccioni/rules_rust that referenced this issue Dec 23, 2022
This is a "temporary" workaround because during consolidation, we may
merge crates in such a way as to cause a cyclic dependency.

For more details, see
rust-lang/rust-analyzer#13349
bazelbuild#1589
@Veykril Veykril added C-support Category: support questions A-rust-project rust-project.json related issues labels Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rust-project rust-project.json related issues C-support Category: support questions
Projects
None yet
Development

No branches or pull requests

3 participants