Skip to content

Commit

Permalink
Auto merge of #14196 - lschuermann:dev/config-include-enable-in-confi…
Browse files Browse the repository at this point in the history
…g, r=weihanglo

Allow enabling `config-include` feature in config

### What does this PR try to resolve?

Prior to this change, it is not possible to enable the unstable `config-include` flag from within a config file itself. This is due to the fact that, while cargo does reload configuration files if this flag is set, it does not parse the top-level configuration file's unstable flags before checking whether this flag is present.

This commit forces cargo to load unstable features before this check, and if the flag is present, re-loads configuration files with `config-include` enabled.

Addresses #7723 (comment).

### How should we test and review this PR?

This PR adds a testcase for this behavior to cargo's testsuite. In general, it can be replicated with a config similar to that in the testcase, or as illustrated in #7723 (comment).

### Additional information

~I'm not sure whether this is the right(TM) fix, or whether the second `self.load_unstable_flags_from_config()?;` at the bottom of `configure()` would still be required. I have a feeling it might not be, as `reload_rooted_at` also calls out to `load_unstable_flags_from_config`, to collect flags from any newly included configs. If that is not called, no other file was loaded, and thus there should not be a need to re-load unstable flags.~

Resolved: #14196 (comment)
  • Loading branch information
bors committed Jul 6, 2024
2 parents 83a5ff4 + 0e35bea commit c7be9e4
Show file tree
Hide file tree
Showing 2 changed files with 170 additions and 2 deletions.
7 changes: 5 additions & 2 deletions src/cargo/util/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1035,6 +1035,11 @@ impl GlobalContext {
self.cli_config = Some(cli_config.iter().map(|s| s.to_string()).collect());
self.merge_cli_args()?;
}

// Load the unstable flags from config file here first, as the config
// file itself may enable inclusion of other configs. In that case, we
// want to re-load configs with includes enabled:
self.load_unstable_flags_from_config()?;
if self.unstable_flags.config_include {
// If the config was already loaded (like when fetching the
// `[alias]` table), it was loaded with includes disabled because
Expand Down Expand Up @@ -1091,8 +1096,6 @@ impl GlobalContext {
let cli_target_dir = target_dir.as_ref().map(|dir| Filesystem::new(dir.clone()));
self.target_dir = cli_target_dir;

self.load_unstable_flags_from_config()?;

Ok(())
}

Expand Down
165 changes: 165 additions & 0 deletions tests/testsuite/config_include.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,171 @@ fn simple() {
assert_eq!(gctx.get::<i32>("key3").unwrap(), 4);
}

#[cargo_test]
fn enable_in_unstable_config() {
// config-include enabled in the unstable config table:
write_config_at(
".cargo/config.toml",
"
include = 'other.toml'
key1 = 1
key2 = 2
[unstable]
config-include = true
",
);
write_config_at(
".cargo/other.toml",
"
key2 = 3
key3 = 4
",
);
let gctx = GlobalContextBuilder::new()
.nightly_features_allowed(true)
.build();
assert_eq!(gctx.get::<i32>("key1").unwrap(), 1);
assert_eq!(gctx.get::<i32>("key2").unwrap(), 2);
assert_eq!(gctx.get::<i32>("key3").unwrap(), 4);
}

#[cargo_test]
fn mix_of_hierarchy_and_include() {
write_config_at(
"foo/.cargo/config.toml",
"
include = 'other.toml'
key1 = 1
# also make sure unstable flags merge in the correct order
[unstable]
features = ['1']
",
);
write_config_at(
"foo/.cargo/other.toml",
"
key1 = 2
key2 = 2
[unstable]
features = ['2']
",
);
write_config_at(
".cargo/config.toml",
"
include = 'other.toml'
key1 = 3
key2 = 3
key3 = 3
[unstable]
features = ['3']
",
);
write_config_at(
".cargo/other.toml",
"
key1 = 4
key2 = 4
key3 = 4
key4 = 4
[unstable]
features = ['4']
",
);
let gctx = GlobalContextBuilder::new()
.unstable_flag("config-include")
.cwd("foo")
.nightly_features_allowed(true)
.build();
assert_eq!(gctx.get::<i32>("key1").unwrap(), 1);
assert_eq!(gctx.get::<i32>("key2").unwrap(), 2);
assert_eq!(gctx.get::<i32>("key3").unwrap(), 3);
assert_eq!(gctx.get::<i32>("key4").unwrap(), 4);
assert_eq!(
gctx.get::<Vec<String>>("unstable.features").unwrap(),
vec![
"4".to_string(),
"3".to_string(),
"2".to_string(),
"1".to_string()
]
);
}

#[cargo_test]
fn mix_of_hierarchy_and_include_with_enable_in_unstable_config() {
// `mix_of_hierarchy_and_include`, but with the config-include
// feature itself enabled in the unstable config table:
write_config_at(
"foo/.cargo/config.toml",
"
include = 'other.toml'
key1 = 1
# also make sure unstable flags merge in the correct order
[unstable]
features = ['1']
config-include = true
",
);
write_config_at(
"foo/.cargo/other.toml",
"
key1 = 2
key2 = 2
[unstable]
features = ['2']
",
);
write_config_at(
".cargo/config.toml",
"
include = 'other.toml'
key1 = 3
key2 = 3
key3 = 3
[unstable]
features = ['3']
",
);
write_config_at(
".cargo/other.toml",
"
key1 = 4
key2 = 4
key3 = 4
key4 = 4
[unstable]
features = ['4']
",
);
let gctx = GlobalContextBuilder::new()
.cwd("foo")
.nightly_features_allowed(true)
.build();
assert_eq!(gctx.get::<i32>("key1").unwrap(), 1);
assert_eq!(gctx.get::<i32>("key2").unwrap(), 2);
assert_eq!(gctx.get::<i32>("key3").unwrap(), 3);
assert_eq!(gctx.get::<i32>("key4").unwrap(), 4);
assert_eq!(
gctx.get::<Vec<String>>("unstable.features").unwrap(),
vec![
"4".to_string(),
"3".to_string(),
"2".to_string(),
"1".to_string()
]
);
}

#[cargo_test]
fn works_with_cli() {
write_config_at(
Expand Down

0 comments on commit c7be9e4

Please sign in to comment.