Skip to content

Commit

Permalink
fix: Fix affected detection aborting early.
Browse files Browse the repository at this point in the history
  • Loading branch information
milesj committed Nov 28, 2024
1 parent 7098d80 commit 8587949
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 48 deletions.
44 changes: 23 additions & 21 deletions crates/action-graph/src/action_graph_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,28 @@ impl<'app> ActionGraphBuilder<'app> {
return Ok(Some(*index));
}

// Compare against touched files if provided
// We should install deps & sync projects *before* running targets
let mut edges = vec![];

if let Some(install_deps_index) = self.install_deps(project, Some(task))? {
edges.push(install_deps_index);
}

edges.push(self.sync_project(project)?);

// And we also need to create edges for task dependencies
if !task.deps.is_empty() {
trace!(
task_target = task.target.as_str(),
dep_targets = ?task.deps.iter().map(|d| d.target.as_str()).collect::<Vec<_>>(),
"Linking dependencies for task",
);

edges.extend(self.run_task_dependencies(task)?);
}

// Compare against touched files if provided. We need to run this *after*
// dependencies, so that they're affected status has been marked!
if let Some(affected) = &mut self.affected {
if let Some(downstream) = &reqs.downstream_target {
affected
Expand All @@ -352,28 +373,9 @@ impl<'app> ActionGraphBuilder<'app> {
}
}

// We should install deps & sync projects *before* running targets
let mut edges = vec![];

if let Some(install_deps_index) = self.install_deps(project, Some(task))? {
edges.push(install_deps_index);
}

edges.push(self.sync_project(project)?);

// Insert the node and create edges
let index = self.insert_node(node);

// And we also need to create edges for task dependencies
if !task.deps.is_empty() {
trace!(
task_target = task.target.as_str(),
dep_targets = ?task.deps.iter().map(|d| d.target.as_str()).collect::<Vec<_>>(),
"Linking dependencies for task",
);

edges.extend(self.run_task_dependencies(task)?);
}

self.link_requirements(index, edges);

// And possibly dependents
Expand Down
9 changes: 8 additions & 1 deletion crates/action-graph/tests/action_graph_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,14 @@ mod action_graph {

let graph = builder.build();

assert!(topo(graph).is_empty());
assert!(topo(graph)
.into_iter()
.find(|node| if let ActionNode::RunTask(inner) = &node {
inner.target.as_str() == "bar:build"
} else {
false
})
.is_none());
}

#[tokio::test]
Expand Down
6 changes: 3 additions & 3 deletions crates/affected/src/affected.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_hash::{FxHashMap, FxHashSet};
use serde::{Deserialize, Serialize};
use std::fmt;

#[derive(Debug, PartialEq)]
#[derive(Debug, Eq, Hash, PartialEq)]
pub enum AffectedBy {
AlreadyMarked,
AlwaysAffected,
Expand Down Expand Up @@ -84,7 +84,7 @@ pub struct AffectedProjectState {
}

impl AffectedProjectState {
pub fn from(list: Vec<AffectedBy>) -> Self {
pub fn from(list: FxHashSet<AffectedBy>) -> Self {
let mut state = Self::default();

for by in list {
Expand Down Expand Up @@ -130,7 +130,7 @@ pub struct AffectedTaskState {
}

impl AffectedTaskState {
pub fn from(list: Vec<AffectedBy>) -> Self {
pub fn from(list: FxHashSet<AffectedBy>) -> Self {
let mut state = Self::default();

for by in list {
Expand Down
48 changes: 33 additions & 15 deletions crates/affected/src/affected_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,18 @@ use moon_task::{Target, Task};
use moon_workspace_graph::{GraphConnections, WorkspaceGraph};
use rustc_hash::{FxHashMap, FxHashSet};
use std::env;
use std::fmt;
use tracing::{debug, trace};

pub struct AffectedTracker<'app> {
workspace_graph: &'app WorkspaceGraph,
touched_files: &'app FxHashSet<WorkspaceRelativePathBuf>,

projects: FxHashMap<Id, Vec<AffectedBy>>,
projects: FxHashMap<Id, FxHashSet<AffectedBy>>,
project_downstream: DownstreamScope,
project_upstream: UpstreamScope,

tasks: FxHashMap<Target, Vec<AffectedBy>>,
tasks: FxHashMap<Target, FxHashSet<AffectedBy>>,
task_downstream: DownstreamScope,
task_upstream: UpstreamScope,
}
Expand Down Expand Up @@ -167,7 +168,7 @@ impl<'app> AffectedTracker<'app> {
self.projects
.entry(project.id.clone())
.or_default()
.push(affected);
.insert(affected);

self.track_project_dependencies(project, 0)?;
self.track_project_dependents(project, 0)?;
Expand Down Expand Up @@ -203,7 +204,7 @@ impl<'app> AffectedTracker<'app> {
self.projects
.entry(dep_config.id.clone())
.or_default()
.push(AffectedBy::DownstreamProject(project.id.clone()));
.insert(AffectedBy::DownstreamProject(project.id.clone()));

if depth == 0 && self.project_upstream == UpstreamScope::Direct {
continue;
Expand All @@ -221,7 +222,7 @@ impl<'app> AffectedTracker<'app> {
if self.project_downstream == DownstreamScope::None {
trace!(
project_id = project.id.as_str(),
"Not tracking dependents as downstream scope is none"
"Not tracking project dependents as downstream scope is none"
);

return Ok(());
Expand All @@ -231,18 +232,21 @@ impl<'app> AffectedTracker<'app> {
if self.project_downstream == DownstreamScope::Direct {
trace!(
project_id = project.id.as_str(),
"Tracking direct dependents"
"Tracking direct project dependents"
);
} else {
trace!(project_id = project.id.as_str(), "Tracking deep dependents");
trace!(
project_id = project.id.as_str(),
"Tracking deep project dependents"
);
}
}

for dep_id in self.workspace_graph.projects.dependents_of(project) {
self.projects
.entry(dep_id.clone())
.or_default()
.push(AffectedBy::UpstreamProject(project.id.clone()));
.insert(AffectedBy::UpstreamProject(project.id.clone()));

if depth == 0 && self.project_downstream == DownstreamScope::Direct {
continue;
Expand Down Expand Up @@ -331,7 +335,7 @@ impl<'app> AffectedTracker<'app> {
self.tasks
.entry(task.target.clone())
.or_default()
.push(affected);
.insert(affected);

self.track_task_dependencies(task, 0)?;
self.track_task_dependents(task, 0)?;
Expand All @@ -340,7 +344,7 @@ impl<'app> AffectedTracker<'app> {
self.projects
.entry(project_id.to_owned())
.or_default()
.push(AffectedBy::Task(task.target.clone()));
.insert(AffectedBy::Task(task.target.clone()));
}

Ok(())
Expand Down Expand Up @@ -374,7 +378,7 @@ impl<'app> AffectedTracker<'app> {
self.tasks
.entry(dep_config.target.clone())
.or_default()
.push(AffectedBy::DownstreamTask(task.target.clone()));
.insert(AffectedBy::DownstreamTask(task.target.clone()));

if depth == 0 && self.task_upstream == UpstreamScope::Direct {
continue;
Expand All @@ -392,7 +396,7 @@ impl<'app> AffectedTracker<'app> {
if self.task_downstream == DownstreamScope::None {
trace!(
task_target = task.target.as_str(),
"Not tracking dependents as downstream scope is none"
"Not tracking task dependents as downstream scope is none"
);

return Ok(());
Expand All @@ -402,12 +406,12 @@ impl<'app> AffectedTracker<'app> {
if self.task_downstream == DownstreamScope::Direct {
trace!(
task_target = task.target.as_str(),
"Tracking direct dependents"
"Tracking direct task dependents"
);
} else {
trace!(
task_target = task.target.as_str(),
"Tracking deep dependents"
"Tracking deep task dependents"
);
}
}
Expand All @@ -416,7 +420,7 @@ impl<'app> AffectedTracker<'app> {
self.tasks
.entry(dep_target.clone())
.or_default()
.push(AffectedBy::UpstreamTask(task.target.clone()));
.insert(AffectedBy::UpstreamTask(task.target.clone()));

if depth == 0 && self.task_downstream == DownstreamScope::Direct {
continue;
Expand All @@ -430,3 +434,17 @@ impl<'app> AffectedTracker<'app> {
Ok(())
}
}

impl<'app> fmt::Debug for AffectedTracker<'app> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("AffectedTracker")
.field("touched_files", &self.touched_files)
.field("projects", &self.projects)
.field("project_downstream", &self.project_downstream)
.field("project_upstream", &self.project_upstream)
.field("tasks", &self.tasks)
.field("task_downstream", &self.task_downstream)
.field("task_upstream", &self.task_upstream)
.finish()
}
}
9 changes: 1 addition & 8 deletions crates/app/src/commands/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,14 +152,7 @@ pub async fn run_target(

if should_run_affected {
action_graph_builder.set_touched_files(&touched_files)?;
action_graph_builder.set_affected_scopes(
UpstreamScope::Deep,
if args.dependents {
DownstreamScope::Direct
} else {
DownstreamScope::None
},
)?;
action_graph_builder.set_affected_scopes(UpstreamScope::Deep, DownstreamScope::Deep)?;
}

// Run targets, optionally based on affected files
Expand Down

0 comments on commit 8587949

Please sign in to comment.