diff --git a/crates/core/action-pipeline/src/actions/run_task.rs b/crates/core/action-pipeline/src/actions/run_task.rs index 6b1cb22e768..bcdc0bc2cbc 100644 --- a/crates/core/action-pipeline/src/actions/run_task.rs +++ b/crates/core/action-pipeline/src/actions/run_task.rs @@ -37,7 +37,7 @@ pub async fn run_task( color::label(&task.target) ); - runner.node = action.node.clone(); + runner.node = Arc::clone(&action.node); action.allow_failure = task.options.allow_failure; // If a dependency failed, we should skip this target diff --git a/crates/core/action-pipeline/src/estimator.rs b/crates/core/action-pipeline/src/estimator.rs index 86daf2561ec..24dc5f23dec 100644 --- a/crates/core/action-pipeline/src/estimator.rs +++ b/crates/core/action-pipeline/src/estimator.rs @@ -47,10 +47,6 @@ impl Estimator { // Bucket every ran target based on task name, // and aggregate all tasks of the same name. for result in results { - let Some(node) = &result.node else { - continue; - }; - let Some(duration) = &result.duration else { continue; }; @@ -66,7 +62,7 @@ impl Estimator { task_duration *= 10; } - match node { + match &*result.node { ActionNode::SetupTool { .. } | ActionNode::InstallDeps { .. } | ActionNode::InstallProjectDeps { .. } => { diff --git a/crates/core/action-pipeline/src/pipeline.rs b/crates/core/action-pipeline/src/pipeline.rs index c5816ba92fb..3ca30fba142 100644 --- a/crates/core/action-pipeline/src/pipeline.rs +++ b/crates/core/action-pipeline/src/pipeline.rs @@ -367,8 +367,8 @@ impl Pipeline { } term.line(label_checkpoint( - match &result.node { - Some(ActionNode::RunTask { target, .. }) => target.as_str(), + match &*result.node { + ActionNode::RunTask { target, .. } => target.as_str(), _ => &result.label, }, Checkpoint::RunFailed, @@ -470,7 +470,7 @@ impl Pipeline { let mut skipped_count = 0; for result in results { - if compact && !matches!(result.node.as_ref().unwrap(), ActionNode::RunTask { .. }) { + if compact && !matches!(*result.node, ActionNode::RunTask { .. }) { continue; } diff --git a/crates/core/action-pipeline/src/processor.rs b/crates/core/action-pipeline/src/processor.rs index af3ed5693a4..2f63140f9a8 100644 --- a/crates/core/action-pipeline/src/processor.rs +++ b/crates/core/action-pipeline/src/processor.rs @@ -3,7 +3,7 @@ use crate::actions::run_task::run_task; use crate::actions::setup_tool::setup_tool; use crate::actions::sync_project::sync_project; use crate::actions::sync_workspace::sync_workspace; -use moon_action::{Action, ActionNode}; +use moon_action::{Action, ActionNode, ActionStatus}; use moon_action_context::ActionContext; use moon_emitter::{Emitter, Event}; use moon_logger::trace; @@ -29,7 +29,7 @@ pub async fn process_action( ) -> miette::Result { action.start(); - let node = action.node.take().unwrap(); + let node = Arc::clone(&action.node); let log_action_label = color::muted_light(&action.label); trace!( @@ -51,7 +51,9 @@ pub async fn process_action( }) .await?; - let result = match &node { + let result = match &*node { + ActionNode::None => Ok(ActionStatus::Skipped), + // Setup and install the specific tool ActionNode::SetupTool { runtime } => { local_emitter @@ -214,7 +216,7 @@ pub async fn process_action( if action.has_failed() { // If these fail, we should abort instead of trying to continue if matches!( - node, + *node, ActionNode::SetupTool { .. } | ActionNode::InstallDeps { .. } ) { action.abort(); @@ -245,8 +247,5 @@ pub async fn process_action( ); } - // Reassign the node for reuse - action.node = Some(node); - Ok(action) } diff --git a/crates/core/action-pipeline/tests/estimator_test.rs b/crates/core/action-pipeline/tests/estimator_test.rs index 80ab2ba684a..337b005c422 100644 --- a/crates/core/action-pipeline/tests/estimator_test.rs +++ b/crates/core/action-pipeline/tests/estimator_test.rs @@ -2,18 +2,19 @@ use moon_action::{Action, ActionNode, ActionStatus}; use moon_action_pipeline::estimator::{Estimator, TaskEstimate}; use moon_platform::Runtime; use rustc_hash::FxHashMap; +use std::sync::Arc; use std::time::Duration; const NANOS_PER_MILLI: u32 = 1_000_000; const HALF_SECOND: u32 = NANOS_PER_MILLI * 500; -fn create_run_task_action(runtime: Runtime, target: &str) -> ActionNode { - ActionNode::RunTask { +fn create_run_task_action(runtime: Runtime, target: &str) -> Arc { + Arc::new(ActionNode::RunTask { interactive: false, persistent: false, runtime, target: target.into(), - } + }) } mod estimator { @@ -40,7 +41,7 @@ mod estimator { let est = Estimator::calculate( &[Action { duration: Some(Duration::new(10, 0)), - node: Some(create_run_task_action(Runtime::system(), "proj:task")), + node: create_run_task_action(Runtime::system(), "proj:task"), ..Action::default() }], Duration::new(5, 0), @@ -67,27 +68,27 @@ mod estimator { &[ Action { duration: Some(Duration::new(10, 0)), - node: Some(create_run_task_action(Runtime::system(), "a:build")), + node: create_run_task_action(Runtime::system(), "a:build"), ..Action::default() }, Action { duration: Some(Duration::new(5, 0)), - node: Some(create_run_task_action(Runtime::system(), "a:lint")), + node: create_run_task_action(Runtime::system(), "a:lint"), ..Action::default() }, Action { duration: Some(Duration::new(15, 0)), - node: Some(create_run_task_action(Runtime::system(), "b:build")), + node: create_run_task_action(Runtime::system(), "b:build"), ..Action::default() }, Action { duration: Some(Duration::new(8, 0)), - node: Some(create_run_task_action(Runtime::system(), "c:test")), + node: create_run_task_action(Runtime::system(), "c:test"), ..Action::default() }, Action { duration: Some(Duration::new(12, 0)), - node: Some(create_run_task_action(Runtime::system(), "d:lint")), + node: create_run_task_action(Runtime::system(), "d:lint"), ..Action::default() }, ], @@ -122,21 +123,21 @@ mod estimator { &[ Action { duration: Some(Duration::new(10, 0)), - node: Some(ActionNode::SetupTool { + node: Arc::new(ActionNode::SetupTool { runtime: Runtime::system(), }), ..Action::default() }, Action { duration: Some(Duration::new(25, 0)), - node: Some(ActionNode::InstallDeps { + node: Arc::new(ActionNode::InstallDeps { runtime: Runtime::system(), }), ..Action::default() }, Action { duration: Some(Duration::new(10, 0)), - node: Some(create_run_task_action(Runtime::system(), "proj:task")), + node: create_run_task_action(Runtime::system(), "proj:task"), ..Action::default() }, ], @@ -166,7 +167,7 @@ mod estimator { let est = Estimator::calculate( &[Action { duration: Some(Duration::new(3, 0)), - node: Some(create_run_task_action(Runtime::system(), "proj:task")), + node: create_run_task_action(Runtime::system(), "proj:task"), status: ActionStatus::Cached, ..Action::default() }], @@ -194,41 +195,41 @@ mod estimator { &[ Action { duration: Some(Duration::new(10, 0)), - node: Some(ActionNode::SetupTool { + node: Arc::new(ActionNode::SetupTool { runtime: Runtime::system(), }), ..Action::default() }, Action { duration: Some(Duration::new(25, 0)), - node: Some(ActionNode::InstallDeps { + node: Arc::new(ActionNode::InstallDeps { runtime: Runtime::system(), }), ..Action::default() }, Action { duration: Some(Duration::new(10, 0)), - node: Some(create_run_task_action(Runtime::system(), "a:build")), + node: create_run_task_action(Runtime::system(), "a:build"), ..Action::default() }, Action { duration: Some(Duration::new(5, 0)), - node: Some(create_run_task_action(Runtime::system(), "a:lint")), + node: create_run_task_action(Runtime::system(), "a:lint"), ..Action::default() }, Action { duration: Some(Duration::new(15, 0)), - node: Some(create_run_task_action(Runtime::system(), "b:build")), + node: create_run_task_action(Runtime::system(), "b:build"), ..Action::default() }, Action { duration: Some(Duration::new(8, 0)), - node: Some(create_run_task_action(Runtime::system(), "c:test")), + node: create_run_task_action(Runtime::system(), "c:test"), ..Action::default() }, Action { duration: Some(Duration::new(12, 0)), - node: Some(create_run_task_action(Runtime::system(), "d:lint")), + node: create_run_task_action(Runtime::system(), "d:lint"), ..Action::default() }, ], @@ -267,41 +268,41 @@ mod estimator { &[ Action { duration: Some(Duration::new(10, 0)), - node: Some(ActionNode::SetupTool { + node: Arc::new(ActionNode::SetupTool { runtime: Runtime::system(), }), ..Action::default() }, Action { duration: Some(Duration::new(25, 0)), - node: Some(ActionNode::InstallDeps { + node: Arc::new(ActionNode::InstallDeps { runtime: Runtime::system(), }), ..Action::default() }, Action { duration: Some(Duration::new(10, 0)), - node: Some(create_run_task_action(Runtime::system(), "a:build")), + node: create_run_task_action(Runtime::system(), "a:build"), ..Action::default() }, Action { duration: Some(Duration::new(5, 0)), - node: Some(create_run_task_action(Runtime::system(), "a:lint")), + node: create_run_task_action(Runtime::system(), "a:lint"), ..Action::default() }, Action { duration: Some(Duration::new(15, 0)), - node: Some(create_run_task_action(Runtime::system(), "b:build")), + node: create_run_task_action(Runtime::system(), "b:build"), ..Action::default() }, Action { duration: Some(Duration::new(8, 0)), - node: Some(create_run_task_action(Runtime::system(), "c:test")), + node: create_run_task_action(Runtime::system(), "c:test"), ..Action::default() }, Action { duration: Some(Duration::new(12, 0)), - node: Some(create_run_task_action(Runtime::system(), "d:lint")), + node: create_run_task_action(Runtime::system(), "d:lint"), ..Action::default() }, ], diff --git a/crates/core/action/src/action.rs b/crates/core/action/src/action.rs index deedebcf05d..35bbd363557 100644 --- a/crates/core/action/src/action.rs +++ b/crates/core/action/src/action.rs @@ -2,6 +2,7 @@ use moon_action_graph::ActionNode; use moon_common::color; use moon_utils::time::{chrono::prelude::*, now_timestamp}; use serde::{Deserialize, Serialize}; +use std::sync::Arc; use std::time::{Duration, Instant}; fn has_failed(status: &ActionStatus) -> bool { @@ -100,7 +101,7 @@ pub struct Action { pub log_target: String, #[serde(skip)] - pub node: Option, + pub node: Arc, #[serde(skip)] pub node_index: usize, @@ -126,7 +127,7 @@ impl Action { flaky: false, label: node.label(), log_target: String::new(), - node: Some(node), + node: Arc::new(node), node_index: 0, started_at: None, start_time: None, diff --git a/crates/core/runner/src/runner.rs b/crates/core/runner/src/runner.rs index 5835f68e8ca..8e292764e93 100644 --- a/crates/core/runner/src/runner.rs +++ b/crates/core/runner/src/runner.rs @@ -22,6 +22,7 @@ use moon_workspace::Workspace; use rustc_hash::FxHashMap; use starbase_styles::color; use starbase_utils::glob; +use std::sync::Arc; use tokio::{ task, time::{sleep, Duration}, @@ -38,7 +39,7 @@ pub enum HydrateFrom { pub struct Runner<'a> { pub cache: CacheItem, - pub node: Option, + pub node: Arc, emitter: &'a Emitter, @@ -70,7 +71,7 @@ impl<'a> Runner<'a> { Ok(Runner { cache, - node: None, + node: Arc::new(ActionNode::None), emitter, project, stderr: Term::buffered_stderr(), @@ -545,15 +546,14 @@ impl<'a> Runner<'a> { let primary_longest_width = context.primary_targets.iter().map(|t| t.id.len()).max(); let is_primary = context.primary_targets.contains(&self.task.target); let is_real_ci = is_ci() && !is_test_env(); - let is_persistent = - self.node.as_ref().is_some_and(|n| n.is_persistent()) || self.task.is_persistent(); + let is_persistent = self.node.is_persistent() || self.task.is_persistent(); let output; let error; // When a task is configured as local (no caching), or the interactive flag is passed, // we don't "capture" stdout/stderr (which breaks stdin) and let it stream natively. let is_interactive = (!self.task.options.cache && context.primary_targets.len() == 1) - || self.node.as_ref().is_some_and(|n| n.is_interactive()) + || self.node.is_interactive() || self.task.is_interactive(); // When the primary target, always stream the output for a better developer experience. diff --git a/nextgen/action-graph/src/action_node.rs b/nextgen/action-graph/src/action_node.rs index 8c15a04ba7b..2baa3f4980f 100644 --- a/nextgen/action-graph/src/action_node.rs +++ b/nextgen/action-graph/src/action_node.rs @@ -4,9 +4,12 @@ use moon_task::Target; use serde::Serialize; use std::hash::{Hash, Hasher}; -#[derive(Clone, Debug, Eq, PartialEq, Serialize)] +#[derive(Clone, Debug, Default, Eq, PartialEq, Serialize)] #[serde(tag = "action", content = "params")] pub enum ActionNode { + #[default] + None, + /// Install tool dependencies in the workspace root. InstallDeps { runtime: Runtime }, @@ -40,7 +43,7 @@ impl ActionNode { Self::RunTask { runtime, .. } => runtime, Self::SetupTool { runtime } => runtime, Self::SyncProject { runtime, .. } => runtime, - Self::SyncWorkspace => unreachable!(), + _ => unreachable!(), } } @@ -97,6 +100,7 @@ impl ActionNode { format!("Sync{runtime}Project({project})") } Self::SyncWorkspace => "SyncWorkspace".into(), + Self::None => "None".into(), } } } diff --git a/packages/cli/CHANGELOG.md b/packages/cli/CHANGELOG.md index 462b3a674e3..c8ec0823846 100644 --- a/packages/cli/CHANGELOG.md +++ b/packages/cli/CHANGELOG.md @@ -12,6 +12,10 @@ ## Unreleased +#### 🐞 Fixes + +- Fixed an issue where interactive/persistent flags weren't always bubbled up the the task runner. + #### ⚙️ Internal - Updated to proto v0.20.