Skip to content

Commit

Permalink
Auto merge of #13992 - epage:config, r=weihanglo
Browse files Browse the repository at this point in the history
fix(config): Ensure `--config net.git-fetch-with-cli=true` is respected

### What does this PR try to resolve?

#13479 changed the global context initialization order so that command line stuff is processed after we read some config.
This had a side effect of breaking `--config net.git-fetch-with-cli=true`.
I reverted the change to restore support for `--config`.

Fixes #13991

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

### Additional information

This reverts commit f525e1f.

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.
  • Loading branch information
bors committed May 31, 2024
2 parents 9ba3894 + 4ce2b61 commit 7a6fad0
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 62 deletions.
50 changes: 25 additions & 25 deletions src/cargo/util/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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(())
Expand Down
69 changes: 33 additions & 36 deletions tests/testsuite/cargo/z_help/stdout.term.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
9 changes: 8 additions & 1 deletion tests/testsuite/config_cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<String>("foo").unwrap(), "bar");
assert_eq!(gctx.net_config().unwrap().git_fetch_with_cli, Some(true));
}

#[cargo_test]
Expand All @@ -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::<i32>("build.jobs").unwrap(), 1);
assert_eq!(gctx.get::<String>("build.rustc").unwrap(), "cli");
assert_eq!(gctx.get::<bool>("term.verbose").unwrap(), true);
assert_eq!(gctx.net_config().unwrap().git_fetch_with_cli, Some(true));

// Setting both term.verbose and term.quiet is invalid and is tested
// in the run test suite.
Expand Down

0 comments on commit 7a6fad0

Please sign in to comment.