diff --git a/src/bin/cargo/main.rs b/src/bin/cargo/main.rs index 1619b487b2ff..abcb87efc41f 100644 --- a/src/bin/cargo/main.rs +++ b/src/bin/cargo/main.rs @@ -220,10 +220,30 @@ fn is_executable>(path: P) -> bool { } fn search_directories(config: &Config) -> Vec { - 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)); - } + let 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. + let mut dirs = if path_dirs.iter().any(|p| p == &home_bin) { + vec![] + } else { + vec![home_bin] + }; + + dirs.extend(path_dirs); dirs } diff --git a/tests/testsuite/cargo_command.rs b/tests/testsuite/cargo_command.rs index a49f56a41539..d50c86a211a4 100644 --- a/tests/testsuite/cargo_command.rs +++ b/tests/testsuite/cargo_command.rs @@ -7,6 +7,8 @@ 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::{ @@ -341,6 +343,87 @@ fn cargo_subcommand_env() { .run(); } +/// 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) +} + +// 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_test] +fn cargo_cmd_bin_prefer_over_path_if_not_included() { + let (_outside_dir, _inside_dir) = set_up_cargo_foo(); + + 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. +#[cfg(unix)] +#[cargo_test] +fn cargo_cmd_bin_respect_path_when_included() { + let (outside_dir, inside_dir) = set_up_cargo_foo(); + + cargo_process("foo") + .env( + "PATH", + format!("{}:{}", inside_dir.display(), outside_dir.display()), + ) + .with_stdout_contains("INSIDE") + .run(); + + cargo_process("foo") + // Note: trailing slash + .env( + "PATH", + format!("{}:{}/", inside_dir.display(), outside_dir.display()), + ) + .with_stdout_contains("INSIDE") + .run(); + + cargo_process("foo") + .env( + "PATH", + format!("{}:{}", outside_dir.display(), inside_dir.display()), + ) + .with_stdout_contains("OUTSIDE") + .run(); + + cargo_process("foo") + // Note: trailing slash + .env( + "PATH", + format!("{}/:{}", outside_dir.display(), inside_dir.display()), + ) + .with_stdout_contains("OUTSIDE") + .run(); +} + +#[test] #[cargo_test] fn cargo_subcommand_args() { let p = echo_subcommand();