[src/lib/{cli,runner}.rs] Refactor pub fn run such that execution_p…

…lan is separated from run ; [src/lib/types.rs] Imbue `pub struct ExecutionPlan` with `steps_to_run` & `name` fields

Cargo.toml

@@ -49,10 +49,7 @@ ci_info = { version = "^0.14.14", features = ["serde-1"] }
 cliparser = "^0.1.2"
 colored = "^2"
 ctrlc = "^3"
-diesel = { version = "^2", optional = true, features = [
-  "serde_json",
-  "r2d2",
-] }
+diesel = { version = "^2", optional = true, features = ["serde_json", "r2d2"] }
 diesel_migrations = { version = "^2", optional = true }
 dirs-next = "^2"
 duckscript = "^0.8.0"

src/lib/cli.rs

@@ -20,7 +20,7 @@ use crate::recursion_level;
 use crate::runner;
 use crate::time_summary;
 use crate::toolchain;
-use crate::types::{CliArgs, GlobalConfig};
+use crate::types::{CliArgs, Config, EnvInfo, GlobalConfig};
 use crate::version;
 use std::time::SystemTime;
 
@@ -32,11 +32,11 @@ pub(crate) static DEFAULT_LOG_LEVEL: &str = "info";
 pub(crate) static DEFAULT_TASK_NAME: &str = "default";
 pub(crate) static DEFAULT_OUTPUT_FORMAT: &str = "default";
 
-pub fn run(
-    cli_args: &CliArgs,
+pub fn prepare_for_execution_plan<'a>(
+    cli_args: &'a CliArgs,
     global_config: &GlobalConfig,
     logger_options: Option<LoggerOptions>,
-) -> Result<(), CargoMakeError> {
+) -> Result<(EnvInfo, Config, &'a String, Vec<(String, u128)>, SystemTime), CargoMakeError> {
     let start_time = SystemTime::now();
 
     recursion_level::increment();
@@ -103,7 +103,7 @@ pub fn run(
     let experimental = cli_args.experimental;
     let config = descriptor::load(&build_file, force_makefile, env, experimental)?;
 
-    let mut time_summary_vec = vec![];
+    let mut time_summary_vec = Vec::<(String, u128)>::new();
     time_summary::add(
         &mut time_summary_vec,
         "[Load Makefiles]",
@@ -130,6 +130,19 @@ pub fn run(
     // ensure profile env was not overridden
     profile::set(&normalized_profile_name);
 
+    Ok((env_info, config, task, time_summary_vec, start_time))
+}
+
+pub fn run(
+    cli_args: &CliArgs,
+    global_config: &GlobalConfig,
+    logger_options: Option<LoggerOptions>,
+) -> Result<(), CargoMakeError> {
+    let (env_info, config, task, time_summary_vec, start_time) =
+        prepare_for_execution_plan(cli_args, global_config, logger_options)?;
+
+    // let execution_plan: ExecutionPlan;
+
     if cli_args.list_all_steps || cli_args.list_category_steps.is_some() {
         cli_commands::list_steps::run(
             &config,
@@ -139,7 +152,8 @@ pub fn run(
             cli_args.hide_uninteresting,
         )
     } else if cli_args.diff_execution_plan {
-        let default_config = descriptor::load_internal_descriptors(true, experimental, None)?;
+        let default_config =
+            descriptor::load_internal_descriptors(true, cli_args.experimental, None)?;
         cli_commands::diff_steps::run(
             &default_config,
             &config,

src/lib/cli_commands/diff_steps.rs

@@ -19,7 +19,11 @@ use std::io::{BufWriter, Write};
 
 fn write_as_string(execution_plan: &ExecutionPlan, file: &File) -> io::Result<()> {
     let mut writer = BufWriter::new(file);
-    writeln!(&mut writer, "{:#?}", &execution_plan.steps)
+    writeln!(
+        &mut writer,
+        "{:#?}",
+        &execution_plan.steps[execution_plan.steps_to_run.clone()]
+    )
 }
 
 /// Runs the execution plan diff

src/lib/cli_commands/print_steps.rs

@@ -50,7 +50,7 @@ fn print_short_description(
     execution_plan: &ExecutionPlan,
 ) -> io::Result<()> {
     let mut counter = 1;
-    for step in &execution_plan.steps {
+    for step in &execution_plan.steps[execution_plan.steps_to_run.clone()] {
         let task = &step.config;
         let description = match &task.description {
             Some(value) => value,
@@ -62,7 +62,7 @@ fn print_short_description(
             counter, &step.name, &description
         )?;
 
-        counter = counter + 1;
+        counter += 1;
     }
     Ok(())
 }
@@ -71,7 +71,20 @@ fn print_default(
     output_buffer: &mut impl io::Write,
     execution_plan: &ExecutionPlan,
 ) -> io::Result<()> {
-    writeln!(output_buffer, "{:#?}", &execution_plan)
+    let steps: Vec<crate::types::Step> = {
+        let mut _steps =
+            Vec::<crate::types::Step>::with_capacity(execution_plan.steps_to_run.len());
+        execution_plan.steps[execution_plan.steps_to_run.clone()].clone_into(&mut _steps);
+        _steps
+    };
+    writeln!(
+        output_buffer,
+        "{:#?}",
+        ExecutionPlan {
+            steps,
+            ..ExecutionPlan::default()
+        }
+    )
 }
 
 /// Only prints the execution plan

src/lib/cli_commands/print_steps_test.rs

@@ -110,7 +110,7 @@ fn print_default_valid() {
         config: Task::new(),
     };
     let steps = vec![step];
-    let execution_plan = ExecutionPlan { steps };
+    let execution_plan = ExecutionPlan::new(steps);
 
     print_default(&mut std::io::stdout(), &execution_plan).expect("print should succeed");
 }
@@ -122,7 +122,7 @@ fn print_short_description_valid() {
         config: Task::new(),
     };
     let steps = vec![step];
-    let execution_plan = ExecutionPlan { steps };
+    let execution_plan = ExecutionPlan::new(steps);
 
     print_short_description(&mut std::io::stdout(), &execution_plan).expect("print should succeed");
 }

src/lib/error.rs

@@ -46,6 +46,9 @@ pub enum CargoMakeError {
     #[strum(to_string = "Task {0:#?} is {1}")]
     TaskIs(String, &'static str) = 110,
 
+    #[strum(to_string = "Task error detected, exit code: {0}")]
+    TaskErrorExitCode(i32) = 111,
+
     #[strum(to_string = "{0}")]
     NotFound(String) = 404,
 

src/lib/execution_plan.rs

@@ -549,6 +549,6 @@ impl<'a> ExecutionPlanBuilder<'a> {
             };
         }
 
-        Ok(ExecutionPlan { steps })
+        Ok(ExecutionPlan::new(steps))
     }
 }

src/lib/execution_plan_test.rs

@@ -809,6 +809,39 @@ fn create_single() {
     assert_eq!(execution_plan.steps[2].name, "end");
 }
 
+#[test]
+fn create_single_serde_json() {
+    let mut config_section = ConfigSection::new();
+    config_section.init_task = Some("init".to_string());
+    config_section.end_task = Some("end".to_string());
+    let mut config = Config {
+        config: config_section,
+        env_files: vec![],
+        env: IndexMap::new(),
+        env_scripts: vec![],
+        tasks: IndexMap::new(),
+        plugins: None,
+    };
+
+    config.tasks.insert("init".to_string(), Task::new());
+    config.tasks.insert("end".to_string(), Task::new());
+
+    let task = Task::new();
+
+    config.tasks.insert("test".to_string(), task);
+
+    let execution_plan = ExecutionPlanBuilder {
+        allow_private: true,
+        ..ExecutionPlanBuilder::new(&config, "test")
+    }
+    .build()
+    .unwrap();
+    assert_eq!(execution_plan.steps.len(), 3);
+    assert_eq!(execution_plan.steps[0].name, "init");
+    assert_eq!(execution_plan.steps[1].name, "test");
+    assert_eq!(execution_plan.steps[2].name, "end");
+}
+
 #[test]
 fn create_single_disabled() {
     let mut config_section = ConfigSection::new();

src/lib/plugin/sdk/cm_plugin_force_plugin_set_test.rs

@@ -85,7 +85,13 @@ set_env FORCE_PLUGIN_SET_AND_CLEAR_FLOW_TEST_SET_4 1
         "FORCE_PLUGIN_SET_AND_CLEAR_FLOW_TEST_SET_4"
     ));
 
-    runner::run_flow(&flow_info, Rc::new(RefCell::new(FlowState::new())), false).unwrap();
+    let execution_plan = runner::prepare_execution_plan(&flow_info, false).unwrap();
+    runner::run_task_flow(
+        &flow_info,
+        Rc::new(RefCell::new(FlowState::new())),
+        &execution_plan,
+    )
+    .unwrap();
 
     assert!(envmnt::is_equal(
         "FORCE_PLUGIN_SET_AND_CLEAR_FLOW_TEST_SET",

src/lib/runner.rs

@@ -229,7 +229,8 @@ fn run_sub_task_and_report(
                 if fork {
                     run_forked_task(&sub_flow_info, flow_state, cleanup_task)
                 } else {
-                    run_flow(&sub_flow_info, flow_state, true)
+                    let execution_plan = prepare_execution_plan(&flow_info, false)?;
+                    run_task_flow(&sub_flow_info, flow_state, &execution_plan)
                 }
             };
 
@@ -494,7 +495,7 @@ pub fn run_task_flow(
     flow_state: Rc<RefCell<FlowState>>,
     execution_plan: &ExecutionPlan,
 ) -> Result<(), CargoMakeError> {
-    for step in &execution_plan.steps {
+    for step in &execution_plan.steps[execution_plan.steps_to_run.clone()] {
         run_task(&flow_info, flow_state.clone(), &step)?;
     }
     Ok(())
@@ -594,11 +595,10 @@ fn create_watch_task(task: &str, options: Option<TaskWatchOptions>, flow_info: &
     task_config
 }
 
-pub fn run_flow(
+pub fn prepare_execution_plan(
     flow_info: &FlowInfo,
-    flow_state: Rc<RefCell<FlowState>>,
     sub_flow: bool,
-) -> Result<(), CargoMakeError> {
+) -> Result<ExecutionPlan, CargoMakeError> {
     let allow_private = sub_flow || flow_info.allow_private;
 
     let execution_plan = ExecutionPlanBuilder {
@@ -611,17 +611,11 @@ pub fn run_flow(
         ..ExecutionPlanBuilder::new(&flow_info.config, &flow_info.task)
     }
     .build()?;
-    debug!("Created execution plan: {:#?}", &execution_plan);
-
-    run_task_flow(&flow_info, flow_state, &execution_plan)?;
-
-    Ok(())
+    debug!("Prepared execution plan: {:#?}", &execution_plan);
+    Ok(execution_plan)
 }
 
-fn run_protected_flow(
-    flow_info: &FlowInfo,
-    flow_state: Rc<RefCell<FlowState>>,
-) -> Result<(), CargoMakeError> {
+fn prepare_protected_execution_plan(flow_info: &FlowInfo) -> Result<ExecutionPlan, CargoMakeError> {
     let proxy_task = create_proxy_task(
         &flow_info.task,
         flow_info.allow_private,
@@ -633,35 +627,36 @@ fn run_protected_flow(
     let exit_code = command::run_command(&proxy_task.command.unwrap(), &proxy_task.args, false)?;
 
     if exit_code != 0 {
-        match flow_info.config.config.on_error_task {
-            Some(ref on_error_task) => {
-                let mut error_flow_info = flow_info.clone();
-                error_flow_info.disable_on_error = true;
-                error_flow_info.task = on_error_task.clone();
-
-                run_flow(&error_flow_info, flow_state, false)?;
-            }
-            _ => (),
-        };
+        Err(CargoMakeError::TaskErrorExitCode(exit_code))
+    } else if flow_info.config.config.on_error_task.is_some() {
+        let execution_plan = prepare_execution_plan(&flow_info, false)?;
+        Ok(execution_plan)
+    } else {
+        Err(CargoMakeError::NotFound(String::from("ExecutionPlan")))
+    }
+}
 
-        error!("Task error detected, exit code: {}", &exit_code);
+fn run_protected_flow(
+    flow_info: &FlowInfo,
+    flow_state: Rc<RefCell<FlowState>>,
+    execution_plan: &ExecutionPlan,
+) -> Result<(), CargoMakeError> {
+    if let Some(ref on_error_task) = flow_info.config.config.on_error_task {
+        let mut error_flow_info = flow_info.clone();
+        error_flow_info.disable_on_error = true;
+        error_flow_info.task = on_error_task.clone();
+        run_task_flow(&error_flow_info, flow_state, &execution_plan)?;
     }
     Ok(())
 }
 
-/// Runs the requested tasks.<br>
-/// The flow is as follows:
-///
-/// * Create an execution plan based on the requested task and its dependencies
-/// * Run all tasks defined in the execution plan
-pub fn run(
+pub fn prepare_for_run(
     config: Config,
     task: &str,
     env_info: EnvInfo,
     cli_args: &CliArgs,
-    start_time: SystemTime,
     time_summary_vec: Vec<(String, u128)>,
-) -> Result<(), CargoMakeError> {
+) -> Result<(FlowInfo, Rc<RefCell<FlowState>>, ExecutionPlan), CargoMakeError> {
     time_summary::init(&config, &cli_args);
 
     let skip_tasks_pattern = match cli_args.skip_tasks_pattern {
@@ -691,10 +686,36 @@ pub fn run(
 
     let flow_state_rc = Rc::new(RefCell::new(flow_state));
 
+    let execution_plan =
+        if flow_info.disable_on_error || flow_info.config.config.on_error_task.is_none() {
+            prepare_execution_plan(&flow_info, false)?
+        } else {
+            prepare_protected_execution_plan(&flow_info)?
+        };
+
+    Ok((flow_info, flow_state_rc, execution_plan))
+}
+
+/// Runs the requested tasks.<br>
+/// The flow is as follows:
+///
+/// * Create an execution plan based on the requested task and its dependencies
+/// * Run all tasks defined in the execution plan
+pub fn run(
+    config: Config,
+    task: &str,
+    env_info: EnvInfo,
+    cli_args: &CliArgs,
+    start_time: SystemTime,
+    time_summary_vec: Vec<(String, u128)>,
+) -> Result<(), CargoMakeError> {
+    let (flow_info, flow_state_rc, execution_plan) =
+        prepare_for_run(config, task, env_info, cli_args, time_summary_vec)?;
+
     if flow_info.disable_on_error || flow_info.config.config.on_error_task.is_none() {
-        run_flow(&flow_info, flow_state_rc.clone(), false)?;
+        run_task_flow(&flow_info, flow_state_rc.clone(), &execution_plan)?;
     } else {
-        run_protected_flow(&flow_info, flow_state_rc.clone())?;
+        run_protected_flow(&flow_info, flow_state_rc.clone(), &execution_plan)?;
     }
 
     let time_string = match start_time.elapsed() {