deps: update V8 to 5.5 #9618
deps: update V8 to 5.5 #9618
Conversation
|
As brought up in nodejs/v8#2 I'm quite skeptical this is a good idea until we get better hooks for native promises. (Which seemed to be the plan to get in before node version 8, where this was originally supposed to land.) |
|
CI run to validate across platforms: https://ci.nodejs.org/job/node-test-commit-v8-linux/421/ |
That's a pretty cryptic comment on its own. I think the missing context is that 5.5 ships async/await without a flag? |
637c4c3
to
bc00ddd
Compare
|
Looks like CI failures across the board. |
|
Afaik smartos14 support is dropped for 5.5 |
|
Updated. There are some new utf-8 fixes (1, 2) that make Example failure: test('utf-8', Buffer.from('F0B841', 'hex'), '\ufffd\ufffdA');
// AssertionError: Expected "\ufffd\ufffd\u41", but got "\ufffd\u41"If I change the expected value, the error is inverted: test('utf-8', Buffer.from('F0B841', 'hex'), '\ufffdA');
// AssertionError: Expected "\ufffd\u41", but got "\ufffd\ufffd\u41" |
|
@targos see discussion here: v8@af842a7#commitcomment-19855022 |
04d75fa
to
cef1cc9
Compare
|
I'm turning the PR into a clean semver-major that can land on master. I'll then open another one to backport to v7.x. |
|
This is still in progress because of v8@af842a7#commitcomment-19855022 |
|
@nodejs/platform-smartos What are we going to do with SmartOS 14? The CI cannot be green because of the incompatibility with this platform. |
|
@targos as far as I know, it is to be dropped for 55 and forward. I can look at skipping testing against smartos14 for a specific node version; for instance 7.3? |
|
@targos FWIW, what @jbergstroem mentioned:
sounds good to me. P.S: sorry for the delay, I was on vacation until today. |
|
CI run one more time: https://ci.nodejs.org/job/node-test-pull-request/6073/ |
PR-URL: #9618 Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
V8 5.5 is not API/ABI compatible with 5.4. This commit increments NODE_MODULE_VERSION by one. Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md PR-URL: #9618 Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
With the upstream fix in V8, function declarations now work fine in the vm module and the test is no longer failing. Refs: https://codereview.chromium.org/2334733002 Fixes: #548 PR-URL: #9618 Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
The issue is fixed upstream in V8. Thus we do not need this workaround in REPL. Fixes: #548 PR-URL: #9618 Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
V8 5.5 changed how invalid characters are handled and it now appears to follow the WHATWG Encoding standard, where all of an invalid character's bytes are replaced by a single replacement character (\ufffd) instead of replacing each invalid byte with separate replacement characters. Example: the byte sequence 0xF0,0xB8,0x41 is decoded as '\ufffdA' in V8 5.5, but is decoded as '\ufffd\ufffdA' in previous versions of V8. PR-URL: #9618 Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
|
Landed in a67a04d...24ef1e6. Thanks everyone! |
|
Is this means we can use async/await safely? |
|
Will it be included in version 8 without flags? |
|
@leodutra It is already in master branch, thus in Node.js 8 |
With the upstream fix in V8, function declarations now work fine in the vm module and the test is no longer failing. Fixes: nodejs#548 Refs: https://codereview.chromium.org/2334733002 Refs: nodejs#9618 PR-URL: nodejs#11029 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Myles Borins <[email protected]>
The issue is fixed upstream in V8. Thus we do not need this workaround in REPL. Fixes: nodejs#548 Refs: nodejs#9618 PR-URL: nodejs#11029 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Myles Borins <[email protected]>
With the upstream fix in V8, function declarations now work fine in the vm module and the test is no longer failing. Fixes: nodejs#548 Refs: https://codereview.chromium.org/2334733002 Refs: nodejs#9618 PR-URL: nodejs#11029 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Myles Borins <[email protected]>
The issue is fixed upstream in V8. Thus we do not need this workaround in REPL. Fixes: nodejs#548 Refs: nodejs#9618 PR-URL: nodejs#11029 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Myles Borins <[email protected]>
|
@targos https://github.com/nodejs/node/blob/master/BUILDING.md#unix still mentions Clang 3.4 as the required version. Do you know what the new required version is? |
|
@joaocgreis I don't know what is the actual required version but since we dropped the workaround from #8343 it must be at least 3.4.2. |
Checklist
make -j8 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
V8
Description of change
This PR updates V8 to the current
5.5-lkgrbranch./cc @nodejs/v8
Previous discussion: nodejs/v8#2
CI: https://ci.nodejs.org/job/node-test-pull-request/4849/V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/417/V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/422/