From 330831c83998f38b9182566d4d1249e8e2107db4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dawid=20Ci=C4=99=C5=BCarkiewicz?= <dpc@dpc.pw>
Date: Thu, 25 Aug 2022 12:15:34 -0700
Subject: [PATCH] Do not add home bin path to PATH if it's already there

This is to allow users to control the order via PATH if they so desire.

Fix #11020
---
 src/bin/cargo/main.rs            | 27 +++++++++--
 tests/testsuite/cargo_command.rs | 80 ++++++++++++++++++++++++++++++++
 2 files changed, 102 insertions(+), 5 deletions(-)

diff --git a/src/bin/cargo/main.rs b/src/bin/cargo/main.rs
index 0ec0cc1f6ad..dead961f44d 100644
--- a/src/bin/cargo/main.rs
+++ b/src/bin/cargo/main.rs
@@ -212,11 +212,28 @@ fn is_executable<P: AsRef<Path>>(path: P) -> bool {
 }
 
 fn search_directories(config: &Config) -> Vec<PathBuf> {
-    let mut dirs = vec![config.home().clone().into_path_unlocked().join("bin")];
-    if let Some(val) = env::var_os("PATH") {
-        dirs.extend(env::split_paths(&val));
-    }
-    dirs
+    let mut path_dirs = if let Some(val) = env::var_os("PATH") {
+        env::split_paths(&val).collect()
+    } else {
+        vec![]
+    };
+
+    let home_bin = config.home().clone().into_path_unlocked().join("bin");
+
+    // If any of that PATH elements contains `home_bin`, do not
+    // add it again. This is so that the users can control priority
+    // of it using PATH, while preserving the historical
+    // behavior of preferring it over system global directories even
+    // when not in PATH at all.
+    // See https://github.com/rust-lang/cargo/issues/11020 for details.
+    //
+    // Note: `p == home_bin` will ignore trailing slash, but we don't
+    // `canonicalize` the paths.
+    if !path_dirs.iter().any(|p| p == &home_bin) {
+        path_dirs.insert(0, home_bin);
+    };
+
+    path_dirs
 }
 
 fn init_git_transports(config: &Config) {
diff --git a/tests/testsuite/cargo_command.rs b/tests/testsuite/cargo_command.rs
index a49f56a4153..694b9a1090b 100644
--- a/tests/testsuite/cargo_command.rs
+++ b/tests/testsuite/cargo_command.rs
@@ -7,11 +7,14 @@ use std::path::{Path, PathBuf};
 use std::process::Stdio;
 use std::str;
 
+use cargo_test_support::basic_manifest;
+use cargo_test_support::paths::CargoPathExt;
 use cargo_test_support::registry::Package;
 use cargo_test_support::tools::echo_subcommand;
 use cargo_test_support::{
     basic_bin_manifest, cargo_exe, cargo_process, paths, project, project_in_home,
 };
+use cargo_util::paths::join_paths;
 
 fn path() -> Vec<PathBuf> {
     env::split_paths(&env::var_os("PATH").unwrap_or_default()).collect()
@@ -341,6 +344,83 @@ fn cargo_subcommand_env() {
         .run();
 }
 
+#[cargo_test]
+fn cargo_cmd_bins_vs_explicit_path() {
+    // Set up `cargo-foo` binary in two places: inside `$HOME/.cargo/bin` and outside of it
+    //
+    // Return paths to both places
+    fn set_up_cargo_foo() -> (PathBuf, PathBuf) {
+        let p = project()
+            .at("cargo-foo")
+            .file("Cargo.toml", &basic_manifest("cargo-foo", "1.0.0"))
+            .file(
+                "src/bin/cargo-foo.rs",
+                r#"fn main() { println!("INSIDE"); }"#,
+            )
+            .file(
+                "src/bin/cargo-foo2.rs",
+                r#"fn main() { println!("OUTSIDE"); }"#,
+            )
+            .build();
+        p.cargo("build").run();
+        let cargo_bin_dir = paths::home().join(".cargo/bin");
+        cargo_bin_dir.mkdir_p();
+        let root_bin_dir = paths::root().join("bin");
+        root_bin_dir.mkdir_p();
+        let exe_name = format!("cargo-foo{}", env::consts::EXE_SUFFIX);
+        fs::rename(p.bin("cargo-foo"), cargo_bin_dir.join(&exe_name)).unwrap();
+        fs::rename(p.bin("cargo-foo2"), root_bin_dir.join(&exe_name)).unwrap();
+
+        (root_bin_dir, cargo_bin_dir)
+    }
+
+    let (outside_dir, inside_dir) = set_up_cargo_foo();
+
+    // If `$CARGO_HOME/bin` is not in a path, prefer it over anything in `$PATH`.
+    //
+    // This is the historical behavior we don't want to break.
+    cargo_process("foo").with_stdout_contains("INSIDE").run();
+
+    // When `$CARGO_HOME/bin` is in the `$PATH`
+    // use only `$PATH` so the user-defined ordering is respected.
+    {
+        cargo_process("foo")
+            .env(
+                "PATH",
+                join_paths(&[&inside_dir, &outside_dir], "PATH").unwrap(),
+            )
+            .with_stdout_contains("INSIDE")
+            .run();
+
+        cargo_process("foo")
+            // Note: trailing slash
+            .env(
+                "PATH",
+                join_paths(&[inside_dir.join(""), outside_dir.join("")], "PATH").unwrap(),
+            )
+            .with_stdout_contains("INSIDE")
+            .run();
+
+        cargo_process("foo")
+            .env(
+                "PATH",
+                join_paths(&[&outside_dir, &inside_dir], "PATH").unwrap(),
+            )
+            .with_stdout_contains("OUTSIDE")
+            .run();
+
+        cargo_process("foo")
+            // Note: trailing slash
+            .env(
+                "PATH",
+                join_paths(&[outside_dir.join(""), inside_dir.join("")], "PATH").unwrap(),
+            )
+            .with_stdout_contains("OUTSIDE")
+            .run();
+    }
+}
+
+#[test]
 #[cargo_test]
 fn cargo_subcommand_args() {
     let p = echo_subcommand();