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

Release proposal: v1.4.2 #995

Closed
rvagg opened this issue Feb 27, 2015 · 47 comments
Closed

Release proposal: v1.4.2 #995

rvagg opened this issue Feb 27, 2015 · 47 comments
Labels
meta Issues and PRs related to the general management of the project.

Comments

@rvagg
Copy link
Member

rvagg commented Feb 27, 2015

This is a quick one, will give it 24 hours or less. Mainly to fix #988, proposed change being in #994.

  • [25da0742ee] - build: improve vcbuild.bat (Bert Belder) #998
  • [b8310cbd3e] - build: reduce tarball size by 8-10% (Johan Bergström) #961
  • [58a612ea9d] - deps: make node-gyp work with io.js (cjihrig) #990
  • [2a2fe5c4f2] - deps: upgrade npm to 2.6.1 (Forrest L Norvell) #990
  • [84ee2722a3] - doc: minor formatting fixes. (Tim Oxley) #996
  • [cf0306cd71] - doc: update stability index (Chris Dickinson) #943
  • [fb2439a699] - doc: add robertkowalski as collaborator (Robert Kowalski) #977
  • [f83d380647] - doc: update os.markdown (Benjamin Gruenbaum) #976
  • [ae7a23351f] - doc: add roadmap, i18n, tracing, evangelism WGs (Mikeal Rogers) #911
  • [14174a95a5] - doc: document roadmap, workgroups (Mikeal Rogers)
  • [865ee313cf] - doc: add tellnes as collaborator (Christian Tellnes) #973
  • [01296923db] - doc: add mscdex as collaborator (Brian White) #972
  • [675cffb33e] - http: don't confuse automatic headers for others (Christian Tellnes) #828
  • [7887e119ed] - install: new performance counters provider guid (Russell Dempsey)
  • [4d1fa2ca97] - src: add check for already defined macro NOMINMAX (Pavel Medvedev) #986
  • [1ab7e80838] - tls: proxy handle.reading back to parent handle (Fedor Indutny) #1004
  • [755461219d] - tls: fix typo handle._reading => handle.reading (Fedor Indutny) #994
@rvagg
Copy link
Member Author

rvagg commented Feb 27, 2015

FYI: Jenkins is being a :trollface: for us and getting different build machines to build on different commits, hiding test failures that crept in before 1.4.1. I also had lots of trouble even getting 1.4.1 out because of this but I think I may have sorted it out (never say never with Jenkins).

CI on v1.x as it is now: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/207/ - expecting to see Windows failures this time. Will try #994 next.

@rvagg
Copy link
Member Author

rvagg commented Feb 27, 2015

And there it is! Human error after all, the test-ci and introduction of TAP changes in CI were causing havoc so I'd reverted the test runs to be just test-simple and test-message but only test-message is running on Windows. I'm not actually sure why since it's doing vcbuild.bat release nosign x64 test-simple test-message and that's supposed to work. Trying just vcbuild.bat release nosign x64 test now.

https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/208/

@indutny
Copy link
Member

indutny commented Feb 28, 2015

Yay! :)

@rvagg
Copy link
Member Author

rvagg commented Feb 28, 2015

2015-02-28, Version 1.4.2, @rvagg

Notable changes

  • tls: A typo introduced in the TLSWrap changes in #840 only encountered as a bug on Windows was not caught by the io.js CI system due to problems with the Windows build script and the Windows CI slave configuration. Fixed in #994 & #1004. (Fedor Indutny)
  • npm: Upgrade npm to 2.6.1. See npm CHANGELOG.md for details. Summary:
    • 8b98f0e
      #4471 npm outdated (and only npm outdated) now defaults to --depth=0. This also has the excellent but unexpected effect of making npm update -g work the way almost everyone wants it to. See the docs for
      --depth

      for the mildly confusing details. (@smikes)
    • aa79194
      #6565 Tweak peerDependency
      deprecation warning to include which peer dependency on which package is
      going to need to change. (@othiym23)
    • 5fa067f
      #7171 Tweak engineStrict
      deprecation warning to include which package.json is using it.
      (@othiym23)
  • Add new collaborators:

Known issues

  • Windows support has some outstanding failures that have not been properly picked up by the io.js CI system due to a combination of factors including human, program and Jenkins errors. See #1005 for details & discussion. Expect these problems to be addressed ASAP.
  • Surrogate pair in REPL can freeze terminal #690
  • Not possible to build io.js as a static library #686
  • process.send() is not synchronous as the docs suggest, a regression introduced in 1.0.2, see #760 and fix in #774
  • Calling dns.setServers() while a DNS query is in progress can cause the process to crash on a failed assertion #894

