Skip to content

Commit

Permalink
Merge pull request sagiegurari#1132 from wmmc88/expand-condition-scri…
Browse files Browse the repository at this point in the history
…pt-args

Expand `condition_script_runner_args`
  • Loading branch information
sagiegurari authored Aug 18, 2024
2 parents 2fb48a6 + 0477016 commit b639e7d
Show file tree
Hide file tree
Showing 10 changed files with 256 additions and 33 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ dump.rdb
/core
/src/**/Cargo.lock
/examples/**/Cargo.lock
*.rustc_info.json
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ strum_macros = "0.26.4"
toml = "^0.8"

[dev-dependencies]
cfg-if = "^1.0.0"
expect-test = "^1"

[target.'cfg(windows)'.dependencies]
Expand Down
34 changes: 17 additions & 17 deletions docs/_includes/content.md
Original file line number Diff line number Diff line change
Expand Up @@ -1268,9 +1268,9 @@ We run task **3** the output would be:

<a name="usage-env"></a>
### Environment Variables
`cargo-make` enabled the definition of environmental variables in several ways, which can later be accessed throughout task execution.
`cargo-make` enabled the definition of environment variables in several ways, which can later be accessed throughout task execution.

Because environmental variables play a significant role in `cargo-make`, it provides multiple declarative ways to provide them at different levels of granularity.
Because environment variables play a significant role in `cargo-make`, it provides multiple declarative ways to provide them at different levels of granularity.

