Skip to content

Commit

Permalink
Auto merge of #14579 - linyihai:issue-14560, r=epage
Browse files Browse the repository at this point in the history
fix(config): Don't double-warn about `$CARGO_HOME/config`

### What does this PR try to resolve?

The core requirements for this bug are
- You have a `$CARGO_HOME/.config`
- Your project is inside of `$HOME`

We have a check to make sure we don't double-walk `$CARGO/config` but
that check is *after* we warn about there being no `.toml` extension.

To fix this, we just need to move that check outside of the file lookup.
This required changing the `seen` check from checking whether walked the
config file to checking if we've walked the config dir.  As we only have
one file per directory, this should work.

Fixes #14560

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

test commit added the test, fix commit fixed the issue.

### Additional information
  • Loading branch information
bors committed Sep 26, 2024
2 parents 01e1ab5 + fe917f2 commit 4b81a83
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 6 deletions.
11 changes: 6 additions & 5 deletions src/cargo/util/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1581,20 +1581,21 @@ impl GlobalContext {
where
F: FnMut(&Path) -> CargoResult<()>,
{
let mut stash: HashSet<PathBuf> = HashSet::new();
let mut seen_dir = HashSet::new();

for current in paths::ancestors(pwd, self.search_stop_path.as_deref()) {
if let Some(path) = self.get_file_path(&current.join(".cargo"), "config", true)? {
let config_root = current.join(".cargo");
if let Some(path) = self.get_file_path(&config_root, "config", true)? {
walk(&path)?;
stash.insert(path);
}
seen_dir.insert(config_root);
}

// Once we're done, also be sure to walk the home directory even if it's not
// in our history to be sure we pick up that standard location for
// information.
if let Some(path) = self.get_file_path(home, "config", true)? {
if !stash.contains(&path) {
if !seen_dir.contains(home) {
if let Some(path) = self.get_file_path(home, "config", true)? {
walk(&path)?;
}
}
Expand Down
22 changes: 21 additions & 1 deletion tests/testsuite/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use cargo::CargoResult;
use cargo_test_support::compare::assert_e2e;
use cargo_test_support::prelude::*;
use cargo_test_support::str;
use cargo_test_support::{paths, project, symlink_supported, t};
use cargo_test_support::{paths, project, project_in_home, symlink_supported, t};
use cargo_util_schemas::manifest::TomlTrimPaths;
use cargo_util_schemas::manifest::TomlTrimPathsValue;
use cargo_util_schemas::manifest::{self as cargo_toml, TomlDebugInfo, VecStringOrBool as VSOB};
Expand Down Expand Up @@ -299,6 +299,26 @@ f1 = 1
assert_e2e().eq(&output, expected);
}

#[cargo_test]
fn home_config_works_without_extension() {
write_config_at(
paths::cargo_home().join("config"),
"\
[foo]
f1 = 1
",
);
let p = project_in_home("foo").file("src/lib.rs", "").build();

p.cargo("-vV")
.with_stderr_data(str![[r#"
[WARNING] `[ROOT]/home/.cargo/config` is deprecated in favor of `config.toml`
[NOTE] if you need to support cargo 1.38 or earlier, you can symlink `config` to `config.toml`
"#]])
.run();
}

#[cargo_test]
fn config_ambiguous_filename_symlink_doesnt_warn() {
// Windows requires special permissions to create symlinks.
Expand Down

0 comments on commit 4b81a83

Please sign in to comment.