Skip to content

Commit

Permalink
fix: remove panic from task graph validation (#5789)
Browse files Browse the repository at this point in the history
### Description

When porting the task graph validation I for some reason added a panic
here instead of copying the [Go
impl](https://github.com/vercel/turbo/blob/main/cli/internal/core/engine.go#L491)
which just skips processing the root node.

This PR also shored up some error formatting differences between
Rust/Go. I fixed the formatting that's directly related to the
validation, but I'll fix the global error formatting differences in a
separate PR.

### Testing Instructions

Existing integration tests that validate now pass modulo error
formatting:
```
[0 olszewski@chriss-mbp] /Users/olszewski/code/vercel/turborepo/turborepo-tests/integration $ EXPERIMENTAL_RUST_CODEPATH=true .cram_env/bin/prysk --shell=bash tests/persistent_dependencies/1-topological.t
!
--- tests/persistent_dependencies/1-topological.t
+++ tests/persistent_dependencies/1-topological.t.err
@@ -14,8 +14,8 @@
 // app-a#dev
 // └── pkg-a#dev
   $ ${TURBO} run dev
-   ERROR  run failed: error preparing engine: Invalid persistent task configuration:
+  ERROR run failed: error preparing engine: Invalid persistent task configuration:
   "pkg-a#dev" is a persistent task, "app-a#dev" cannot depend on it
-  Turbo error: error preparing engine: Invalid persistent task configuration:
+  ERROR error preparing engine: Invalid persistent task configuration:
   "pkg-a#dev" is a persistent task, "app-a#dev" cannot depend on it
   [1]

# Ran 1 tests, 0 skipped, 1 failed.
```

Closes TURBO-1248

---------

Co-authored-by: Chris Olszewski <Chris Olszewski>
  • Loading branch information
chris-olszewski committed Aug 24, 2023
1 parent 431279e commit 0c6ad99
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 2 deletions.
3 changes: 2 additions & 1 deletion crates/turborepo-lib/src/engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ impl Engine<Built> {
.node_weight(dep_index)
.expect("index comes from iterating the graph and must be present")
else {
panic!("{task_id} depends on root task node");
// No need to check the root node
continue;
};

let task_definition = self.task_definitions.get(dep_id).ok_or_else(|| {
Expand Down
7 changes: 6 additions & 1 deletion crates/turborepo-lib/src/run/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,12 @@ impl Run {

engine
.validate(&pkg_dep_graph, opts.run_opts.concurrency)
.map_err(|errors| anyhow!("Validation failed:\n{}", errors.into_iter().join("\n")))?;
.map_err(|errors| {
anyhow!(
"error preparing engine: Invalid persistent task configuration:\n{}",
errors.into_iter().join("\n")
)
})?;

if let Some(graph_opts) = opts.run_opts.graph {
match graph_opts {
Expand Down

0 comments on commit 0c6ad99

Please sign in to comment.