mapbox / node-pre-gyp

Node.js tool for easy binary deployment of C++ addons
BSD 3-Clause "New" or "Revised" License
1.11k stars 259 forks source link

node-gyp failure with npm 5.8.0 #362

Open mapsam opened 6 years ago

mapsam commented 6 years ago

node-pre-gyp fails in the node-gyp step with the following error (example using the vtquery module):

Error: Failed to execute 'node-gyp configure ...

Stack trace

V=1 ./node_modules/.bin/node-pre-gyp configure build --error_on_warnings=true --loglevel=error
NPM BASE /Users/mapsam/.nvm/versions/node/v6.13.0/lib/node_modules/npm/
NPM GYP BIN /Users/mapsam/.nvm/versions/node/v6.13.0/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js
node-pre-gyp ERR! configure error 
node-pre-gyp ERR! stack Error: Failed to execute 'node-gyp configure --error_on_warnings=true --loglevel=error --module=/Users/mapsam/mapbox/vtquery/lib/binding/vtquery.node --module_name=vtquery --module_path=/Users/mapsam/mapbox/vtquery/lib/binding' (Error: spawn node-gyp ENOENT)
node-pre-gyp ERR! stack     at ChildProcess.<anonymous> (/Users/mapsam/mapbox/vtquery/node_modules/node-pre-gyp/lib/util/compile.js:80:29)
node-pre-gyp ERR! stack     at emitOne (events.js:96:13)
node-pre-gyp ERR! stack     at ChildProcess.emit (events.js:188:7)
node-pre-gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:217:12)
node-pre-gyp ERR! stack     at onErrorNT (internal/child_process.js:376:16)
node-pre-gyp ERR! stack     at _combinedTickCallback (internal/process/next_tick.js:80:11)
node-pre-gyp ERR! stack     at process._tickCallback (internal/process/next_tick.js:104:9)
node-pre-gyp ERR! stack     at Module.runMain (module.js:613:11)
node-pre-gyp ERR! stack     at run (bootstrap_node.js:387:7)
node-pre-gyp ERR! stack     at startup (bootstrap_node.js:153:9)
node-pre-gyp ERR! System Darwin 17.4.0
node-pre-gyp ERR! command "/Users/mapsam/.nvm/versions/node/v6.13.0/bin/node" "/Users/mapsam/mapbox/vtquery/node_modules/.bin/node-pre-gyp" "configure" "build" "--error_on_warnings=true" "--loglevel=error"
node-pre-gyp ERR! cwd /Users/mapsam/mapbox/vtquery
node-pre-gyp ERR! node -v v6.13.0
node-pre-gyp ERR! node-pre-gyp -v v0.6.39
node-pre-gyp ERR! not ok 
Failed to execute 'node-gyp configure --error_on_warnings=true --loglevel=error --module=/Users/mapsam/mapbox/vtquery/lib/binding/vtquery.node --module_name=vtquery --module_path=/Users/mapsam/mapbox/vtquery/lib/binding' (Error: spawn node-gyp ENOENT)

It appears this is an issue with the most recent npm release, version 5.8.0, which was released on 03/08/2018 and removed the node-gyp module unexpectedly. https://github.com/npm/npm/issues/20163. The following path is created in node-pre-gyp with the assumption that node-gyp exists in npm.

/Users/mapsam/.nvm/versions/node/v6.13.0/lib/node_modules/npm/node_modules/node-gyp/bin/

We'll continue to monitor over there to learn whether we need a fix here or if we can just update to a future version.

cc @springmeyer @millzpaugh

mapsam commented 6 years ago

There is a fix in place for this at https://github.com/npm/npm/pull/20276!

mapsam commented 6 years ago

Per comment from @nicolasnoble -

As a resolution, I would recommend probing for both node_modules/node-gyp/bin/node-gyp.js and node_modules/npm-lifecycle/node_modules/node-gyp/bin/node-gyp.js. Maybe even just probing the node_modules/npm/bin/node-gyp-bin directory instead, since this is what npm uses internally.

nicolasnoble commented 6 years ago

Right, we probably don't want to just wait on npm to have an update for this. There will be people out there in production who have npm 5.8.0 installed, and they will fail to install node-pre-gyp enabled packages because of this issue without understanding why. It'd be a better user experience to have node-pre-gyp workarounding bad installations of npm.

springmeyer commented 6 years ago

Looks like npm is moving to fix this (https://github.com/npm/npm/pull/20276), so I'm hesitant to apply a workaround in node-pre-gyp since I presume it is easier to upgrade npm than it is to upgrade node-pre-gyp (given node-pre-gyp needs to be bundled).

mapsam commented 6 years ago

This was fixed in https://github.com/npm/npm/commit/cfe562c5803db08a8d88957828a2cd1cc51a8dd5 and is part of the 6.0.0 release (with a possible backport planned), confirmed locally that the error is no longer present (double confirmed with no global installation of node-gyp).

Closing since it seems unlikely we'll add a fix to the node-gyp search function every time a bug is introduced via npm.

sssoleileraaa commented 6 years ago

Hey! I just ran into this while using npm 5.8.0 and was able to fix this issue by doing:

npm install npm@latest -g

Thanks for making this issue @mapsam!

nicolasnoble commented 6 years ago

Right, node 8.11.2 is still pinned on npm 5.6.0 when installing it fresh, so it seems this issue is going to stay relevant for anything that installs nodejs with the npm associated with it; only node 10 is pinned on npm 6 right now.

This means that any CI that uses nvm for version switching will have this bug here. Right now for gRPC we are installing node-gyp globally in our CI to alleviate the problem, but it's fairly sad to have to do this.