This repository was archived by the owner on Aug 11, 2022. It is now read-only.
Repository navigation
This repository was archived by the owner on Aug 11, 2022. It is now read-only.
npm creates incorrect cmd shim for jezebel package on windows #3380
Description
When trying to run jezebel on windows after installing it with npm, it fails. C:\Users\AppData\Roaming\npm\jezebel.cmd looks like this:
:: Created by npm, please don't edit manually.
@IF EXIST "%~dp0\NODE_PATH=./lib:$NODE_PATH.exe" (
"%~dp0\NODE_PATH=./lib:$NODE_PATH.exe" node %~dp0\node_modules\jezebel\bin\jezebel" %*
) ELSE (
NODE_PATH=./lib:$NODE_PATH node "%~dp0\node_modules\jezebel\bin\jezebel" %*
)
Which is syntactically incorrect.
Looking at the jezebel repository the bin\jezebel file has the following first line:
#!/usr/bin/env NODE_PATH=./lib:$NODE_PATH node
Looking at the npm source code, specifically node_modules\cmd-shim:22 shows:
, shebangExpr = /^#\!\s*(?:\/usr\/bin\/env)?\s*([^ \t]+)(.*)$/
This regular expression was not built with the possibillity of variable declarations in mind.
It should be adapted to extract the variable declarations and translate them to the proper cmd syntax:
set NODE_PATH=.lib:%NODE_PATH%
in this case.
Activity
ForbesLindesay commentedon Apr 28, 2013
The source code for cmd-shim is here and a pull request to fix this would be welcome. However, it should be easier in this case to just make jezebel not do that, since there's no real reason why it would need to do that.
basbossink commentedon Apr 28, 2013
I'll try to create a fix. I see your point but I think it's best if the shim creation either fails if it can not extract the command from the file successfully or handles the given case correctly. My proposal would be to go for the latter option.
isaacs commentedon Apr 29, 2013
It looks like Jezebel is trying to use NODE_PATH as a polyfill for the now-removed require.paths.
That is not supported, recommended, safe, or guaranteed to work reliably. You should report the bug to them.
isaacs commentedon Apr 29, 2013
Note: I do acknowledge that this is a bug in cmd-shim, but like many bugs, it's not worth fixing until and unless there's a real need for it, because the fix adds complexity.
basbossink commentedon Apr 29, 2013
As you can see I reported the issue to jezebel, do you see a legitimate reason to add support for the use of variables in cmd-shim?
ForbesLindesay commentedon Apr 29, 2013
Yes, in this case it's definitely a higher priority to just fix jezebel.
One option would be to just fix
cmd-shimso that it reports this as an error and states that it's not supported?fix: Handle environment variables properly