search

npm / cmd-shim

The cmd-shim used in npm
ISC License
76 stars 40 forks source link

Syntax error on Windows #17

Closed kgryte closed 8 years ago

kgryte commented 8 years ago

This issue seems to be related to #16, and possibly to the change introduced in #4 by @copenhas.

Description

Running Windows builds on AppVeyor with MinGW and MSYS. When executing test commands of the sort,

/c/projects/foo/node_modules/.bin/istanbul cover \
  --no-default-excludes \
  -x 'node_modules/**' \
  -x 'build/**' \
  -x '**/test/**' \
  -x '**/tests/**' \
  -x 'reports/**' \
  --dir /c/projects/foo/reports/coverage \
  --report lcov \
  /c/projects/foo/node_modules/.bin/tape -- /c/projects/foo/test/test.js /c/projects/foo/test/test.a.js

I encounter the following error

basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')")
          ^^^^^^^

SyntaxError: missing ) after argument list
    at exports.runInThisContext (vm.js:53:16)
    at Module._compile (module.js:387:25)
    at Object.Module._extensions..js (module.js:422:10)
    at Object.Module._extensions.(anonymous function) [as .js] (c:\projects\foo\node_modules\istanbul\lib\hook.js:109:37)
    at Module.load (module.js:357:32)
    at Function.Module._load (module.js:314:12)
    at Function.Module.runMain (module.js:447:10)
    at runFn (c:\projects\foo\node_modules\istanbul\lib\command\common\run-with-cover.js:122:16)
    at c:\projects\foo\node_modules\istanbul\lib\command\common\run-with-cover.js:251:17
    at c:\projects\foo\node_modules\istanbul\lib\util\file-matcher.js:68:16
make[1]: *** [test-istanbul] Error 1
make[1]: Leaving directory `/c/projects/foo'
make: *** [test-cov] Error 2

Environments

OS:

Node/npm versions:

copenhas commented 8 years ago

Sorry for such a late response. I don't have a Windows setup at home. I got a basic setup going and tried something real quick.

Setup:

Windows 7
git for Windows 2.8.1
node 4.4.3
MinGW with MSYS installed (grabbed the installer from sourceforge)

I installed istanbul globally:

npm install -g istanbul

Then in git bash, cmd.exe, and MSYS bash this is what I got:

$ istanbul
Need a command to run
Try "istanbul help" for usage

Just to make sure the line in question was in the script I ran the following in git bash:

$ cat `which istanbul`
#!/bin/sh
basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')")

case `uname` in
    *CYGWIN*) basedir=`cygpath -w "$basedir"`;;
esac

if [ -x "$basedir/node" ]; then
  "$basedir/node"  "$basedir/node_modules/istanbul/lib/cli.js" "$@"
  ret=$?
else
  node  "$basedir/node_modules/istanbul/lib/cli.js" "$@"
  ret=$?
fi
exit $ret

Looks like this needs some more investigation to figure out why this is causing issues for you. I'll try to test some git extensions at work when I get a chance (it might take me a few days though). They were the reason originally #4 was opened.

kgryte commented 8 years ago

@copenhas Thanks for the response. Might be worth trying to run some of the istanbul commands (e.g., istanbul cover) to see if things work on your end. And would there be any difference in a local installation as opposed to installing globally? Also, not sure if this is an istanbul specific issue.

khelmar commented 8 years ago

I'm seeing the same issue when trying to run sass-lint so I don't think it's istanbul specific

kgryte commented 8 years ago

@copenhas wanted to follow-up and see if you had any updates.

pallxk commented 8 years ago

Out of curiosity, I made some investigation on this.

In fact, SyntaxError: missing ) after argument list is reported by node.js and is not reported by shell. So this does not necessarily mean something goes wrong with the shim shell script.

With some reading of the code of istanbul and testing, I think this is caused by running the shim shell script via node. That is, in your particular case, node /c/projects/foo/node_modules/.bin/tape was run under the hood, where ./bin/tap is the shim shell script rather than the actual node.js script. You may check this with --verbose added to your istanbul cover command.

See istanbul/run-with-cover.js#L117 for related code.

So even if this is not istanbul specific, I thinks it's not caused by #4, and not related to #16.

(I'm new to node.js and not using istanbul, so chances are I missed or misunderstood something. Please tell me if i'm wrong.)

copenhas commented 8 years ago

I have not had a chance to look into again. I'm afraid I got caught up with work long enough to forget about this issue. I apologize.

I did test out our internal git extensions (written in node and the reason I submitted #4 ), and they work on latest node.js. After reading @pallxk comment (thanks for looking into it), I looked at the Istanbul documentation here. It looks like the command is expecting a node.js javascript file and not a shell command.

Perhaps instead of passing istanbul /c/projects/foo/node_modules/.bin/tape you could try figuring out what the main module is and pass it that. Based on tape's package.json it looks like it would be something like this: /c/projects/foo/node_modules/tape/main.js.

ghost commented 8 years ago

I've got the exact same issue, driving me crazy. The same exact code runs on my Mac w/o this error, but on Windows it just won't run.

Don't have any idea where to start looking.

copenhas commented 8 years ago

@kdawg1406 I believe the difference between macOS and Windows is that macOS supports she-bang scripts, while in Windows the commands have to be wrapped up into the cmd shim. So when you pass in ./node_modules/.bin/mocha on macOS the file probably contains JavaScript, but on Windows it contains shell commands.

Per the comments above you'll find that Istanbul is actually expecting a JavaScript file as the argument and not shell script or command. I believe that's what's causing the syntax error for you. You'll need to find out what the main module file path is so you can pass it in cross platform.

copenhas commented 8 years ago

Looks like Istanbul documents it some at README.md#usage-on-windows

ghost commented 8 years ago

@copenhas Bingo!

This works for mocha on Windows:

./node_modules/.bin/istanbul cover ./node_modules/mocha/bin/_mocha

Thank you for your outstanding support and very quick response.

Best to you,

Karl

kgryte commented 8 years ago

@copenhas Was able to resolve the issue using a solution similar to the solution described above. Thanks for the help. Closing the issue.

ReinsBrain commented 6 years ago

@kdawg1406 - your solution doesn't work for me - when i use: ./node_modules/.bin/istanbul cover ./node_modules/.bin/_mocha error is:

\node_modules\.bin\_mocha:2
basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')")
          ^^^^^^^
SyntaxError: missing ) after argument list

when i use (as you suggest): ./node_modules/.bin/istanbul cover ./node_modules/bin/_mocha error is: Unable to resolve file [./node_modules/bin/_mocha]

I don't understand how it can't resolve the file because i can see the file at ./node_modules/bin/_mocha]

ghost commented 6 years ago

@ReinsBrain Not sure, this was a long time ago, so much as changed in the toolchain. Sorry, I don't have an answer for you.

mihaisavezi commented 5 years ago

This doesn't work for storybook. trying to debug via node --inspect node_modules/.bin/storybook-start -p 9001 throws the same error.

Sawtaytoes commented 4 years ago

@ReinsBrain You might need to change that line to npx _mocha.

Also, this error is happening even in the Webpack project. I get it when I run from a child process started with node.

Looks like something these libraries depend on is running this code in a way that Windows doesn't understand.

Whoever wrote the original code (cmd-shim?) has an incorrect assumption about the OS when commands are run from Node.js rather than from the OS's CLI.