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

Future Node.js versions for V8 6.3/6.4? #505

Closed
zcbenz opened this issue Mar 13, 2018 · 16 comments
Closed

Future Node.js versions for V8 6.3/6.4? #505

zcbenz opened this issue Mar 13, 2018 · 16 comments

Comments

@zcbenz
Copy link

zcbenz commented Mar 13, 2018

Hi Node.js team,

At Electron team we are going to upgrade Electron to use Chrome 63/64, which uses V8 6.3/6.4, while Node.js does not have a release for those V8 versions yet. This leaves us the question what version should we use for process.version in Node.js.

Since the release of Node.js is not going to be aligned with the timeline of V8 stable releases, at Electron we only have the option to use a fake version for process.version. But we don't want to break toolchains when future Node.js releases use different versions for V8 6.3/6.4, so I wonder if it is possible for Node.js to have a plan for future versions.

Node.js Version Module Version V8 Version
8.9.x 57 6.1
8.10.x 57 6.2
9.0.x 59 6.2
9.8.0 59 6.2
??? ??? 6.3
??? ??? 6.4

/cc @alexeykuzmin @nodejs/delivery-channels @MylesBorins @bnoordhuis @addaleax

Thanks,
Cheng

@ljharb
Copy link
Member

ljharb commented Mar 13, 2018

Is waiting on a chrome upgrade for the corresponding node v8 upgrade not an option?

@zcbenz
Copy link
Author

zcbenz commented Mar 13, 2018

@ljharb That would be way too long. Using old Chrome versions leaves Electron apps to security problems, so we want to align Electron releases with Chrome stable releases.

Currently the latest Electron release is on Chrome 62 (V8 6.2), while the latest stable release of Chrome is 64, this has already raised security concerns from users of Electron apps.

@ljharb
Copy link
Member

ljharb commented Mar 13, 2018

Should perhaps electron have separate explicit chrome and node versions, and hide the generic version var entirely?

@zcbenz
Copy link
Author

zcbenz commented Mar 13, 2018

@ljharb Hiding process.version would cause compatibility problems with existing Node.js code, we can't require Node.js module authors to detect the actual runtime the code is running on. Our only choice is to use a fake Node.js version here, and we do not want to break things in future.

@ljharb
Copy link
Member

ljharb commented Mar 13, 2018

Could it exist but with a non numeric prefix, or something semver can parse but would never be a valid node version, like a prerelease?

@zcbenz
Copy link
Author

zcbenz commented Mar 13, 2018

Having a non-parsable version has the compatibility problem, because modules may throw when parsing it, or end up with corrupted state.

Using a prerelease version still requires us to guess a version number, like 10.0.0-prelease, and it does not really solve the problem when a future Node.js 10.0.0 release uses a different V8 version.

We had the same problem the time before the io.js, when Node.js was largely behind the V8 releases. We tried a few ways like using an existing Node.js version, or guessing a future Node.js version, but all ended up with confusions from users.

@ljharb
Copy link
Member

ljharb commented Mar 13, 2018

what about v10.0.0-electron.0, and just incrementing the prerelease number until node and chrome are back in alignment?

It doesn’t really matter if the version numbers match, as long as they don’t collide and are explicit in what they signify.

@zcbenz
Copy link
Author

zcbenz commented Mar 13, 2018

It might be something we can try. But to implement it we have to add new patches to our Node fork, while I'm more hoping to be able to just use the version number in Node's master branch.

And real purpose on aligning version numbers is to have native modules compatibility. Currently if you install a native module targeting Node.js 9.x, you can just use it in Electron 2.x (V8 6.2) because we have aligned Node.js versions and module versions. What I want is when users install native modules for Node.js 10.x in future, users can still use it in Electron 3.x (V8 6.3).

To achieve this level of compatibility we need to have aligned version numbers between Electron and Node.js, and using a prerelease number can not help that.

@ljharb
Copy link
Member

ljharb commented Mar 13, 2018

If the build team can commit to future v8 and node versions, then you’re fine, but otherwise i feel like you won’t have any options given your criteria besides risking deviating the electron version, or blocking chrome updates on node updates.

@zcbenz
Copy link
Author

zcbenz commented Mar 13, 2018

It would be great if we can know future Node versions, but if not I still have some question to be able to make decisions on our side:

  1. Which V8 version will Node.js 10 use?
  2. Will there be releases of Node.js using V8 6.3/6.4/6.5 (aka. will you skip V8 updates)?
  3. Will Node.js update V8 while keeping ABI compatibility (like Node.js 8.10.0)?
  4. What module version number would Node.js choose for V8 6.3/6.4/6.5?

@Trott
Copy link
Member

Trott commented Mar 13, 2018

