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

[WIP] V7.0.0 proposal (beta) #8503

Merged
merged 213 commits into from
Oct 14, 2016
Merged

[WIP] V7.0.0 proposal (beta) #8503

merged 213 commits into from
Oct 14, 2016

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Sep 12, 2016

This is the start of the v7.0.0 release proposal. Each week until the release a new beta build will be generated from this branch for testing.

Note: this branch includes the beta V8 5.4 commits from #8317. *This PR has not yet landed in master but is expected to do so before v7.0.0 is released. Those commits are included here in order to allow us more time to test v7.0.0 with V8 5.4

This currently does not yet include the release commit that switches to a release build. Nor does it currently include the changedocs. I will generate both of those later.

I had planned on doing a beta build this week but will wait until next week as my schedule is rather busy at the moment.

This is a work in progress PR. Do not land!

/cc @nodejs/ctc @nodejs/citgm @nodejs/testing

@jasnell jasnell added wip Issues and PRs that are still a work in progress. meta Issues and PRs related to the general management of the project. labels Sep 12, 2016
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. v8 engine Issues and PRs related to the V8 dependency. v7.x labels Sep 12, 2016
@jasnell jasnell mentioned this pull request Sep 12, 2016
2 tasks
@mscdex mscdex removed build Issues and PRs related to build files or the CI. v8 engine Issues and PRs related to the V8 dependency. labels Sep 12, 2016
@kevinmartin
Copy link

@jasnell where can we find the weekly beta builds?

@jasnell
Copy link
Member Author

jasnell commented Sep 21, 2016

I have not yet generated the first one. The first will be available in https://nodejs.org/download/rc/ later on today.

@jasnell
Copy link
Member Author

jasnell commented Sep 21, 2016

Ok, change that... the first beta build is available here: https://nodejs.org/download/test/v7.0.0-test2016092198fd4cb7d5/

the only thing missing is the raspbian build.

@kevinmartin
Copy link

kevinmartin commented Sep 21, 2016

For anyone coming to this thread wanting to install these builds with nvm:

NVM_NODEJS_ORG_MIRROR=https://nodejs.org/download/test nvm install v7

@targos
Copy link
Member

targos commented Sep 21, 2016

Should we bump the NODE_MODULE_VERSION for the beta builds ?

@addaleax
Copy link
Member

Should we bump the NODE_MODULE_VERSION for the beta builds ?

If that’s easily possible, I think so, yes. We might want to bump straight to 51 to avoid conflicts with electron’s updates from the last few months, see electron/electron#5851 (comment)

(/cc @zcbenz fyi)

@jbergstroem
Copy link
Member

@addaleax said:
We might want to bump straight to 51 to avoid conflicts

Sounds good to me.

@jasnell
Copy link
Member Author

jasnell commented Sep 21, 2016

I'll do that in the next beta build on Monday

On Wednesday, September 21, 2016, Johan Bergström [email protected]
wrote:

@addaleax https://github.com/addaleax said:
We might want to bump straight to 51 to avoid conflicts

Sounds good to me.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#8503 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAa2eYWsxMAJ3xJXs7pw4CYSatYzrbTzks5qsaFcgaJpZM4J7EQ3
.

@jasnell
Copy link
Member Author

jasnell commented Sep 21, 2016

At this point it looks like there may be four dependency version updates pending that could land in v7, all of which are significant, all of which will need @nodejs/ctc signoff in order to land:

  1. The V8 5.4 Update. This one seems uncontroversial at this point.
  2. libuv 1.10 Update. @saghul is working on this.
  3. icu 58 Update. @srl295 is working on this. (This may include the auto-discover mechanism for full-icu at startup)
  4. npm v4. @zkat mentions here that it'll go stable on 2016-10-06. announced on stage at Node Interactive EU that npm v4 was coming in October but did not specify precisely when. @zkat did indicate that it would likely go into v7 but so far there's been no indication of when we'll see a PR coming.

As a reminder, we are looking to cut the v7.0.0 release in just about one month from now. If these are going to land, we need the PRs ASAP so that they can be included in the beta builds for testing.

I will run the next beta build on Monday Sept 26th.

@addaleax
Copy link
Member

libuv 1.10 Update

Just to make sure I’m not misunderstanding anything – that’s not semver-major, so it doesn’t really need CTC signoff, right?

@zkat
Copy link
Contributor

zkat commented Sep 21, 2016

alright so: We're tagging npm@4 on October 6th. The earliest we are likely to put up a PR, even as an "early release" for the sake of cooperating with y'all's process is on October 7th. I had a brief convo with @addaleax before I saw the notification for this (and before she pinged me about this note), and I think what we concluded was that npm@4 is unlikely to be accepted into node@7 but that's really quite alright, cause npm@3 is something we're still supporting through the lifetime of node@7 anyway? We're much more concerned about what goes into LTS releases anyway. Sorry about making the assumption that npm@4's timing + stuff would work out, but please don't worry about it :)

@jasnell
Copy link
Member Author

jasnell commented Sep 21, 2016

@addaleax ... hopefully the libuv 1.10 update won't be semver-major, but given the trouble that we had with the bump up to 1.9 right before v6, we'll want to be careful and test everything thoroughly.

@saghul
Copy link
Member

saghul commented Sep 21, 2016

Just to make sure I’m not misunderstanding anything – that’s not semver-major, so it doesn’t really need CTC signoff, right?

@addaleax In principle any minor libuv update should not result in a semver-major bump in Node. Now, mistakes / the unexpected / etc can and do happen. I'll update the libuv update PR tomorrow since there is just 1 patch pending, which is related to z/OS, so we should be able to run the tests and CITGM to see if smoke comes out.

EDIT: typos.

@jasnell
Copy link
Member Author

jasnell commented Sep 21, 2016

@zkat ... if the delta between npm3 and npm4 is small enough then even 2016-10-07 should be ok, especially since it'll likely be a week or two later when we get the v8 stable. I just don't want to drop in a new npm right before release without folks having the opportunity to test things out fully.

As you said, however, v7 only has a lifespan of up to 9 months and v6 with npm3 will outlive it so if it works, great, but if it doesn't that should be ok also.

If I recall correctly, you said that npm v5 is expected before April next year?

@zkat
Copy link
Contributor

zkat commented Sep 21, 2016

the current plan is for npm@5 to get tagged Q1 2017, but that's a pretty fresh plan and I wouldn't be surprised if the date slips some, and I don't know what the date for that particular tag would be, since it depends on work that we haven't quite started yet.

