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

Detect shebangs absolutely to node, wrap those too #13

Merged
merged 4 commits into from
Apr 5, 2016
Merged

Conversation

isaacs
Copy link
Collaborator

@isaacs isaacs commented Apr 3, 2016

When there's a program that doesn't match one of the other criteria, read its contents, and if it's a shebang to node, then wrap it.

This lets nyc wrap npm when npm is a shebang to an absolute path to Node.

Closes #12

@bcoe
Copy link
Member

bcoe commented Apr 3, 2016

this doesn't quite do the trick, because the npm bin is doing something quite wonky:

#!/bin/sh
// 2>/dev/null; exec "`dirname "$0"`/node" "$0" "$@"

looks like it's dynamically generating the absolute path to node, I used the following shim to solve the problem:

https://github.com/tapjs/spawn-wrap/pull/14/files#diff-168726dbe96b3ce427e7fedce31bb0bcR151

it uses which in a similar way to your approach, and is more generic than @addaleax's initial fact finding mission.

@addaleax
Copy link
Member

addaleax commented Apr 3, 2016

Yup, this hack that the Node.js people install in the npm executable is one of the reasons I decided to try the special check for "npm" instead of shebang matching.

+1 for using which, I thought of that but didn’t dare add a dependency here. 😄

@bcoe
Copy link
Member

bcoe commented Apr 3, 2016

@isaacs you should be able to rebase with master now and appveyor will pass. I turned off node@0.10.x which has never worked. I can boot Windows 7 at some point and dig into this, I think it's mostly exit-code edge-cases.

addaleax and others added 4 commits April 4, 2016 21:04
As laid out in istanbuljs/nyc#190, spawn-wrap’ing `npm` does currently
not work when using the `npm` binary that comes bundled with Node.js.

This adds special handler code for the case that `npm` is the
program that should be executed, looking for the `npm` executable
in PATH using `which` and invoking the shim with it as an argument.
Skip on windows, because shebangs basically don't exist there.
@isaacs isaacs merged commit 57473f8 into master Apr 5, 2016
@isaacs isaacs deleted the fix-12 branch April 5, 2016 04:19
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 this pull request may close these issues.

3 participants