@nodejs/release @nodejs/v8-update

@ofrobots
Copy link
Contributor

Which V8 version will Node.js 10 use?

Most likely Node 10.0.0 will ship with V8 6.6. See nodejs/node#19201.

Will there be releases of Node.js using V8 6.3/6.4/6.5 (aka. will you skip V8 updates)?

Unlikely. We are not going to be updating the version of V8 in Node 9.x. Node 10 will ship with 6.6.

Will Node.js update V8 while keeping ABI compatibility (like Node.js 8.10.0)?

Yes, likely. This depends on the active API/ABI departure in subsequent V8 versions.

What module version number would Node.js choose for V8 6.3/6.4/6.5?

nodejs/node#19201 picks module version 63. We try to align module versions to afford easier sync with electron. Would that work for you?

@ofrobots
Copy link
Contributor

Err.. I answered the last question differently from what you asked.

We aren't going to have a MODULE_VERSION for 6.3/6.4/6.5 because those are not going to be released as part of a Node release. Node 10.x will use MODULE_VERSION 63 – to match V8 6.6 – for the lifetime of 10.x regardless of if we update V8 during the lifetime of 10.x. We update V8 in a API/ABI compatible way in a semver major line.

@zcbenz
Copy link
Author

zcbenz commented Mar 13, 2018

Thanks for answering. Yes V8 6.6 with module version 63 works for Electron.

@MylesBorins
Copy link
Contributor

@zcbenz we have been keeping V8 integrated into master, and bumping the modules version number. If you wanted to use one of those versions of V8 you could simply float those patches on top fo the version of node you plan to ship

6.3 --> nodejs/node#16271
6.4 --> nodejs/node#17489
6.5 --> nodejs/node#18453
6.6 --> nodejs/node#19201

We've also been documenting the Node Version number in node_version.h, so referencing that file on master could serve as a source of truth.

We have a tool we use to update V8, which you should be able to use to automate updating V8 in your embedded node if you prefer to not float our versions (or have issues with it) --> https://www.npmjs.com/package/update-v8

One thing I would do if you are going to use a version of V8 not in a release would be to look at a log of changed in deps/v8 to make sure you grab all the patches we float.

For example here is a list of all of the patches we floated on top of 6.4.388.40, the initial version of 6.4 landed on master

* 8a54f4f676 - deps: cherry-pick 0bcb1d6f from upstream V8 (9 days ago)<Jakob Kummerow>
* 13cb056e4c - deps: cherry-pick 46c4979e86 from upstream v8 (3 weeks ago)<Ben Noordhuis>
* 81232320aa - deps: patch V8 to 6.4.388.46 (4 weeks ago)<Myles Borins>
* 36386dc4e3 - deps: patch V8 to 6.4.388.45 (4 weeks ago)<Myles Borins>
* b6000d8285 - deps: patch V8 to 6.4.388.44 (4 weeks ago)<Myles Borins>
* d0e4d4e0a1 - deps: patch V8 to 6.4.388.42 (5 weeks ago)<Myles Borins>
* 1f7648272e - deps: patch V8 to 6.4.388.41 (5 weeks ago)<Myles Borins>
* eb82b21138 - v8: add missing ',' in OpenBSD's 'sources' section. (6 weeks ago)<Aaron Bieber>
* 70277d6170 - deps: V8: resolve remaining whitespace diff (6 weeks ago)<Myles Borins>
* 142d6237b6 - deps: V8: reintroduce missing whitespace in test (7 weeks ago)<Ali Ijaz Sheikh>
* b06440356d - deps: cherry-pick c3bb73f from upstream V8 (7 weeks ago)<Ali Ijaz Sheikh>
* a1c5dddbb2 - deps: cherry-pick 814577e from upstream V8 (7 weeks ago)<Ali Ijaz Sheikh>
* 990959d2f6 - deps: cherry-pick 0c35b72 from upstream V8 (7 weeks ago)<Gus Caplan>
* 4c4af643e5 - deps: update V8 to 6.4.388.40 (7 weeks ago)<Michaël Zasso>

this was generated using an alias I have git lg

Please feel free to ping me directly any time you have question related to this stuff, that being said there is no reason to not open an issue as well!

@zcbenz
Copy link
Author

zcbenz commented Mar 15, 2018

Thanks for all your help!

Since Node.js won't have releases for V8 6.3/6.4/6.5, we are going to do following in Electron:

  1. Stay at Node 9.x for Chrome 63/64/65.
  2. Add patch to bump NODE_MODULE_VERSION in our Node fork.
  3. Apply Node's V8 patches on the V8 in Chromium repo (we are using V8 from Chrome instead of the one in Node repo).

@zcbenz zcbenz closed this as completed Mar 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants