Skip to content
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 3.6.0 #4958

Closed
wants to merge 2 commits into from
Closed

deps: upgrade npm to 3.6.0 #4958

wants to merge 2 commits into from

Conversation

iarna
Copy link
Member

@iarna iarna commented Jan 29, 2016

This is the big one! After this I anticipate getting back into a weekly cadence of upstreaming things to Node.js.

Contains the changes in these releases:

Notable inclusions are:

  • The clarified split between Terms of Service and the CLI's own license (see the discussion in https://github.com/npm/npm/releases/tag/v3.5.1 and Potential Licensing issues with npm #3959 for more context).
  • Several important changes to how edge cases involving bundleDependencies are handled.
  • Tests don't implode when running under a node prerelease.
  • You also only get one warning now about using a prerelease version rather than dozens and dozens from failed engine checks.
  • A handful of minor new user affordances in 3.6.0– in particular npm outdated reports linked modules & npm version can now take from-git as an argument.

r: @Fishrock123
r: @jasnell
r: @mikeal

@iarna iarna added the npm Issues and PRs related to the npm client dependency or the npm registry. label Jan 29, 2016
@MylesBorins
Copy link
Member

@iarna
Copy link
Member Author

iarna commented Jan 29, 2016

Ah I may have stepped on some of your CI– I just repushed with PR-URL in the commit messages.

@zkat
Copy link
Contributor

zkat commented Jan 29, 2016

info note: Most of the licensing stuff covered here was done in LTS with #4110 and #4872 -- these should be the same changes. :)

@MylesBorins
Copy link
Member

Doesn't look like it got in the way of the CI job. restarted citgm.

I'm also running make test-npm locally. First run through had two failures, but I think they were related to my settings for progress bar. I've removed that line from my .npmrc and am running again. I'll edit this comment with the results when it is done

@Fishrock123
Copy link
Member

@iarna I'm getting what appears to be a new failure:

test/tap/progress-config.js ........................... 4/5
  default
  not ok should be enabled
    +++ found                                                          
    --- wanted                                                         
    -true                                                              
    +false                                                             
    compare: ===
    at:
      line: 34
      column: 7
      file: test/tap/progress-config.js
      type: EventEmitter
    source: |
      t.is(log.progressEnabled, true, 'should be enabled')
    stack: |
      EventEmitter.<anonymous> (test/tap/progress-config.js:34:7)

@MylesBorins
Copy link
Member

@Fishrock123 that's the same failure I was getting. Do you have your progress config set off globally atm?

@MylesBorins
Copy link
Member

After removing my global setting for progress everything passes as expected

screen shot 2016-01-29 at 2 27 06 pm

@Fishrock123
Copy link
Member

Yeah my progress is disabled. I don't really think it should be picking it up globally though?

@iarna
Copy link
Member Author

iarna commented Jan 29, 2016

I'll make a note to make that test agnostic to your configuration.

@Fishrock123
Copy link
Member

@iarna I'm not really sure how npm tests work, my assumption is that it would be ideal to run them as isolated as possible, but perhaps that's not true?

@iarna
Copy link
Member Author

iarna commented Jan 29, 2016

@Fishrock123 The tests broadly assume that you haven't fiddled with the defaults to various config values, which is not fantastic, but is fine for CI (and for the rest of us it's easy to tweak our configs while testing). When we see things where behavior is gonna be substantially different than expect if you change your config we've been trying to explicitly set the config that the test is testing, but it's been an as-we-go thing, not comprehensive.

@zkat
Copy link
Contributor

zkat commented Jan 29, 2016

LGTM here. I assume we'll catch that particular config thing next time around.

@jasnell
Copy link
Member

jasnell commented Jan 29, 2016

LGTM as well. Incremental steps :)
On Jan 29, 2016 3:20 PM, "Kat Marchán" notifications@github.com wrote:

LGTM from me. I assume we'll catch that particular config thing next time
around.


Reply to this email directly or view it on GitHub
#4958 (comment).

Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Feb 1, 2016
PR-URL: nodejs#4958
Reviewed-By: Myles Borins <mborins@us.ibm.com>
Reviewed-By: Kat Marchán <kzm@sykosomatic.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Feb 1, 2016
PR-URL: nodejs#4958
Reviewed-By: Myles Borins <mborins@us.ibm.com>
Reviewed-By: Kat Marchán <kzm@sykosomatic.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@Fishrock123
Copy link
Member

Thanks, landed in d5d301f...18c12bb!

@iarna Just a nit, I had to change the license commit message, it was a bit over 50 chars. :)

@iarna
Copy link
Member Author

iarna commented Feb 1, 2016

@Fishrock123 Ah! I'll watch for that in future

rvagg pushed a commit that referenced this pull request Feb 8, 2016
PR-URL: #4958
Reviewed-By: Myles Borins <mborins@us.ibm.com>
Reviewed-By: Kat Marchán <kzm@sykosomatic.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
rvagg pushed a commit that referenced this pull request Feb 8, 2016
PR-URL: #4958
Reviewed-By: Myles Borins <mborins@us.ibm.com>
Reviewed-By: Kat Marchán <kzm@sykosomatic.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
jasnell added a commit that referenced this pull request Feb 9, 2016
This is an important security release. All Node.js users should
consult the security release summary at nodejs.org for details on
patched vulnerabilities.

Notable changes

* http: fix defects in HTTP header parsing for requests and responses
  that can allow request smuggling (CVE-2016-2086) or response
  splitting (CVE-2016-2216). HTTP header parsing now aligns more
  closely with the HTTP spec including restricting the acceptable
  characters.
* http-parser: upgrade from 2.6.0 to 2.6.1
* npm: upgrade npm from 3.3.12 to 3.6.0
  (Rebecca Turner) #4958
* openssl: upgrade from 1.0.2e to 1.0.2f. To mitigate against the
  Logjam attack, TLS clients now reject Diffie-Hellman handshakes with
  parameters shorter than 1024-bits, up from the previous limit of
  768-bits.
jasnell added a commit that referenced this pull request Feb 9, 2016
This is an important security release. All Node.js users should
consult the security release summary at nodejs.org for details on
patched vulnerabilities.

Notable changes

* http: fix defects in HTTP header parsing for requests and responses
  that can allow request smuggling (CVE-2016-2086) or response
  splitting (CVE-2016-2216). HTTP header parsing now aligns more
  closely with the HTTP spec including restricting the acceptable
  characters.
* http-parser: upgrade from 2.6.0 to 2.6.1
* npm: upgrade npm from 3.3.12 to 3.6.0
  (Rebecca Turner) #4958
* openssl: upgrade from 1.0.2e to 1.0.2f. To mitigate against the
  Logjam attack, TLS clients now reject Diffie-Hellman handshakes with
  parameters shorter than 1024-bits, up from the previous limit of
  768-bits.
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
PR-URL: nodejs#4958
Reviewed-By: Myles Borins <mborins@us.ibm.com>
Reviewed-By: Kat Marchán <kzm@sykosomatic.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
PR-URL: nodejs#4958
Reviewed-By: Myles Borins <mborins@us.ibm.com>
Reviewed-By: Kat Marchán <kzm@sykosomatic.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
npm Issues and PRs related to the npm client dependency or the npm registry.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants