FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

deps: update npm to v5.5.1 by MylesBorins · Pull Request #16509 · nodejs/node · GitHub

/ node Public

deps: update npm to v5.5.1 - #16509

Closed
MylesBorins wants to merge 3 commits into
nodejs:masterfrom
MylesBorins:npm-5.5.1
ConversationCommits (3)ChecksFiles changed
Closed

deps: update npm to v5.5.1#16509
MylesBorins wants to merge 3 commits into
nodejs:masterfrom
MylesBorins:npm-5.5.1

Conversation

MylesBorins commented Oct 26, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

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 added this to the 9.0.0 milestone Oct 26, 2017
nodejs-github-bot added the npm Issues and PRs related to the npm client dependency or the npm registry. label Oct 26, 2017

MylesBorins commented Oct 26, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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 approved these changes Oct 26, 2017
View reviewed changes

addaleax left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

MylesBorins commented Oct 27, 2017

Copy link
Copy Markdown
Contributor Author

@addaleax done 😄

MylesBorins commented Oct 27, 2017

Copy link
Copy Markdown
Contributor Author

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

MylesBorins commented Oct 27, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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 commented Oct 29, 2017

Copy link
Copy Markdown
Member

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

targos approved these changes Oct 29, 2017
View reviewed changes

targos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

jasnell commented Oct 29, 2017

Copy link
Copy Markdown
Member

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

MylesBorins commented Oct 30, 2017

Copy link
Copy Markdown
Contributor Author

landed in 64168eb...b8888f5

MylesBorins added a commit that referenced this pull request Oct 30, 2017
deps: update npm to 5.5.1
ace4fe5 
Closes: #16280

PR-URL: #16509
Fixes: #14161
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins added a commit that referenced this pull request Oct 30, 2017
deps: backport 4ca695819 from npm upstream
9f33a24 
Original commit message:

    minizlib@1.0.4

    Fixes Node 9 compatibility.

    Credit: @isaacs

PR-URL: #16509
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins added a commit that referenced this pull request Oct 30, 2017
deps: manually add 9.x support to npm
b8888f5 
refs: npm/npm#18964

PR-URL: #16509
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins added a commit that referenced this pull request Oct 30, 2017
deps: update npm to 5.5.1
0d7e4d2 
Closes: #16280

PR-URL: #16509
Fixes: #14161
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
chrmoritz mentioned this pull request Oct 31, 2017
Closed
4 tasks
Qard pushed a commit to ayojs/ayo that referenced this pull request Nov 2, 2017
deps: update npm to 5.5.1
ba243dd 
Closes: nodejs/node#16280

PR-URL: nodejs/node#16509
Fixes: nodejs/node#14161
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Qard pushed a commit to ayojs/ayo that referenced this pull request Nov 2, 2017
deps: backport 4ca695819 from npm upstream
dd07440 
Original commit message:

    minizlib@1.0.4

    Fixes Node 9 compatibility.

    Credit: @isaacs

PR-URL: nodejs/node#16509
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Qard pushed a commit to ayojs/ayo that referenced this pull request Nov 2, 2017
deps: manually add 9.x support to npm
34b126c 
refs: npm/npm#18964

PR-URL: nodejs/node#16509
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Qard pushed a commit to ayojs/ayo that referenced this pull request Nov 2, 2017
2017-10-31, Version 8.9.0 'Carbon' (LTS) Release
781098d 
This LTS release comes with 87 commits. This includes 30 that are
updates to lib/ or src/, 20 that are test related, 13 that are doc
related, 19 which are build / tools related, and 4 commits which are
updates to dependencies.

Notable Changes:

* doc:
  - add Gibson Fahnestock to Release team (Gibson Fahnestock)
    nodejs/node#16620
* deps:
  - update npm to 5.5.1 (Myles Borins)
    nodejs/node#16509
* http2:
  - The exposed http2 socket is no longer manipulatable
    (Anatoli Papirovski)
    nodejs/node#16330
* module:
  - support custom paths to require.resolve() (cjihrig)
    nodejs/node#16397
* util:
  - util.TextEncoder and util.TextDecoder are no longer experimental.
    There will no longer be a warning when they are used
    (James M Snell)
    nodejs/node#15743

PR-URL: nodejs/node#16630
Qard pushed a commit to ayojs/ayo that referenced this pull request Nov 2, 2017
deps: update npm to 5.5.1
803e41e 
Closes: nodejs/node#16280

PR-URL: nodejs/node#16509
Fixes: nodejs/node#14161
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Qard pushed a commit to ayojs/ayo that referenced this pull request Nov 2, 2017
deps: backport 4ca695819 from npm upstream
36bffa8 
Original commit message:

    minizlib@1.0.4

    Fixes Node 9 compatibility.

    Credit: @isaacs

PR-URL: nodejs/node#16509
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Qard pushed a commit to ayojs/ayo that referenced this pull request Nov 2, 2017
deps: manually add 9.x support to npm
b95da1e 
refs: npm/npm#18964

PR-URL: nodejs/node#16509
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Qard pushed a commit to ayojs/ayo that referenced this pull request Nov 2, 2017
2017-10-31, Version 8.9.0 'Carbon' (LTS) Release
ad502ca 
This LTS release comes with 87 commits. This includes 30 that are
updates to lib/ or src/, 20 that are test related, 13 that are doc
related, 19 which are build / tools related, and 4 commits which are
updates to dependencies.

Notable Changes:

* doc:
  - add Gibson Fahnestock to Release team (Gibson Fahnestock)
    nodejs/node#16620
* deps:
  - update npm to 5.5.1 (Myles Borins)
    nodejs/node#16509
* http2:
  - The exposed http2 socket is no longer manipulatable
    (Anatoli Papirovski)
    nodejs/node#16330
* module:
  - support custom paths to require.resolve() (cjihrig)
    nodejs/node#16397
* util:
  - util.TextEncoder and util.TextDecoder are no longer experimental.
    There will no longer be a warning when they are used
    (James M Snell)
    nodejs/node#15743

PR-URL: nodejs/node#16630
MylesBorins deleted the npm-5.5.1 branch November 14, 2017 17:44
This was referenced Nov 19, 2017
Closed
Closed
addaleax pushed a commit to ayojs/ayo that referenced this pull request Dec 7, 2017
deps: update npm to 5.5.1
9391afd 
Closes: nodejs/node#16280

PR-URL: nodejs/node#16509
Fixes: nodejs/node#14161
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Dec 7, 2017
deps: backport 4ca695819 from npm upstream
17403a6 
Original commit message:

    minizlib@1.0.4

    Fixes Node 9 compatibility.

    Credit: @isaacs

PR-URL: nodejs/node#16509
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Dec 7, 2017
deps: manually add 9.x support to npm
637fed7 
refs: npm/npm#18964

PR-URL: nodejs/node#16509
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Dec 7, 2017
2017-10-31, Version 8.9.0 'Carbon' (LTS) Release
ceddd58 
This LTS release comes with 87 commits. This includes 30 that are
updates to lib/ or src/, 20 that are test related, 13 that are doc
related, 19 which are build / tools related, and 4 commits which are
updates to dependencies.

Notable Changes:

* doc:
  - add Gibson Fahnestock to Release team (Gibson Fahnestock)
    nodejs/node#16620
* deps:
  - update npm to 5.5.1 (Myles Borins)
    nodejs/node#16509
* http2:
  - The exposed http2 socket is no longer manipulatable
    (Anatoli Papirovski)
    nodejs/node#16330
* module:
  - support custom paths to require.resolve() (cjihrig)
    nodejs/node#16397
* util:
  - util.TextEncoder and util.TextDecoder are no longer experimental.
    There will no longer be a warning when they are used
    (James M Snell)
    nodejs/node#15743

PR-URL: nodejs/node#16630

kasicka commented Feb 5, 2018

Copy link
Copy Markdown

Can we backport npm/npm@4ca6958 to v8.x branch too? I get npm ERR! invalid: minizlib@1.0.3 /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 commented Feb 5, 2018

Copy link
Copy Markdown
Member

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

cc @nodejs/lts @nodejs/npm

kasicka commented Feb 5, 2018

Copy link
Copy Markdown

@richardlau npm@5.6.0 is included in v8.9.4, yet the error is present.. tl;dr is minizlib should be updated to 1.0.4

richardlau commented Feb 6, 2018

Copy link
Copy Markdown
Member

npm@5.6.0 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 commented Feb 6, 2018

Copy link
Copy Markdown
Member

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

kasicka commented Feb 6, 2018

Copy link
Copy Markdown

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

gibfahn commented Feb 6, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

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

kasicka commented Feb 7, 2018

Copy link
Copy Markdown

Should b8888f5 be backported too?

richardlau commented Feb 7, 2018

Copy link
Copy Markdown
Member

Should b8888f5 be backported too?

IMO yes it should, because 8.x is supposed to contain npm@5.6.0 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

kasicka mentioned this pull request Feb 7, 2018
Closed
4 tasks
MylesBorins added a commit to MylesBorins/node that referenced this pull request Feb 7, 2018
deps: re land npm 5.6.0
9e81a50 
When the original backport was landed there were some missing commits
on v8.x. This commit completely re lands npm 5.6.0 to make sure the
version on 8.x has the correct file tree

Refs: nodejs#16509 (comment)
MylesBorins mentioned this pull request Feb 7, 2018
Closed
gibfahn pushed a commit that referenced this pull request Feb 8, 2018
deps: re land npm 5.6.0
96f85e4 
When the original backport was landed there were some missing commits
on v8.x. This commit completely re lands npm 5.6.0 to make sure the
version on 8.x has the correct file tree

Refs: #16509 (comment)
PR-URL: #18625
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
richardlau mentioned this pull request Feb 9, 2018
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

addaleax addaleax approved these changes
watilde watilde approved these changes
jasnell Awaiting requested review from jasnell
iarna Awaiting requested review from iarna
+1 more reviewer
targos targos approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

npm Issues and PRs related to the npm client dependency or the npm registry.

Projects

None yet

Milestone

9.0.0

Development

Successfully merging this pull request may close these issues.

9 participants

Back | FazBrowse Home | New Git URL