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

[v8.x] deps: update npm to 5.4.2 #15600

Closed
wants to merge 1 commit into
base: v8.x-staging
from

Conversation

Projects
None yet
7 participants
@targos
Member

targos commented Sep 25, 2017

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.
  • #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.

Changelogs

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

@targos targos added the npm label Sep 25, 2017

@targos targos referenced this pull request Sep 25, 2017

Closed

deps: update npm to 5.4.2 #15486

2 of 2 tasks complete
@bzoz

This comment has been minimized.

Show comment
Hide comment
@bzoz

bzoz Sep 25, 2017

Contributor

Looks like this will fix #15095

Contributor

bzoz commented Sep 25, 2017

Looks like this will fix #15095

@bricss

This comment has been minimized.

Show comment
Hide comment
@bricss

bricss Sep 25, 2017

I can confirm that it will definitely fix #15095

bricss commented Sep 25, 2017

I can confirm that it will definitely fix #15095

@gibfahn

This comment has been minimized.

Show comment
Hide comment
@gibfahn

gibfahn Sep 25, 2017

Member

cc/ @nodejs/npm , would be good to get @zkat and @iarna to give this a look (AIUI it's #15486 (comment) but for 8.x).

@targos did you manage to run make test-npm on this? It should work on Darwin/Linux, but in my experience (not up to date) you get the odd test failure.

Member

gibfahn commented Sep 25, 2017

cc/ @nodejs/npm , would be good to get @zkat and @iarna to give this a look (AIUI it's #15486 (comment) but for 8.x).

@targos did you manage to run make test-npm on this? It should work on Darwin/Linux, but in my experience (not up to date) you get the odd test failure.

@targos

This comment has been minimized.

Show comment
Hide comment
@targos

targos Sep 25, 2017

Member

@gibfahn make test-npm seems to block on

> npm@5.4.2 test-node /home/mzasso/git/nodejs/node-v8/test-npm
> tap --timeout 240 "test/tap/*.js" "test/network/*.js" "test/broken-under-nyc*/*.js"

Reading TAP data from stdin (use "-" argument to suppress)

CPU is not working

Member

targos commented Sep 25, 2017

@gibfahn make test-npm seems to block on

> npm@5.4.2 test-node /home/mzasso/git/nodejs/node-v8/test-npm
> tap --timeout 240 "test/tap/*.js" "test/network/*.js" "test/broken-under-nyc*/*.js"

Reading TAP data from stdin (use "-" argument to suppress)

CPU is not working

@gibfahn

This comment has been minimized.

Show comment
Hide comment
@gibfahn

gibfahn Sep 25, 2017

Member

You could try the js one:

./node tools/test-npm-package --install deps/npm test

Or I could just land #11540 and we could run it through CI. Really needs some more reviews, but 🤷‍♂️ .

FWIW that output usually means that tap didn't find any files. Chances are the directory didn't get set up properly, you can just look through the bash script, it just copies deps/npm to test-npm and then does an npm install && npm test on it.

Member

gibfahn commented Sep 25, 2017

You could try the js one:

./node tools/test-npm-package --install deps/npm test

Or I could just land #11540 and we could run it through CI. Really needs some more reviews, but 🤷‍♂️ .

FWIW that output usually means that tap didn't find any files. Chances are the directory didn't get set up properly, you can just look through the bash script, it just copies deps/npm to test-npm and then does an npm install && npm test on it.

@targos

This comment has been minimized.

Show comment
Hide comment
@targos

targos Sep 25, 2017

Member

I tried test-npm-package and manual run. Both also hang. It's weird because I don't have this issue with a fresh clone of the npm repo.

Member

targos commented Sep 25, 2017

I tried test-npm-package and manual run. Both also hang. It's weird because I don't have this issue with a fresh clone of the npm repo.

@iarna

This comment has been minimized.

Show comment
Hide comment
@iarna

iarna Sep 26, 2017

Member

I'm ok with 5.4.2 going in (previous 5.4 releases weren't stable enough for me to feel comfortable).

I'll take a look at the tests, this should be pretty straight forward. The blocking happens when tap is given an empty list of tests (it falls back to reading them from STDIN, which results in the process never doing anything).

Member

iarna commented Sep 26, 2017

I'm ok with 5.4.2 going in (previous 5.4 releases weren't stable enough for me to feel comfortable).

I'll take a look at the tests, this should be pretty straight forward. The blocking happens when tap is given an empty list of tests (it falls back to reading them from STDIN, which results in the process never doing anything).

@gibfahn

This comment has been minimized.

Show comment
Hide comment
@gibfahn

gibfahn Sep 30, 2017

Member

@targos the version of npm in your PR doesn't seem to have a test directory, which I assume is why the test is hanging.

See https://github.com/targos/node/tree/npm-5.4.2-v8.x/deps/npm
cf: https://github.com/nodejs/node/tree/master/deps/npm

Member

gibfahn commented Sep 30, 2017

@targos the version of npm in your PR doesn't seem to have a test directory, which I assume is why the test is hanging.

See https://github.com/targos/node/tree/npm-5.4.2-v8.x/deps/npm
cf: https://github.com/nodejs/node/tree/master/deps/npm

@targos

This comment has been minimized.

Show comment
Hide comment
@targos

targos Sep 30, 2017

Member

OK. That makes sense... Then the issue is that npm doesn't include the test directory anymore in the release tarball.

Member

targos commented Sep 30, 2017

OK. That makes sense... Then the issue is that npm doesn't include the test directory anymore in the release tarball.

@targos

This comment has been minimized.

Show comment
Hide comment
@targos

targos Sep 30, 2017

Member

This is because the version of npm-packlist present in npm 5.4.2 ignores the test directory. This has been reverted in npm/npm-packlist#3 and should be OK in the next version of npm.
@iarna do you know when the release is scheduled?

Member

targos commented Sep 30, 2017

This is because the version of npm-packlist present in npm 5.4.2 ignores the test directory. This has been reverted in npm/npm-packlist#3 and should be OK in the next version of npm.
@iarna do you know when the release is scheduled?

deps: update npm to 5.4.2
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`.
* [#17844](npm/npm#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.

Changelogs

* [`v5.4.0`](https://github.com/npm/npm/releases/tag/v5.4.0)
* [`v5.4.1`](https://github.com/npm/npm/releases/tag/v5.4.1)
* [`v5.4.2`](https://github.com/npm/npm/releases/tag/v5.4.2)
@MylesBorins

This comment has been minimized.

Show comment
Hide comment
@MylesBorins

MylesBorins Oct 3, 2017

Member

landed in 547fe58

Member

MylesBorins commented Oct 3, 2017

landed in 547fe58

@MylesBorins MylesBorins closed this Oct 3, 2017

MylesBorins added a commit that referenced this pull request Oct 11, 2017

2017-10-11, Node.js Version 8.7.0 (Current)
Notable Changes:

* deps:
  * update npm to 5.4.2
    #15600
  * upgrade libuv to 1.15.0
    #15745
  * update V8 to 6.1.534.42
    #15393
* dgram:
  * support for setting dgram socket buffer size
    #13623
* fs:
  * add support O_DSYNC file open constant
    #15451
* util:
  * deprecate obj.inspect for custom inspection
    #15631
* tools, build:
  * there is a fancy new macOS installer
    #15179
* Added new collaborator
  * bmeurer - Benedikt Meurer - https://github.com/bmeurer
  * kfarnung - Kyle Farnung - https://github.com/kfarnung

PR-URL: #15762

@MylesBorins MylesBorins referenced this pull request Oct 11, 2017

Merged

v8.7.0 proposal #15762

MylesBorins added a commit that referenced this pull request Oct 11, 2017

2017-10-11, Node.js Version 8.7.0 (Current)
Notable Changes:

* deps:
  * update npm to 5.4.2
    #15600
  * upgrade libuv to 1.15.0
    #15745
  * update V8 to 6.1.534.42
    #15393
* dgram:
  * support for setting dgram socket buffer size
    #13623
* fs:
  * add support O_DSYNC file open constant
    #15451
* util:
  * deprecate obj.inspect for custom inspection
    #15631
* tools, build:
  * there is a fancy new macOS installer
    #15179
* Added new collaborator
  * bmeurer - Benedikt Meurer - https://github.com/bmeurer
  * kfarnung - Kyle Farnung - https://github.com/kfarnung

PR-URL: #15762

addaleax added a commit to addaleax/ayo that referenced this pull request Oct 12, 2017

2017-10-11, Node.js Version 8.7.0 (Current)
Notable Changes:

* deps:
  * update npm to 5.4.2
    nodejs/node#15600
  * upgrade libuv to 1.15.0
    nodejs/node#15745
  * update V8 to 6.1.534.42
    nodejs/node#15393
* dgram:
  * support for setting dgram socket buffer size
    nodejs/node#13623
* fs:
  * add support O_DSYNC file open constant
    nodejs/node#15451
* util:
  * deprecate obj.inspect for custom inspection
    nodejs/node#15631
* tools, build:
  * there is a fancy new macOS installer
    nodejs/node#15179
* Added new collaborator
  * bmeurer - Benedikt Meurer - https://github.com/bmeurer
  * kfarnung - Kyle Farnung - https://github.com/kfarnung

PR-URL: nodejs/node#15762

@targos targos deleted the targos:npm-5.4.2-v8.x branch Oct 23, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment