From 8bfc31f975f92bd1b411f66d45a728bd021574e9 Mon Sep 17 00:00:00 2001 From: Leon Schuermann Date: Fri, 5 Jul 2024 20:16:15 -0400 Subject: [PATCH 1/3] test(add): show config-include cannot be enabled in top-level config --- tests/testsuite/config_include.rs | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/testsuite/config_include.rs b/tests/testsuite/config_include.rs index d8f1dab972b..a5c46a56655 100644 --- a/tests/testsuite/config_include.rs +++ b/tests/testsuite/config_include.rs @@ -48,6 +48,37 @@ fn simple() { assert_eq!(gctx.get::("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(); + // On this commit, enabling `config-include` in the top-level + // configuration does not yet work. + assert_eq!(gctx.get::("key1").unwrap(), 1); + assert_eq!(gctx.get::("key2").unwrap(), 2); + assert_eq!(gctx.get::("key3").ok(), None); +} + #[cargo_test] fn works_with_cli() { write_config_at( From 45e5cb49641aae97669061296ea39ef5f9a6f349 Mon Sep 17 00:00:00 2001 From: Leon Schuermann Date: Fri, 5 Jul 2024 20:38:10 -0400 Subject: [PATCH 2/3] test(add): show config-include correctly merges unstable configs Co-authored-by: Weihang Lo --- tests/testsuite/config_include.rs | 133 ++++++++++++++++++++++++++++++ 1 file changed, 133 insertions(+) diff --git a/tests/testsuite/config_include.rs b/tests/testsuite/config_include.rs index a5c46a56655..846bd4cd41a 100644 --- a/tests/testsuite/config_include.rs +++ b/tests/testsuite/config_include.rs @@ -79,6 +79,139 @@ fn enable_in_unstable_config() { assert_eq!(gctx.get::("key3").ok(), None); } +#[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::("key1").unwrap(), 1); + assert_eq!(gctx.get::("key2").unwrap(), 2); + assert_eq!(gctx.get::("key3").unwrap(), 3); + assert_eq!(gctx.get::("key4").unwrap(), 4); + assert_eq!( + gctx.get::>("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(); + // On this commit, enabling `config-include` in the top-level + // configuration does not yet work. + assert_eq!(gctx.get::("key1").unwrap(), 1); + assert_eq!(gctx.get::("key2").unwrap(), 3); + assert_eq!(gctx.get::("key3").unwrap(), 3); + assert_eq!(gctx.get::("key4").ok(), None); + assert_eq!( + gctx.get::>("unstable.features").unwrap(), + vec!["3".to_string(), "1".to_string()] + ); +} + #[cargo_test] fn works_with_cli() { write_config_at( From 0e35bea6c3f54f9e9557bab186b7e1ea5f987814 Mon Sep 17 00:00:00 2001 From: Leon Schuermann Date: Fri, 5 Jul 2024 20:35:24 -0400 Subject: [PATCH 3/3] Allow enabling `config-include` feature in config 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. --- src/cargo/util/context/mod.rs | 7 +++++-- tests/testsuite/config_include.rs | 17 +++++++++-------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/cargo/util/context/mod.rs b/src/cargo/util/context/mod.rs index f275c23877a..c4fa1a5947d 100644 --- a/src/cargo/util/context/mod.rs +++ b/src/cargo/util/context/mod.rs @@ -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 @@ -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(()) } diff --git a/tests/testsuite/config_include.rs b/tests/testsuite/config_include.rs index 846bd4cd41a..502ac7e919f 100644 --- a/tests/testsuite/config_include.rs +++ b/tests/testsuite/config_include.rs @@ -72,11 +72,9 @@ fn enable_in_unstable_config() { let gctx = GlobalContextBuilder::new() .nightly_features_allowed(true) .build(); - // On this commit, enabling `config-include` in the top-level - // configuration does not yet work. assert_eq!(gctx.get::("key1").unwrap(), 1); assert_eq!(gctx.get::("key2").unwrap(), 2); - assert_eq!(gctx.get::("key3").ok(), None); + assert_eq!(gctx.get::("key3").unwrap(), 4); } #[cargo_test] @@ -200,15 +198,18 @@ fn mix_of_hierarchy_and_include_with_enable_in_unstable_config() { .cwd("foo") .nightly_features_allowed(true) .build(); - // On this commit, enabling `config-include` in the top-level - // configuration does not yet work. assert_eq!(gctx.get::("key1").unwrap(), 1); - assert_eq!(gctx.get::("key2").unwrap(), 3); + assert_eq!(gctx.get::("key2").unwrap(), 2); assert_eq!(gctx.get::("key3").unwrap(), 3); - assert_eq!(gctx.get::("key4").ok(), None); + assert_eq!(gctx.get::("key4").unwrap(), 4); assert_eq!( gctx.get::>("unstable.features").unwrap(), - vec!["3".to_string(), "1".to_string()] + vec![ + "4".to_string(), + "3".to_string(), + "2".to_string(), + "1".to_string() + ] ); }