Skip to content

Commit d47a293

Browse files
authored
refactor: replace ExecutionGraph with generic AcyclicGraph and show full cycle path in errors (#158)
## Summary - Refactors `ExecutionGraph` into a generic `AcyclicGraph<N, Ix>` with `ExecutionGraph` kept as a type alias for backward compatibility - Removes precomputed topological order from the graph struct; computes it on-the-fly via `compute_topological_order()` - Replaces `petgraph::algo::toposort`-based cycle detection with DFS + predecessor tracking that reconstructs the **full cycle path** - Cycle errors now display all participating tasks as an arrow chain (e.g. `Cycle dependency detected: pkg#task-a -> pkg#task-b -> pkg#task-a`) instead of just one task - Correctly handles graphs with multiple disconnected components - Adds 14 unit tests for `AcyclicGraph` covering DAGs, cycles, self-loops, and multi-component graphs
1 parent 3a631c8 commit d47a293

File tree

7 files changed

+439
-81
lines changed

7 files changed

+439
-81
lines changed

crates/vite_task/src/session/execute/mod.rs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ struct ExecutionContext<'a> {
5656
impl ExecutionContext<'_> {
5757
/// Execute all tasks in an execution graph in dependency order.
5858
///
59-
/// `ExecutionGraph` guarantees acyclicity at construction time and caches a
60-
/// topological order. We iterate `toposort()` in reverse to get execution order
59+
/// `ExecutionGraph` guarantees acyclicity at construction time.
60+
/// We compute a topological order and iterate in reverse to get execution order
6161
/// (dependencies before dependents).
6262
///
6363
/// The `path_prefix` tracks our position within nested execution graphs. For the
@@ -71,14 +71,15 @@ impl ExecutionContext<'_> {
7171
graph: &ExecutionGraph,
7272
path_prefix: &LeafExecutionPath,
7373
) {
74-
// `toposort()` returns nodes in topological order: for every edge A→B,
75-
// A appears before B. Since our edges mean "A depends on B", dependencies
76-
// (B) appear after their dependents (A). We iterate in reverse to get
77-
// execution order where dependencies run first.
74+
// `compute_topological_order()` returns nodes in topological order: for every
75+
// edge A→B, A appears before B. Since our edges mean "A depends on B",
76+
// dependencies (B) appear after their dependents (A). We iterate in reverse
77+
// to get execution order where dependencies run first.
7878

7979
// Execute tasks in dependency-first order. Each task may have multiple items
8080
// (from `&&`-split commands), which are executed sequentially.
81-
for &node_ix in graph.toposort().iter().rev() {
81+
let topo_order = graph.compute_topological_order();
82+
for &node_ix in topo_order.iter().rev() {
8283
let task_execution = &graph[node_ix];
8384

8485
for (item_idx, item) in task_execution.items.iter().enumerate() {

crates/vite_task_bin/tests/e2e_snapshots/fixtures/error_cycle_dependency/snapshots/cycle dependency error.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@ source: crates/vite_task_bin/tests/e2e_snapshots/main.rs
33
expression: e2e_outputs
44
---
55
[1]> vp run task-a # task-a -> task-b -> task-a cycle
6-
Error: Cycle dependency detected involving task error-cycle-dependency-test#task-b
6+
Error: Cycle dependency detected: error-cycle-dependency-test#task-a -> error-cycle-dependency-test#task-b -> error-cycle-dependency-test#task-a

crates/vite_task_plan/src/error.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -146,11 +146,11 @@ pub enum Error {
146146

147147
/// A cycle was detected in the task dependency graph during planning.
148148
///
149-
/// This is caught by `ExecutionGraph::try_from_graph`, which validates that the
150-
/// graph is a DAG. The contained `TaskDisplay` identifies one of the tasks
151-
/// involved in the cycle.
152-
#[error("Cycle dependency detected involving task {0}")]
153-
CycleDependencyDetected(TaskDisplay),
149+
/// This is caught by `AcyclicGraph::try_from_graph`, which validates that the
150+
/// graph is a DAG. The contained path lists all tasks forming the cycle
151+
/// as a closed loop (the first and last elements are the same).
152+
#[error("Cycle dependency detected: {}", _0.iter().map(std::string::ToString::to_string).collect::<Vec<_>>().join(" -> "))]
153+
CycleDependencyDetected(Vec<TaskDisplay>),
154154
}
155155

156156
impl Error {

0 commit comments

Comments
 (0)