Commits

@rvagg
Copy link
Member Author

rvagg commented Feb 28, 2015

I might just do this today, give me a few hours to work on Jenkins to make sure it's working properly and I'll go ahead and do a release.

PLEASE DON'T INTRODUCE SEMVER-MINOR COMMITS UNTIL THIS IS RELEASED

@rvagg
Copy link
Member Author

rvagg commented Feb 28, 2015

@indutny @piscisaureus the real problem atm is that Jenkins isn't getting a !0 exit code from vcbuild. Check this out: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/209/nodes=iojs-win2012r2/console - note the SUCCESS at the bottom but 79 errors. So it's always going to be blue. Any clues what that might be about? Full command on this is this atm: vcbuild.bat release nosign x64 test.

@piscisaureus
Copy link
Contributor

@rvagg

Check this out: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/209/nodes=iojs-win2012r2/console -

But these look like genuine failures. Or am I missing something?

@rvagg
Copy link
Member Author

rvagg commented Feb 28, 2015

@piscisaureus genuine failures but the build is blue (not red), look at the last line:

Finished: SUCCESS

It's not reporting back. I'm about to hop on one of these machines and investigate whether it's the use of -J for test.py that's doing this (current theory), if you have any better ideas then let me know. Basically with the current test setup we can have failures on Windows that you won't see unless you're looking at console output.

@piscisaureus
Copy link
Contributor

@rvagg I'm guessing that the exit code is zero because after the tests, jslint runs and succeds.

Batch processing isn't aborted when a command returns a nonzero exit code, So even if there are test failures, the jslint step will run after that and set the exit code to 0.

@rvagg
Copy link
Member Author

rvagg commented Feb 28, 2015

@piscisaureus also vcbuild.bat release nosign x64 test-simple test-message only runs test-message and I don't understand why.

@piscisaureus
Copy link
Contributor

@piscisaureus also vcbuild.bat release nosign x64 test-simple test-message only runs test-message and I don't understand why.

Huh, do you have a jenkins link? vcbuild.bat looks like it does the right thing.

@rvagg
Copy link
Member Author

rvagg commented Feb 28, 2015

https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/200/nodes=iojs-win2012r2/console for instance was run with that string. I'm also on one of the servers now running it myself and get the same behaviour. As if test-simple is completely ignored as an argument. Switch them around and it seems to work (although tbh I can't tell just at the moment if test-message is being run or not after test-simple).

@piscisaureus
Copy link
Contributor

@rvagg oops. vcbuild.bat is the culprit:

https://github.com/iojs/io.js/blob/v1.x/vcbuild.bat#L59

@piscisaureus
Copy link
Contributor

@rvagg #998

@rvagg
Copy link
Member Author

rvagg commented Feb 28, 2015

confirmed that #988 is fixed in the above nightly build, just doing another CI run because I'm still not happy that vcbuild is fixed, and then will move on to a release if all's good.

@rvagg
Copy link
Member Author

rvagg commented Feb 28, 2015

No good, https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/212/ is not happy on Windows.

@indutny, you have some work to do, sorry: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/212/nodes=iojs-win2008r2/console & https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/212/nodes=iojs-win2012r2/console - 12 failures on both Windows machines, seem to be focused on tls_wrap.

1.4.2 is going to have to wait some more.

@indutny
Copy link
Member

indutny commented Feb 28, 2015

@rvagg many tests are using curl without checking for platform...

@indutny
Copy link
Member

indutny commented Feb 28, 2015

Ah, I see that it is a pre-requisite...

@indutny
Copy link
Member

indutny commented Feb 28, 2015

Leaves it with 3 failures for me #1003

@indutny
Copy link
Member

indutny commented Feb 28, 2015

  • test-stdin-from-file
  • test-fs-access
  • test-child-process-stdio-big-write-end

The latter one seems to be triggering some ENOTSUP on writev... @piscisaureus ?

@piscisaureus
Copy link
Contributor

@indutny libuv-windows doesn't support writev on named and ipc pipes, and will fail with ENOTSUP. Maybe that's causing the failures?

@rvagg
Copy link
Member Author

rvagg commented Feb 28, 2015

@piscisaureus I really need to get a release out to deal with the tls-wrap bug, can you give me an assessment of whether it's worth holding it up to deal with these too? If not, then a "Known issues" item would be good but I don't know what that should be; something about "ENOTSUP on writev errors on Windows"

