From c3d29dc842141fa962c375888523d1092eb7e2e3 Mon Sep 17 00:00:00 2001 From: arookieofc <2128194521@qq.com> Date: Sat, 31 Oct 2026 04:22:58 +0800 Subject: [PATCH] fix(sdk): list every Node on the search path, not only the first one (#502) Detection stopped at the first `node` on the search path. Over SSH the minimal `PATH` puts `/usr/bin` ahead of `~/.local/bin`, so a Node installed there was never listed and refreshing could not help. Every `node` on the search path is now listed, and installs keep nvm, Volta or fnm as their source. Co-authored-by: a-rookie-of-C-language <186315048+a-rookie-of-C-language@users.noreply.github.com> --- crates/ide-core/src/tool_path.rs | 6 ++++++ crates/ide-sdk/src/discover.rs | 32 +++++++++++++++++++++++++------- crates/ide-sdk/src/exec.rs | 11 +++++++++++ crates/ide-sdk/src/tests.rs | 28 ++++++++++++++++++++++++++++ 4 files changed, 70 insertions(+), 7 deletions(-) diff --git a/crates/ide-core/src/tool_path.rs b/crates/ide-core/src/tool_path.rs index 14760d56..51dc4ee4 100644 --- a/crates/ide-core/src/tool_path.rs +++ b/crates/ide-core/src/tool_path.rs @@ -98,6 +98,12 @@ const USER_DIRS: &[&str] = &[ const SYSTEM_DIRS: &[&str] = &["/opt/homebrew/bin", "/usr/local/go/bin", "/usr/local/bin", "/usr/bin", "/bin"]; +/// Whether `dir` is one of the system's own bin directories, which [`search_path`] adds +/// whatever `PATH` holds. +pub fn is_system_dir(dir: &Path) -> bool { + SYSTEM_DIRS.iter().any(|d| Path::new(d) == dir) +} + /// The `bin` directories of nvm's Node versions in `dir`, newest first. fn nvm_bins(dir: &Path) -> Vec { let mut versions: Vec<(Vec, PathBuf)> = std::fs::read_dir(dir) diff --git a/crates/ide-sdk/src/discover.rs b/crates/ide-sdk/src/discover.rs index 5aea9ea4..a9dec42e 100644 --- a/crates/ide-sdk/src/discover.rs +++ b/crates/ide-sdk/src/discover.rs @@ -1,6 +1,6 @@ //! Finding installed interpreters and toolchains. -use crate::exec::{query, search_path, which}; +use crate::exec::{executables_in, query, search_path, which}; use crate::{Sdk, SdkKind, bin_dir, exe, home_dir, pinned, venv_of}; use ide_core::version_managers::{self, Tool}; use std::collections::HashSet; @@ -236,11 +236,32 @@ fn subdirs(dir: &Path) -> Vec { // ------------------------------------------------------------------ node +/// Node's own version managers, by where they keep installs below the home directory. +const NODE_MANAGERS: &[(&str, &str)] = + &[(".nvm/versions/node", "nvm"), (".volta/tools/image/node", "volta"), (".local/share/fnm/node-versions", "fnm")]; + +/// Where a Node on the search path comes from: its version manager, "system" for one +/// of the system's own bin directories, "path" for anything else the user put on `PATH`. +pub(crate) fn node_source(path: &Path, home: Option<&Path>) -> &'static str { + let managed = home.and_then(|home| { + NODE_MANAGERS.iter().find(|(dir, _)| path.starts_with(home.join(dir))).map(|(_, manager)| *manager) + }); + managed.or_else(|| version_managers::manager_of(path)).unwrap_or_else(|| { + if path.parent().is_some_and(ide_core::tool_path::is_system_dir) { "system" } else { "path" } + }) +} + fn node() -> Vec { let mut found: Vec<(PathBuf, &'static str)> = Vec::new(); - // A mise / asdf shim picks the version by directory: its installs are listed instead. - if let Some(p) = which("node").filter(|p| !version_managers::is_shim(p)) { - let source = version_managers::manager_of(&p).unwrap_or("path"); + // Every Node on the search path, not only the first one `which` finds: an SSH + // session's minimal PATH puts /usr/bin ahead of the user's own ~/.local/bin, so + // /usr/bin/node would be the only install listed (#440). + for p in executables_in("node", std::env::split_paths(&search_path())) { + // A mise / asdf shim picks the version by directory: its installs are listed instead. + if version_managers::is_shim(&p) { + continue; + } + let source = node_source(&p, home_dir().as_deref()); found.push((p, source)); } if let Some(home) = home_dir() { @@ -257,9 +278,6 @@ fn node() -> Vec { for (v, manager) in version_managers::installs(Tool::Node).into_iter().rev() { found.push((v.join("bin").join(exe("node")), manager)); } - for p in ["/opt/homebrew/bin/node", "/usr/local/bin/node", "/usr/bin/node"] { - found.push((PathBuf::from(p), "system")); - } let found = dedupe_by_real_path(found); par_map(&found, |(path, source)| { let version = query(path, &["--version"], VERSION_TIMEOUT)?; diff --git a/crates/ide-sdk/src/exec.rs b/crates/ide-sdk/src/exec.rs index aef10b05..c7d036b7 100644 --- a/crates/ide-sdk/src/exec.rs +++ b/crates/ide-sdk/src/exec.rs @@ -26,6 +26,17 @@ pub(crate) fn which(name: &str) -> Option { ide_core::process::find_executable(name, &path) } +/// Every install of `name` on `dirs`, one per directory holding it, in `dirs` order (the +/// first is what `which` returns). Tool detection lists them all: a login shell's `PATH` +/// and an SSH exec channel's minimal one disagree on which comes first (#440). +pub(crate) fn executables_in(name: &str, dirs: impl Iterator) -> Vec { + let pathext = ide_core::process::host_var_os("PATHEXT"); + let names = ide_core::process::executable_names(name, cfg!(windows), pathext.as_deref().and_then(|p| p.to_str())); + dirs.filter(|d| !d.as_os_str().is_empty()) + .filter_map(|dir| names.iter().map(|n| dir.join(n)).find(|p| p.is_file())) + .collect() +} + /// Runs `program args` and returns trimmed stdout (stderr when stdout is empty) /// on success; `None` on failure or after `timeout`. pub(crate) fn query(program: &Path, args: &[&str], timeout: Duration) -> Option { diff --git a/crates/ide-sdk/src/tests.rs b/crates/ide-sdk/src/tests.rs index 7792808a..59adefe0 100644 --- a/crates/ide-sdk/src/tests.rs +++ b/crates/ide-sdk/src/tests.rs @@ -939,3 +939,31 @@ fn gradle_settings_precedence() { assert_eq!(gradle_command(root, root, &settings), if local { expected } else { wrapper.clone() }); } } + +/// The reporter of #440 has Node 26 in `~/.local/bin` and an 18 in `/usr/bin`; an SSH +/// exec channel's PATH puts `/usr/bin` first, so only the system's Node was listed. +#[test] +fn node_lists_every_install_on_the_search_path_not_only_the_first() { + let tmp = tempfile::tempdir().unwrap(); + let (system, user) = (tmp.path().join("usr-bin"), tmp.path().join("local-bin")); + for dir in [&system, &user] { + std::fs::create_dir(dir).unwrap(); + std::fs::write(dir.join(exe("node")), "").unwrap(); + } + let found = crate::exec::executables_in("node", [system.clone(), user.clone()].into_iter()); + assert_eq!(found, [system.join(exe("node")), user.join(exe("node"))]); + // A directory without the executable contributes nothing. + let found = crate::exec::executables_in("node", [tmp.path().join("empty"), user.clone()].into_iter()); + assert_eq!(found, [user.join(exe("node"))]); +} + +#[test] +fn node_on_the_search_path_keeps_its_version_manager_as_source() { + let home = Path::new("/home/me"); + let source = |p: &str| crate::discover::node_source(Path::new(p), Some(home)); + assert_eq!(source("/home/me/.nvm/versions/node/v24.0.0/bin/node"), "nvm"); + assert_eq!(source("/home/me/.volta/tools/image/node/24.0.0/bin/node"), "volta"); + assert_eq!(source("/home/me/.local/share/fnm/node-versions/v24/installation/bin/node"), "fnm"); + assert_eq!(source("/home/me/.local/bin/node"), "path"); + assert_eq!(source("/usr/bin/node"), "system"); +}