Expand condition_script_runner_args

.gitignore

@@ -10,3 +10,4 @@ dump.rdb
 /core
 /src/**/Cargo.lock
 /examples/**/Cargo.lock
+*.rustc_info.json

Cargo.toml

@@ -80,8 +80,9 @@ strum_macros = "0.26.4"
 toml = "^0.8"
 
 [dev-dependencies]
-expect-test = "^1"
 cfg-if = "^1.0.0"
+expect-test = "^1"
+tempfile = "^3.10.1"
 
 [target.'cfg(windows)'.dependencies]
 nu-ansi-term = "^0.50"

src/lib/runner.rs

@@ -6,37 +6,30 @@
 //! * Load env variables
 //! * Create an execution plan based on the requested task and its dependencies
 //! * Run all tasks defined in the execution plan
-//!
 
 #[cfg(test)]
 #[path = "runner_test.rs"]
 mod runner_test;
 
-use std::cell::RefCell;
-use std::rc::Rc;
-use std::thread;
-use std::time::SystemTime;
+use std::{cell::RefCell, rc::Rc, thread, time::SystemTime};
 
 use indexmap::IndexMap;
 use regex::Regex;
 
-use crate::command;
-use crate::condition;
-use crate::environment;
-use crate::error::CargoMakeError;
-use crate::execution_plan::ExecutionPlanBuilder;
-use crate::functions;
-use crate::installer;
-use crate::logger;
-use crate::plugin::runner::run_task as run_task_plugin;
-use crate::profile;
-use crate::proxy_task::create_proxy_task;
-use crate::scriptengine;
-use crate::time_summary;
-use crate::types::{
-    CliArgs, Config, DeprecationInfo, EnvInfo, EnvValue, ExecutionPlan, FlowInfo, FlowState,
-    MaybeArray, RunTaskInfo, RunTaskName, RunTaskOptions, RunTaskRoutingInfo, Step, Task,
-    TaskWatchOptions,
+use crate::{
+    command, condition, environment,
+    error::CargoMakeError,
+    execution_plan::ExecutionPlanBuilder,
+    functions, installer, logger,
+    plugin::runner::run_task as run_task_plugin,
+    profile,
+    proxy_task::create_proxy_task,
+    scriptengine, time_summary,
+    types::{
+        CliArgs, Config, DeprecationInfo, EnvInfo, EnvValue, ExecutionPlan, FlowInfo, FlowState,
+        MaybeArray, RunTaskInfo, RunTaskName, RunTaskOptions, RunTaskRoutingInfo, Step, Task,
+        TaskWatchOptions,
+    },
 };
 
 fn do_in_task_working_directory<F>(step: &Step, mut action: F) -> Result<(), CargoMakeError>
@@ -183,7 +176,8 @@ fn run_forked_task(
     }
 }
 
-/// runs a sub task and returns true/false based if a sub task was actually invoked
+/// runs a sub task and returns true/false based if a sub task was actually
+/// invoked
 fn run_sub_task_and_report(
     flow_info: &FlowInfo,
     flow_state: Rc<RefCell<FlowState>>,
@@ -361,8 +355,11 @@ pub(crate) fn run_task_with_options(
             None => (),
         };
 
+        let env_before_current_task_env_expansion = std::env::vars();
+        // expand env now in case condition_script_runner_args has a value that needs to
+        // be expanded
         let step = {
-            //get profile
+            // get profile
             let profile_name = profile::get();
 
             match step.config.env_files {
@@ -376,7 +373,7 @@ pub(crate) fn run_task_with_options(
 
             envmnt::set("CARGO_MAKE_CURRENT_TASK_NAME", &step.name);
 
-            //make sure profile env is not overwritten
+            // make sure profile env is not overwritten
             profile::set(&profile_name);
 
             // modify step using env and functions
@@ -444,11 +441,8 @@ pub(crate) fn run_task_with_options(
                     None => {
                         do_in_task_working_directory(&step, || -> Result<bool, CargoMakeError> {
                             // run script
-                            let script_runner_done = scriptengine::invoke(
-                                &step.config,
-                                flow_info,
-                                flow_state.clone(),
-                            )?;
+                            let script_runner_done =
+                                scriptengine::invoke(&step.config, flow_info, flow_state.clone())?;
 
                             // run command
                             if !script_runner_done {
@@ -466,6 +460,11 @@ pub(crate) fn run_task_with_options(
                 };
             }
         } else {
+            // Restore env to state before expansion for condition evaluation
+            for (key, value) in env_before_current_task_env_expansion {
+                envmnt::set(key, value);
+            }
+
             let fail_message = match step.config.condition {
                 Some(ref condition) => match condition.fail_message {
                     Some(ref value) => value.to_string(),

src/lib/runner_test.rs

@@ -1,15 +1,17 @@
-use super::*;
-use crate::test;
-use crate::types::{
-    ConditionScriptValue, ConfigSection, CrateInfo, EnvFile, RunTaskDetails, ScriptValue,
-    TaskCondition,
-};
 use cfg_if::cfg_if;
 use git_info::types::GitInfo;
 use rust_info::types::RustInfo;
 
+use super::*;
 #[cfg(target_os = "linux")]
 use crate::types::WatchOptions;
+use crate::{
+    test,
+    types::{
+        ConditionScriptValue, ConfigSection, CrateInfo, EnvFile, RunTaskDetails, ScriptValue,
+        TaskCondition,
+    },
+};
 
 #[test]
 #[ignore]
@@ -895,6 +897,55 @@ fn run_task_deprecated_flag() {
     run_task(&flow_info, Rc::new(RefCell::new(FlowState::new())), &step).unwrap();
 }
 
+#[test]
+#[ignore]
+fn run_task_failed_condition_script_doesnt_change_env() {
+    const TEST_ENV_VAR: &str = "TEST_VAR";
+    const ORIGINAL_ENV_VAR_VALUE: &str = "ORIGINAL-VALUE";
+    std::env::set_var(TEST_ENV_VAR, ORIGINAL_ENV_VAR_VALUE);
+
+    let flow_info = FlowInfo {
+        config: Config::default(),
+        task: "test".to_string(),
+        env_info: EnvInfo {
+            rust_info: RustInfo::new(),
+            crate_info: CrateInfo::new(),
+            git_info: GitInfo::new(),
+            ci_info: ci_info::get(),
+        },
+        disable_workspace: false,
+        disable_on_error: false,
+        allow_private: false,
+        skip_init_end_tasks: false,
+        skip_tasks_pattern: None,
+        cli_arguments: None,
+    };
+
+    let step = Step {
+        name: "test".to_string(),
+        config: Task {
+            // condition for this task always evaluates to false
+            condition_script: Some(ConditionScriptValue::SingleLine("exit 1".to_string())),
+            script_runner: Some("@duckscript".to_string()),
+            script: Some(ScriptValue::SingleLine(format!(
+                "set_env {TEST_ENV_VAR} NEW-VALUE-FROM-SCRIPT"
+            ))),
+            env: Some(
+                [(
+                    TEST_ENV_VAR.to_string(),
+                    EnvValue::Value("NEW-VALUE-FROM-ENV".to_string()),
+                )]
+                .into(),
+            ),
+            ..Default::default()
+        },
+    };
+
+    run_task(&flow_info, Rc::new(RefCell::new(FlowState::new())), &step).unwrap();
+
+    assert_eq!(std::env::var(TEST_ENV_VAR).unwrap(), ORIGINAL_ENV_VAR_VALUE);
+}
+
 #[test]
 #[ignore]
 fn should_watch_none_and_env_not_set() {
@@ -1062,7 +1113,8 @@ fn create_watch_task_with_makefile_with_spaces_in_path() {
     make_command_line.push_str(" --profile=");
     make_command_line.push_str(&profile::get());
     make_command_line.push_str(
-        " --allow-private --skip-init-end-tasks --makefile \"/path with spaces/mymakefile.toml\" some_task"
+        " --allow-private --skip-init-end-tasks --makefile \"/path with spaces/mymakefile.toml\" \
+         some_task",
     );
 
     let args = task.args.unwrap();

src/lib/types.rs

@@ -2359,7 +2359,7 @@ impl ConfigSection {
     }
 }
 
-#[derive(Serialize, Deserialize, Debug, Clone)]
+#[derive(Serialize, Deserialize, Debug, Clone, Default)]
 /// Holds the entire configuration such as task definitions and env vars
 pub struct Config {
     /// Runtime config