-
-
Notifications
You must be signed in to change notification settings - Fork 124
fix(node,bun): include toolchain in PATH during install #1492
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.
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
Conversation
PR j178#1488 removed symlinks from bin_dir but only updated run() to prepend the toolchain directory to PATH. The install() functions still only prepended bin_dir, causing npm/bun install to fail when no system node/bun is in PATH. This fix mirrors the run() PATH setup in install() by including the toolchain's parent directory.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1492 +/- ##
=======================================
Coverage 91.51% 91.51%
=======================================
Files 87 87
Lines 18153 18169 +16
=======================================
+ Hits 16612 16627 +15
- Misses 1541 1542 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
📦 Cargo Bloat ComparisonBinary size change: +0.00% (22.5 MiB → 22.5 MiB) Expand for cargo-bloat outputHead Branch ResultsBase Branch Results |
Verifies npm install works without system node in PATH. Regression test for j178#1492.
|
Note: The existing Added |
|
Thanks! |
Summary
PR #1488 removed symlinks from
bin_dirbut only updatedrun()to prepend the toolchain directory to PATH. Theinstall()functions still only prependedbin_dir, causingnpm installto fail when no system node is available in PATH.MRE
With
.pre-commit-config.yaml:When no system node is in PATH (can be simulated with
PATH=/usr/bin:/bin, running:Fails with
With system node in PATH,
npm install"works" but uses wrong node version (system instead of requested).Fix
Mirror the
run()PATH setup ininstall()by including the toolchain's parent directory:Note: Bun fix is for consistency/edge cases (bun is a native binary, not a shebang script like npm).