Skip to content

Commit 4d852f0

Browse files
fix(engine): no longer error if provided task is omitted by filter (#10051)
### Description Fixes #9619 This PR makes it so we no longer error if the filter excludes a task provided by the user. This is especially helpful when filters are not static such as `--affected` or SCM based filters. ### Testing Instructions Updated unit tests. Before ``` # Task only defined on `@turbo/types` [0 olszewski@macbookpro] /Users/olszewski/code/vercel/turborepo $ turbo copy-schema --filter=cli WARNING No locally installed `turbo` found. Using version: 2.4.4. turbo 2.4.4 × Missing tasks in project ╰─▶ × Could not find task `copy-schema` in project # Task only defined at root [1 olszewski@macbookpro] /Users/olszewski/code/vercel/turborepo $ turbo build check:toml --filter='!cli' WARNING No locally installed `turbo` found. Using version: 2.4.4. turbo 2.4.4 × Missing tasks in project ╰─▶ × Could not find task `check:toml` in project ``` After ``` # Task only defined on `@turbo/types` [0 olszewski@macbookpro] /Users/olszewski/code/vercel/turborepo $ turbo_dev copy-schema --filter=cli WARNING No locally installed `turbo` found. Using version: 2.4.4. turbo 2.4.4 • Packages in scope: cli • Running copy-schema in 1 packages • Remote caching enabled No tasks were executed as part of this run. Tasks: 0 successful, 0 total Cached: 0 cached, 0 total Time: 113ms # Task only defined at root [0 olszewski@macbookpro] /Users/olszewski/code/vercel/turborepo $ turbo_dev check:toml --filter='!cli' WARNING No locally installed `turbo` found. Using version: 2.4.4. turbo 2.4.4 • Packages in scope: @turbo-internal/top-issues-gh-action, @turbo/benchmark, @turbo/codemod, @turbo/eslint-config, @turbo/exe-stub, @turbo/gen, @turbo/releaser, @turbo/telemetry, @turbo/test-utils, @turbo/tsconfig, @turbo/types, @turbo/utils, @turbo/workspaces, @turborepo-examples-tests/basic-npm, @turborepo-examples-tests/basic-pnpm, @turborepo-examples-tests/basic-yarn, @turborepo-examples-tests/kitchen-sink-npm, @turborepo-examples-tests/kitchen-sink-pnpm, @turborepo-examples-tests/kitchen-sink-yarn, @turborepo-examples-tests/non-monorepo-npm, @turborepo-examples-tests/non-monorepo-pnpm, @turborepo-examples-tests/non-monorepo-yarn, @turborepo-examples-tests/with-svelte-npm, @turborepo-examples-tests/with-svelte-pnpm, @turborepo-examples-tests/with-svelte-yarn, @turborepo-examples-tests/with-tailwind-npm, @turborepo-examples-tests/with-tailwind-pnpm, @turborepo-examples-tests/with-tailwind-yarn, cargo-sweep-action, create-turbo, eslint-config-turbo, eslint-plugin-turbo, prysk, turbo-ignore, turbo-vsc, turborepo-examples, turborepo-repository, turborepo-tests-helpers, turborepo-tests-integration • Running check:toml in 39 packages • Remote caching enabled No tasks were executed as part of this run. Tasks: 0 successful, 0 total Cached: 0 cached, 0 total Time: 123ms ```
1 parent e85b0d3 commit 4d852f0

File tree

1 file changed

+102
-12
lines changed

1 file changed

+102
-12
lines changed

crates/turborepo-lib/src/engine/builder.rs

Lines changed: 102 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ impl<'a> EngineBuilder<'a> {
259259
.task_id()
260260
.unwrap_or_else(|| TaskId::new(workspace.as_ref(), task.task()));
261261

262-
if Self::has_task_definition(turbo_json_loader, workspace, task, &task_id)? {
262+
if Self::has_task_definition_in_run(turbo_json_loader, workspace, task, &task_id)? {
263263
missing_tasks.remove(task.as_inner());
264264

265265
// Even if a task definition was found, we _only_ want to add it as an entry
@@ -276,6 +276,33 @@ impl<'a> EngineBuilder<'a> {
276276
}
277277
}
278278

279+
{
280+
// We can encounter IO errors trying to load turbo.jsons which prevents using
281+
// `retain` in the standard way. Instead we store the possible error
282+
// outside of the loop and short circuit checks if we've encountered an error.
283+
let mut error = None;
284+
missing_tasks.retain(|task_name, _| {
285+
// If we've already encountered an error skip checking the rest.
286+
if error.is_some() {
287+
return true;
288+
}
289+
match Self::has_task_definition_in_repo(
290+
turbo_json_loader,
291+
self.package_graph,
292+
task_name,
293+
) {
294+
Ok(has_defn) => !has_defn,
295+
Err(e) => {
296+
error.get_or_insert(e);
297+
true
298+
}
299+
}
300+
});
301+
if let Some(err) = error {
302+
return Err(err);
303+
}
304+
}
305+
279306
if !missing_tasks.is_empty() {
280307
let missing_pkgs: HashMap<_, _> = missing_tasks
281308
.iter()
@@ -463,8 +490,25 @@ impl<'a> EngineBuilder<'a> {
463490
}
464491

465492
// Helper methods used when building the engine
493+
/// Checks if there's a task definition somewhere in the repository
494+
fn has_task_definition_in_repo(
495+
loader: &TurboJsonLoader,
496+
package_graph: &PackageGraph,
497+
task_name: &TaskName<'static>,
498+
) -> Result<bool, Error> {
499+
for (package, _) in package_graph.packages() {
500+
let task_id = task_name
501+
.task_id()
502+
.unwrap_or_else(|| TaskId::new(package.as_str(), task_name.task()));
503+
if Self::has_task_definition_in_run(loader, package, task_name, &task_id)? {
504+
return Ok(true);
505+
}
506+
}
507+
Ok(false)
508+
}
466509

467-
fn has_task_definition(
510+
/// Checks if there's a task definition in the current run
511+
fn has_task_definition_in_run(
468512
loader: &TurboJsonLoader,
469513
workspace: &PackageName,
470514
task_name: &TaskName<'static>,
@@ -485,7 +529,12 @@ impl<'a> EngineBuilder<'a> {
485529

486530
let Some(turbo_json) = turbo_json else {
487531
// If there was no turbo.json in the workspace, fallback to the root turbo.json
488-
return Self::has_task_definition(loader, &PackageName::Root, task_name, task_id);
532+
return Self::has_task_definition_in_run(
533+
loader,
534+
&PackageName::Root,
535+
task_name,
536+
task_id,
537+
);
489538
};
490539

491540
let task_id_as_name = task_id.as_task_name();
@@ -502,7 +551,7 @@ impl<'a> EngineBuilder<'a> {
502551
{
503552
Ok(true)
504553
} else if !matches!(workspace, PackageName::Root) {
505-
Self::has_task_definition(loader, &PackageName::Root, task_name, task_id)
554+
Self::has_task_definition_in_run(loader, &PackageName::Root, task_name, task_id)
506555
} else {
507556
Ok(false)
508557
}
@@ -788,7 +837,8 @@ mod test {
788837
let task_id = TaskId::try_from(task_id).unwrap();
789838

790839
let has_def =
791-
EngineBuilder::has_task_definition(&loader, &workspace, &task_name, &task_id).unwrap();
840+
EngineBuilder::has_task_definition_in_run(&loader, &workspace, &task_name, &task_id)
841+
.unwrap();
792842
assert_eq!(has_def, expected);
793843
}
794844

@@ -1460,14 +1510,9 @@ mod test {
14601510
let engine = EngineBuilder::new(&repo_root, &package_graph, &loader, false)
14611511
.with_tasks(vec![Spanned::new(TaskName::from("app1#another"))])
14621512
.with_workspaces(vec![PackageName::from("libA")])
1463-
.build();
1464-
assert!(engine.is_err());
1465-
let report = miette::Report::new(engine.unwrap_err());
1466-
let mut msg = String::new();
1467-
miette::JSONReportHandler::new()
1468-
.render_report(&mut msg, report.as_ref())
1513+
.build()
14691514
.unwrap();
1470-
assert_json_snapshot!(msg);
1515+
assert_eq!(engine.tasks().collect::<Vec<_>>(), &[&TaskNode::Root]);
14711516
}
14721517

14731518
#[test]
@@ -1505,4 +1550,49 @@ mod test {
15051550
.unwrap();
15061551
assert_snapshot!(msg);
15071552
}
1553+
1554+
#[test]
1555+
fn test_filter_removes_task_def() {
1556+
let repo_root_dir = TempDir::with_prefix("repo").unwrap();
1557+
let repo_root = AbsoluteSystemPathBuf::new(repo_root_dir.path().to_str().unwrap()).unwrap();
1558+
let package_graph = mock_package_graph(
1559+
&repo_root,
1560+
package_jsons! {
1561+
repo_root,
1562+
"app1" => ["libA"],
1563+
"libA" => []
1564+
},
1565+
);
1566+
let turbo_jsons = vec![
1567+
(
1568+
PackageName::Root,
1569+
turbo_json(json!({
1570+
"tasks": {
1571+
"build": { "dependsOn": ["^build"] },
1572+
}
1573+
})),
1574+
),
1575+
(
1576+
PackageName::from("app1"),
1577+
turbo_json(json!({
1578+
"tasks": {
1579+
"app1-only": {},
1580+
}
1581+
})),
1582+
),
1583+
]
1584+
.into_iter()
1585+
.collect();
1586+
let loader = TurboJsonLoader::noop(turbo_jsons);
1587+
let engine = EngineBuilder::new(&repo_root, &package_graph, &loader, false)
1588+
.with_tasks(vec![Spanned::new(TaskName::from("app1-only"))])
1589+
.with_workspaces(vec![PackageName::from("libA")])
1590+
.build()
1591+
.unwrap();
1592+
assert_eq!(
1593+
engine.tasks().collect::<Vec<_>>(),
1594+
&[&TaskNode::Root],
1595+
"only the root task node should be present"
1596+
);
1597+
}
15081598
}

0 commit comments

Comments
 (0)