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 1.4.29 #3639

Closed
wants to merge 1 commit into from
Closed

Conversation

othiym23
Copy link
Contributor

@othiym23 othiym23 commented Nov 3, 2015

See https://github.com/npm/npm/releases/tag/v1.4.29 for details.

Encourage users to upgrade to a newer npm that supports scopes, and lays the groundwork for getting npm@2 into Node 0.10 LTS.

r: @jasnell
r: @Fishrock123

See https://github.com/npm/npm/releases/tag/v1.4.29 for details.
Encourage users to upgrade to a newer npm, and lays the groundwork for
getting npm@2 into Node 0.10 LTS.
@othiym23 othiym23 added npm Issues and PRs related to the npm client dependency or the npm registry. lts Issues and PRs related to Long Term Support releases. labels Nov 3, 2015
@ChALkeR ChALkeR added the v0.10 label Nov 3, 2015
@rvagg
Copy link
Member

rvagg commented Nov 3, 2015

lgtm

@Fishrock123
Copy link
Contributor

Refs: nodejs/Release#37

@othiym23 You mentioned npm/npm@b9474a8 in that thread -- was the patch that prevented user credentials or something from being leaked?

@othiym23
Copy link
Contributor Author

othiym23 commented Nov 3, 2015

@Fishrock123: Yes, and that change is meant to be included by upgrading the version of npm included with 0.10 LTS to npm@2, not this (temporary and transitional) version of npm@1. This version of npm includes only the two changes proposed for [email protected] in nodejs/Release#37.

@Fishrock123
Copy link
Contributor

@othiym23 I understand. is there any actual reason that patch wouldn't apply though? It's relatively serious, I think.

@jasnell
Copy link
Member

jasnell commented Nov 3, 2015

hmm.. that's a tough one. Part of the reason for doing this is that npm v1 won't be getting any more support, including security updates but then we deliver it with a security update ;-) ... but until the updated v0.10 with npm2 is out, those users are still impacted. I think I tend to agree with @Fishrock123 on it tho, it would be best if this included that patch since it's an existing known issue.

@othiym23
Copy link
Contributor Author

othiym23 commented Nov 3, 2015

In my judgment, it is no more or less serious than the other security fixes included in npm@2+, and the sole reason for this version of npm's existence is to get 0.10 users off of unsupported npm and onto at least npm@2. I will not be making any security fixes to this version of npm.

@jasnell
Copy link
Member

jasnell commented Nov 3, 2015

Fair enough. This change LGTM

@jasnell
Copy link
Member

jasnell commented Nov 4, 2015

@Fishrock123 @zkat @othiym23 ... would it be possible to have y'all double check that this won't have the same kind of hiccups we were having with the recent npm updates in master? I'll likely get this landed in v0.10-staging by end of week.

@ChALkeR
Copy link
Member

ChALkeR commented Nov 5, 2015

Actual changes LGTM. I skipped the npm tests.
Note that this PR actually removes one man page (which is also LGTM, though).

@othiym23
Copy link
Contributor Author

othiym23 commented Nov 5, 2015

@zkat is in East Asia for conferencing, and I'm pretty sure this will be free of the shenanigans we were seeing with debug et al elsewhere, because the changes are 80% just version number changes in the documentation. @ChALkeR, that manpage was an npm@2 manpage that sneaked into [email protected] by accident, so removing it was for the best.

@Fishrock123, could I prevail upon you to double-check that the npm tests run for you with a clean build? You'll kind of have to do it by hand, because this version doesn't include the fixes to the test scripts we've landed in npm@2 / newer versions of Node.

@Fishrock123
Copy link
Contributor

Yeah I'll do the tests thing tomorrow. :)

On Nov 5, 2015, at 12:20 AM, Forrest L Norvell [email protected] wrote:

@zkat is in East Asia for conferencing, and I'm pretty sure this will be free of the shenanigans we were seeing with debug et al elsewhere, because the changes are 80% just version number changes in the documentation. @ChALkeR, that manpage was an npm@2 manpage that sneaked into [email protected] by accident, so removing it was for the best.

@Fishrock123, could I prevail upon you to double-check that the npm tests run for you with a clean build? You'll kind of have to do it by hand, because this version doesn't include the fixes to the test scripts we've landed in npm@2 / newer versions of Node.


Reply to this email directly or view it on GitHub.

@jasnell
Copy link
Member

jasnell commented Nov 5, 2015

Ok, quick test run on my end shows everything green also. Will get this landed now into v0.10-staging but can back out if anything comes up amiss in other tests

othiym23 added a commit that referenced this pull request Nov 5, 2015
See https://github.com/npm/npm/releases/tag/v1.4.29 for details.
Encourage users to upgrade to a newer npm, and lays the groundwork for
getting npm@2 into Node 0.10 LTS.

PR-URL: #3639
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@jasnell
Copy link
Member

jasnell commented Nov 5, 2015

Landed in v0.10-staging in b68781e

@Fishrock123
Copy link
Contributor

@jasnell did you sudo make install before running the tests? Otherwise that test run is not valid.

@MylesBorins
Copy link
Contributor

is there an environment in CI for running the npm tests?

@Fishrock123
Copy link
Contributor

@thealphanerd no, see nodejs/build#234 for more details.

@Fishrock123
Copy link
Contributor

LGTM though, but need to be careful about that.

@jasnell
Copy link
Member

jasnell commented Nov 6, 2015

Yes I did. Didn't see anything of concern but we still need to double and triple check. At this point there's nothing else in v0.10-staging so we're safe. Let's get everything verified.

@Fishrock123
Copy link
Contributor

Ok, should be fine then.

@rvagg
Copy link
Member

rvagg commented Dec 3, 2015

screen shot 2015-12-04 at 10 22 35 am

Going out in today's release. 👍

@othiym23
Copy link
Contributor Author

othiym23 commented Dec 4, 2015

🎉 Thanks to all, and a PR upgrading to npm@2 will be inbound to v0.10-staging just as soon as I help @Fishrock123 figure out why npm@3 is causing test falures against master / v5.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lts Issues and PRs related to Long Term Support releases. 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.

6 participants