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

deps: upgrade npm to v6.5.0 #25234

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
6 participants
@ljharb
Copy link
Contributor

ljharb commented Dec 26, 2018

I noticed that node v11.6 ships with an npm that reports a version of v6.5.0-next.0. This PR updates that to actual v6.5.0.

Checklist
@ljharb

This comment has been minimized.

Copy link
Contributor Author

ljharb commented Dec 26, 2018

@ljharb

This comment has been minimized.

Copy link
Contributor Author

ljharb commented Dec 26, 2018

Local test failures:

        not ok 9 - config properties list: projectData: # of elements
          ---
          found: 2
          wanted: 0
          compare: ===
          at:
            line: 66
            column: 5
            file: test/tap/config-basic.js
            type: global
            function: isDeeplyDetails
          stack: |
            isDeeplyDetails (test/tap/config-basic.js:66:5)
            test/tap/config-basic.js:77:7
            Array.forEach (<anonymous>)
            test/tap/config-basic.js:76:17
            lib/config/core.js:83:7
            Array.forEach (<anonymous>)
            lib/config/core.js:82:13
            f (node_modules/once/once.js:25:25)
            finalize (lib/config/core.js:193:5)
            afterExtras (lib/config/core.js:184:5)
            node_modules/mkdirp/index.js:48:26
          source: |
            t.is(Object.keys(aa).length, Object.keys(bb).length, msg + ': # of elements')
          ...

        not ok 51 - config by source -> project -> data: # of elements
          ---
          found: 2
          wanted: 0
          compare: ===
          at:
            line: 66
            column: 5
            file: test/tap/config-basic.js
            type: global
            function: isDeeplyDetails
          stack: |
            isDeeplyDetails (test/tap/config-basic.js:66:5)
            test/tap/config-basic.js:68:5
            Array.forEach (<anonymous>)
            isDeeplyDetails (test/tap/config-basic.js:67:19)
            test/tap/config-basic.js:68:5
            Array.forEach (<anonymous>)
            isDeeplyDetails (test/tap/config-basic.js:67:19)
            test/tap/config-basic.js:79:5
            lib/config/core.js:83:7
            Array.forEach (<anonymous>)
            lib/config/core.js:82:13
            f (node_modules/once/once.js:25:25)
            finalize (lib/config/core.js:193:5)
            afterExtras (lib/config/core.js:184:5)
            node_modules/mkdirp/index.js:48:26
          source: |
            t.is(Object.keys(aa).length, Object.keys(bb).length, msg + ': # of elements')
          ...
@MylesBorins

This comment has been minimized.

Copy link
Member

MylesBorins commented Dec 27, 2018

@ljharb is that test failing on master as well?

@ljharb

This comment has been minimized.

Copy link
Contributor Author

ljharb commented Dec 27, 2018

Hmm, I'm seeing many more of the npm tests failing on master - but maybe I'm not running the tests properly. Some help, or independent confirmation, would be appreciated.

@Trott

This comment has been minimized.

Copy link
Member

Trott commented Dec 29, 2018

Definitely tests on master are failing. It appears to have been this way for years. I tried to bisect to find out when the last time make test-npm passed. It's possible I got it wrong because maybe it had multiple periods of being fixed amidst long stretches of being broken, but the earliest time I could find was 3 years ago! 😱

Going through failures now and seeing about fixing them, but it's going to be slow-going. For example: #22519 (comment)

If you're seeing fewer test failures with the newer npm, that's probably a sign that a bunch of things have been fixed. I'd call that success.

@jasnell

This comment has been minimized.

Copy link
Member

jasnell commented Dec 29, 2018

Might be about time to revisit the question of whether we really need npm in our source tree. Sure, we can still huddle it in builds, but having it in the repo likely no longer makes sense (if it ever did).

@ljharb

This comment has been minimized.

Copy link
Contributor Author

ljharb commented Dec 29, 2018

In that case, can this land and go out in an 11.6.1? :-D

@Trott

Trott approved these changes Dec 30, 2018

Copy link
Member

Trott left a comment

RSLGTM

@Trott

This comment has been minimized.

Copy link
Member

Trott commented Dec 30, 2018

@Trott Trott added the author ready label Dec 30, 2018

@ljharb

This comment has been minimized.

Copy link
Contributor Author

ljharb commented Dec 31, 2018

@Trott any idea why that one FreeBSD job failed?

@Trott

This comment has been minimized.

Copy link
Member

Trott commented Dec 31, 2018

@Trott any idea why that one FreeBSD job failed?

Known problem with the test that makes it fail from time to time.

Resume Build CI: https://ci.nodejs.org/job/node-test-pull-request/19889/

@Trott

This comment has been minimized.

Copy link
Member

Trott commented Dec 31, 2018

Hmmm...same FreeBSD failure. That's not reassuring. But it seems very unlikely to actually be related. Let's try one more time: https://ci.nodejs.org/job/node-test-pull-request/19890/

@ljharb

This comment has been minimized.

Copy link
Contributor Author

ljharb commented Dec 31, 2018

Yay, looks like that passed

@Trott

This comment has been minimized.

Copy link
Member

Trott commented Jan 3, 2019

This is now 7 days old, has one approval, and a green CI. It can land. However, I'd really like it if it had a second pair of eyes giving it a review. Anyone? @nodejs/npm @lance

@lance
Copy link
Contributor

lance left a comment

Looks good to me. I had the same failures, plus an additional three due to the fact that I have git configured to sign my commits. Updating my config with commit.gpgsign=false for deps/npm seemed to resolve the additional three I was seeing.

@lance

This comment has been minimized.

Copy link
Contributor

lance commented Jan 3, 2019

Landed in aea5229

@lance lance closed this Jan 3, 2019

lance added a commit that referenced this pull request Jan 3, 2019

deps: upgrade npm to v6.5.0
PR-URL: #25234
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Lance Ball <lball@redhat.com>

@ljharb ljharb deleted the ljharb:npm-6.5.0 branch Jan 3, 2019

addaleax added a commit that referenced this pull request Jan 5, 2019

deps: upgrade npm to v6.5.0
PR-URL: #25234
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Lance Ball <lball@redhat.com>

refack added a commit to refack/node that referenced this pull request Jan 14, 2019

deps: upgrade npm to v6.5.0
PR-URL: nodejs#25234
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Lance Ball <lball@redhat.com>

@BridgeAR BridgeAR referenced this pull request Jan 16, 2019

Merged

v11.7.0 proposal #25537

BridgeAR added a commit to BridgeAR/node that referenced this pull request Jan 16, 2019

deps: upgrade npm to v6.5.0
PR-URL: nodejs#25234
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Lance Ball <lball@redhat.com>

BridgeAR added a commit to BridgeAR/node that referenced this pull request Jan 17, 2019

2019-01-17, Version 11.7.0 (Current), @BridgeAR
Notable Changes

* compression / zlib:
  * Added brotli support (Anna Henningsen and Zach Vacura)
    nodejs#24938
* console:
  * Added `inspectOptions` option (Ruben Bridgewater)
    nodejs#24978
* crypto:
  * Always accept private keys as public keys (Tobias Nießen)
    nodejs#25217
* deps:
  * Upgrade npm to v6.5.0 (Jordan Harband)
    nodejs#25234
* fs:
  * Use internalBinding('fs') internally instead of
    process.binding('fs') (Masashi Hirano)
    nodejs#22478
* http(s):
  * Support overriding http\\s.globalAgent (Roy Sommer)
    nodejs#25170
* util:
  * Inspect ArrayBuffers contents closely (Ruben Bridgewater)
    nodejs#25006

PR-URL: nodejs#25537

BridgeAR added a commit to BridgeAR/node that referenced this pull request Jan 17, 2019

2019-01-17, Version 11.7.0 (Current), @BridgeAR
Notable Changes

* compression / zlib:
  * Added brotli support (Anna Henningsen and Zach Vacura)
    nodejs#24938
* console:
  * Added `inspectOptions` option (Ruben Bridgewater)
    nodejs#24978
* crypto:
  * Always accept private keys as public keys (Tobias Nießen)
    nodejs#25217
* deps:
  * Upgrade npm to v6.5.0 (Jordan Harband)
    nodejs#25234
* fs:
  * Use internalBinding('fs') internally instead of
    process.binding('fs') (Masashi Hirano)
    nodejs#22478
* http(s):
  * Support overriding http\\s.globalAgent (Roy Sommer)
    nodejs#25170
* util:
  * Inspect ArrayBuffers contents closely (Ruben Bridgewater)
    nodejs#25006
* worker:
  * Expose workers by default and remove `--experimental-worker` flag
    (Anna Henningsen) nodejs#25361

PR-URL: nodejs#25537

BridgeAR added a commit that referenced this pull request Jan 18, 2019

2019-01-17, Version 11.7.0 (Current), @BridgeAR
Notable Changes

* compression / zlib:
  * Added brotli support (Anna Henningsen and Zach Vacura)
    #24938
* console:
  * Added `inspectOptions` option (Ruben Bridgewater)
    #24978
* crypto:
  * Always accept private keys as public keys (Tobias Nießen)
    #25217
* deps:
  * Upgrade npm to v6.5.0 (Jordan Harband)
    #25234
* fs:
  * Use internalBinding('fs') internally instead of
    process.binding('fs') (Masashi Hirano)
    #22478
* http(s):
  * Support overriding http\\s.globalAgent (Roy Sommer)
    #25170
* util:
  * Inspect ArrayBuffers contents closely (Ruben Bridgewater)
    #25006
* worker:
  * Expose workers by default and remove `--experimental-worker` flag
    (Anna Henningsen) #25361

PR-URL: #25537

@MylesBorins MylesBorins referenced this pull request Jan 24, 2019

Merged

v11.8.0 proposal #25687

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