The delta between npm@3 and npm@4 is fairly small though -- the only change of actual significance (imo) is the one related to prepublish, and even that is only breaking in a really small way: an extra deprecation warning when publishing for users using prepublish scripts, plus a conflict with any users using prepare (there's only 182 packages currently in the registry that use this, and most of them belong to literally a single person).

@Fishrock123
Copy link
Contributor

I will note it may be pretty odd if npm jumps from 3 to 5.

@zkat will that deprecation still exist in npm@5, or will we likely be jumping into new behavior by then for prepublish?

@zkat
Copy link
Contributor

zkat commented Sep 22, 2016

@Fishrock123 I'm not sure how long we plan on keeping the deprecation around. I assume it'll be there for a while, but I'm gonna cc @othiym23 to see if he has any concrete timeline for it.

@yosuke-furukawa
Copy link
Member

FYI, Ship async-await on V8. https://codereview.chromium.org/2356943002/
Node v7 supports async-await?

@jasnell
Copy link
Member Author

jasnell commented Sep 24, 2016

@yosuke-furukawa ... I wouldn't say "supports" in any kind of official way, but async-await is there behind a V8 harmony flag.

@MylesBorins
Copy link
Contributor

it appears that patch landed on master, I would assume the async / await
stuff will likely come in 5.5

/cc @nodejs/v8

On Sat, Sep 24, 2016, 1:55 PM James M Snell [email protected]
wrote:

@yosuke-furukawa https://github.com/yosuke-furukawa ... I wouldn't say
"supports" in any kind of official way, but async-await is there behind a
V8 harmony flag.


You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
#8503 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAecVwwFyW2MjiKhyT7ltsUQ2NBBO8rTks5qtWQNgaJpZM4J7EQ3
.

PR-URL: nodejs#7162
Refs: nodejs#6413
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
@jasnell
Copy link
Member Author

jasnell commented Oct 10, 2016

Fourth beta build is underway: https://ci-release.nodejs.org/job/reis-iojs+release/8/console

@jasnell
Copy link
Member Author

jasnell commented Oct 11, 2016

The fourth beta build is available here: https://nodejs.org/download/test/v7.0.0-test201610107f7d1d385d/

@jasnell
Copy link
Member Author

jasnell commented Oct 11, 2016

Trott and others added 8 commits October 12, 2016 10:33
PR-URL: nodejs#8941
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
PR-URL: nodejs#8486
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ilkka Myller <[email protected]>
Reviewed-By: James M Snell <[email protected]>
rval never existed, it was added as that in 077f9d7

Fixes: nodejs#9001
PR-URL: nodejs#9023
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Implements WHATWG URL support. Example:

```
var u = new url.URL('http://example.org');
```

Currently passing all WHATWG url parsing tests and all but two of the
setter tests. The two setter tests are intentionally skipped for now
but will be revisited.

PR-URL: nodejs#7448
Reviewed-By: Ilkka Myller <[email protected]>
Some commit links in the changelogs were pointing to incorrect/missing
shas.

PR-URL: nodejs#8122
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Fix compile bug when building with the --without-intl option
(introduced by 4b31238)

PR-URL: nodejs#9041
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
SSL_get_SSL_CTX returns the SSL_CTX for an SSL. Previously the code
accessed |ssl->ctx| directly, but that's no longer possible with OpenSSL
1.1.0.

SSL_get_SSL_CTX exists all the way back to (at least) OpenSSL 0.9.8 and
so this change should be fully compatible.

PR-URL: nodejs#8995
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
PR-URL: nodejs#9011
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@MylesBorins
Copy link
Contributor

MylesBorins commented Oct 13, 2016

FYI Chromium 54 is stable

https://googlechromereleases.blogspot.com/2016/10/stable-channel-update-for-desktop.html

Checking Omaha Proxy shows that 5.4.500.31 is the official version of V8 being shipped as stable, this is the same version we currently have on v7.x-staging 🎉

@jasnell
Copy link
Member Author

jasnell commented Oct 13, 2016

+1

On Wednesday, October 12, 2016, Myles Borins [email protected]
wrote:

FYI Chromium 54 is stable

https://googlechromereleases.blogspot.com/2016/10/stable-
channel-update-for-desktop.html

Checking Omaha Proxy shows that 5.4.500.31 is the official version of V8
being shipped as stable, this is the same version we currently have on
v7.x-staging 🎉


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#8503 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAa2eZrkFryJtAOnkBdAIbvjfiWDba1tks5qzb33gaJpZM4J7EQ3
.

Trott and others added 13 commits October 14, 2016 09:36
As the CTC grows and has representation from more time zones, we need to
embrace asynchronous decision making and rely less on the actual
meeting. This change is a proposal for that which, ironically, probably
has to be approved at a meeting.

PR-URL: nodejs#8945
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Julien Gilli <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
Unsanitized paths containing line feed characters can be used for
header injection and request splitting so reject them with an exception.

There seems to be no reasonable use case for allowing control characters
(characters <= 31) while there are several scenarios where they can be
used to exploit software bugs so reject control characters altogether.

PR-URL: nodejs#8923
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: not-an-aardvark <[email protected]>
This replaces TickObject with an object literal. This offers
performance improvements of up to ~20%.

PR-URL: nodejs#8932
Reviewed-By: Claudio Rodriguez <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Ilkka Myller <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Trevor Norris <[email protected]>
* var to const
* add check that expected error is ENOENT
* indexOf() to includes()

PR-URL: nodejs#8999
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
PR-URL: nodejs#9089
Reviewed-By: Colin Ihrig <[email protected]>
Wrapped the timer into class to ensure it is cleaned up properly.

PR-URL: nodejs#8870
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Adds documentation and explicit reasons on why
the GitHub web interface button is not used. This was
explained in the referenced issue by @thealphanerd.

Fixes: nodejs#8893
PR-URL: nodejs#9044
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Adds verbose reasons to the documentation on why the
Reviewed-By metadata on a pull request is important.
This was loosely mentioned as an issue in the referenced
issue below, and answered by @addaleax.

Ref: nodejs#8893
PR-URL: nodejs#9044
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
tick-processor-base.js is a module used by three other tests. It is not
a test fixture so move it out of the fixture directory. (One downside to
having it in the fixture directory is that fixture code is not currently
linted.)

It is possible that the code in tick-processor-base.js should be
integrated into common.js. This can potentially happen subsequently (and
might make a reasonable good first contribution for a new contributor).

PR-URL: nodejs#9022
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Highlight deprecated API methods/properties in "Table of Contents" for
increasing understandability. Adapted code to eslint standards.

PR-URL: nodejs#7189
Fixes: nodejs/nodejs.org#772
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
The config.gypi target has a recipe that uses the control function error
to report if the config.gypi file is missing or if it is stale (the
configure file was updated which is a prerequisite of this rule).

GNU make has two phases, immediate and deferred. During the first phase
 it will expand any variables or functions as the makefile is parsed.
The recipe in this case is a shell if statement, which is a deferred
construct. But the control function $(error) is an immediate construct
which will cause the makefile processing to stop during the first phase
of the Make process.

If I understand this correctly the only possible outcome of this rule is
the "Stale config.gypi, please re-run ./configure"  message which will
be done in the first phase and then exit. The shell condition will not
be considered. So it will never report that the config.gypi is missing.

bnoordhuis suggested that we simply change this into a single error
message:
"Missing or stale config.gypi, please run configure"

PR-URL: nodejs#9053
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
`end` MUST always be emitted **before** `close`. However, if a handle
will invoke `uv_close_cb` immediately, or in the same JS tick - `close`
may be emitted first.

PR-URL: nodejs#9066
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ilkka Myller <[email protected]>
@jasnell jasnell merged commit 6eece77 into nodejs:v7.x Oct 14, 2016
@jasnell
Copy link
Member Author

jasnell commented Oct 14, 2016

Merged all of the cherry-picked commits into v7.x branch to prepare for the actual release commits. Opening a new PR to work on the actual release proposal commit.

@targos targos mentioned this pull request Oct 14, 2016
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. wip Issues and PRs that are still a work in progress.
Projects
None yet
Development

Successfully merging this pull request may close these issues.