From aeec88c77ffcb5c3c201f771d5079ebdb199ea88 Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Thu, 4 Jun 2026 13:34:42 +0530 Subject: [PATCH] fix(installer): symlink bundled node/npm into command bin dir for FHS root installs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root installs on Linux (FHS layout, #15608) put the `hermes` command in `/usr/local/bin` (on PATH) but symlinked the bundled node/npm/npx into `~/.local/bin`, which isn't on PATH for a stock root shell. `node`/`npm` were 'command not found' and `hermes dashboard` failed with 'npm is not available' because its build-on-demand fallback couldn't find npm. Fix: `install_node()` now symlinks into `get_command_link_dir()` — the same helper the `hermes` command link already uses — so node/npm/npx land wherever the command does (`/usr/local/bin` on FHS root, `~/.local/bin` otherwise, `$PREFIX/bin` on Termux). Non-root and Termux installs are unchanged. Also fixes: - `scripts/lib/node-bootstrap.sh`: adds `_nb_get_link_dir()` mirroring the same root/Termux/user logic for the standalone bootstrap path (used by `hermes update`, TUI node bootstrap, etc.) - `hermes_cli/uninstall.py`: `remove_node_symlinks()` now checks all candidate directories (`~/.local/bin`, `/usr/local/bin`, `$PREFIX/bin`) so root FHS uninstalls don't leave orphan symlinks Regression from #15608, which created the FHS path for the command but left `install_node` pointed at the legacy user-local dir. --- hermes_cli/uninstall.py | 70 ++++++++++++------- scripts/install.sh | 14 ++-- scripts/lib/node-bootstrap.sh | 23 ++++-- .../test_uninstall_node_symlinks.py | 34 +++++++++ 4 files changed, 105 insertions(+), 36 deletions(-) diff --git a/hermes_cli/uninstall.py b/hermes_cli/uninstall.py index 430abd47b..2c666ddd2 100644 --- a/hermes_cli/uninstall.py +++ b/hermes_cli/uninstall.py @@ -9,6 +9,7 @@ Provides options for: import os import shutil import subprocess +import sys from pathlib import Path from hermes_constants import get_hermes_home @@ -117,45 +118,60 @@ def remove_wrapper_script(): return removed +def _node_symlink_candidate_dirs() -> "list[Path]": + """Directories where the installer may have placed node/npm/npx symlinks.""" + dirs: list[Path] = [Path.home() / ".local" / "bin"] + # Root FHS installs put links in /usr/local/bin. + if sys.platform == "linux": + dirs.append(Path("/usr/local/bin")) + # Termux installs put links in $PREFIX/bin. + prefix = os.environ.get("PREFIX", "") + if prefix and "com.termux" in prefix: + dirs.append(Path(prefix) / "bin") + return dirs + + def remove_node_symlinks(hermes_home: Path) -> list: - """Remove the node/npm/npx symlinks the installer drops in ~/.local/bin. + """Remove the node/npm/npx symlinks the installer placed on PATH. The POSIX installer (``scripts/install.sh`` / ``scripts/lib/node-bootstrap.sh``) - creates:: + symlinks node/npm/npx into the same directory as the ``hermes`` command: - ~/.local/bin/node -> $HERMES_HOME/node/bin/node - ~/.local/bin/npm -> $HERMES_HOME/node/bin/npm - ~/.local/bin/npx -> $HERMES_HOME/node/bin/npx + - ``/usr/local/bin/`` on root FHS installs (Linux, uid 0) + - ``$PREFIX/bin/`` on Termux + - ``~/.local/bin/`` otherwise (the common non-root case) - and prepends ``~/.local/bin`` to PATH, so these shadow an existing Node - manager such as nvm. Symmetrically remove them on uninstall, but *only* - when the link still resolves into this Hermes home's ``node`` directory. - A link the user has since repointed at nvm (or anything else outside - Hermes) is left untouched so we never break unrelated tooling. + We check all candidate directories so that uninstall works regardless of + how the install was done (e.g. a root FHS install that placed links in + ``/usr/local/bin``, or an older install that used ``~/.local/bin`` before + the FHS fix). Only symlinks that resolve into this Hermes home's ``node`` + directory are removed — links the user has repointed elsewhere (nvm, fnm, + etc.) are left untouched. """ node_dir = (hermes_home / "node").resolve() removed = [] for name in ("node", "npm", "npx"): - link = Path.home() / ".local" / "bin" / name - try: - # Only act on symlinks — never delete a real binary the user put here. - if not link.is_symlink(): - continue + for bin_dir in _node_symlink_candidate_dirs(): + link = bin_dir / name + try: + # Only act on symlinks — never delete a real binary the user put here. + if not link.is_symlink(): + continue - # Resolve the link target and confirm it points into our node dir. - # os.readlink + manual join handles broken (dangling) links too; - # Path.resolve() on a dangling link still returns the target path. - target = Path(os.readlink(link)) - if not target.is_absolute(): - target = (link.parent / target) - target = target.resolve() + # Resolve the link target and confirm it points into our node dir. + # os.readlink + manual join handles broken (dangling) links too; + # Path.resolve() on a dangling link still returns the target path. + target = Path(os.readlink(link)) + if not target.is_absolute(): + target = (link.parent / target) + target = target.resolve() - if target == node_dir or node_dir in target.parents: - link.unlink() - removed.append(link) - except Exception as e: - log_warn(f"Could not remove {link}: {e}") + if target == node_dir or node_dir in target.parents: + link.unlink() + removed.append(link) + except Exception as e: + log_warn(f"Could not remove {link}: {e}") return removed diff --git a/scripts/install.sh b/scripts/install.sh index 7f278e2c3..c5d6732e4 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -836,16 +836,20 @@ install_node() { return 0 fi - # Place into ~/.hermes/node/ and symlink binaries to ~/.local/bin/ + # Place into ~/.hermes/node/ and symlink binaries into the same bin dir + # the hermes command uses (get_command_link_dir): /usr/local/bin for root + # FHS installs, $PREFIX/bin on Termux, ~/.local/bin otherwise. rm -rf "$HERMES_HOME/node" mkdir -p "$HERMES_HOME" mv "$extracted_dir" "$HERMES_HOME/node" rm -rf "$tmp_dir" - mkdir -p "$HOME/.local/bin" - ln -sf "$HERMES_HOME/node/bin/node" "$HOME/.local/bin/node" - ln -sf "$HERMES_HOME/node/bin/npm" "$HOME/.local/bin/npm" - ln -sf "$HERMES_HOME/node/bin/npx" "$HOME/.local/bin/npx" + local node_link_dir + node_link_dir="$(get_command_link_dir)" + mkdir -p "$node_link_dir" + ln -sf "$HERMES_HOME/node/bin/node" "$node_link_dir/node" + ln -sf "$HERMES_HOME/node/bin/npm" "$node_link_dir/npm" + ln -sf "$HERMES_HOME/node/bin/npx" "$node_link_dir/npx" export PATH="$HERMES_HOME/node/bin:$PATH" diff --git a/scripts/lib/node-bootstrap.sh b/scripts/lib/node-bootstrap.sh index 9eadc479d..02e568733 100644 --- a/scripts/lib/node-bootstrap.sh +++ b/scripts/lib/node-bootstrap.sh @@ -44,6 +44,19 @@ _nb_is_termux() { [ -n "${TERMUX_VERSION:-}" ] || [[ "${PREFIX:-}" == *"com.termux/files/usr"* ]] } +# Where to symlink node/npm/npx so they land on PATH. +# Mirrors get_command_link_dir() from install.sh: root FHS → /usr/local/bin, +# Termux → $PREFIX/bin, otherwise ~/.local/bin. +_nb_get_link_dir() { + if _nb_is_termux && [ -n "${PREFIX:-}" ]; then + echo "$PREFIX/bin" + elif [ "$(id -u)" = 0 ] && [ "$(uname -s)" = "Linux" ]; then + echo "/usr/local/bin" + else + echo "$HOME/.local/bin" + fi +} + _nb_node_major() { local v v=$(node --version 2>/dev/null | sed 's/^v//' | cut -d. -f1) @@ -187,10 +200,12 @@ _nb_install_bundled_node() { mv "$extracted" "$HERMES_HOME/node" rm -rf "$tmp" - mkdir -p "$HOME/.local/bin" - ln -sf "$HERMES_HOME/node/bin/node" "$HOME/.local/bin/node" - ln -sf "$HERMES_HOME/node/bin/npm" "$HOME/.local/bin/npm" - ln -sf "$HERMES_HOME/node/bin/npx" "$HOME/.local/bin/npx" + local _link_dir + _link_dir="$(_nb_get_link_dir)" + mkdir -p "$_link_dir" + ln -sf "$HERMES_HOME/node/bin/node" "$_link_dir/node" + ln -sf "$HERMES_HOME/node/bin/npm" "$_link_dir/npm" + ln -sf "$HERMES_HOME/node/bin/npx" "$_link_dir/npx" export PATH="$HERMES_HOME/node/bin:$PATH" _nb_have_modern_node || return 1 diff --git a/tests/hermes_cli/test_uninstall_node_symlinks.py b/tests/hermes_cli/test_uninstall_node_symlinks.py index 316e6d646..e6d62d4c7 100644 --- a/tests/hermes_cli/test_uninstall_node_symlinks.py +++ b/tests/hermes_cli/test_uninstall_node_symlinks.py @@ -130,3 +130,37 @@ def test_only_some_links_present(fake_home): assert (local_bin / "node").exists() assert not (local_bin / "npm").is_symlink() assert not (local_bin / "npx").is_symlink() + + +def test_removes_fhs_symlinks_in_usr_local_bin(fake_home, tmp_path, monkeypatch): + """Root FHS installs place node symlinks in /usr/local/bin. + + We monkeypatch _node_symlink_candidate_dirs to return a temp dir standing + in for /usr/local/bin so the test doesn't need real root privileges. + """ + hermes_home = fake_home / ".hermes" + node_bin = _make_hermes_node(hermes_home) + + # Fake /usr/local/bin as a temp dir with our symlinks. + fhs_bin = tmp_path / "usr_local_bin" + fhs_bin.mkdir() + for name in ("node", "npm", "npx"): + (fhs_bin / name).symlink_to(node_bin / name) + + # Ensure ~/.local/bin has NO symlinks (simulate pure FHS install). + local_bin = fake_home / ".local" / "bin" + for name in ("node", "npm", "npx"): + p = local_bin / name + if p.exists() or p.is_symlink(): + p.unlink() + + # Return only our fake FHS dir as a candidate. + monkeypatch.setattr( + uninstall, "_node_symlink_candidate_dirs", lambda: [fhs_bin] + ) + + removed = uninstall.remove_node_symlinks(hermes_home) + + assert sorted(p.name for p in removed) == ["node", "npm", "npx"] + for name in ("node", "npm", "npx"): + assert not (fhs_bin / name).is_symlink()