deps: update npm to 5.4.2

[targos]

Followed the guide to update npm to 5.4.2.

/cc @nodejs/npm @zkat@Fishrock123

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

Affected core subsystem(s)

deps

[targos]

I don't know if make test-npm is supposed to pass now on Linux, but I'm getting a scary error:

/home/mzasso/git/nodejs/node/out/Release/node[3473]: ../src/node_zlib.cc:438:static void node::{anonymous}::ZCtx::Init(const v8::FunctionCallbackInfo<v8::Value>&): Assertion `args.Length() == 7 && "init(windowBits, level, memLevel, strategy, writeResult, writeCallback," " dictionary)"' failed. 1: node::Abort() [npm] 2: node::Assert(char const* const (*) [4]) [npm] 3: 0x12e40aa [npm] 4: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&)) [npm] 5: 0xb05dec [npm] 6: v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*) [npm] 7: 0x1010d2f0463d tools/test-npm.sh: line 38: 3473 Aborted (core dumped) node bin/npm-cli.js install --ignore-scripts --no-save make: *** [Makefile:406: test-npm] Error 134 

[targos]

I found the problem. The CHECK is triggered by a call from minizlib:

this[_handle].init(opts.windowBits||constants.Z_DEFAULT_WINDOWBITS,level,opts.memLevel||constants.Z_DEFAULT_MEMLEVEL,strategy,opts.dictionary)

The library is directly calling the internal function with 5 arguments. The function's signature was changed four months ago from 4/5 arguments to 7 in add4b0a (zlib: improve performance).

[targos]

minizlib is a dependency of the tar package (that has 9M downloads/month and 764 dependents, including npm).
/cc @nodejs/tsc

[Fishrock123]

Let's let the npm folks chime in before deciding anything. Also looping in @isaacs who is the author of minizlib.

[addaleax]

I guess that might be the root cause of #14161 then … sigh.

[MylesBorins]

@targos could you open an issue on minizlib with the findings?

[gibfahn]

Given that #13322 hasn't landed on 8.x, could this be retargeted to 8.x for now?

I know it's not ideal for npm on 8.x to be ahead of master, but it sounds like this update fixes a bunch of serious bugs people are hitting, so getting this out would be a priority.

[zkat]

This LGTM -- would be good for @iarna to eyeball it too.

This includes three npm releases, one of which is pretty big (5.4.0).

Changes of Note

• Significant performance boost for installations from cache (~10%+)
• A number of bugfixes for important and severe bugs affecting installation on all supported platforms, but particularly noticeable on Windows.
• More Windows-related fixes for npx.
• b6d5549d2
  #17844
  Make package-lock.json sorting locale-agnostic. This will cause some users to see seemingly spurious diffs on their pkglocks if they were using previous versions of npm5, because, for example, JSONStream is sorted into a different location. This is fine and will go away as people upgrade.
  (@LotharSee)

Changelogs

• v5.4.0
• v5.4.1
• v5.4.2

[targos]

Thanks @zkat! I updated the commit message with you notes.

PR against v8.x: #15600

[MylesBorins]

ping regarding getting this to work on master

[MylesBorins]

@targos is there a PR to replace this?

[targos]

@MylesBorins No. There is no stable npm version that fixes the zlib issue at the moment.

[MylesBorins]

@targos this should likely be kept open then in case we end up reverting the zlib fix

[targos]

If that happens, we should update to the current version. It's not difficult to do. Feel free to ping me in case I miss it.