Skip to content

Commit

Permalink
Do not add home bin path to PATH if it's already there
Browse files Browse the repository at this point in the history
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
  • Loading branch information
dpc committed Aug 29, 2022
1 parent 4ed54ce commit 4f81a49
Show file tree
Hide file tree
Showing 2 changed files with 115 additions and 5 deletions.
28 changes: 24 additions & 4 deletions src/bin/cargo/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,10 +220,30 @@ 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));
}
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
}

Expand Down
92 changes: 91 additions & 1 deletion tests/testsuite/cargo_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

use std::env;
use std::fs;
use std::io::Read;
use std::io::{Read, Write};
use std::os::unix::fs::PermissionsExt;
use std::path::{Path, PathBuf};
use std::process::Stdio;
use std::str;
Expand Down Expand Up @@ -341,6 +342,95 @@ 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 cargo_home_dir = paths::home().join(".cargo");
let bin_dir = cargo_home_dir.join("bin");
fs::create_dir_all(&bin_dir).unwrap();
fs::File::create(&bin_dir.join("cargo-foo"))
.unwrap()
.write_all(
br#"!/usr/bin/env bash
echo INSIDE
"#,
)
.unwrap();

fs::File::create(&cargo_home_dir.join("cargo-foo"))
.unwrap()
.write_all(
br#"!/usr/bin/env bash
echo OUTSIDE
"#,
)
.unwrap();

fs::set_permissions(
cargo_home_dir.join("cargo-foo"),
fs::Permissions::from_mode(0o555),
)
.unwrap();
fs::set_permissions(bin_dir.join("cargo-foo"), fs::Permissions::from_mode(0o555)).unwrap();

(cargo_home_dir, 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.
#[cfg(unix)]
#[cargo_test]
fn cargo_cmd_bin_prefer_over_path_if_not_included() {
let (_cargo_home_dir, _bin_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 (cargo_home_dir, bin_dir) = set_up_cargo_foo();

cargo_process("foo")
.env(
"PATH",
format!("{}:{}", bin_dir.display(), cargo_home_dir.display()),
)
.with_stdout_contains("INSIDE")
.run();

cargo_process("foo")
// Note: trailing slash
.env(
"PATH",
format!("{}:{}/", bin_dir.display(), cargo_home_dir.display()),
)
.with_stdout_contains("INSIDE")
.run();

cargo_process("foo")
.env(
"PATH",
format!("{}:{}", cargo_home_dir.display(), bin_dir.display()),
)
.with_stdout_contains("OUTSIDE")
.run();

cargo_process("foo")
// Note: trailing slash
.env(
"PATH",
format!("{}/:{}", cargo_home_dir.display(), bin_dir.display()),
)
.with_stdout_contains("OUTSIDE")
.run();
}

#[test]
#[cargo_test]
fn cargo_subcommand_args() {
let p = echo_subcommand();
Expand Down

0 comments on commit 4f81a49

Please sign in to comment.