configure.js: escape '<' #1506
configure.js: escape '<' #1506
Conversation
|
I'm not confident enough to pull this in atm, does someone else with deep enough expertise want to confirm this is the right thing to do? |
|
I'm experiencing this bug as well, any update on reviews? Applying the patch manually seems to fix the issue for me. |
|
@yeerkkiller1 if you're still interested, would you mind rebasing and squashing your two commits? |
On windows invoking spawn on a batch file results in additional argument processing. Special characters need to be escaped (twice). fixes nodejs#1501 (comment)
|
@rvagg, no problem, it's squashed and pushed now. |
|
SGTM but I would love a +1 from @bzoz, @joaocgreis, @refack or one of our other core Windows experts, I've never had experience escaping in .bat with |
|
@rvagg This is indeed the correct fix. I just spent an hour debugging this issue myself. It would be great if this fix would be merged. |
|
@bzoz, @joaocgreis, please confirm |
|
I think this is not needed anymore. #1582 made a change that extracts the Python .exe filename even from batch files and then uses it. We are no longer using the batch file to spawn Python executables. I've also was not able to trigger the original issue. @yeerkkiller1, @jlennox could you verify that the issue is still present in master? |
|
#1582 went out with v5.0.0 so should be in npm already, maybe update npm: |
Fixes: #1501
On windows invoking spawn on a batch file results in additional
argument processing. Special characters need to be escaped (twice).
Checklist
npm install && npm testpassesDescription of change
Fixes issue where npm install fails if python is a batch file. This occurs because the batch file tries to interpret the '<' specially.
I tested this change on my windows machine (with and without python.bat in my path). I don't think this change will impact other platforms, as it only applies changes when the platform is win32, and when python is a batch file (which would have broken anyway).