Skip to content

Add .toml file extension restriction for -Zconfig-include #12298

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 21, 2023
Merged

Add .toml file extension restriction for -Zconfig-include #12298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3f82ec7
feat: check if `config-include` file ends with `.toml`
weihanglo Jun 21, 2023
33b0a24
test(config-include): assert config merged in DFS
weihanglo Jun 21, 2023
26b7725
doc(unstable): merge precedence of `-Zconfig-include`
weihanglo Jun 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/cargo/util/config/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1273,6 +1273,16 @@ impl Config {
return Ok(Vec::new());
}
};

for (path, abs_path, def) in &includes {
if abs_path.extension() != Some(OsStr::new("toml")) {
bail!(
"expected a config include path ending with `.toml`, \
but found `{path}` from `{def}`",
)
}
}

Ok(includes)
}

Expand Down
33 changes: 18 additions & 15 deletions src/doc/src/reference/unstable.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,27 +450,30 @@ cargo check --keep-going -Z unstable-options
### config-include
* Tracking Issue: [#7723](https://github.com/rust-lang/cargo/issues/7723)

This feature requires the `-Zconfig-include` command-line option.

The `include` key in a config file can be used to load another config file. It
takes a string for a path to another file relative to the config file, or a
list of strings. It requires the `-Zconfig-include` command-line option.
takes a string for a path to another file relative to the config file, or an
array of config file paths. Only path ending with `.toml` is accepted.

```toml
# .cargo/config
include = '../../some-common-config.toml'
```

The config values are first loaded from the include path, and then the config
file's own values are merged on top of it.
# a path ending with `.toml`
include = "path/to/mordor.toml"

This can be paired with [config-cli](#config-cli) to specify a file to load
from the command-line. Pass a path to a config file as the argument to
`--config`:

```console
cargo +nightly -Zunstable-options -Zconfig-include --config somefile.toml build
# or an array of paths
include = ["frodo.toml", "samwise.toml"]
```

CLI paths are relative to the current working directory.
Unlike other config values, the merge behavior of the `include` key is
different. When a config file contains an `include` key:

1. The config values are first loaded from the `include` path.
* If the value of the `include` key is an array of paths, the config values
are loaded and merged from left to right for each path.
* Recurse this step if the config values from the `include` path also
contain an `include` key.
2. Then, the config file's own values are merged on top of the config
from the `include` path.

### target-applies-to-host
* Original Pull Request: [#9322](https://github.com/rust-lang/cargo/pull/9322)
Expand Down
112 changes: 77 additions & 35 deletions tests/testsuite/config_include.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ use cargo_test_support::{no_such_file_err_msg, project};
#[cargo_test]
fn gated() {
// Requires -Z flag.
write_config("include='other'");
write_config("include='other.toml'");
write_config_at(
".cargo/other",
".cargo/other.toml",
"
othervalue = 1
",
Expand All @@ -25,13 +25,13 @@ fn simple() {
write_config_at(
".cargo/config",
"
include = 'other'
include = 'other.toml'
key1 = 1
key2 = 2
",
);
write_config_at(
".cargo/other",
".cargo/other.toml",
"
key2 = 3
key3 = 4
Expand Down Expand Up @@ -84,39 +84,63 @@ fn works_with_cli() {
}

#[cargo_test]
fn left_to_right() {
// How it merges multiple includes.
fn left_to_right_bottom_to_top() {
// How it merges multiple nested includes.
write_config_at(
".cargo/config",
"
include = ['one', 'two']
primary = 1
include = ['left-middle.toml', 'right-middle.toml']
top = 1
",
);
write_config_at(
".cargo/right-middle.toml",
"
include = 'right-bottom.toml'
top = 0
right-middle = 0
",
);
write_config_at(
".cargo/one",
".cargo/right-bottom.toml",
"
one = 1
primary = 2
top = -1
right-middle = -1
right-bottom = -1
",
);
write_config_at(
".cargo/two",
".cargo/left-middle.toml",
"
two = 2
primary = 3
include = 'left-bottom.toml'
top = -2
right-middle = -2
right-bottom = -2
left-middle = -2
",
);
write_config_at(
".cargo/left-bottom.toml",
"
top = -3
right-middle = -3
right-bottom = -3
left-middle = -3
left-bottom = -3
",
);
let config = ConfigBuilder::new().unstable_flag("config-include").build();
assert_eq!(config.get::<i32>("primary").unwrap(), 1);
assert_eq!(config.get::<i32>("one").unwrap(), 1);
assert_eq!(config.get::<i32>("two").unwrap(), 2);
assert_eq!(config.get::<i32>("top").unwrap(), 1);
assert_eq!(config.get::<i32>("right-middle").unwrap(), 0);
assert_eq!(config.get::<i32>("right-bottom").unwrap(), -1);
assert_eq!(config.get::<i32>("left-middle").unwrap(), -2);
assert_eq!(config.get::<i32>("left-bottom").unwrap(), -3);
}

#[cargo_test]
fn missing_file() {
// Error when there's a missing file.
write_config("include='missing'");
write_config("include='missing.toml'");
let config = ConfigBuilder::new()
.unstable_flag("config-include")
.build_err();
Expand All @@ -127,10 +151,10 @@ fn missing_file() {
could not load Cargo configuration

Caused by:
failed to load config include `missing` from `[..]/.cargo/config`
failed to load config include `missing.toml` from `[..]/.cargo/config`

Caused by:
failed to read configuration file `[..]/.cargo/missing`
failed to read configuration file `[..]/.cargo/missing.toml`

Caused by:
{}",
Expand All @@ -139,12 +163,30 @@ Caused by:
);
}

#[cargo_test]
fn wrong_file_extension() {
// Error when it doesn't end with `.toml`.
write_config("include='config.png'");
let config = ConfigBuilder::new()
.unstable_flag("config-include")
.build_err();
assert_error(
config.unwrap_err(),
"\
could not load Cargo configuration

Caused by:
expected a config include path ending with `.toml`, but found `config.png` from `[..]/.cargo/config`
",
);
}

#[cargo_test]
fn cycle() {
// Detects a cycle.
write_config_at(".cargo/config", "include='one'");
write_config_at(".cargo/one", "include='two'");
write_config_at(".cargo/two", "include='config'");
write_config_at(".cargo/config.toml", "include='one.toml'");
write_config_at(".cargo/one.toml", "include='two.toml'");
write_config_at(".cargo/two.toml", "include='config.toml'");
let config = ConfigBuilder::new()
.unstable_flag("config-include")
.build_err();
Expand All @@ -154,16 +196,16 @@ fn cycle() {
could not load Cargo configuration

Caused by:
failed to load config include `one` from `[..]/.cargo/config`
failed to load config include `one.toml` from `[..]/.cargo/config.toml`

Caused by:
failed to load config include `two` from `[..]/.cargo/one`
failed to load config include `two.toml` from `[..]/.cargo/one.toml`

Caused by:
failed to load config include `config` from `[..]/.cargo/two`
failed to load config include `config.toml` from `[..]/.cargo/two.toml`

Caused by:
config `include` cycle detected with path `[..]/.cargo/config`",
config `include` cycle detected with path `[..]/.cargo/config.toml`",
);
}

Expand All @@ -178,10 +220,10 @@ fn cli_include() {
bar = 2
",
);
write_config_at(".cargo/config-foo", "foo = 2");
write_config_at(".cargo/config-foo.toml", "foo = 2");
let config = ConfigBuilder::new()
.unstable_flag("config-include")
.config_arg("include='.cargo/config-foo'")
.config_arg("include='.cargo/config-foo.toml'")
.build();
assert_eq!(config.get::<i32>("foo").unwrap(), 2);
assert_eq!(config.get::<i32>("bar").unwrap(), 2);
Expand Down Expand Up @@ -209,7 +251,7 @@ fn cli_include_failed() {
// Error message when CLI include fails to load.
let config = ConfigBuilder::new()
.unstable_flag("config-include")
.config_arg("include='foobar'")
.config_arg("include='foobar.toml'")
.build_err();
assert_error(
config.unwrap_err(),
Expand All @@ -218,10 +260,10 @@ fn cli_include_failed() {
failed to load --config include

Caused by:
failed to load config include `foobar` from `--config cli option`
failed to load config include `foobar.toml` from `--config cli option`

Caused by:
failed to read configuration file `[..]/foobar`
failed to read configuration file `[..]/foobar.toml`

Caused by:
{}",
Expand All @@ -235,14 +277,14 @@ fn cli_merge_failed() {
// Error message when CLI include merge fails.
write_config("foo = ['a']");
write_config_at(
".cargo/other",
".cargo/other.toml",
"
foo = 'b'
",
);
let config = ConfigBuilder::new()
.unstable_flag("config-include")
.config_arg("include='.cargo/other'")
.config_arg("include='.cargo/other.toml'")
.build_err();
// Maybe this error message should mention it was from an include file?
assert_error(
Expand All @@ -251,7 +293,7 @@ fn cli_merge_failed() {
failed to merge --config key `foo` into `[..]/.cargo/config`

Caused by:
failed to merge config value from `[..]/.cargo/other` into `[..]/.cargo/config`: \
failed to merge config value from `[..]/.cargo/other.toml` into `[..]/.cargo/config`: \
expected array, but found string",
);
}
Expand Down