Skip to content

Commit

Permalink
Auto merge of #16871 - Veykril:dev-dependency-cycles, r=Veykril
Browse files Browse the repository at this point in the history
fix: Skip problematic cyclic dev-dependencies

Implements a workaround for #14167, notably it does not implement the ideas surfaced in the issue, but takes a simpler to implement approach (and one that is more consistent).

Effectively, all this does is discard dev-dependency edges that go from a workspace library target to another workspace library target. This means, using a dev-dependency to another workspace member inside unit tests will always fail to resolve for r-a now, (instead of being order dependent and causing problems elsewhere) while things will work out fine in integration tests, benches, examples etc. This effectively acknowledges package cycles to be okay, but crate graph cycles to be invalid:

Quoting #14167 (comment)
> Though, if you have “package cycle” in integration tests, you’d have “crate cycle” in unit test.

We disallow the latter here, while continuing to support the former

(What's missing is to supress diagnostics for such unit tests, though not doing so might be a good deterrent, making devs avoid the pattern altogether)
  • Loading branch information
bors committed Mar 18, 2024
2 parents d3eeadc + 76fb73a commit 59b9cc1
Showing 1 changed file with 18 additions and 0 deletions.
18 changes: 18 additions & 0 deletions crates/project-model/src/workspace.rs
Expand Up @@ -1091,6 +1091,24 @@ fn cargo_to_crate_graph(
continue;
}

// If the dependency is a dev-dependency with both crates being member libraries of
// the workspace we discard the edge. The reason can be read up on in
// https://github.com/rust-lang/rust-analyzer/issues/14167
// but in short, such an edge usually causes some form of cycle in the crate graph
// wrt to unit tests. Something we cannot reasonable support.
if dep.kind == DepKind::Dev
&& matches!(kind, TargetKind::Lib { .. })
&& cargo[dep.pkg].is_member
&& cargo[pkg].is_member
{
tracing::warn!(
"Discarding dev-dependency edge from library target `{}` to library target `{}` to prevent potential cycles",
cargo[dep.pkg].name,
cargo[pkg].name
);
continue;
}

add_dep(crate_graph, from, name.clone(), to)
}
}
Expand Down

0 comments on commit 59b9cc1

Please sign in to comment.