Skip to content

fix: support running sh shim from wsl#178

Merged
owlstronaut merged 2 commits into
npm:mainfrom
nadalaba:fix-wsl
Jun 8, 2026
Merged

fix: support running sh shim from wsl#178
owlstronaut merged 2 commits into
npm:mainfrom
nadalaba:fix-wsl

Conversation

@nadalaba

@nadalaba nadalaba commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

What:

Support running sh shim from WSL.

Details:

  • Previously, the generated sh shim called windows binary progs without .exe which works from msys/cygwin shells but not from wsl. Also, the the target passed to the windows binary prog as a parameter had a unix path on WSL.
  • Now, we detect if we are in WSL, and check for the existence of an executable with and without .exe. We also use wslpath to convert parameter's path to a Windows path.

@nadalaba nadalaba requested a review from a team as a code owner April 29, 2026 18:46
@nadalaba nadalaba changed the title fix: support running windows node from wsl fix: support running binary targets from wsl May 29, 2026
@nadalaba

Copy link
Copy Markdown
Contributor Author

hey @owlstronaut can you help with the failing CodeQL CI? Not sure what I can do on my end

@nadalaba nadalaba changed the title fix: support running binary targets from wsl fix: support running binary progs from wsl May 30, 2026

@owlstronaut owlstronaut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I don't know that this will work for us in all cases. As-is, the new basedir_win rewrite kicks in for any Linux host with wslpath on PATH and then unconditionally hands a windows-form path to whatever interpreter runs. That works for your node.exe+shim case you are targeting, but breaks the common WSL cases where the selected interpreter is a POSIX binary.

@nadalaba

nadalaba commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

I don't think there's an issue with assuming the interpreter isn't POSIX if we're using cmd-shim. If the interpreter were POSIX, all shims created here would already break when invoked from intended shells (cmd, pwsh, cygwin, msys) since they currently pass Windows-form paths to the interpreter.

On WSL with POSIX interpreters, users shouldn't be using cmd-shim in the first place, the README explicitly recommends symlinks instead.

the new basedir_win rewrite kicks in for any Linux host with wslpath on PATH

I used npm/cli's approach for WSL detection, but I can narrow it down using uname -a and check for *WSL2* instead of *Linux*. Would you prefer that?

@owlstronaut

Copy link
Copy Markdown
Contributor

Fair point on the posix interpreter concern. But yes, I think it can be narrowed to WSL2 with uname -a. I think also the probe order needs to be flipped to .exe first to match the npm/cli. Could also use a guard on the wslpath for non-zero exits

@nadalaba nadalaba changed the title fix: support running binary progs from wsl fix: support running sh shim from wsl Jun 6, 2026
@nadalaba

nadalaba commented Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

For the wslpath guard, I'm exiting with an error if wslpath fails since a unix fallback path won't work in WSL anyway.

I also added a 4th path check for PROG_EXE (e.g., node.exe) for non-global packages shims (node is not on the same directory as the shim), since WSL doesn't auto append .exe to PATH binaries like other Windows shells do.

@owlstronaut owlstronaut merged commit 1ca32a3 into npm:main Jun 8, 2026
23 checks passed
@github-actions github-actions Bot mentioned this pull request Jun 8, 2026
owlstronaut pushed a commit that referenced this pull request Jun 8, 2026
🤖 I have created a release *beep* *boop*
---


## [9.0.1](v9.0.0...v9.0.1)
(2026-06-08)
### Bug Fixes
*
[`1ca32a3`](1ca32a3)
[#178](#178) support running sh shim
from wsl (#178) (@nadalaba)
### Chores
*
[`30cad4d`](30cad4d)
[#183](#183) bump
@npmcli/eslint-config from 6.0.1 to 7.0.0 (@dependabot[bot])
*
[`3b7e7dc`](3b7e7dc)
[#182](#182) postinstall for
dependabot template-oss PR (@npm-cli-bot)
*
[`c64c525`](c64c525)
[#182](#182) bump
@npmcli/template-oss from 5.0.0 to 5.1.0 (@dependabot[bot])

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants