-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
<(target_arch) not resolving, causing npm install to fail #1501
Comments
The Could you try running |
One more thing: in the log I see |
Hi, thanks for the investigation! I didn't even notice C:\src\depot_tools is installed as part of building chromium, from their instructions here: https://chromium.googlesource.com/chromium/src/+/master/docs/windows_build_instructions.md. `npm install --verbose`
When I was testing before I made a fork of node-gyp with the However, removing I found I could reproduce the issue simply by redirecting python from a batch file (putting python.bat in my path, instead of python.exe). For example, with (and the @echo off is important, or else the python version detection code in configure.js breaks) this as my batch file:
So it looks like using a batch file to redirect inputs breaks when there is a "<" in the arguments. Works: Fails: Fails: Works: Although it sort of seems like this is a chromium problem, if there is only one line we using "<" in an argument it would completely get rid of this type of problem in the future by changing that one line. It is also possible to check for python not accepting "<" in arguments, which would require developers to change their PATH when switching between chromium source development, and development that requires node-gyp. I'm going to keep investigating the "<" issue, and see if there is a way chromium can change their python.BAT file to work with "<" arguments. |
Hi @yeerkkiller1. Thanks for the informative submission. Simplest solution might be to write |
On windows invoking spawn on a batch file results in additional argument processing. Special characters need to be escaped (twice). fixes nodejs#1501 (comment)
On windows invoking spawn on a batch file results in additional argument processing. Special characters need to be escaped (twice). fixes nodejs#1501 (comment)
On windows invoking spawn on a batch file results in additional argument processing. Special characters need to be escaped (twice). fixes nodejs#1501 (comment)
node: v10.6.0, npm: 6.1.0
MSYS_NT-10.0 DESKTOP-E7DLHGV 2.8.2(0.313/5/3) 2017-07-12 15:35 x86_64 Msys
Microsoft (R) Build Engine version 15.6.85.37198 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.
15.6.85.37198
Microsoft (R) C/C++ Optimizing Compiler Version 19.13.26131.1 for x86
Copyright (C) Microsoft Corporation. All rights reserved.
usage: cl [ option... ] filename... [ /link linkoption... ]
https://github.com/nodejs/abi-stable-node-addon-examples/blob/master/1_hello_world/napi/hello.cc
npm install
node "C:\Program Files\nodejs\node_modules\npm\node_modules\npm-lifecycle\node-gyp-bin\\..\..\node-gyp\bin\node-gyp.js" rebuild
The specific problem is
<(target_arch)
, which comes from this line: https://github.com/nodejs/node-gyp/blob/master/lib/configure.js#L296 .It appears as if
<(target_arch)
(process substitution) is being used, which I believe is only valid syntax on linux? I am not very familiar with shell scripting, but it looks like$(target_arch)
would work? And at the very least be preferable on windows?But there could also be another issue causing
<(target_arch)
to not work on my machine? Because it looks like that line is years old.The text was updated successfully, but these errors were encountered: