internal: Document when crate cycles can occur

[Wilfred]

This is legal when there are dev-dependencies, and rust-analyzer itself even does this. This causes spurious warnings in several cases, such as generating SCIP for rust-analyzer:

$ cargo run --bin rust-analyzer --release -- scip .
2026-03-12T18:40:33.824092Z  WARN cyclic deps: cfg(Idx::<CrateBuilder>(21)) -> cfg(Idx::<CrateBuilder>(21)), alternative path: cfg(Idx::<CrateBuilder>(21))

In this case, the cfg crate enables its tt feature by depending on itself in dev-dependencies.

[ChayimFriedman2]

So we will not add the dependency, right? Can't this break code?

[Veykril]

We cannot fix this, the issue is this is a package cycle (not a crate dependency cycle). Cargo allows these, but rust-analyzers crate graph can't model these as that would duplicate a bunch of crates in the graph (for the dev-dependency and without. See #14167, instead of logging each cycle as warn we should log ones that the crate graph has a cycle or more I think as a warning still. Reason being that if people report related issues we do want to know at a glance from the logs if there were cycles as that can cause issues for r-a (we just can't do anything about it)

[ChayimFriedman2]

I believe this can indeed cause issues if cyclic crates are used e.g. in tests and we can't fix that because duplicating analysis for tests will be awful.

[Wilfred]

Can't this break code?

FWIW this is only a logging change, it's not a behaviour change.

Perhaps it would make sense to downgrade self-dependency logs to info, but treat the rest as warnings?

If not, happy to abandon this PR.

[Veykril]

instead of logging each cycle as warn we should log ones that the crate graph has a cycle or more I think as a warning still. Reason being that if people report related issues we do want to know at a glance from the logs if there were cycles as that can cause issues for r-a (we just can't do anything about it)

[Wilfred]

we should log ones that the crate graph has a cycle or more

Sorry, I'm struggling to parse this. I think you mean a cycle with length two or more?

I've updated the PR assuming I've understood correctly.

[ChayimFriedman2]

That's not what @Veykril meant as I understand them, they meant that we should only log the first cycle error, ignoring the rest. But honestly I'm not sure this is a wise idea; if the cycle is unexpected/lead to degradation in service, we might need to know any cycle that exists (just as an example, take the cycle we're yet to decipher in the Miri codebase).

[Veykril]

We could hide the remaining ones behind a debug log in that case

[ChayimFriedman2]

I feel like it's not hurting and can help as warn log, but no strong opinion.

[rustbot]

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[Wilfred]

OK, I agree that logging subtle warnings is better than risking no logs when there are real bugs. This conversation has been super useful though, so I've just changed this PR to summarise the discussion about when cycles can occur and when they're bugs or harmless.