@piscisaureus
Copy link
Contributor

@rvagg

To be honest I think that the test-child-process-stdio-big-write-end failure is very serious. I can't trivially implement writev() for pipes in libuv, so I would suggest reverting the StreamBase patches for the time being.

@piscisaureus
Copy link
Contributor

I'm sorry, I don't have an opportunity today to find a better solution ...
Maybe we can change the "current version" on the website back to 1.3.x ?

@indutny
Copy link
Member

indutny commented Feb 28, 2015

@piscisaureus I doubt it is related to StreamBase.

@indutny
Copy link
Member

indutny commented Feb 28, 2015

@piscisaureus Aaaah, I think you actually talked about StreamWrap::AddMethods...

@rvagg
Copy link
Member Author

rvagg commented Feb 28, 2015

@piscisaureus @indutny I have a 1.4.2 built and ready to release and seems to address most of the concerns related to tls, should I just push this out while we find a way to fix things up for a 1.4.3? That's my vote but I really don't understand the details of the other problems.

@indutny
Copy link
Member

indutny commented Feb 28, 2015

Will fix it in a bit.

@rvagg
Copy link
Member Author

rvagg commented Feb 28, 2015

OK, my vote: release 1.4.2 now, get a 1.4.3 out ASAP, even if it's tomorrow (or even today?). 1.4.2 much less broken than 1.4.1 for Windows.

@indutny
Copy link
Member

indutny commented Feb 28, 2015

@rvagg I have a fix coming in a minute.

indutny added a commit to indutny/io.js that referenced this issue Feb 28, 2015
Only TCP and JSStream do support `.writev()` on all platforms at the
moment. Ensure that it won't be enabled everywhere.

Fix: nodejs#995
@indutny
Copy link
Member

indutny commented Feb 28, 2015

@rvagg done #1008

@rvagg
Copy link
Member Author

rvagg commented Feb 28, 2015

I'm still voting for a 1.4.2 now, it's ready to go, 1.4.3 can be soon after as far as I can see this isn't quite as serious

@indutny
Copy link
Member

indutny commented Feb 28, 2015

@rvagg please put it into known problems then, and let's do a 1.4.3 right after it :) I'm quite sure that people might hit it in their code so releasing fix would be quite important.

@piscisaureus
Copy link
Contributor

  • test-fs-access fails only if the working directory isn't clean. This is a bug in the test.
  • test-stdin-from-file failes because on windows there is no 'fd' property on a libuv handle object.

Neither failure is indicative of a serious issue.

So +1 on releasing after fixing test-child-process-stdio-big-write-end

@rvagg
Copy link
Member Author

rvagg commented Feb 28, 2015

@piscisaureus to clarify: you're -1 on 1.4.2 going out and want test-child-process-stdio-big-write-end fixed before a release? we're in the same situation as 1.4.0 now that this has been tagged and built and ready to release so I can't bundle more in to a 1.4.2.

@rvagg
Copy link
Member Author

rvagg commented Feb 28, 2015

going to release 1.4.2, it's tagged and I'm now -1 on deleting tags so may as well get this out and work on 1.4.3

@rvagg
Copy link
Member Author

rvagg commented Feb 28, 2015

https://iojs.org/dist/latest/ it's out

ping @iojs/website and not we will have a 1.4.3 soon too

@rvagg rvagg closed this as completed Feb 28, 2015
@piscisaureus
Copy link
Contributor

Agreed, don't delete tags.

indutny added a commit that referenced this issue Feb 28, 2015
Only TCP and JSStream do support `.writev()` on all platforms at the
moment. Ensure that it won't be enabled everywhere.

Fix: #995
PR-URL: #1008
Reviewed-by: Bert Belder <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
@Fishrock123
Copy link
Contributor

I don't have access to a computer right now. Cc @therebelrobot / @snostorm / @mikeal

@snostorm
Copy link

snostorm commented Mar 1, 2015

I'm online, I'll take a look at updating the site

@snostorm
Copy link

snostorm commented Mar 1, 2015

FYI slight delay as I'm doing this one "right" with a long overdue switch to handlebars for version/v8 version (since I am find/replacing those same fields anyway.) Almost to the finish line.

@snostorm
Copy link

snostorm commented Mar 1, 2015

Sitting at nodejs/iojs.org#251, I'll ping @iojs/website for a +1

@snostorm
Copy link

snostorm commented Mar 1, 2015

Heads up @rvagg I put up a second PR for 1.4.3 as well nodejs/iojs.org#252

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

No branches or pull requests

6 participants