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

uv: let's discuss the backport policy for LTS #6806

Closed
4 tasks
saghul opened this issue May 17, 2016 · 20 comments
Closed
4 tasks

uv: let's discuss the backport policy for LTS #6806

saghul opened this issue May 17, 2016 · 20 comments
Labels
libuv Issues and PRs related to the libuv dependency or the uv binding.

Comments

@saghul
Copy link
Member

saghul commented May 17, 2016

Right now Node 4.x is running libuv 1.8.0. Since libuv follows semver, any further versions in the v1.x branch have a backwards compatible API and a stable ABI.

There won't be another libuv 1.8.X, because we are in the v1.9.x cycle now, so Node v4.x should adopt the latest and greatest, as a general rule.

Now, given the current stdio situation (#6456) let's wait for that to settle before taking action, but I wanted to get the ball rolling.

Edit: The Plan

@rvagg
Copy link
Member

rvagg commented May 17, 2016

I'm +1 on backporting given:

  1. enough delay to let it prove in Node Current
  2. review by LTS team of the changes being introduced and explicit sign-off by them

@mscdex mscdex added the libuv Issues and PRs related to the libuv dependency or the uv binding. label May 17, 2016
@Fishrock123
Copy link
Contributor

cc @nodejs/lts

@jasnell
Copy link
Member

jasnell commented May 17, 2016

This works for me in general but... my chief concern would be subtle behavior changes such as the stdio small block / flushing issue . Given the impact that has had on the ecosystem I would rather hold off until those issues are adequately resolved. (As suggested by @saghul ;)...)

@mhdawson
Copy link
Member

I think it should come down to a risk/reward analysis which would be what I think the conditions @rvagg lists are meant to address. If there is a benefit to the LTS release in terms of fixing defects/problems and the level of change is acceptable, then I believe we should pull it in. If instead it is only new features and there is significant change then maybe its better to wait as LTS users likely value stability over "new". The balance might also depend on how far into the LTS cycle a release is. For example I don't think pulling in the latest into an LTS release a month or two before it goes out of service would make sense.

@saghul
Copy link
Member Author

saghul commented May 17, 2016

@mhdawson it's not only the new features, it's the bugfixes that matter here. LTS won't get any Node code backported which uses the new APIs, so that's fine. As it stands, Node 4 is running an unmaintained version of libuv, not even security bugs will make a v1.8.x happen.

@jasnell bugs do and will happen. the stdio thing was by no means an intended behavior change, it's still under investigation.

@jasnell
Copy link
Member

jasnell commented May 17, 2016

@saghul ... yep, understood. The point is only that for the LTS stream we need to be much more conservative about how "up to date" we want to be. Letting things sit in current for a while to ensure that there are no significant regressions makes the most sense here.

@MylesBorins
Copy link
Contributor

I'm not entirely opposed to the update but like others I am concerned about the various subtle changes introduced in 1.9.x

@saghul we should likely use this issue to keep track of the problems / solutions. Would you be willing to do so in the top comment?

@saghul
Copy link
Member Author

saghul commented May 18, 2016

I'm only aware of a single problem. Which I'm willing to help resolve (see #6456 (comment)).

Now, this is an interesting dilemma. From libuv's perspective, we fixed a bug, and tty writes are as non-blocking as they were, but Node was relying on "undefined behavior".

Let's try to get this solved together. Here is a tentative patch: libuv/libuv#878

@saghul
Copy link
Member Author

saghul commented May 29, 2016

Since Debian is currently wondering about the status of this, I'll make a status update. Note that as far as I'm concerned, there is only one problem caused fater the 1.9.X upgrade.

The inadvertent change in behavior only affects OSX, and once #6895 lands, thus restoring the old behavior in OSX we should be able to backport the libuv upgrade plus that workaround to LTS.

@saghul
Copy link
Member Author

saghul commented Jun 1, 2016

#6895 has landed, I'll wait until it makes it to a v6 release and then back port the libuv upgrade + that patch to v4. Is everyone OK with this?

@rvagg
Copy link
Member

rvagg commented Jun 2, 2016

yes, but I'd still like to be very conservative on timing for this, it's possible other things might arise in v6

@rvagg
Copy link
Member

rvagg commented Jun 2, 2016

but yes, a PR against v4.x-staging would be good, we can deal with timing separate to that

@jasnell
Copy link
Member

jasnell commented Jun 2, 2016

Once such a pr is landed in v4.x-staging, we can generate a number of release candidates with the update that can be tested for further breakage.

@saghul
Copy link
Member Author

saghul commented Jun 3, 2016

Updated my initial comment with the plan / issues I'll backport. I'll get the PR worked out over the weekend, hopefully.

@MylesBorins
Copy link
Contributor

MylesBorins commented Jun 3, 2016

@saghul it seems reasonable that this should be able to be mixed in with the v4.5.0 RC

thoughts @nodejs/lts?

@saghul
Copy link
Member Author

saghul commented Jun 3, 2016

@thealphanerd when is it supposed to come out?

@MylesBorins
Copy link
Contributor

tentatively June 14th.

@saghul
Copy link
Member Author

saghul commented Jun 3, 2016

Ok. I'll get the PR ASAP and we take it from there.

@saghul
Copy link
Member Author

saghul commented Jun 7, 2016

PR: #7205

@jasnell
Copy link
Member

jasnell commented May 30, 2017

Closing as there does not appear to be anything more to do

@jasnell jasnell closed this as completed May 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libuv Issues and PRs related to the libuv dependency or the uv binding.
Projects
None yet
Development

No branches or pull requests

7 participants