deps: update npm to v5.5.1

[MylesBorins]

Followed guide

This PR manually floats 4ca6958196 from npm upstream to update zlib. This can obviously be trumped by a 5.5.2 release if we can get it in before 9.x, but it seemed like to best option to be able to ship today and remove a bit of the pressure for next weeks release (and get 2 factor into the LTS release 🎉).

Refs: npm/npm@4ca6958
Closes: #16280
Fixes: #14161

/cc @iarna @zkat

[MylesBorins]

I had to float another patch to explicitly support Node 9.x

sent a pr upstream npm/npm#18964

edit: it is worth mentioning that the tests pass on my local machine once these two extra patches are floated on the 5.5.1 release

[addaleax]

Kind of feel like floating a patch should happen in a separate commit, but this should be fine either way. Thanks!

[MylesBorins]

@addaleax done 😄

[MylesBorins]

could someone test locally and see if it works for them?

[MylesBorins]

so I'm a bit concerned about a couple things.

when I followed the instructions from the npm wiki there were a whole bunch of files that git ignored when I tried to add them. I ended up adding the entire npm folder with "add -f"

Now we are seeing +118,079 −43,651 in the diff... which may be correct, and may not be correct.

What it could mean is that we have been shipping a broken npm in other versions. I'm going to dig in and review the process to see if we have been accidentally shipping a bad tree

edit: one of the culprits may be my global git ignore... especially ignoring a folder called tar

edit 2: verified that 5.3.0 is fine, but heed this warning future backporters. We should likely downstream the updating npm guide... PR incoming

edit 3: By using whitespace fix I took the diff down to +109,421 −35,117

[targos]

CI: https://ci.nodejs.org/job/node-test-npm/32/

[targos]

LGTM. I have about the same amount of changes if I do the update on my side.

[jasnell]

This has to land before noon pacific time today in order to make it in to 9.0.0

[MylesBorins]

landed in 64168eb...b8888f5

[kasicka]

Can we backport npm/npm@4ca6958 to v8.x branch too? I get npm ERR! invalid: [email protected] /usr/local/lib/node_modules/npm/node_modules/tar/node_modules/minizlib when I try npm ls -g with npm bundled with v8.9.4, backporting 4ca695819 fixes the error.. Or should people from npm fix this?

[richardlau]

@kasicka We usually try not to float patches on top of npm. npm/npm@4ca6958 is included in [email protected] so it probably makes more sense to backport either #17535 or #17777 to Node.js 8.

cc @nodejs/lts @nodejs/npm

[kasicka]

@richardlau [email protected] is included in v8.9.4, yet the error is present.. tl;dr is minizlib should be updated to 1.0.4

[richardlau]

[email protected] is included in v8.9.4, yet the error is present.. tl;dr is minizlib should be updated to 1.0.4

@gibfahn Did something get left out of the npm update for v8.9.4?

[gibfahn]

It was backported by @MylesBorins according to #16509 (comment), so question for him 🤷‍♂️

[kasicka]

Seems like only 0d7e4d2 was backported to v8.x branches

[gibfahn]

Okay, sounds like we need to backport npm/npm@4ca6958 (9f33a24) in the upcoming 8.x release.

[kasicka]

Should b8888f5 be backported too?

[richardlau]

Should b8888f5 be backported too?

IMO yes it should, because 8.x is supposed to contain [email protected] and without it the files in our v8.x branch do not match the equivalent in npm's source tree:

https://github.com/npm/npm/blob/v5.6.0/lib/utils/unsupported.js#L3-L9

https://github.com/nodejs/node/blob/v8.9.4/deps/npm/lib/utils/unsupported.js#L3-L8

We're obviously going to have to be careful with backports when we float patches on top of npm on master/current (e.g., 9f33a24 and b8888f5) because it means those diffs aren't included in the subsequent "update npm to vX" PRs.

cc @nodejs/lts