Release proposal: v2.3.1 by rvagg · Pull Request #1996 · nodejs/node · GitHub

As pointed out by @silverwind, 671e64a contains an important fix for Windows users that we should get out ASAP. Let me know if there objections to a 24-hour window for this, I figure that a patch release with a limited list of commits should be straightforward.

[e56758a5e0] - async-wrap: add provider id and object info cb (Trevor Norris) #1896
[d5637e67c9] - buffer: fix cyclic dependency with util (Brendan Ashworth) #1988
[c207e8d223] - build: fix pkg-config output parsing in configure (Ben Noordhuis) #1986
[8d8a26e8f7] - build: don't run lint from test-ci (Johan Bergström) #1965
[1ec53c044d] - build: simplify execution of built binary (Johan Bergström) #1955
[3beb880716] - crypto: add cert check to CNNIC Whitelist (Shigeki Ohtsu) #1895
[6aab2f3b9a] - deps: make node-gyp work with io.js (cjihrig) iojs/io.js#990
[3e12561b55] - deps: upgrade to npm 2.11.2 (Rebecca Turner) #1956
[1f93b63b11] - doc: change the info to the same as in gitconfig (Christian Tellnes) #2000
[0cf94e6856] - doc: mention CI in Collaborator Guide (Rich Trott) #1995
[7a3006efe4] - doc: add TOC links to Collaborator Guide (Rich Trott) #1994
[30638b150f] - doc: add TSC meeting notes 2015-06-10 (Bert Belder) #1943
[c4ec04136b] - doc: reformat authors section (Johan Bergström) #1966
[96165f9be2] - doc: minor clarification in the modules API doc. (Сковорода Никита Андреевич) #1983
[5c2707c1b2] - doc: benchmark/README.md copyedit (Rich Trott) #1970
[74fdf732d0] - doc: copyedit COLLABORATOR_GUIDE.md (Rich Trott) #1964
[5fe6e83640] - doc: copyedit GOVERNANCE.md (Rich Trott) #1963
[428526544c] - doc: add ChALkeR as collaborator (Сковорода Никита Андреевич) #1927
[5dfe0d5d61] - doc: remove irrelevant SEMVER-MINOR & MAJOR (Rod Vagg)
[fb8811d95e] - lib,test: fix whitespace issues (Roman Reiss) #1971
[a71ee93afe] - module: reduce syscalls during require search (Pierre Inglebert) #1920
[671e64ac73] - module: allow long paths for require on Windows (Michaël Zasso)
[061342a500] - net: Defer reading until listeners could be added (James Hartig) #1496
[0abcf44d6b] - test: add Buffer slice UTF-8 test (Rich Trott) #1989
[88c1831ff4] - test: tmpdir creation failures should fail tests (Rich Trott) #1976
[52a822d944] - test: fix test-cluster-worker-disconnect (Santiago Gimeno) #1919
[7c79490bfb] - test: only refresh tmpDir for tests that need it (Rich Trott) #1954
[88d7904c0b] - test: remove test repetition (Rich Trott) #1874
[91dfb5e094] - tools: make test-npm work without global npm (Jeremiah Senkpiel) #1926
[3777f41562] - tools: enable whitespace related rules in eslint (Roman Reiss) #1971
[626432d843] - util: dont repeat isBuffer (Brendan Ashworth) #1988
[1d79f572f1] - util: move deprecate() to internal module (Brendan Ashworth) #1988
[4b4b1760b5] - v8: cherry-pick uclibc build patch from upstream (Ben Noordhuis) #1974
[5d0cee46bb] - vm: remove unnecessary HandleScopes (Ben Noordhuis) #2001
[953b3e75e8] - win,node-gyp: enable delay-load hook by default (Bert Belder) iojs/io.js#1433
[3806d875d3] - zlib: prevent uncaught exception in zlibBuffer (Michaël Zasso) #1811

Noteable changes besides the npm update should be 671e64a, 3806d87 and 671e64a...626432d.

LGTM

Tangentially related - would it be okay to transliterate Russian names? Or is this super sensitive?
I'm thinking that "Skovoroda Nikita Andreevich" would be easier to read for 90% of the audience than "Сковорода Никита Андреевич". @ChALkeR

@piscisaureus there was conversation about that in #1927, but I don't think a decision was reached.

@piscisaureus It would be ok from my side. If you do that, you can use «Nikita Skovoroda» (first and last name correspondingly). The current variant was copied from my git settings and/or GitHub user profile.

Previous discussion here: #1927.
See commit 4285265, it has the transliterated name in the message.

Could we also add this to release? #1982

that'll have to go in to .mailmap to make it happen @piscisaureus and @ChALkeR

@bricss That one is not finished yet and has an alternate version in #1998.

It requires further discussion and there is no need to delay v2.3.1 release for that, v2.3.1 fixes more serious bugs. #1982/#1998 atm fixes one module using the API in an incorrect and undocumented way (the other one got patched already, there might be more, but I know of only two for sure).

I would recommend fixing your problem by merging dtabuenc/karma-html-reporter#27 instead, if you want a fix asap.

@ChALkeR #1982 looks much better then #1998 I guess it would be possible to choose one and merge before release, cause this issue could be very sensitive for a lot of people and these projects.

@bricss, This is not the correct place to discuss which of #1998 and #1982 should be merged.
And I see no reason holding a release that fixes more serious io.js-side issues (namely 3806d87 and 671e64a) that affect a lot of people because of the fix that addresses an incorrect API usage by one (until you name the others) third-party module (not io.js side) is not being reviewed yet.
That one should go into the next patch release, if there is nothing else holding off the release now. It shouldn't block the release.

@piscisaureus, @rvagg, @brendanashworth

If the question on how you should write my name is holding the release, just remove those two commits from the above list and put them in some later release when a consensus is reached. Those two commits are insignificant, they target the documentation only.

3806d87 is strictly speaking a security fix and should not be delayed,
671e64a fixes a problem that, while being a Windows-only bug, caused three bugreports in io.js to the date.

«Nikita Skovoroda» sounds pretty good.

Updated the commit list in the OP. @rvagg lmk if you're able to get this out today, otherwise I'll see about doing it later today.

@Fishrock123 This should not be released as-is with the updated commit list.
Either remove a71ee93 from 2.3.1, or wait until #2011 is merged.
If it was up to me, I would exclude a71ee93 from this release because it should not be delayed.

Either #2011 may have to land (with or without a test?) or a71ee93 may have to be reverted.

Could we just tag 061342a as 2.3.1? There are three commits after that one — a71ee93 and two documentation-related.

Edit: Ah, each release requires a commit, so we can't. Without making a separate branch, that is.

@chrisdickinson thoughts on branching to get a 2.3.1 out? I have no problem with it but perhaps there's something special we need to be considering here?

@Fishrock123 @ChALkeR also an option is forcing a commit in there and forcing a rebase of everything that's in after and have their shas change. I also don't have a great problem with that but then again I'm a git cowboy.

... and sorry for being tardy with this, I said I'd release this yesterday but didn't so this is my fault.

[1f93b63b11] - doc: change the info to the same as in gitconfig (Christian Tellnes) #2000
[0cf94e6856] - doc: mention CI in Collaborator Guide (Rich Trott) #1995
[a71ee93afe] - module: reduce syscalls during require search (Pierre Inglebert) #1920

there's only these 3 that would be changed, so perhaps not a big deal?

Now also:

[c5353d7c62] - build: remove lint from test-ci on windows (Johan Bergström) #2004

Imma do a release and force push before any more commits land. Stop me now if you think this is a terrible idea!

CI @ 061342a https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/54/

I've not been following this closely, but it seems #2011 or #2013 should land for the windows path fix to really be complete. cc @Fishrock123

I was going on this:

Either #2011 may have to land (with or without a test?) or a71ee93 may have to be reverted.

Is that not your reading @silverwind?

Yeah, that's the gist. I think the choice is between a somewhat risky landing of #2011 or #2013, or to delay the release another day until this is sorted out.

What is happenning now with 2.3.1 brings up a question: why are current patch releases based on the master branch?
The usual way of doing things would be to create a branch for each minor release (including the current one) and cherry-pick fixes to that branch.

FYI I've opted to sit on this while we further investigate the zlib bug (wasted a chunk of my afternoon doing so!), should get a release out tomorrow, @Fishrock123 || @chrisdickinson are welcome to get this out if they feel the need when they're around before I show up.

bug reports

My computer is currently out of service so I'm afraid there's not much I can do to help. :/

Edit: got it working again for a while..

#2013 is a more complete fix for a71ee93 (includes a test for it)

Looks like we are too late on this now, we should not put this out until Monday. ((Weekend))

agree on putting it off till the end of the weekend, I'm happy to take on board the suggestion to avoid releasing just before or during a weekend to make it easier for people who work in this area as a day-job and don't want weekend drama

This is the Notable Changes I'm going with, critique welcome.

module: The number of syscalls made during a require() have been significantly reduced which should lead to a performance improvement (Pierre Inglebert) #1920, (Michaël Zasso) #1991.
npm:
- Upgrade to v2.11.2 (Rebecca Turner) #1956.
- Upgrade to v2.11.3 (Forrest L Norvell) #2018.
zlib: A bug was discovered where the process would abort if the final part of a zlib decompression results in a buffer that would exceed the maximum length of 0x3fffffff bytes (~1GiB). This was likely to only occur during synchronous decompression. This is now fixed and will instead result in a thrown RangeError. (Michaël Zasso) #1811.

@targos could you take a look at these since they are mainly yours?

module: The number of syscalls made during a require() have been significantly reduced which should lead to a performance improvement (Pierre Inglebert) #1920, (Michaël Zasso) #1991.

#1991 is a different issue, isn't it?

#2013 is not merged yet, FYI.

You're right @ChALkeR, I was mixing up require() speedups, thanks

#2013 is landed

CI @ https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/66/

Running smoke tests (again)

For the zlib bug, it is incorrect.
The synchronous version is almost unchanged (it was already throwing, there is just a different error message to match the async one).
The async version was throwing inside async code, resulting in an uncaught exception (and potentially process abort). It is now passing an error to the callback instead.

@targos thanks! What I actually meant to refer to was "buffering" vs "streaming", not "sync" vs "async". I've been tinkering with this bug and so far I'm unable to trigger it under any streaming situation but I'd love to hear if you have different results on this.

OK, I'm not going to move forward with this release and going to bed instead. There are crypto errors on Windows 2008 that are beyond the random timeouts we might expect. Also, only a few of them are failing with a duration of 60 seconds (the timeout length), so there's something more sinister going on here.

@shigeki could this be to do with the headers work, and/or the CNNIC whitelist cert check?

/cc @nodejs/crypto

not ok 422 - test-https-foafssl.js
  ---
  duration_ms: 60.178
not ok 675 - test-tls-alert.js
  ---
  duration_ms: 60.177
  ...
not ok 698 - test-tls-dhe.js
  ---
  duration_ms: 60.173
  ...
not ok 699 - test-tls-ecdh.js
#c:\workspace\iojs+pr+win\nodes\win2008r2\test\parallel\test-tls-ecdh.js:40
#    if (err) throw err;
#                   ^
#Error: Command failed: C:\Windows\system32\cmd.exe /s /c ""c:\workspace\iojs+pr+win\nodes\win2008r2\Release\openssl-cli.exe" s_client -cipher -ALL:ECDHE-RSA-RC4-SHA -connect 127.0.0.1:12346"
#WARNING: can't open config file: /usr/local/ssl/openssl.cnf
#Loading 'screen' into random state -
#    at ChildProcess.exithandler (child_process.js:196:12)
#    at emitTwo (events.js:87:13)
#    at ChildProcess.emit (events.js:172:7)
#    at maybeClose (internal/child_process.js:764:16)
#    at Socket.<anonymous> (internal/child_process.js:319:11)
#    at emitOne (events.js:77:13)
#    at Socket.emit (events.js:169:7)
#    at Pipe._onclose (net.js:467:12)
  ---
  duration_ms: 0.496
  ...
not ok 700 - test-tls-ecdh-disable.js
#
#assert.js:89
#  throw new assert.AssertionError({
#        ^
#AssertionError: -1 != -1
#    at c:\workspace\iojs+pr+win\nodes\win2008r2\test\parallel\test-tls-ecdh-disable.js:39:12
#    at ChildProcess.exithandler (child_process.js:203:5)
#    at emitTwo (events.js:87:13)
#    at ChildProcess.emit (events.js:172:7)
#    at maybeClose (internal/child_process.js:764:16)
#    at Socket.<anonymous> (internal/child_process.js:319:11)
#    at emitOne (events.js:77:13)
#    at Socket.emit (events.js:169:7)
#    at Pipe._onclose (net.js:467:12)
  ---
  duration_ms: 0.499
  ...
