diff --git a/CHANGELOG.md b/CHANGELOG.md index 3eef1d6f..73f5ac6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Changelog +- **Changed** Arguments passed after a task name (e.g. `vp run test some-filter`) are now forwarded only to that task. Tasks pulled in via `dependsOn` no longer receive them ([#324](https://github.com/voidzero-dev/vite-task/issues/324)) - **Fixed** Windows file access tracking no longer panics when a task touches malformed paths that cannot be represented as workspace-relative inputs ([#330](https://github.com/voidzero-dev/vite-task/pull/330)) - **Fixed** `vp run --cache` now supports running without a task specifier and opens the interactive task selector, matching bare `vp run` behavior ([#312](https://github.com/voidzero-dev/vite-task/pull/313)) - **Fixed** Ctrl-C now prevents future tasks from being scheduled and prevents caching of in-flight task results ([#309](https://github.com/voidzero-dev/vite-task/pull/309)) diff --git a/crates/vite_task_graph/src/query/mod.rs b/crates/vite_task_graph/src/query/mod.rs index 7a090315..39cbfb68 100644 --- a/crates/vite_task_graph/src/query/mod.rs +++ b/crates/vite_task_graph/src/query/mod.rs @@ -26,9 +26,26 @@ use crate::{IndexedTaskGraph, TaskDependencyType, TaskId, TaskNodeIndex}; /// A task execution graph queried from a `TaskQuery`. /// -/// Nodes are `TaskNodeIndex` values into the full `TaskGraph`. +/// Nodes in `graph` are `TaskNodeIndex` values into the full `TaskGraph`. /// Edges represent the final dependency relationships between tasks (no weights). -pub type TaskExecutionGraph = DiGraphMap; +/// +/// `requested` is the subset of nodes the user typed on the CLI — i.e. the +/// nodes added by `map_subgraph_to_tasks` (stage 2), not the ones reached +/// only via `dependsOn` expansion in `IndexedTaskGraph::add_dependencies` (stage 3). +/// +/// For example, given `test` with `dependsOn: ["build"]` and the command +/// `vp run test some-filter`: +/// +/// - `graph` contains both `test` and `build` with an edge between them. +/// - `requested` contains only `test`. +/// +/// The planner uses this distinction to forward `some-filter` to `test` +/// while running `build` with no extra args. +#[derive(Debug, Default, Clone)] +pub struct TaskExecutionGraph { + pub graph: DiGraphMap, + pub requested: FxHashSet, +} /// A query for which tasks to run. /// @@ -167,13 +184,17 @@ impl IndexedTaskGraph { // Map remaining nodes and their edges to task nodes. // Every node still in `subgraph` is in `pkg_to_task`; the index operator // panics on a missing key — that would be a bug in the loop above. + // + // All nodes added here are explicitly-requested tasks, so they are + // inserted into both the inner graph and the `requested` set. for &task_idx in pkg_to_task.values() { - execution_graph.add_node(task_idx); + execution_graph.graph.add_node(task_idx); + execution_graph.requested.insert(task_idx); } for (src, dst, ()) in subgraph.all_edges() { let st = pkg_to_task[&src]; let dt = pkg_to_task[&dst]; - execution_graph.add_edge(st, dt, ()); + execution_graph.graph.add_edge(st, dt, ()); } } @@ -187,9 +208,13 @@ impl IndexedTaskGraph { execution_graph: &mut TaskExecutionGraph, mut filter_edge: impl FnMut(TaskDependencyType) -> bool, ) { - let mut frontier: FxHashSet = execution_graph.nodes().collect(); + let mut frontier: FxHashSet = execution_graph.graph.nodes().collect(); // Continue until no new nodes are added to the frontier. + // + // Nodes added here are dependency-only tasks and must NOT be marked as + // `requested` — the planner uses that distinction to decide whether to + // forward CLI extra args to a task. while !frontier.is_empty() { let mut next_frontier = FxHashSet::::default(); @@ -198,8 +223,8 @@ impl IndexedTaskGraph { let to_node = edge_ref.target(); let dep_type = *edge_ref.weight(); if filter_edge(dep_type) { - let is_new = !execution_graph.contains_node(to_node); - execution_graph.add_edge(from_node, to_node, ()); + let is_new = !execution_graph.graph.contains_node(to_node); + execution_graph.graph.add_edge(from_node, to_node, ()); if is_new { next_frontier.insert(to_node); } diff --git a/crates/vite_task_plan/src/plan.rs b/crates/vite_task_plan/src/plan.rs index e9436680..8e657d84 100644 --- a/crates/vite_task_plan/src/plan.rs +++ b/crates/vite_task_plan/src/plan.rs @@ -653,6 +653,7 @@ fn plan_spawn_execution( /// `vp run build` produces a different query than the script's `vp run -r build`, /// so the skip rule doesn't fire, but the prune rule catches root in the result). /// Like the skip rule, extra args don't affect this — only the `TaskQuery` matters. +#[expect(clippy::too_many_lines, reason = "sequential planning steps are clearer in one function")] pub async fn plan_query_request( query: Arc, plan_options: PlanOptions, @@ -696,7 +697,15 @@ pub async fn plan_query_request( let parallel = plan_options.parallel; - context.set_extra_args(plan_options.extra_args); + // Extra args are applied per-task below, not globally on the context. + // Tasks explicitly requested by the query receive `extra_args`; tasks + // only reached via `dependsOn` expansion receive an empty slice so that + // caller-specific CLI args don't pollute dependency tasks. + // See https://github.com/voidzero-dev/vite-task/issues/324. + let extra_args = plan_options.extra_args; + // Allocated once and shared across every dep-only task's context below, + // instead of calling `Arc::new([])` inside the per-task loop. + let empty_extra_args: Arc<[Str]> = Arc::from([]); context.set_parent_query(Arc::clone(&query)); // Query matching tasks from the task graph. @@ -715,35 +724,43 @@ pub async fn plan_query_request( // This handles cases like root `"build": "vp run build"` — the root's build // task is in the result but expanding it would recurse, so we remove it and // reconnect its predecessors directly to its successors. - let pruned_task = context.expanding_task().filter(|t| task_node_index_graph.contains_node(*t)); + let pruned_task = + context.expanding_task().filter(|t| task_node_index_graph.graph.contains_node(*t)); let mut execution_node_indices_by_task_index = FxHashMap::::with_capacity_and_hasher( - task_node_index_graph.node_count(), + task_node_index_graph.graph.node_count(), rustc_hash::FxBuildHasher, ); // Build the inner DiGraph first, then validate acyclicity at the end. let mut inner_graph = InnerExecutionGraph::with_capacity( - task_node_index_graph.node_count(), - task_node_index_graph.edge_count(), + task_node_index_graph.graph.node_count(), + task_node_index_graph.graph.edge_count(), ); // Plan each task node as execution nodes, skipping the pruned task - for task_index in task_node_index_graph.nodes() { + for task_index in task_node_index_graph.graph.nodes() { if Some(task_index) == pruned_task { continue; } - let task_execution = plan_task_as_execution_node(task_index, context.duplicate(), true) - .boxed_local() - .await?; + let mut task_context = context.duplicate(); + // Only the explicitly requested tasks receive CLI extra args. + // Dep-only tasks (pulled in via `dependsOn`) run with empty extras. + if task_node_index_graph.requested.contains(&task_index) { + task_context.set_extra_args(Arc::clone(&extra_args)); + } else { + task_context.set_extra_args(Arc::clone(&empty_extra_args)); + } + let task_execution = + plan_task_as_execution_node(task_index, task_context, true).boxed_local().await?; execution_node_indices_by_task_index .insert(task_index, inner_graph.add_node(task_execution)); } // Add edges between execution nodes according to task dependencies, // skipping edges involving the pruned task. - for (from_task_index, to_task_index, ()) in task_node_index_graph.all_edges() { + for (from_task_index, to_task_index, ()) in task_node_index_graph.graph.all_edges() { if Some(from_task_index) == pruned_task || Some(to_task_index) == pruned_task { continue; } @@ -757,9 +774,9 @@ pub async fn plan_query_request( // Reconnect through the pruned node: wire each predecessor directly to each successor. if let Some(pruned) = pruned_task { let preds: Vec<_> = - task_node_index_graph.neighbors_directed(pruned, Direction::Incoming).collect(); + task_node_index_graph.graph.neighbors_directed(pruned, Direction::Incoming).collect(); let succs: Vec<_> = - task_node_index_graph.neighbors_directed(pruned, Direction::Outgoing).collect(); + task_node_index_graph.graph.neighbors_directed(pruned, Direction::Outgoing).collect(); for &pred in &preds { for &succ in &succs { if let (Some(&pe), Some(&se)) = ( diff --git a/crates/vite_task_plan/tests/plan_snapshots/fixtures/extra-args-not-forwarded-to-depends-on/package.json b/crates/vite_task_plan/tests/plan_snapshots/fixtures/extra-args-not-forwarded-to-depends-on/package.json new file mode 100644 index 00000000..7f0805cc --- /dev/null +++ b/crates/vite_task_plan/tests/plan_snapshots/fixtures/extra-args-not-forwarded-to-depends-on/package.json @@ -0,0 +1,3 @@ +{ + "name": "@test/extra-args-not-forwarded-to-depends-on" +} diff --git a/crates/vite_task_plan/tests/plan_snapshots/fixtures/extra-args-not-forwarded-to-depends-on/snapshots.toml b/crates/vite_task_plan/tests/plan_snapshots/fixtures/extra-args-not-forwarded-to-depends-on/snapshots.toml new file mode 100644 index 00000000..f705c2d9 --- /dev/null +++ b/crates/vite_task_plan/tests/plan_snapshots/fixtures/extra-args-not-forwarded-to-depends-on/snapshots.toml @@ -0,0 +1,7 @@ +# Tests that extra args (`vt run test some-filter`) are forwarded only to the +# explicitly requested task (`test`), not to `dependsOn` tasks (`build`). +# https://github.com/voidzero-dev/vite-task/issues/324 + +[[plan]] +name = "extra args only reach requested task" +args = ["run", "test", "some-filter"] diff --git a/crates/vite_task_plan/tests/plan_snapshots/fixtures/extra-args-not-forwarded-to-depends-on/snapshots/query - extra args only reach requested task.snap b/crates/vite_task_plan/tests/plan_snapshots/fixtures/extra-args-not-forwarded-to-depends-on/snapshots/query - extra args only reach requested task.snap new file mode 100644 index 00000000..55f40a0d --- /dev/null +++ b/crates/vite_task_plan/tests/plan_snapshots/fixtures/extra-args-not-forwarded-to-depends-on/snapshots/query - extra args only reach requested task.snap @@ -0,0 +1,182 @@ +--- +source: crates/vite_task_plan/tests/plan_snapshots/main.rs +expression: "&plan_json" +info: + args: + - run + - test + - some-filter +input_file: crates/vite_task_plan/tests/plan_snapshots/fixtures/extra-args-not-forwarded-to-depends-on +--- +{ + "graph": [ + { + "key": [ + "/", + "build" + ], + "node": { + "task_display": { + "package_name": "@test/extra-args-not-forwarded-to-depends-on", + "task_name": "build", + "package_path": "/" + }, + "items": [ + { + "execution_item_display": { + "task_display": { + "package_name": "@test/extra-args-not-forwarded-to-depends-on", + "task_name": "build", + "package_path": "/" + }, + "command": "vt tool print build", + "and_item_index": null, + "cwd": "/" + }, + "kind": { + "Leaf": { + "Spawn": { + "cache_metadata": { + "spawn_fingerprint": { + "cwd": "", + "program_fingerprint": { + "OutsideWorkspace": { + "program_name": "vtt" + } + }, + "args": [ + "print", + "build" + ], + "env_fingerprints": { + "fingerprinted_envs": {}, + "untracked_env_config": [ + "" + ] + } + }, + "execution_cache_key": { + "UserTask": { + "task_name": "build", + "and_item_index": 0, + "extra_args": [], + "package_path": "" + } + }, + "input_config": { + "includes_auto": true, + "positive_globs": [], + "negative_globs": [] + } + }, + "spawn_command": { + "program_path": "/vtt", + "args": [ + "print", + "build" + ], + "all_envs": { + "NO_COLOR": "1", + "PATH": "/node_modules/.bin:" + }, + "cwd": "/" + } + } + } + } + } + ] + }, + "neighbors": [] + }, + { + "key": [ + "/", + "test" + ], + "node": { + "task_display": { + "package_name": "@test/extra-args-not-forwarded-to-depends-on", + "task_name": "test", + "package_path": "/" + }, + "items": [ + { + "execution_item_display": { + "task_display": { + "package_name": "@test/extra-args-not-forwarded-to-depends-on", + "task_name": "test", + "package_path": "/" + }, + "command": "vt tool print test some-filter", + "and_item_index": null, + "cwd": "/" + }, + "kind": { + "Leaf": { + "Spawn": { + "cache_metadata": { + "spawn_fingerprint": { + "cwd": "", + "program_fingerprint": { + "OutsideWorkspace": { + "program_name": "vtt" + } + }, + "args": [ + "print", + "test", + "some-filter" + ], + "env_fingerprints": { + "fingerprinted_envs": {}, + "untracked_env_config": [ + "" + ] + } + }, + "execution_cache_key": { + "UserTask": { + "task_name": "test", + "and_item_index": 0, + "extra_args": [ + "some-filter" + ], + "package_path": "" + } + }, + "input_config": { + "includes_auto": true, + "positive_globs": [], + "negative_globs": [] + } + }, + "spawn_command": { + "program_path": "/vtt", + "args": [ + "print", + "test", + "some-filter" + ], + "all_envs": { + "NO_COLOR": "1", + "PATH": "/node_modules/.bin:" + }, + "cwd": "/" + } + } + } + } + } + ] + }, + "neighbors": [ + [ + "/", + "build" + ] + ] + } + ], + "concurrency_limit": 4 +} diff --git a/crates/vite_task_plan/tests/plan_snapshots/fixtures/extra-args-not-forwarded-to-depends-on/snapshots/task graph.snap b/crates/vite_task_plan/tests/plan_snapshots/fixtures/extra-args-not-forwarded-to-depends-on/snapshots/task graph.snap new file mode 100644 index 00000000..043c2812 --- /dev/null +++ b/crates/vite_task_plan/tests/plan_snapshots/fixtures/extra-args-not-forwarded-to-depends-on/snapshots/task graph.snap @@ -0,0 +1,80 @@ +--- +source: crates/vite_task_plan/tests/plan_snapshots/main.rs +expression: task_graph_json +input_file: crates/vite_task_plan/tests/plan_snapshots/fixtures/extra-args-not-forwarded-to-depends-on +--- +[ + { + "key": [ + "/", + "build" + ], + "node": { + "task_display": { + "package_name": "@test/extra-args-not-forwarded-to-depends-on", + "task_name": "build", + "package_path": "/" + }, + "resolved_config": { + "command": "vt tool print build", + "resolved_options": { + "cwd": "/", + "cache_config": { + "env_config": { + "fingerprinted_envs": [], + "untracked_env": [ + "" + ] + }, + "input_config": { + "includes_auto": true, + "positive_globs": [], + "negative_globs": [] + } + } + } + }, + "source": "TaskConfig" + }, + "neighbors": [] + }, + { + "key": [ + "/", + "test" + ], + "node": { + "task_display": { + "package_name": "@test/extra-args-not-forwarded-to-depends-on", + "task_name": "test", + "package_path": "/" + }, + "resolved_config": { + "command": "vt tool print test", + "resolved_options": { + "cwd": "/", + "cache_config": { + "env_config": { + "fingerprinted_envs": [], + "untracked_env": [ + "" + ] + }, + "input_config": { + "includes_auto": true, + "positive_globs": [], + "negative_globs": [] + } + } + } + }, + "source": "TaskConfig" + }, + "neighbors": [ + [ + "/", + "build" + ] + ] + } +] diff --git a/crates/vite_task_plan/tests/plan_snapshots/fixtures/extra-args-not-forwarded-to-depends-on/vite-task.json b/crates/vite_task_plan/tests/plan_snapshots/fixtures/extra-args-not-forwarded-to-depends-on/vite-task.json new file mode 100644 index 00000000..d52e0377 --- /dev/null +++ b/crates/vite_task_plan/tests/plan_snapshots/fixtures/extra-args-not-forwarded-to-depends-on/vite-task.json @@ -0,0 +1,12 @@ +{ + "cache": true, + "tasks": { + "build": { + "command": "vt tool print build" + }, + "test": { + "command": "vt tool print test", + "dependsOn": ["build"] + } + } +}