Skip to content
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

node-gyp-build fails on windows if path to node.exe contains a space #71

Closed
Lesstat opened this issue May 24, 2024 · 5 comments · Fixed by #72
Closed

node-gyp-build fails on windows if path to node.exe contains a space #71

Lesstat opened this issue May 24, 2024 · 5 comments · Fixed by #72

Comments

@Lesstat
Copy link

Lesstat commented May 24, 2024

Since upgrading to node 18.20 and node-gyp-build 4.8.1 node-gyp-build fails on a clean npm install on windows with the message:

'C:\Program' is not recognized as an internal or extrernal command, operable program or batch file

After debugging the issue locally I believe the issue originates in the build function in bin.js.

For windows, the command is run inside a shell and the path to node.exe is passed unquoted. Therefore, the shell interprets and splits the path. Locally, I could fix the issue by replacing line 25 with:

`"${process.execPath}"`,

I am hesitant to make a PR as I cannot currently test this on other platforms, but I am happy to do so if you want me to.

@maheshsivast
Copy link

maheshsivast commented Jun 3, 2024

Facing the same problem
This commit should be the cause 692860e

Revert line 30 to proc.spawn(args[0], args.slice(1), { stdio: 'inherit' }) is working
value of args[0] in my case is 'C:\Program Files\nodejs\node.exe'

Installing the previous version solves the problem
npm install --save-dev node-gyp-build@4.8.0

JovannMC added a commit to JovannMC/haritorax-interpreter that referenced this issue Jun 7, 2024
+ Stay on node-gyp-build@4.8.0 to prevent issues building due to spaces (prebuild/node-gyp-build#71)
+ Update @abandonware/noble to 1.9.2-25
cb1kenobi added a commit to cb1kenobi/node-gyp-build that referenced this issue Jun 29, 2024
@Lesstat
Copy link
Author

Lesstat commented Jul 8, 2024

I just tested the fix in #72 on Windows with node versions 18.16.0, 18.18.2, 18.20.3 and 20.11.1. It worked for all those versions.

@lolPlatinumPlayer
Copy link

Its Crazy! This package weekly downloads 12,530,231 times. Surprisingly, there is such a fundamental and serious mistake. And this error message in Chinese computer will show like npm error 'C:\Program' �����ڲ����ⲿ���Ҳ���ǿ����еij��� npm error ���������ļ���. Its real hard for me to get to the root of the problem.

petetronic pushed a commit to posit-dev/positron that referenced this issue Aug 22, 2024
For reasons that we haven't yet fully ascertained, Jupyter Adapter
builds are broken on Windows for dev environments, after #4283.

```
error C:\Users\jmcph\git\positron\extensions\jupyter-adapter\node_modules\lzma-native: Command failed.
Exit code: 1
Command: node-gyp-build
Arguments:
Directory: C:\Users\jmcph\git\positron\extensions\jupyter-adapter\node_modules\lzma-native
Output:
'C:\Program' is not recognized as an internal or external command,
operable program or batch file.
```

After some investigation, it appears we're hitting
prebuild/node-gyp-build#71 (caused by
prebuild/node-gyp-build@692860e),
which is still unresolved in node-gyp-build 4.8.1.

The fix is to downgrade node-gyp-build to 4.8.0 for the Jupyter Adapter,
which is an older version that doesn't contain the offending commit.

> [!NOTE]
> I also tried downgrading node-gyp-build to 4.8.0 across Positron, but
it turns out that other places _only_ work with 4.8.1. What a delight.
@GustavPS
Copy link

How come this issue has not been resolved yet? It has huge consequences and sounds like an easy fix?

@mafintosh
Copy link
Collaborator

There is a PR with the fix we can merge? If so happy to take a look.

mafintosh pushed a commit that referenced this issue Aug 28, 2024
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 a pull request may close this issue.

5 participants