update npm to 5.8.0

[MylesBorins]

Another attempt after #19560 broke master. Because npm removed node-gyp we now need to vendor it. I've added it to tools/node_modules similar to our other vendored dependencies. I've also gone ahead and make a script to update the dep, update the license builder, update the license, and update the Makefile.

I'm going to run CITGM on this... I'm curious if this will have any effect on native modules at all.

/cc @iarna why did y'all remove node-gyp, and are there any edge cases we should expect?

[MylesBorins]

Also, it would be possible to move the node-gyp folder out of the npm directory before updating npm. This may make the diff a bit smaller when updating gyp via the script... although I'm not sure if this really makes a difference for git internals... just wanted to make an offer to do so

[MylesBorins]

CI: https://ci.nodejs.org/job/node-test-pull-request/14081/
CITGM: https://ci.nodejs.org/job/citgm-smoker/1356/

[MylesBorins]

/cc @nodejs/build and @nodejs/tsc as this change has gotten a bit bigger in scope

[jasnell]

Wait... 5.8 removed node-gyp in a minor release? Ugh.

[addaleax]

@jasnell No, the files just moved around in-tree

[jasnell]

No, the files just moved around in-tree

Phew... ok good.... :-) gave me a slight mini heart attack there.

[rvagg]

so wait, was it removed or moved? did @MylesBorinsadd it to node_modules or .. what is going on? is there a reference for what this is all about?

[addaleax]

$ grep version tools/node_modules/node-gyp/package.json "version": "3.6.2"
$ grep version deps/npm/node_modules/npm-lifecycle/node_modules/node-gyp/package.json "version": "3.6.2"

This PR is adding a second copy.

I think we could just hoist node-gyp back to its old location in the npm tree manually

[MylesBorins]

Ahhh I don't know why I thought it had been removed instead of just moved. I'll update the PR I prefer to not change the location in the tree

…

On Thu, Apr 5, 2018, 7:49 PM Anna Henningsen ***@***.***> wrote: $ grep version tools/node_modules/node-gyp/package.json "version": "3.6.2" $ grep version deps/npm/node_modules/npm-lifecycle/node_modules/node-gyp/package.json "version": "3.6.2" This PR is adding a second copy. I think we could just hoist node-gyp back to its old location in the npm tree manually — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#19840 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAecVzKY-tJXNzMAqFpRho6zZ6lDEx_yks5tlq13gaJpZM4TJQlN> .

[MylesBorins]

rolled back the node-gyp stuff and simply updated the Makefile to reference the new location in the npm node_modules tree.

CI: https://ci.nodejs.org/job/node-test-pull-request/14084/
CI for master: https://ci.nodejs.org/job/node-test-commit/17479/ (there were weird failures in last CI)

[richardlau]

vcbuild.bat will also need to be updated with the new node-gyp location.

[richardlau]

The AIX failure is real. node-gyp's lib/find-node-directory.js assumes it lives in npm/node_modules/node-gyp/lib.

Cc @nodejs/platform-aix

[rvagg]

The AIX failure is real. node-gyp's lib/find-node-directory.js assumes it lives in npm/node_modules/node-gyp/lib.

I've seen a lot of code (and maybe I've written some ...) that makes this assumption, moving it could cause some yelping. +1 to hoisting it although we should probably have this discussion with npm folks.

[ChALkeR]

I just observed Atom text editor failing to install or update any plugins because of this with system-wide npm@5.8.0 install.

https://github.com/npm/npm/blob/master/bin/node-gyp-bin/node-gyp — npm itself fails on 5.8.0 when executing this file.

Related: npm/npm#20163, mapbox/node-pre-gyp#362

[zkat]

As pointed out: we didn't remove node-gyp, but instead moved it so it lives under npm-lifecycle now, as part of refactoring these bits into separate pieces.

[MylesBorins]

@zkat I think the issue is that node-gyp itself has code internally expecting to live at that location... And there is a non trivial amount of code in the ecosystem making similar assumption

[zkat]

@MylesBorinsnpm/npm#20276

If this works, it should be pretty easy to backport as well.

[MylesBorins]

@zkat 🎉

[ChALkeR]

Yes, npm/npm#20276 looks like an ideal solution (especially the tests part). Thanks, @zkat!

[BridgeAR]

Should this stay open due to #20190?

[MylesBorins]

We are likely going to need to update the 5.x release line before 6.x lands At the very least this can re target 8.x

…

On Sun, Apr 22, 2018, 1:59 PM Ruben Bridgewater ***@***.***> wrote: Should this stay open due to #20190 <#20190>? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#19840 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAecV5QHmnZW126g4nqlF6Fe4QnCZX3rks5trMT4gaJpZM4TJQlN> .

[p0psicles]

@MylesBorins does this mean there is a chance that npm 5.8.x get's released (packaged) with a node 8.x.x version?

[BridgeAR]

Ping @MylesBorins about retargeting this PR

[MylesBorins]

@BridgeAR I'm going to go ahead and close this for now. We would need a different 5.x version to fix the node-gyp issue. If 6.x lands on master in a timely fashion we can backport in an 8.x minor, which is coming in the next month