not ok 719 - test-tls-no-sslv3.js
#WARNING: can't open config file: /usr/local/ssl/openssl.cnf
#Loading 'screen' into random state -
#assert.js:89
#  throw new assert.AssertionError({
#        ^
#AssertionError: 3221226505 == 1
#    at ChildProcess.<anonymous> (c:\workspace\iojs+pr+win\nodes\win2008r2\test\parallel\test-tls-no-sslv3.js:34:12)
#    at ChildProcess.<anonymous> (c:\workspace\iojs+pr+win\nodes\win2008r2\test\common.js:367:15)
#    at ChildProcess.g (events.js:260:16)
#    at emitTwo (events.js:87:13)
#    at ChildProcess.emit (events.js:172:7)
#    at Process.ChildProcess._handle.onexit (internal/child_process.js:200:12)
  ---
  duration_ms: 0.499
  ...
not ok 730 - test-tls-securepair-server.js
#
#assert.js:89
#  throw new assert.AssertionError({
#        ^
#AssertionError: 1 == 0
#    at process.<anonymous> (c:\workspace\iojs+pr+win\nodes\win2008r2\test\parallel\test-tls-securepair-server.js:127:10)
#    at emitOne (events.js:82:20)
#    at process.emit (events.js:169:7)
  ---
  duration_ms: 0.496
  ...
not ok 732 - test-tls-session-cache.js
#done
#
#assert.js:89
#  throw new assert.AssertionError({
#        ^
#AssertionError: 3221226505 == 0
#    at ChildProcess.<anonymous> (c:\workspace\iojs+pr+win\nodes\win2008r2\test\parallel\test-tls-session-cache.js:92:14)
#    at emitTwo (events.js:87:13)
#    at ChildProcess.emit (events.js:172:7)
#    at Process.ChildProcess._handle.onexit (internal/child_process.js:200:12)
  ---
  duration_ms: 0.495
  ...
not ok 733 - test-tls-set-ciphers.js
#c:\workspace\iojs+pr+win\nodes\win2008r2\test\parallel\test-tls-set-ciphers.js:44
#    if (err) throw err;
#                   ^
#Error: Command failed: C:\Windows\system32\cmd.exe /s /c ""c:\workspace\iojs+pr+win\nodes\win2008r2\Release\openssl-cli.exe" s_client -cipher RC4-MD5 -connect 127.0.0.1:12346"
#WARNING: can't open config file: /usr/local/ssl/openssl.cnf
#Loading 'screen' into random state -
#    at ChildProcess.exithandler (child_process.js:196:12)
#    at emitTwo (events.js:87:13)
#    at ChildProcess.emit (events.js:172:7)
#    at maybeClose (internal/child_process.js:764:16)
#    at Socket.<anonymous> (internal/child_process.js:319:11)
#    at emitOne (events.js:77:13)
#    at Socket.emit (events.js:169:7)
#    at Pipe._onclose (net.js:467:12)
  ---
  duration_ms: 0.497
  ...

https://jenkins-iojs.nodesource.com/job/iojs+pr+win/53/nodes=win2008r2/ is the failing machine, all other slaves are OK, including the Windows 2012 one.

Kicking off another one before I head off: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/67/

@rvagg hmm, new CI doesn't have those.

Let's try again then https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/68/

I'm still concerned with those failures but I have zero to go on so if anyone has any clues then please fill us in.

@rvagg still looks green..

smoke testing, I've been tuning my smoke test suite and it's coming along nicely!

success on most fronts, the outstanding smoke test I still can't make play nicely is npm, details here: npm/npm#8648

pushing ahead with release

PR-URL: #1996 Notable changes * module: The number of syscalls made during a require() have been significantly reduced again (see #1801 from v2.2.0 for previous work), which should lead to a performance improvement (Pierre Inglebert) #1920. * npm: - Upgrade to v2.11.2 (Rebecca Turner) #1956. - Upgrade to v2.11.3 (Forrest L Norvell) #2018. * zlib: A bug was discovered where the process would abort if the final part of a zlib decompression results in a buffer that would exceed the maximum length of 0x3fffffff bytes (~1GiB). This was likely to only occur during buffered decompression (rather than streaming). This is now fixed and will instead result in a thrown RangeError (Michaël Zasso) #1811.

https://jenkins-iojs.nodesource.com/job/iojs+release/75/ building

done, https://iojs.org/dist/latest/

armv6 taking its time as usual, will promote later

Armv6 is up. 🍻

