From b398b9ccd58b252cede1f6789f582ec6045de64d Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 24 Apr 2024 10:31:56 +0100 Subject: [PATCH] config reading: use same_file for suppressing "both files" warning 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 --- Cargo.toml | 1 + src/cargo/util/context/mod.rs | 18 +++++++----------- tests/testsuite/config.rs | 12 ++---------- 3 files changed, 10 insertions(+), 21 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d0f2afb3972e..38d0f5bb4e0c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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 diff --git a/src/cargo/util/context/mod.rs b/src/cargo/util/context/mod.rs index 6da9569bb140..a7b127b8af2c 100644 --- a/src/cargo/util/context/mod.rs +++ b/src/cargo/util/context/mod.rs @@ -1542,28 +1542,25 @@ 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(), @@ -1571,7 +1568,6 @@ impl GlobalContext { 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`"), )?; - } } } diff --git a/tests/testsuite/config.rs b/tests/testsuite/config.rs index 33f360c04471..822db4cbf860 100644 --- a/tests/testsuite/config.rs +++ b/tests/testsuite/config.rs @@ -348,12 +348,8 @@ f1 = 1 assert_eq!(gctx.get::>("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] @@ -378,13 +374,9 @@ f1 = 1 assert_eq!(gctx.get::>("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]