Skip to content

Commit

Permalink
Add dependents.
Browse files Browse the repository at this point in the history
  • Loading branch information
milesj committed Oct 6, 2023
1 parent 2b57577 commit e579e30
Show file tree
Hide file tree
Showing 11 changed files with 206 additions and 37 deletions.
58 changes: 46 additions & 12 deletions nextgen/action-graph/src/action_graph_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ use tracing::{debug, trace};

type TouchedFilePaths = FxHashSet<WorkspaceRelativePathBuf>;

// TODO: run task dependents

pub struct ActionGraphBuilder<'app> {
pub include_dependents: bool,

all_query: Option<Criteria>,
graph: DiGraph<ActionNode, ()>,
indices: FxHashMap<ActionNode, NodeIndex>,
Expand All @@ -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,
Expand Down Expand Up @@ -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<Vec<NodeIndex>> {
// 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<Vec<NodeIndex>> {
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
Expand All @@ -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<Vec<NodeIndex>> {
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<T: AsRef<Target>>(
&mut self,
target: T,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
tasks:
external:
deps: ['deps:base']
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,14 @@ tasks:
deps: [b, c, a]
options:
runDepsInParallel: false

chain1:
deps: ['chain2']
chain2:
deps: ['chain3']
chain3: {}

base: {}

internal:
deps: ['~:base']
54 changes: 52 additions & 2 deletions nextgen/action-graph/tests/action_graph_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 [ ]
}

Original file line number Diff line number Diff line change
@@ -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 [ ]
}

Original file line number Diff line number Diff line change
@@ -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 [ ]
}

Original file line number Diff line number Diff line change
@@ -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 [ ]
}

Original file line number Diff line number Diff line change
Expand Up @@ -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 [ ]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 [ ]
Expand Down
14 changes: 14 additions & 0 deletions packages/cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit e579e30

Please sign in to comment.