diff --git a/crates/ide-agent/src/native/mod.rs b/crates/ide-agent/src/native/mod.rs index 665e9a049..d7e34e77c 100644 --- a/crates/ide-agent/src/native/mod.rs +++ b/crates/ide-agent/src/native/mod.rs @@ -27,7 +27,8 @@ pub(crate) fn newest_install(command: &str) -> String { let path = ide_core::process::host_var_os("PATH") .or_else(|| ide_core::process::shell_env().find(|(k, _)| *k == "PATH").map(|(_, v)| v.to_owned())) .unwrap_or_default(); - newest_install_in(command, &path) + // Windows: read afresh, so a tool the user installed since the IDE started is found too. + newest_install_in(command, &ide_core::process::with_user_path(&path)) } fn newest_install_in(command: &str, search: &std::ffi::OsStr) -> String { diff --git a/crates/ide-core/Cargo.toml b/crates/ide-core/Cargo.toml index af0d6b992..a2a26d945 100644 --- a/crates/ide-core/Cargo.toml +++ b/crates/ide-core/Cargo.toml @@ -19,4 +19,4 @@ tokio-util.workspace = true tempfile.workspace = true [target.'cfg(windows)'.dependencies] -windows-sys = { version = "0.61", features = ["Win32_Globalization"] } +windows-sys = { version = "0.61", features = ["Win32_Globalization", "Win32_System_Registry", "Win32_Foundation"] } diff --git a/crates/ide-core/src/process.rs b/crates/ide-core/src/process.rs index f06ea74ff..85ee20f5d 100644 --- a/crates/ide-core/src/process.rs +++ b/crates/ide-core/src/process.rs @@ -237,10 +237,110 @@ pub fn find_executable(name: &str, search: &OsStr) -> Option find_executable_in(name, search, cfg!(windows), pathext.as_deref().and_then(|p| p.to_str())) } +/// The user's own `PATH` entries on Windows (`HKCU\Environment`), the part of the +/// merged `PATH` a process may not inherit — an IDE started before the variable was +/// edited, or as a different user, sees only the machine entries. `%VAR%` references +/// (`REG_EXPAND_SZ`) are expanded against the current environment. `None` elsewhere: +/// there a login shell already supplies the user's whole `PATH`. +#[cfg(windows)] +pub fn user_path() -> Option { + use windows_sys::Win32::Foundation::ERROR_SUCCESS; + use windows_sys::Win32::System::Registry::{ + HKEY, HKEY_CURRENT_USER, KEY_READ, RRF_RT_REG_EXPAND_SZ, RRF_RT_REG_SZ, RegCloseKey, RegGetValueW, + RegOpenKeyExW, + }; + + fn wide(s: &str) -> Vec { + s.encode_utf16().chain(std::iter::once(0)).collect() + } + + unsafe { + let mut key: HKEY = std::ptr::null_mut(); + if RegOpenKeyExW(HKEY_CURRENT_USER, wide("Environment").as_ptr(), 0, KEY_READ, &mut key) != ERROR_SUCCESS { + return None; + } + let read = || { + // Only `Path`; `REG_EXPAND_SZ` is accepted so `%USERPROFILE%\…` entries expand. + let mut kind = 0u32; + let mut len = 0u32; + let name = wide("Path"); + let status = RegGetValueW( + key, + std::ptr::null(), + name.as_ptr(), + RRF_RT_REG_SZ | RRF_RT_REG_EXPAND_SZ, + &mut kind, + std::ptr::null_mut(), + &mut len, + ); + if status != ERROR_SUCCESS || len == 0 { + return None; + } + let mut buf = vec![0u16; (len as usize).div_ceil(2)]; + let status = RegGetValueW( + key, + std::ptr::null(), + name.as_ptr(), + RRF_RT_REG_SZ | RRF_RT_REG_EXPAND_SZ, + &mut kind, + buf.as_mut_ptr() as *mut _, + &mut len, + ); + if status != ERROR_SUCCESS { + return None; + } + let text = String::from_utf16_lossy(&buf); + Some(text.trim_end_matches('\0').to_owned()) + }; + let value = read(); + RegCloseKey(key); + value.filter(|v| !v.trim().is_empty()).map(OsString::from) + } +} + +/// The user's own `PATH` entries; `None` outside Windows (see the Windows impl). +#[cfg(not(windows))] +pub fn user_path() -> Option { + None +} + +/// `path` followed by the [`user_path`] entries it lacks. +pub fn with_user_path(path: &OsStr) -> OsString { + user_path().map_or_else(|| path.to_owned(), |user| append_path_entries(path, &user)) +} + +fn append_path_entries(path: &OsStr, extra: &OsStr) -> OsString { + let mut dirs: Vec = std::env::split_paths(path).filter(|d| !d.as_os_str().is_empty()).collect(); + for dir in std::env::split_paths(extra).filter(|d| !d.as_os_str().is_empty()) { + if !dirs.contains(&dir) { + dirs.push(dir); + } + } + std::env::join_paths(dirs).unwrap_or_else(|_| path.to_owned()) +} + #[cfg(test)] mod tests { use super::*; + #[cfg(windows)] + #[test] + fn user_path_reads_the_current_users_path() { + // This host has a user `Path` only when the test account set one; either way the call + // must not fail, and a present value is a non-empty `;`-joined list of directories. + if let Some(path) = user_path() { + let dirs: Vec<_> = std::env::split_paths(&path).collect(); + assert!(!dirs.is_empty(), "user PATH should hold at least one directory"); + } + } + + #[test] + fn append_path_entries_keeps_the_order_and_adds_what_is_missing() { + let join = |dirs: &[&str]| std::env::join_paths(dirs).unwrap(); + assert_eq!(append_path_entries(&join(&["a", "b"]), &join(&["b", "c"])), join(&["a", "b", "c"])); + assert_eq!(append_path_entries(OsStr::new(""), &join(&["c"])), join(&["c"])); + } + #[test] fn shell_env_adds_only_what_the_process_lacks_and_the_overlay_wins() { set_shell_env(vec![("OPENIDEA_SHELL_ONLY_VAR".into(), "1".into()), ("PATH".into(), "/from/shell".into())]); diff --git a/crates/ide-core/src/tool_path.rs b/crates/ide-core/src/tool_path.rs index 51dc4ee4d..266f5bae0 100644 --- a/crates/ide-core/src/tool_path.rs +++ b/crates/ide-core/src/tool_path.rs @@ -17,7 +17,16 @@ pub fn search_path() -> OsString { let home = std::env::var_os("HOME").or_else(|| std::env::var_os("USERPROFILE")).map(PathBuf::from); let shell_path = login_env().and_then(|env| env.iter().find(|(k, _)| k == "PATH").map(|(_, v)| v.clone())); let shims: Vec = version_managers::managers().iter().map(|m| m.shims()).collect(); - let dirs = search_dirs(host_var_os("PATH").as_deref(), shell_path.as_deref(), home.as_deref(), &shims); + // The Windows user `PATH` may not reach a process started before it was set; append its + // entries after the inherited ones so missing user tools still resolve. + let user_path = crate::process::user_path(); + let dirs = search_dirs( + host_var_os("PATH").as_deref(), + shell_path.as_deref(), + user_path.as_deref(), + home.as_deref(), + &shims, + ); std::env::join_paths(dirs).unwrap_or_default() } @@ -56,10 +65,12 @@ fn is_executable(path: &Path) -> bool { fn search_dirs( process_path: Option<&OsStr>, shell_path: Option<&OsStr>, + user_path: Option<&OsStr>, home: Option<&Path>, shims: &[PathBuf], ) -> Vec { - let mut dirs: Vec = process_path.into_iter().chain(shell_path).flat_map(std::env::split_paths).collect(); + let mut dirs: Vec = + process_path.into_iter().chain(shell_path).chain(user_path).flat_map(std::env::split_paths).collect(); if let Some(home) = home { dirs.extend(USER_DIRS.iter().map(|d| home.join(d))); dirs.extend(nvm_bins(&home.join(".nvm/versions/node"))); @@ -172,6 +183,7 @@ mod tests { Some(OsStr::new("/usr/bin:.:node_modules/.bin:/opt/tools")), // nushell prints `$PATH` as is. Some(OsStr::new("$PATH:/opt/tools:/home/me/.asdf/shims")), + None, Some(Path::new("/home/me")), &shims, ); @@ -185,10 +197,28 @@ mod tests { let unique: HashSet<&PathBuf> = dirs.iter().collect(); assert_eq!(unique.len(), dirs.len()); // A `:` can't be in a `PATH` entry: those below such a home are left out. - let dirs = search_dirs(Some(OsStr::new("/usr/bin")), None, Some(Path::new("/home/a:b")), &[]); + let dirs = search_dirs(Some(OsStr::new("/usr/bin")), None, None, Some(Path::new("/home/a:b")), &[]); assert!(dirs.contains(&PathBuf::from("/usr/bin")) && !dirs.iter().any(|d| d.starts_with("/home"))); } + #[test] + fn search_dirs_append_user_path_entries_once_after_the_inherited_path() { + let (system, dup, usertool) = + if cfg!(windows) { ("C:/system", "C:/dup", "C:/usertool") } else { ("/system", "/dup", "/usertool") }; + let home = Path::new(if cfg!(windows) { "C:/home/me" } else { "/home/me" }); + let join = |dirs: &[&str]| std::env::join_paths(dirs.iter().map(PathBuf::from)).unwrap(); + let inherited = join(&[system, dup]); + let user = join(&[dup, usertool]); + let dirs = search_dirs(Some(inherited.as_os_str()), None, Some(user.as_os_str()), Some(home), &[]); + let at = |d: &str| dirs.iter().position(|p| p == Path::new(d)).unwrap(); + // User entries follow the inherited PATH, de-duplicated, ahead of the fallbacks. + assert_eq!(dirs.iter().filter(|d| *d == Path::new(dup)).count(), 1); + assert!(at(dup) < at(usertool)); + // And before this platform's home/system fallbacks. + let fallback = dirs.iter().position(|d| d == &home.join(USER_DIRS[0])).unwrap(); + assert!(at(usertool) < fallback); + } + #[test] fn nvm_versions_newest_first() { let dir = std::env::temp_dir().join(format!("openidea-nvm-{}", std::process::id())); diff --git a/crates/ide-tools/src/lib.rs b/crates/ide-tools/src/lib.rs index 317eea3ee..d5cd36c0d 100644 --- a/crates/ide-tools/src/lib.rs +++ b/crates/ide-tools/src/lib.rs @@ -151,11 +151,13 @@ fn managed_bin_dir(root: &Path, spec: &ToolSpec, platform: Platform) -> Option