* [Declaration](#env-declaration)
* [Global Configuration](#usage-env-config)
Expand All @@ -1285,7 +1285,7 @@ Because environmental variables play a significant role in `cargo-make`, it prov
<a name="env-declaration"></a>
#### Declaration

There are multiple ways to declare environmental variables, all of which are suited for specific suitcases.
There are multiple ways to declare environment variables, all of which are suited for specific suitcases.

##### Simple

Expand All @@ -1308,9 +1308,9 @@ LIST_VALUE = [ "VALUE1", "VALUE2", "VALUE3" ]

##### Script

`cargo-make` supports the use of simple scripts. The output of the said script will then determine the value of the environmental variable.
`cargo-make` supports the use of simple scripts. The output of the said script will then determine the value of the environment variable.

The script's object has two additional arguments: `multiline` and `depends_on`. If `multiple` is set to `true`, the supplied script will be evaluated as a script with multiple lines. `depends_on` is a list of environmental variables this script depends on, which is taken into account during reordering if unset `cargo-make` will try to guess the variables used during reordering.
The script's object has two additional arguments: `multiline` and `depends_on`. If `multiple` is set to `true`, the supplied script will be evaluated as a script with multiple lines. `depends_on` is a list of environment variables this script depends on, which is taken into account during reordering if unset `cargo-make` will try to guess the variables used during reordering.

> **Note:** This uses the default OS command runner (`cmd` on Windows, `sh` on UNIX systems), other runners like `duckscript`, `rust`, etc. are **not** supported.
Expand All @@ -1320,7 +1320,7 @@ EVALUATED_VAR = { script = ["echo SOME VALUE"] }

##### Decode Map

`cargo-make` supports the use of mappings where a `source` is matched against a dictionary of possible `mapping`s, where each key of the `mapping` is compared against the evaluated `source` value. Should the key and `source` be the same, the corresponding value to the key will be the value of the environmental variable. If no key is matched, the `default_value` is used if provided. Otherwise, it will default to an empty string instead.
`cargo-make` supports the use of mappings where a `source` is matched against a dictionary of possible `mapping`s, where each key of the `mapping` is compared against the evaluated `source` value. Should the key and `source` be the same, the corresponding value to the key will be the value of the environment variable. If no key is matched, the `default_value` is used if provided. Otherwise, it will default to an empty string instead.

```toml
LIBRARY_EXTENSION = { source = "${CARGO_MAKE_RUST_TARGET_OS}", default_value = "unknown", mapping = {"linux" = "so", "macos" = "dylib", "windows" = "dll", "openbsd" = "so" } }
Expand Down Expand Up @@ -1385,7 +1385,7 @@ PROD = true
<a name="usage-env-task"></a>
#### Task

Environmental variables can be set in a task's scope, and will be merged with the global environment when that task gets executed. This means that the evaluation of environmental variables takes place after all dependencies have run, but _before_ the task itself runs.
Environmental variables can be set in a task's scope, and will be merged with the global environment when that task gets executed. This means that the evaluation of environment variables takes place after all dependencies have run, but _before_ the task itself runs.

> **Note:** Reordering of task variables with global variables will **not** take place. Tasks simply overwrite previously declared variables.
Expand Down Expand Up @@ -1421,7 +1421,7 @@ It is also possible to provide an env file path as part of the CLI args as follo
cargo make --env-file=./env/production.env
```

This allows using the same `Makefile.toml`, but with a different set of environmental variables loaded from the env file.
This allows using the same `Makefile.toml`, but with a different set of environment variables loaded from the env file.

The env file is a simple `key=value`, which is similar to [dotenv](https://www.npmjs.com/package/dotenv), but only supports variable interpolation using the `${}` syntax.

Expand All @@ -1443,7 +1443,7 @@ env_files = [
]
```

To only load environmental variables whenever a variable hasn't been defined yet, use the `defaults_only` property.
To only load environment variables whenever a variable hasn't been defined yet, use the `defaults_only` property.

```toml
env_files = [
Expand All @@ -1452,7 +1452,7 @@ env_files = [
]
```

Use the `profile` property to only load environmental variables whenever a specific profile is active.
Use the `profile` property to only load environment variables whenever a specific profile is active.

> To learn more about profiles, check the [profiles section](#usage-profiles).
Expand Down Expand Up @@ -1514,16 +1514,16 @@ These scripts use that value to create a new environment variable **`COMPOSITE_2
* Load global environment variables defined in the **env.\[current profile\]** block.
* Load global environment setup scripts defined in the **env_scripts** attribute.
* **Per Task**
* Load environment files defined in the **env_files** attribute (relative paths are treated differently than global env_files).
* Setup **per task** internal environment variables (see [Global](#usage-env-global) section).
* Load environment files defined in the **env_files** attribute (relative paths are treated differently than global env_files).
* Load environment variables defined in the **env** block (same behavior as global env block).

During each step, variables can be reordered to ensure all dependencies are specified. The environmental variables will be interpolated before every task run.
During each step, variables can be reordered to ensure all dependencies are specified. The environment variables will be interpolated before every task run.

<a name="env-note-about-ordering"></a>
#### Note about Ordering

The ordering of environmental variables in `cargo-make` is not necessarily the same between definition and evaluation. `cargo-make` instead looks at the values and reorders variables depending on the variables they mention.
The ordering of environment variables in `cargo-make` is not necessarily the same between definition and evaluation. `cargo-make` instead looks at the values and reorders variables depending on the variables they mention.

This behavior has many benefits, like the ability to reference other variables freely or redefine them, in different scopes.

Expand Down Expand Up @@ -1576,7 +1576,7 @@ This is an extended example, which would not work using the naive implementation

<a name="usage-env-global"></a>
#### Global
In addition to manually setting environment variables, cargo-make will also automatically add a few environmental variables, which can be helpful when running task scripts, commands, conditions, and more.
In addition to manually setting environment variables, cargo-make will also automatically add a few environment variables, which can be helpful when running task scripts, commands, conditions, and more.

* **`CARGO_MAKE`** - Set to "true" to help sub-processes identify they are running from `cargo` make.
* **`CARGO_MAKE_TASK`** - Holds the name of the main task being executed.
Expand Down Expand Up @@ -4044,15 +4044,15 @@ install_crate = { rustup_component_name = "rust-src" }
<a name="e001"></a>
### E001: Environment Variables Cycle Detected

A cycle between different environmental variables has been detected;
A cycle between different environment variables has been detected;
This can happen during the merging of environments (at every loading step).
Due to reordering and to make sure that no circular references exist,
this error is emitted.

You can fix this issue, by looking at your env config, and seeing if at any point a circular reference could have occurred.
The error message mentions the environment variables that are likely candidates for the cause of the cycle.

Your best bet is to try to break the cycle, by creating a new environmental variable or use a static value multiple times.
Your best bet is to try to break the cycle, by creating a new environment variable or use a static value multiple times.
Cycles are usually caused by rapidly changing configs, forgotten and unused env variables or design problems,
even without cycle detection or no reordering this would likely cause hidden issues during
execution, as `cargo-make` would need to otherwise set instances to an empty value instead.
Expand All @@ -4061,7 +4061,7 @@ hidden and hard to debug issue.

> **Note:** Scripts are known to sometimes cause false-positives.
> In that case use the `depends_on` property, to explicitly tell `cargo-make`, which
> environmental variables should be considered a dependency instead of trying to guess from the script.
> environment variables should be considered a dependency instead of trying to guess from the script.

<a name="articles"></a>
Expand Down
52 changes: 48 additions & 4 deletions src/lib/command_test.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use super::*;
use crate::test;
use crate::types::Task;
use cfg_if::cfg_if;

#[test]
fn validate_exit_code_unable_to_fetch() {
Expand Down Expand Up @@ -94,8 +95,29 @@ fn run_no_command() {
#[test]
fn run_command() {
let mut task = Task::new();
task.command = Some("echo".to_string());
task.args = Some(vec!["test".to_string()]);

// echo is not a binary on windows, so cmd.exe's echo command is used
task.command = Some({
cfg_if! {
if #[cfg(target_os = "windows")] {
"cmd.exe".to_string()
} else {
"echo".to_string()
}
}
});
task.args = Some({
cfg_if! {
if #[cfg(target_os = "windows")] {
["/c", "echo", "test"]
.iter()
.map(ToString::to_string)
.collect()
} else {
vec!["test".to_string()]
}
}
});

let step = Step {
name: "test_command_output_env".to_string(),
Expand All @@ -111,8 +133,30 @@ fn run_command_for_toolchain() {
let toolchain = test::get_toolchain();

let mut task = Task::new();
task.command = Some("echo".to_string());
task.args = Some(vec!["test".to_string()]);

// echo is not a binary on windows, so cmd.exe's echo command is used
task.command = Some({
cfg_if! {
if #[cfg(target_os = "windows")] {
"cmd.exe".to_string()
} else {
"echo".to_string()
}
}
});
task.args = Some({
cfg_if! {
if #[cfg(target_os = "windows")] {
["/c", "echo", "test"]
.iter()
.map(ToString::to_string)
.collect()
} else {
vec!["test".to_string()]
}
}
});

task.toolchain = Some(toolchain.into());

let step = Step {
Expand Down
13 changes: 12 additions & 1 deletion src/lib/environment/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,7 @@ fn expand_env_for_script_runner_arguments(task: &mut Task) {
}

fn expand_env_for_arguments(task: &mut Task) {
//update args by replacing any env vars
// update args by replacing any env vars
let updated_args = match task.args {
Some(ref args) => {
let mut expanded_args = vec![];
Expand Down Expand Up @@ -886,3 +886,14 @@ pub(crate) fn expand_env(step: &Step) -> Step {
config,
}
}

pub(crate) fn expand_condition_script_runner_arguments(step: &Step) -> Step {
let mut modified_step = step.clone();

modified_step.config.condition_script_runner_args = step
.config
.condition_script_runner_args
.as_ref()
.map(|args| args.iter().map(|arg| expand_value(arg)).collect());
modified_step
}
33 changes: 31 additions & 2 deletions src/lib/environment/mod_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1157,7 +1157,7 @@ fn setup_cargo_home() {
#[ignore]
fn setup_cargo_home_overwrite() {
let path = Path::new("path");
envmnt::set("CARGO_HOME", path);
let old_cargo_home = envmnt::get_set("CARGO_HOME", path);

setup_cwd(None);

Expand All @@ -1168,7 +1168,8 @@ fn setup_cargo_home_overwrite() {
cargo_home
);

envmnt::remove("CARGO_HOME");
// Restore old CARGO_HOME value to avoid breaking other tests
envmnt::set_or_remove("CARGO_HOME", &old_cargo_home);
}

#[test]
Expand Down Expand Up @@ -1846,6 +1847,34 @@ fn expand_env_with_env_vars_and_empty_task_args() {
assert_eq!(args[3], "arg3-ENV1-ENV2".to_string());
}

#[test]
#[ignore]
fn expand_condition_script_runner_args() {
envmnt::set("TEST_ENV_EXPAND", "ENV_VALUE");

let mut task = Task::new();
task.condition_script_runner_args = Some(vec![
"sr1".to_string(),
"sr2-${TEST_ENV_EXPAND}-end".to_string(),
"sr3".to_string(),
]);
let step: Step = Step {
name: "test".to_string(),
config: task,
};
let updated_step = expand_condition_script_runner_arguments(&step);

assert_eq!(updated_step.name, "test".to_string());
assert_eq!(
updated_step.config.condition_script_runner_args.unwrap(),
vec![
"sr1".to_string(),
"sr2-ENV_VALUE-end".to_string(),
"sr3".to_string(),
]
);
}

#[test]
#[ignore]
fn set_current_task_meta_info_env_mixed() {
Expand Down
5 changes: 4 additions & 1 deletion src/lib/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,10 @@ pub(crate) fn run_task_with_options(
None => (),
};

if validate_condition(&flow_info, &step)? {
if validate_condition(
&flow_info,
&environment::expand_condition_script_runner_arguments(&step),
)? {
if logger::should_reduce_output(&flow_info) && step.config.script.is_none() {
debug!("Running Task: {}", &step.name);
} else {
Expand Down
Loading

0 comments on commit b639e7d

Please sign in to comment.