Skip to content

Commit 9df4728

Browse files
committed
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. Tested by preparing two different `cargo-foo` scripts in `$HOME/.cargo/bin` and `$HOME/bin`: ``` > env PATH="/usr/bin/:$HOME/bin:$HOME/.cargo/bin" ./target/debug/cargo foo Inside ~/bin/ > env PATH="$HOME/.cargo/bin:/usr/bin/:$HOME/bin" ./target/debug/cargo foo Inside ~/.cargo/bin/ > env PATH="/usr/bin/:$HOME/bin" ./target/debug/cargo foo Inside ~/.cargo/bin/ ``` and trailing slash: ``` > env PATH="$HOME/.cargo/bin/:/usr/bin/:$HOME/bin" ./target/debug/cargo foo Inside ~/.cargo/bin/ > env PATH="/usr/bin/:$HOME/bin:$HOME/.cargo/bin/" ./target/debug/cargo foo Inside ~/bin/ ``` Fix rust-lang#11020
1 parent d705fb3 commit 9df4728

File tree

2 files changed

+108
-4
lines changed

2 files changed

+108
-4
lines changed

src/bin/cargo/main.rs

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -220,10 +220,30 @@ fn is_executable<P: AsRef<Path>>(path: P) -> bool {
220220
}
221221

222222
fn search_directories(config: &Config) -> Vec<PathBuf> {
223-
let mut dirs = vec![config.home().clone().into_path_unlocked().join("bin")];
224-
if let Some(val) = env::var_os("PATH") {
225-
dirs.extend(env::split_paths(&val));
226-
}
223+
let path_dirs = if let Some(val) = env::var_os("PATH") {
224+
env::split_paths(&val).collect()
225+
} else {
226+
vec![]
227+
};
228+
229+
let home_bin = config.home().clone().into_path_unlocked().join("bin");
230+
231+
// If any of that PATH elements contains `home_bin`, do not
232+
// add it again. This is so that the users can control priority
233+
// of it using PATH, while preserving the historical
234+
// behavior of preferring it over system global directories even
235+
// when not in PATH at all.
236+
// See https://github.com/rust-lang/cargo/issues/11020 for details.
237+
//
238+
// Note: `p == home_bin` will ignore trailing slash, but we don't
239+
// `canonicalize` the paths.
240+
let mut dirs = if path_dirs.iter().any(|p| p == &home_bin) {
241+
vec![]
242+
} else {
243+
vec![home_bin]
244+
};
245+
246+
dirs.extend(path_dirs);
227247
dirs
228248
}
229249

tests/testsuite/cargo_command.rs

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ use std::path::{Path, PathBuf};
77
use std::process::Stdio;
88
use std::str;
99

10+
use cargo_test_support::basic_manifest;
11+
use cargo_test_support::paths::CargoPathExt;
1012
use cargo_test_support::registry::Package;
1113
use cargo_test_support::tools::echo_subcommand;
1214
use cargo_test_support::{
@@ -341,6 +343,88 @@ fn cargo_subcommand_env() {
341343
.run();
342344
}
343345

346+
/// Set up `cargo-foo` binary in two places: inside `$HOME/.cargo/bin` and outside of it
347+
///
348+
/// Return paths to both places
349+
fn set_up_cargo_foo() -> (PathBuf, PathBuf) {
350+
let p = project()
351+
.at("cargo-foo")
352+
.file("Cargo.toml", &basic_manifest("cargo-foo", "1.0.0"))
353+
.file(
354+
"src/bin/cargo-foo.rs",
355+
r#"fn main() { println!("INSIDE"); }"#,
356+
)
357+
.file(
358+
"src/bin/cargo-foo2.rs",
359+
r#"fn main() { println!("OUTSIDE"); }"#,
360+
)
361+
.build();
362+
p.cargo("build").run();
363+
let cargo_bin_dir = paths::home().join(".cargo/bin");
364+
cargo_bin_dir.mkdir_p();
365+
let root_bin_dir = paths::root().join("bin");
366+
root_bin_dir.mkdir_p();
367+
let exe_name = format!("cargo-foo{}", env::consts::EXE_SUFFIX);
368+
fs::rename(p.bin("cargo-foo"), cargo_bin_dir.join(&exe_name)).unwrap();
369+
fs::rename(p.bin("cargo-foo2"), root_bin_dir.join(&exe_name)).unwrap();
370+
371+
(root_bin_dir, cargo_bin_dir)
372+
}
373+
374+
// If `$CARGO_HOME/bin` is not in a path, prefer it over anything in `$PATH`.
375+
//
376+
// This is the historical behavior, we don't want to break.
377+
#[cfg(unix)]
378+
#[cargo_test]
379+
fn cargo_cmd_bin_prefer_over_path_if_not_included() {
380+
let (_outside_dir, _inside_dir) = set_up_cargo_foo();
381+
382+
cargo_process("foo").with_stdout_contains("INSIDE").run();
383+
}
384+
385+
// When `$CARGO_HOME/bin` is in the `$PATH`
386+
// use only `$PATH` so the user-defined ordering is respected.
387+
#[cfg(unix)]
388+
#[cargo_test]
389+
fn cargo_cmd_bin_respect_path_when_included() {
390+
let (outside_dir, inside_dir) = set_up_cargo_foo();
391+
392+
cargo_process("foo")
393+
.env(
394+
"PATH",
395+
format!("{}:{}", inside_dir.display(), outside_dir.display()),
396+
)
397+
.with_stdout_contains("INSIDE")
398+
.run();
399+
400+
cargo_process("foo")
401+
// Note: trailing slash
402+
.env(
403+
"PATH",
404+
format!("{}:{}/", inside_dir.display(), outside_dir.display()),
405+
)
406+
.with_stdout_contains("INSIDE")
407+
.run();
408+
409+
cargo_process("foo")
410+
.env(
411+
"PATH",
412+
format!("{}:{}", outside_dir.display(), inside_dir.display()),
413+
)
414+
.with_stdout_contains("OUTSIDE")
415+
.run();
416+
417+
cargo_process("foo")
418+
// Note: trailing slash
419+
.env(
420+
"PATH",
421+
format!("{}/:{}", outside_dir.display(), inside_dir.display()),
422+
)
423+
.with_stdout_contains("OUTSIDE")
424+
.run();
425+
}
426+
427+
#[test]
344428
#[cargo_test]
345429
fn cargo_subcommand_args() {
346430
let p = echo_subcommand();

0 commit comments

Comments
 (0)