Skip to content

Append Windows "bin" directory to PATH #4249

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 14, 2025
Merged

Append Windows "bin" directory to PATH #4249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 88 additions & 3 deletions src/env_var.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@ use crate::process::Process;

pub const RUST_RECURSION_COUNT_MAX: u32 = 20;

pub(crate) fn prepend_path(
pub(crate) fn insert_path(
name: &str,
prepend: Vec<PathBuf>,
append: Option<PathBuf>,
cmd: &mut Command,
process: &Process,
) {
let old_value = process.var_os(name);
let parts = if let Some(ref v) = old_value {
let mut parts = if let Some(v) = &old_value {
let mut tail = env::split_paths(v).collect::<VecDeque<_>>();
for path in prepend.into_iter().rev() {
if !tail.contains(&path) {
Expand All @@ -26,6 +27,12 @@ pub(crate) fn prepend_path(
prepend.into()
};

if let Some(path) = append {
if !parts.contains(&path) {
parts.push_back(path);
}
}

if let Ok(new_value) = env::join_paths(parts) {
cmd.env(name, new_value);
}
Expand Down Expand Up @@ -77,7 +84,7 @@ mod tests {
let path_z = PathBuf::from(z);
path_entries.push(path_z);

prepend_path("PATH", path_entries, &mut cmd, &tp.process);
insert_path("PATH", path_entries, None, &mut cmd, &tp.process);
let envs: Vec<_> = cmd.get_envs().collect();

assert_eq!(
Expand All @@ -99,4 +106,82 @@ mod tests {
),]
);
}

#[test]
fn append_unique_path() {
let mut vars = HashMap::new();
vars.env(
"PATH",
env::join_paths(["/home/a/.cargo/bin", "/home/b/.cargo/bin"].iter()).unwrap(),
);
let tp = TestProcess::with_vars(vars);
#[cfg(windows)]
let _path_guard = RegistryGuard::new(&USER_PATH).unwrap();

#[track_caller]
fn check(tp: &TestProcess, path_entries: Vec<PathBuf>, append: &str, expected: &[&str]) {
let mut cmd = Command::new("test");

insert_path(
"PATH",
path_entries,
Some(append.into()),
&mut cmd,
&tp.process,
);
let envs: Vec<_> = cmd.get_envs().collect();

assert_eq!(
envs,
&[(
OsStr::new("PATH"),
Some(env::join_paths(expected.iter()).unwrap().as_os_str())
),]
);
}

check(
&tp,
Vec::new(),
"/home/z/.cargo/bin",
&[
"/home/a/.cargo/bin",
"/home/b/.cargo/bin",
"/home/z/.cargo/bin",
],
);
check(
&tp,
Vec::new(),
"/home/a/.cargo/bin",
&["/home/a/.cargo/bin", "/home/b/.cargo/bin"],
);
check(
&tp,
Vec::new(),
"/home/b/.cargo/bin",
&["/home/a/.cargo/bin", "/home/b/.cargo/bin"],
);
check(
&tp,
Vec::from(["/home/c/.cargo/bin".into()]),
"/home/c/.cargo/bin",
&[
"/home/c/.cargo/bin",
"/home/a/.cargo/bin",
"/home/b/.cargo/bin",
],
);
check(
&tp,
Vec::from(["/home/c/.cargo/bin".into()]),
"/home/z/.cargo/bin",
&[
"/home/c/.cargo/bin",
"/home/a/.cargo/bin",
"/home/b/.cargo/bin",
"/home/z/.cargo/bin",
],
);
}
}
58 changes: 39 additions & 19 deletions src/toolchain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ impl<'a> Toolchain<'a> {
new_path.push(PathBuf::from("/usr/lib"));
}

env_var::prepend_path(sysenv::LOADER_PATH, new_path, cmd, self.cfg.process);
env_var::insert_path(sysenv::LOADER_PATH, new_path, None, cmd, self.cfg.process);

// Prepend CARGO_HOME/bin to the PATH variable so that we're sure to run
// cargo/rustc via the proxy bins. There is no fallback case for if the
Expand All @@ -228,30 +228,50 @@ impl<'a> Toolchain<'a> {
path_entries.push(cargo_home.join("bin"));
}

// Historically rustup included the bin directory in PATH to
// work around some bugs (see
// https://github.com/rust-lang/rustup/pull/3178 for more
// information). This shouldn't be needed anymore, and it causes
// On Windows, we append the "bin" directory to PATH by default.
// Windows loads DLLs from PATH and the "bin" directory contains DLLs
// that proc macros and other tools not in the sysroot use.
// It's appended rather than prepended so that the exe files in "bin"
// do not take precedence over anything else in PATH.
//
// Historically rustup prepended the bin directory in PATH but doing so causes
// problems because calling tools recursively (like `cargo
// +nightly metadata` from within a cargo subcommand). The
// recursive call won't work because it is not executing the
// proxy, so the `+` toolchain override doesn't work.
// See: https://github.com/rust-lang/rustup/pull/3178
//
// The RUSTUP_WINDOWS_PATH_ADD_BIN env var was added to opt-in to
// testing the fix. The default is now off, but this is left here
// just in case there are problems. Consider removing in the
// future if it doesn't seem necessary.
#[cfg(target_os = "windows")]
if self
.cfg
.process
.var_os("RUSTUP_WINDOWS_PATH_ADD_BIN")
.is_some_and(|s| s == "1")
{
path_entries.push(self.path.join("bin"));
}
// This behaviour was then changed to not add the bin directory at all.
// But this caused another set of problems due to the sysroot DLLs
// not being found by the loader, e.g. for proc macros.
// See: https://github.com/rust-lang/rustup/issues/3825
//
// Which is how we arrived at the current default described above.
//
// The `RUSTUP_WINDOWS_PATH_ADD_BIN` environment variable allows
// users to opt-in to one of the old behaviours in case the new
// default causes any new issues.
//
// FIXME: The `RUSTUP_WINDOWS_PATH_ADD_BIN` environment variable can
// be removed once we're confident that the default behaviour works.
let append = if cfg!(target_os = "windows") {
let add_bin = self.cfg.process.var("RUSTUP_WINDOWS_PATH_ADD_BIN");
match add_bin.as_deref().unwrap_or("append") {
// Don't add to PATH at all
"0" => None,
// Prepend to PATH
"1" => {
path_entries.push(self.path.join("bin"));
None
}
// Append to PATH (the default)
_ => Some(self.path.join("bin")),
}
} else {
None
};

env_var::prepend_path("PATH", path_entries, cmd, self.cfg.process);
env_var::insert_path("PATH", path_entries, append, cmd, self.cfg.process);
}

/// Infallible function that describes the version of rustc in an installed distribution
Expand Down