PR-URL #70 Related: nodejs/node#1996 Signed-off-by: Hans Kristian Flaatten <hans.kristian.flaatten@turistforeningen.no>

brendanashworth mentioned this pull request Jun 16, 2015

util: fix isBuffer #1988

Closed

mscdex added the meta Issues and PRs related to the general management of the project. label Jun 16, 2015

targos reviewed Jun 19, 2015
View reviewed changes

rvagg force-pushed the v2.3.1-release branch from 6e1500a to b4064fc Compare June 19, 2015 12:26

rvagg force-pushed the v2.3.1-release branch from b4064fc to 46a0445 Compare June 22, 2015 11:35

rvagg force-pushed the v2.3.1-release branch from 46a0445 to 559a80d Compare June 22, 2015 12:13

2015-06-19 io.js v2.3.1 Release

f11d7c5

rvagg force-pushed the v2.3.1-release branch from 559a80d to f11d7c5 Compare June 22, 2015 12:47

Fishrock123 closed this Jun 24, 2015

Starefossen pushed a commit to nodejs/docker-iojs that referenced this pull request Jun 24, 2015

Update to v2.3.1

8481b5a

PR-URL #70 Related: nodejs/node#1996 Signed-off-by: Hans Kristian Flaatten <hans.kristian.flaatten@turistforeningen.no>

Starefossen mentioned this pull request Jun 24, 2015

Update io.js 2.x to v2.3.1 nodejs/docker-iojs#71

Merged

Havoooo mentioned this pull request Sep 1, 2023

2015-06-19 io.js v2.3.1 Release Havoooo/undoing-the-hack#11

Merged

4 tasks

Release proposal: v2.3.1 by rvagg · Pull Request #1996 · nodejs/node · GitHub

Uh oh!

Uh oh!

Release proposal: v2.3.1 - #1996

Release proposal: v2.3.1#1996rvagg wants to merge 1 commit intonodejs:masternodejs/node:masterfrom rvagg:v2.3.1-releaservagg/io.js:v2.3.1-releaseCopy head branch name to clipboard

Conversation

rvagg commented Jun 19, 2015

Uh oh!

silverwind commented Jun 16, 2015

Uh oh!

Fishrock123 commented Jun 17, 2015

Uh oh!

piscisaureus commented Jun 17, 2015

Uh oh!

brendanashworth commented Jun 17, 2015

Uh oh!

ChALkeR commented Jun 17, 2015

Uh oh!

bricss commented Jun 17, 2015

Uh oh!

rvagg commented Jun 17, 2015

Uh oh!

ChALkeR commented Jun 17, 2015

Uh oh!

bricss commented Jun 17, 2015

Uh oh!

ChALkeR commented Jun 17, 2015

Uh oh!

ChALkeR commented Jun 17, 2015

Uh oh!

bricss commented Jun 17, 2015

Uh oh!

Fishrock123 commented Jun 18, 2015

Uh oh!

ChALkeR commented Jun 18, 2015

Uh oh!

Fishrock123 commented Jun 18, 2015

Uh oh!

ChALkeR commented Jun 18, 2015

Uh oh!

rvagg commented Jun 19, 2015

Uh oh!

rvagg commented Jun 19, 2015

Uh oh!

rvagg commented Jun 19, 2015

Uh oh!

rvagg commented Jun 19, 2015

Uh oh!

silverwind commented Jun 19, 2015

Uh oh!

rvagg commented Jun 19, 2015

Uh oh!

silverwind commented Jun 19, 2015

Uh oh!

ChALkeR commented Jun 19, 2015

Uh oh!

rvagg commented Jun 19, 2015

Uh oh!

targos Jun 19, 2015

Choose a reason for hiding this comment

Uh oh!

Fishrock123 commented Jun 19, 2015

Uh oh!

Fishrock123 commented Jun 19, 2015

Uh oh!

Fishrock123 commented Jun 20, 2015

Uh oh!

rvagg commented Jun 21, 2015

Uh oh!

rvagg commented Jun 22, 2015

Uh oh!

ChALkeR commented Jun 22, 2015

Uh oh!

ChALkeR commented Jun 22, 2015

Uh oh!

rvagg commented Jun 22, 2015

Uh oh!

rvagg commented Jun 22, 2015

Uh oh!

targos commented Jun 22, 2015

Release proposal: v2.3.1#1996
rvagg wants to merge 1 commit into
nodejs:masterfrom
rvagg:v2.3.1-release