Skip to content

Commit

Permalink
config reading: use same_file for suppressing "both files" warning
Browse files Browse the repository at this point in the history
This is 100% reliable on Unix, and better on Windows.

(In this commit I avoid reindenting things to make review easier; the
formatting will be fixed in the next commit.)

Fixes #13667
  • Loading branch information
ijackson committed Apr 24, 2024
1 parent 16f83db commit b398b9c
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 21 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ rand.workspace = true
regex.workspace = true
rusqlite.workspace = true
rustfix.workspace = true
same-file.workspace = true
semver.workspace = true
serde = { workspace = true, features = ["derive"] }
serde-untagged.workspace = true
Expand Down
18 changes: 7 additions & 11 deletions src/cargo/util/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1542,36 +1542,32 @@ impl GlobalContext {
//
// Instead, if we got an error, we should bail, but perhaps
// that might be too much risk of being a breaking change.
if possible.exists() {
if let Ok(possible_handle) = same_file::Handle::from_path(&possible) {
if warn {
if let Ok(possible_with_extension_handle) =
same_file::Handle::from_path(&possible_with_extension)
{
// We don't want to print a warning if the version
// without the extension is just a symlink to the version
// WITH an extension, which people may want to do to
// support multiple Cargo versions at once and not
// get a warning.
let skip_warning = if let Ok(target_path) = fs::read_link(&possible) {
target_path == possible_with_extension
} else {
false
};

if !skip_warning {
if possible_with_extension.exists() {
if possible_handle != possible_with_extension_handle {
self.shell().warn(format!(
"both `{}` and `{}` exist. Using `{}`",
possible.display(),
possible_with_extension.display(),
possible.display()
))?;
} else {
}
} else {
self.shell().warn(format!(
"`{}` is deprecated in favor of `{filename_without_extension}.toml`",
possible.display(),
))?;
self.shell().note(
format!("if you need to support cargo 1.38 or earlier, you can symlink `{filename_without_extension}` to `{filename_without_extension}.toml`"),
)?;
}
}
}

Expand Down
12 changes: 2 additions & 10 deletions tests/testsuite/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,12 +348,8 @@ f1 = 1
assert_eq!(gctx.get::<Option<i32>>("foo.f1").unwrap(), Some(1));

// It should NOT have warned for the symlink.
// But, currently it does!
let output = read_output(gctx);
let expected = "\
[WARNING] both `[..]/.cargo/config` and `[..]/.cargo/config.toml` exist. Using `[..]/.cargo/config`
";
assert_match(expected, &output);
assert_match("", &output);
}

#[cargo_test]
Expand All @@ -378,13 +374,9 @@ f1 = 1
assert_eq!(gctx.get::<Option<i32>>("foo.f1").unwrap(), Some(1));

// It should NOT have warned for this situation.
// But, currently it does!
let output = read_output(gctx);
assert_match("", &output);
let expected = "\
[WARNING] both `[..]/.cargo/config` and `[..]/.cargo/config.toml` exist. Using `[..]/.cargo/config`
";
assert_match(expected, &output);
assert_match("", &output);
}

#[cargo_test]
Expand Down

0 comments on commit b398b9c

Please sign in to comment.