diff --git a/nextgen/action-graph/src/action_graph_builder.rs b/nextgen/action-graph/src/action_graph_builder.rs index 1f2a7f495d5..ee51775871d 100644 --- a/nextgen/action-graph/src/action_graph_builder.rs +++ b/nextgen/action-graph/src/action_graph_builder.rs @@ -12,9 +12,9 @@ use tracing::{debug, trace}; type TouchedFilePaths = FxHashSet; -// TODO: run task dependents - pub struct ActionGraphBuilder<'app> { + pub include_dependents: bool, + all_query: Option, graph: DiGraph, indices: FxHashMap, @@ -34,6 +34,7 @@ impl<'app> ActionGraphBuilder<'app> { Ok(ActionGraphBuilder { all_query: None, graph: DiGraph::new(), + include_dependents: false, indices: FxHashMap::default(), platform_manager, project_graph, @@ -171,28 +172,28 @@ impl<'app> ActionGraphBuilder<'app> { "Adding dependencies for task {}", color::label(&task.target), ); - - // We don't pass touched files to dependencies, because if the parent - // task is affected/going to run, then so should all of these! - reqs.extend(self.run_task_dependencies(task, None)?); + reqs.extend(self.run_task_dependencies(task)?); } self.link_requirements(index, reqs); + // And possibly dependents + if self.include_dependents { + self.run_task_dependents(task)?; + } + Ok(Some(index)) } - pub fn run_task_dependencies( - &mut self, - task: &Task, - touched_files: Option<&TouchedFilePaths>, - ) -> miette::Result> { + // We don't pass touched files to dependencies, because if the parent + // task is affected/going to run, then so should all of these! + pub fn run_task_dependencies(&mut self, task: &Task) -> miette::Result> { let parallel = task.options.run_deps_in_parallel; let mut indices = vec![]; let mut previous_target_index = None; for dep_target in &task.deps { - let (_, dep_indices) = self.run_task_by_target(dep_target, touched_files)?; + let (_, dep_indices) = self.run_task_by_target(dep_target, None)?; for dep_index in dep_indices { // When parallel, parent depends on child @@ -215,6 +216,39 @@ impl<'app> ActionGraphBuilder<'app> { Ok(indices) } + // This is costly, is there a better way to do this? + pub fn run_task_dependents(&mut self, task: &Task) -> miette::Result> { + let mut indices = vec![]; + + if let TargetScope::Project(project_locator) = &task.target.scope { + let project = self.project_graph.get(project_locator)?; + + // From self project + for dep_task in project.tasks.values() { + if dep_task.deps.contains(&task.target) { + if let Some(index) = self.run_task(&project, dep_task, None)? { + indices.push(index); + } + } + } + + // From other projects + for dependent_id in self.project_graph.dependents_of(&project)? { + let dep_project = self.project_graph.get(dependent_id)?; + + for dep_task in dep_project.tasks.values() { + if dep_task.deps.contains(&task.target) { + if let Some(index) = self.run_task(&dep_project, dep_task, None)? { + indices.push(index); + } + } + } + } + } + + Ok(indices) + } + pub fn run_task_by_target>( &mut self, target: T, diff --git a/nextgen/action-graph/tests/__fixtures__/tasks/deps-external/moon.yml b/nextgen/action-graph/tests/__fixtures__/tasks/deps-external/moon.yml new file mode 100644 index 00000000000..bd188ab4774 --- /dev/null +++ b/nextgen/action-graph/tests/__fixtures__/tasks/deps-external/moon.yml @@ -0,0 +1,3 @@ +tasks: + external: + deps: ['deps:base'] diff --git a/nextgen/action-graph/tests/__fixtures__/tasks/cases/moon.yml b/nextgen/action-graph/tests/__fixtures__/tasks/deps/moon.yml similarity index 59% rename from nextgen/action-graph/tests/__fixtures__/tasks/cases/moon.yml rename to nextgen/action-graph/tests/__fixtures__/tasks/deps/moon.yml index be0e0b3cb2e..98363d5b0a0 100644 --- a/nextgen/action-graph/tests/__fixtures__/tasks/cases/moon.yml +++ b/nextgen/action-graph/tests/__fixtures__/tasks/deps/moon.yml @@ -13,3 +13,14 @@ tasks: deps: [b, c, a] options: runDepsInParallel: false + + chain1: + deps: ['chain2'] + chain2: + deps: ['chain3'] + chain3: {} + + base: {} + + internal: + deps: ['~:base'] diff --git a/nextgen/action-graph/tests/action_graph_test.rs b/nextgen/action-graph/tests/action_graph_test.rs index 2066cb8c29d..b99da4cdadf 100644 --- a/nextgen/action-graph/tests/action_graph_test.rs +++ b/nextgen/action-graph/tests/action_graph_test.rs @@ -443,7 +443,7 @@ mod action_graph { let container = ActionGraphContainer::new(sandbox.path()).await; let mut builder = container.create_builder(); - let project = container.project_graph.get("cases").unwrap(); + let project = container.project_graph.get("deps").unwrap(); let task = project.get_task("parallel").unwrap(); builder.run_task(&project, task, None).unwrap(); @@ -459,7 +459,7 @@ mod action_graph { let container = ActionGraphContainer::new(sandbox.path()).await; let mut builder = container.create_builder(); - let project = container.project_graph.get("cases").unwrap(); + let project = container.project_graph.get("deps").unwrap(); let task = project.get_task("serial").unwrap(); builder.run_task(&project, task, None).unwrap(); @@ -468,6 +468,56 @@ mod action_graph { assert_snapshot!(graph.to_dot()); } + + #[tokio::test] + async fn can_create_a_chain() { + let sandbox = create_sandbox("tasks"); + let container = ActionGraphContainer::new(sandbox.path()).await; + let mut builder = container.create_builder(); + + let project = container.project_graph.get("deps").unwrap(); + let task = project.get_task("chain1").unwrap(); + + builder.run_task(&project, task, None).unwrap(); + + let graph = builder.build().unwrap(); + + assert_snapshot!(graph.to_dot()); + } + + #[tokio::test] + async fn doesnt_include_dependents() { + let sandbox = create_sandbox("tasks"); + let container = ActionGraphContainer::new(sandbox.path()).await; + let mut builder = container.create_builder(); + + let project = container.project_graph.get("deps").unwrap(); + let task = project.get_task("base").unwrap(); + + builder.run_task(&project, task, None).unwrap(); + + let graph = builder.build().unwrap(); + + assert_snapshot!(graph.to_dot()); + } + + #[tokio::test] + async fn includes_dependents() { + let sandbox = create_sandbox("tasks"); + let container = ActionGraphContainer::new(sandbox.path()).await; + let mut builder = container.create_builder(); + + builder.include_dependents = true; + + let project = container.project_graph.get("deps").unwrap(); + let task = project.get_task("base").unwrap(); + + builder.run_task(&project, task, None).unwrap(); + + let graph = builder.build().unwrap(); + + assert_snapshot!(graph.to_dot()); + } } mod run_task_by_target { diff --git a/nextgen/action-graph/tests/snapshots/action_graph_test__action_graph__run_task_by_target__runs_all.snap b/nextgen/action-graph/tests/snapshots/action_graph_test__action_graph__run_task_by_target__runs_all.snap index 86a4829917c..e03fe506210 100644 --- a/nextgen/action-graph/tests/snapshots/action_graph_test__action_graph__run_task_by_target__runs_all.snap +++ b/nextgen/action-graph/tests/snapshots/action_graph_test__action_graph__run_task_by_target__runs_all.snap @@ -7,25 +7,25 @@ digraph { 1 [ label="SetupSystemTool" ] 2 [ label="SyncSystemProject(base)" ] 3 [ label="RunTask(base:build)" ] - 4 [ label="SyncSystemProject(common)" ] - 5 [ label="RunTask(common:build)" ] - 6 [ label="SyncSystemProject(client)" ] - 7 [ label="SyncSystemProject(server)" ] - 8 [ label="RunTask(client:build)" ] - 9 [ label="RunTask(server:build)" ] + 4 [ label="SyncSystemProject(server)" ] + 5 [ label="RunTask(server:build)" ] + 6 [ label="SyncSystemProject(common)" ] + 7 [ label="RunTask(common:build)" ] + 8 [ label="SyncSystemProject(client)" ] + 9 [ label="RunTask(client:build)" ] 1 -> 0 [ ] 2 -> 1 [ ] 3 -> 2 [ ] 4 -> 1 [ ] - 4 -> 2 [ ] 5 -> 4 [ ] - 7 -> 1 [ ] 6 -> 1 [ ] - 6 -> 7 [ ] - 6 -> 4 [ ] - 9 -> 7 [ ] + 6 -> 2 [ ] + 7 -> 6 [ ] + 8 -> 1 [ ] + 8 -> 4 [ ] 8 -> 6 [ ] - 8 -> 5 [ ] - 8 -> 9 [ ] + 9 -> 8 [ ] + 9 -> 7 [ ] + 9 -> 5 [ ] } diff --git a/nextgen/action-graph/tests/snapshots/action_graph_test__action_graph__run_task_dependencies__can_create_a_chain.snap b/nextgen/action-graph/tests/snapshots/action_graph_test__action_graph__run_task_dependencies__can_create_a_chain.snap new file mode 100644 index 00000000000..290953fed5f --- /dev/null +++ b/nextgen/action-graph/tests/snapshots/action_graph_test__action_graph__run_task_dependencies__can_create_a_chain.snap @@ -0,0 +1,20 @@ +--- +source: nextgen/action-graph/tests/action_graph_test.rs +expression: graph.to_dot() +--- +digraph { + 0 [ label="SyncWorkspace" ] + 1 [ label="SetupSystemTool" ] + 2 [ label="SyncSystemProject(deps)" ] + 3 [ label="RunTask(deps:chain1)" ] + 4 [ label="RunTask(deps:chain2)" ] + 5 [ label="RunTask(deps:chain3)" ] + 1 -> 0 [ ] + 2 -> 1 [ ] + 5 -> 2 [ ] + 4 -> 2 [ ] + 4 -> 5 [ ] + 3 -> 2 [ ] + 3 -> 4 [ ] +} + diff --git a/nextgen/action-graph/tests/snapshots/action_graph_test__action_graph__run_task_dependencies__doesnt_include_dependents.snap b/nextgen/action-graph/tests/snapshots/action_graph_test__action_graph__run_task_dependencies__doesnt_include_dependents.snap new file mode 100644 index 00000000000..7de75775a1d --- /dev/null +++ b/nextgen/action-graph/tests/snapshots/action_graph_test__action_graph__run_task_dependencies__doesnt_include_dependents.snap @@ -0,0 +1,14 @@ +--- +source: nextgen/action-graph/tests/action_graph_test.rs +expression: graph.to_dot() +--- +digraph { + 0 [ label="SyncWorkspace" ] + 1 [ label="SetupSystemTool" ] + 2 [ label="SyncSystemProject(deps)" ] + 3 [ label="RunTask(deps:base)" ] + 1 -> 0 [ ] + 2 -> 1 [ ] + 3 -> 2 [ ] +} + diff --git a/nextgen/action-graph/tests/snapshots/action_graph_test__action_graph__run_task_dependencies__includes_dependents.snap b/nextgen/action-graph/tests/snapshots/action_graph_test__action_graph__run_task_dependencies__includes_dependents.snap new file mode 100644 index 00000000000..7375595d48b --- /dev/null +++ b/nextgen/action-graph/tests/snapshots/action_graph_test__action_graph__run_task_dependencies__includes_dependents.snap @@ -0,0 +1,23 @@ +--- +source: nextgen/action-graph/tests/action_graph_test.rs +expression: graph.to_dot() +--- +digraph { + 0 [ label="SyncWorkspace" ] + 1 [ label="SetupSystemTool" ] + 2 [ label="SyncSystemProject(deps)" ] + 3 [ label="RunTask(deps:base)" ] + 4 [ label="RunTask(deps:internal)" ] + 5 [ label="SyncSystemProject(deps-external)" ] + 6 [ label="RunTask(deps-external:external)" ] + 1 -> 0 [ ] + 2 -> 1 [ ] + 3 -> 2 [ ] + 4 -> 2 [ ] + 4 -> 3 [ ] + 5 -> 1 [ ] + 5 -> 2 [ ] + 6 -> 5 [ ] + 6 -> 3 [ ] +} + diff --git a/nextgen/action-graph/tests/snapshots/action_graph_test__action_graph__run_task_dependencies__runs_deps_in_parallel.snap b/nextgen/action-graph/tests/snapshots/action_graph_test__action_graph__run_task_dependencies__runs_deps_in_parallel.snap index 4e3221ce9b1..f5d5002c747 100644 --- a/nextgen/action-graph/tests/snapshots/action_graph_test__action_graph__run_task_dependencies__runs_deps_in_parallel.snap +++ b/nextgen/action-graph/tests/snapshots/action_graph_test__action_graph__run_task_dependencies__runs_deps_in_parallel.snap @@ -5,11 +5,11 @@ expression: graph.to_dot() digraph { 0 [ label="SyncWorkspace" ] 1 [ label="SetupSystemTool" ] - 2 [ label="SyncSystemProject(cases)" ] - 3 [ label="RunTask(cases:parallel)" ] - 4 [ label="RunTask(cases:c)" ] - 5 [ label="RunTask(cases:a)" ] - 6 [ label="RunTask(cases:b)" ] + 2 [ label="SyncSystemProject(deps)" ] + 3 [ label="RunTask(deps:parallel)" ] + 4 [ label="RunTask(deps:c)" ] + 5 [ label="RunTask(deps:a)" ] + 6 [ label="RunTask(deps:b)" ] 1 -> 0 [ ] 2 -> 1 [ ] 4 -> 2 [ ] diff --git a/nextgen/action-graph/tests/snapshots/action_graph_test__action_graph__run_task_dependencies__runs_deps_in_serial.snap b/nextgen/action-graph/tests/snapshots/action_graph_test__action_graph__run_task_dependencies__runs_deps_in_serial.snap index 26ec9e0deb6..e444a36cc07 100644 --- a/nextgen/action-graph/tests/snapshots/action_graph_test__action_graph__run_task_dependencies__runs_deps_in_serial.snap +++ b/nextgen/action-graph/tests/snapshots/action_graph_test__action_graph__run_task_dependencies__runs_deps_in_serial.snap @@ -5,11 +5,11 @@ expression: graph.to_dot() digraph { 0 [ label="SyncWorkspace" ] 1 [ label="SetupSystemTool" ] - 2 [ label="SyncSystemProject(cases)" ] - 3 [ label="RunTask(cases:serial)" ] - 4 [ label="RunTask(cases:b)" ] - 5 [ label="RunTask(cases:c)" ] - 6 [ label="RunTask(cases:a)" ] + 2 [ label="SyncSystemProject(deps)" ] + 3 [ label="RunTask(deps:serial)" ] + 4 [ label="RunTask(deps:b)" ] + 5 [ label="RunTask(deps:c)" ] + 6 [ label="RunTask(deps:a)" ] 1 -> 0 [ ] 2 -> 1 [ ] 4 -> 2 [ ] diff --git a/packages/cli/CHANGELOG.md b/packages/cli/CHANGELOG.md index 7fd477bdb97..235fb309fbf 100644 --- a/packages/cli/CHANGELOG.md +++ b/packages/cli/CHANGELOG.md @@ -36,6 +36,20 @@ We're marking this as a breaking change as this could subtly introduce cycles in the project graph that weren't present before, and for Node.js projects, this may inject `peerDependencies`. +#### 🚀 Updates + +- Rewrote the dependency graph ground the ground-up: + - Now known as the action graph. + - All actions now depend on the `SyncWorkspace` action, instead of this action running + arbitrarily. + - Cleaned up dependency chains between actions, greatly reducing the number of nodes in the graph. + - Renamed `RunTarget` to `RunTask`, including interactive and persistent variants. + +#### 🐞 Fixes + +- Fixed an issue where task dependents (via `moon ci` or `moon run --dependents`) wouldn't always + locate all downstream tasks. + ## 1.14.5 #### 🐞 Fixes