From d53880db274b43689fa6ad67299fe7254ea203b9 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 31 May 2024 14:56:12 -0500 Subject: [PATCH 1/3] test(config): Show current git-fetch-with-cli behavior --- tests/testsuite/config_cli.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/testsuite/config_cli.rs b/tests/testsuite/config_cli.rs index 1b90d58b918..84068bb65d5 100644 --- a/tests/testsuite/config_cli.rs +++ b/tests/testsuite/config_cli.rs @@ -11,8 +11,12 @@ use std::{collections::HashMap, fs}; #[cargo_test] fn basic() { // Simple example. - let gctx = GlobalContextBuilder::new().config_arg("foo='bar'").build(); + let gctx = GlobalContextBuilder::new() + .config_arg("foo='bar'") + .config_arg("net.git-fetch-with-cli=true") + .build(); assert_eq!(gctx.get::("foo").unwrap(), "bar"); + assert_eq!(gctx.net_config().unwrap().git_fetch_with_cli, None); } #[cargo_test] @@ -39,13 +43,16 @@ fn cli_priority() { .env("CARGO_BUILD_JOBS", "2") .env("CARGO_BUILD_RUSTC", "env") .env("CARGO_TERM_VERBOSE", "false") + .env("CARGO_NET_GIT_FETCH_WITH_CLI", "false") .config_arg("build.jobs=1") .config_arg("build.rustc='cli'") .config_arg("term.verbose=true") + .config_arg("net.git-fetch-with-cli=true") .build(); assert_eq!(gctx.get::("build.jobs").unwrap(), 1); assert_eq!(gctx.get::("build.rustc").unwrap(), "cli"); assert_eq!(gctx.get::("term.verbose").unwrap(), true); + assert_eq!(gctx.net_config().unwrap().git_fetch_with_cli, Some(false)); // Setting both term.verbose and term.quiet is invalid and is tested // in the run test suite. From 37647308e4d0b9635fe19dc3a1e8dd71c093dc23 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 31 May 2024 14:45:43 -0500 Subject: [PATCH 2/3] Revert "fix(context): Configure Shell before emitting messages" This reverts commit f525e1f383fc0d0a7adbdfb1bac6d6228ae79b33. This removes color control from warnings for unstable features. For some reason this removed color support from `cargo -Zhelp` in the tests but I can't reproduce it locally. The most important thing was getting the config fix in. There are two follow ups - Can we have the config working *and* color? - Why did this fail for this field and not the others we already had tests for? I ran out my immediate time box for looking into these. Fixes #13991 --- src/cargo/util/context/mod.rs | 50 +++++++------- tests/testsuite/cargo/z_help/stdout.term.svg | 71 ++++++++++---------- tests/testsuite/config_cli.rs | 4 +- 3 files changed, 61 insertions(+), 64 deletions(-) diff --git a/src/cargo/util/context/mod.rs b/src/cargo/util/context/mod.rs index 914d57f2138..a1d655fe164 100644 --- a/src/cargo/util/context/mod.rs +++ b/src/cargo/util/context/mod.rs @@ -1020,6 +1020,31 @@ impl GlobalContext { unstable_flags: &[String], cli_config: &[String], ) -> CargoResult<()> { + for warning in self + .unstable_flags + .parse(unstable_flags, self.nightly_features_allowed)? + { + self.shell().warn(warning)?; + } + if !unstable_flags.is_empty() { + // store a copy of the cli flags separately for `load_unstable_flags_from_config` + // (we might also need it again for `reload_rooted_at`) + self.unstable_flags_cli = Some(unstable_flags.to_vec()); + } + if !cli_config.is_empty() { + self.cli_config = Some(cli_config.iter().map(|s| s.to_string()).collect()); + self.merge_cli_args()?; + } + 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 + // the `unstable_flags` hadn't been set up, yet. Any values + // fetched before this step will not process includes, but that + // should be fine (`[alias]` is one of the only things loaded + // before configure). This can be removed when stabilized. + self.reload_rooted_at(self.cwd.clone())?; + } + // Ignore errors in the configuration files. We don't want basic // commands like `cargo version` to error out due to config file // problems. @@ -1066,31 +1091,6 @@ impl GlobalContext { let cli_target_dir = target_dir.as_ref().map(|dir| Filesystem::new(dir.clone())); self.target_dir = cli_target_dir; - for warning in self - .unstable_flags - .parse(unstable_flags, self.nightly_features_allowed)? - { - self.shell().warn(warning)?; - } - if !unstable_flags.is_empty() { - // store a copy of the cli flags separately for `load_unstable_flags_from_config` - // (we might also need it again for `reload_rooted_at`) - self.unstable_flags_cli = Some(unstable_flags.to_vec()); - } - if !cli_config.is_empty() { - self.cli_config = Some(cli_config.iter().map(|s| s.to_string()).collect()); - self.merge_cli_args()?; - } - 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 - // the `unstable_flags` hadn't been set up, yet. Any values - // fetched before this step will not process includes, but that - // should be fine (`[alias]` is one of the only things loaded - // before configure). This can be removed when stabilized. - self.reload_rooted_at(self.cwd.clone())?; - } - self.load_unstable_flags_from_config()?; Ok(()) diff --git a/tests/testsuite/cargo/z_help/stdout.term.svg b/tests/testsuite/cargo/z_help/stdout.term.svg index cde7a959502..e476b29ce3f 100644 --- a/tests/testsuite/cargo/z_help/stdout.term.svg +++ b/tests/testsuite/cargo/z_help/stdout.term.svg @@ -2,13 +2,10 @@