diff --git a/crates/turborepo-lib/src/engine/builder.rs b/crates/turborepo-lib/src/engine/builder.rs index 5ba325c7c567f..1157c7e4714e3 100644 --- a/crates/turborepo-lib/src/engine/builder.rs +++ b/crates/turborepo-lib/src/engine/builder.rs @@ -259,7 +259,7 @@ impl<'a> EngineBuilder<'a> { .task_id() .unwrap_or_else(|| TaskId::new(workspace.as_ref(), task.task())); - if Self::has_task_definition(turbo_json_loader, workspace, task, &task_id)? { + if Self::has_task_definition_in_run(turbo_json_loader, workspace, task, &task_id)? { missing_tasks.remove(task.as_inner()); // Even if a task definition was found, we _only_ want to add it as an entry @@ -276,6 +276,33 @@ impl<'a> EngineBuilder<'a> { } } + { + // We can encounter IO errors trying to load turbo.jsons which prevents using + // `retain` in the standard way. Instead we store the possible error + // outside of the loop and short circuit checks if we've encountered an error. + let mut error = None; + missing_tasks.retain(|task_name, _| { + // If we've already encountered an error skip checking the rest. + if error.is_some() { + return true; + } + match Self::has_task_definition_in_repo( + turbo_json_loader, + self.package_graph, + task_name, + ) { + Ok(has_defn) => !has_defn, + Err(e) => { + error.get_or_insert(e); + true + } + } + }); + if let Some(err) = error { + return Err(err); + } + } + if !missing_tasks.is_empty() { let missing_pkgs: HashMap<_, _> = missing_tasks .iter() @@ -463,8 +490,25 @@ impl<'a> EngineBuilder<'a> { } // Helper methods used when building the engine + /// Checks if there's a task definition somewhere in the repository + fn has_task_definition_in_repo( + loader: &TurboJsonLoader, + package_graph: &PackageGraph, + task_name: &TaskName<'static>, + ) -> Result { + for (package, _) in package_graph.packages() { + let task_id = task_name + .task_id() + .unwrap_or_else(|| TaskId::new(package.as_str(), task_name.task())); + if Self::has_task_definition_in_run(loader, package, task_name, &task_id)? { + return Ok(true); + } + } + Ok(false) + } - fn has_task_definition( + /// Checks if there's a task definition in the current run + fn has_task_definition_in_run( loader: &TurboJsonLoader, workspace: &PackageName, task_name: &TaskName<'static>, @@ -485,7 +529,12 @@ impl<'a> EngineBuilder<'a> { let Some(turbo_json) = turbo_json else { // If there was no turbo.json in the workspace, fallback to the root turbo.json - return Self::has_task_definition(loader, &PackageName::Root, task_name, task_id); + return Self::has_task_definition_in_run( + loader, + &PackageName::Root, + task_name, + task_id, + ); }; let task_id_as_name = task_id.as_task_name(); @@ -502,7 +551,7 @@ impl<'a> EngineBuilder<'a> { { Ok(true) } else if !matches!(workspace, PackageName::Root) { - Self::has_task_definition(loader, &PackageName::Root, task_name, task_id) + Self::has_task_definition_in_run(loader, &PackageName::Root, task_name, task_id) } else { Ok(false) } @@ -788,7 +837,8 @@ mod test { let task_id = TaskId::try_from(task_id).unwrap(); let has_def = - EngineBuilder::has_task_definition(&loader, &workspace, &task_name, &task_id).unwrap(); + EngineBuilder::has_task_definition_in_run(&loader, &workspace, &task_name, &task_id) + .unwrap(); assert_eq!(has_def, expected); } @@ -1460,14 +1510,9 @@ mod test { let engine = EngineBuilder::new(&repo_root, &package_graph, &loader, false) .with_tasks(vec![Spanned::new(TaskName::from("app1#another"))]) .with_workspaces(vec![PackageName::from("libA")]) - .build(); - assert!(engine.is_err()); - let report = miette::Report::new(engine.unwrap_err()); - let mut msg = String::new(); - miette::JSONReportHandler::new() - .render_report(&mut msg, report.as_ref()) + .build() .unwrap(); - assert_json_snapshot!(msg); + assert_eq!(engine.tasks().collect::>(), &[&TaskNode::Root]); } #[test] @@ -1505,4 +1550,49 @@ mod test { .unwrap(); assert_snapshot!(msg); } + + #[test] + fn test_filter_removes_task_def() { + let repo_root_dir = TempDir::with_prefix("repo").unwrap(); + let repo_root = AbsoluteSystemPathBuf::new(repo_root_dir.path().to_str().unwrap()).unwrap(); + let package_graph = mock_package_graph( + &repo_root, + package_jsons! { + repo_root, + "app1" => ["libA"], + "libA" => [] + }, + ); + let turbo_jsons = vec![ + ( + PackageName::Root, + turbo_json(json!({ + "tasks": { + "build": { "dependsOn": ["^build"] }, + } + })), + ), + ( + PackageName::from("app1"), + turbo_json(json!({ + "tasks": { + "app1-only": {}, + } + })), + ), + ] + .into_iter() + .collect(); + let loader = TurboJsonLoader::noop(turbo_jsons); + let engine = EngineBuilder::new(&repo_root, &package_graph, &loader, false) + .with_tasks(vec![Spanned::new(TaskName::from("app1-only"))]) + .with_workspaces(vec![PackageName::from("libA")]) + .build() + .unwrap(); + assert_eq!( + engine.tasks().collect::>(), + &[&TaskNode::Root], + "only the root task node should be present" + ); + } }