deps: update V8 to 6.2.414.44 #16848
Conversation
nodejs-github-bot
added
the
v8 engine
|
This doesn't build on windows (#16513). |
|
@refack please feel free to push it to this branch |
|
@refack can you bump the V8 embedder string in https://github.com/nodejs/node/blob/master/common.gypi#L30 I have really mixed feelings about landing an unreviewed patch into V8. Has this patch landed upstream in gyp? /cc @nodejs/v8 |
It's not a change in |
|
@refack we've been trying to avoid floating things on V8 if at all possible. is there a reason to not send this upstream? (we can still float this for now, but revert / update if it lands upstream) |
|
Opted to rerun full CI |
The plan is to upstream, just reprioritized (though I had more time till next V8 bump) |
BTW, as far as I understand this is exactly the bits that are going to be handed over to us once V8 drop
|
|
@maclover7 arm-fanned completely failed in this one but the above report is all green https://ci.nodejs.org/job/node-test-pull-request/11237/ /cc @nodejs/platform-arm any idea what is going on here? one more build of arm in case this was infra: https://ci.nodejs.org/job/node-test-commit-arm-fanned/12354/ |
|
It seems like the gcc / clang on our arm machines is too old for this update 😅 |
|
We're kind of screwed then. See nodejs/build#829 for further discussion. The way forward with this is probably to switch to building on Debian 8 (Jessie) or Ubuntu 14.04 depending on what we can get our hands on. That would mean pulling in Pi3's to do releases for ARMv6 and extra machines on Scaleway for ARMv7 which could be a problem given our limit there. We're locked in to using what we have for Node 4,6,8 for their lifetime however so this is an awkward complication. |
|
I'm not sure. That message is just a warning from |
|
Test build seems to work. Kicked off a new CI: https://ci.nodejs.org/job/node-test-commit/13801/ |
|
I'd like to hear if there are any breakages or if that warning is just a heads-up. I've noted over in nodejs/build#829 that we should probably drop support for Wheezy & 12.04 in Node 10 which would alleviate this problem. The headache would come if V8 6.2 had problems with gcc 4.9 and we wanted to include it with Node 8. What's the status of that discussion btw? |
|
@refack looks good 👍 |
|
CI has been a bit odd tonight, but this looks good except for an AIX failure that is probably unrelated. Likely worth a re-run though. CI on AIX only: https://ci.nodejs.org/job/node-test-commit-aix/10058/ |
|
@refack which upstream did that commit come from? We should include the meta data |
Please include the metadata of that commit. Example of V8 backport: c087502 |
I agree the whole pass-arguments-to-python-just-to-save-a-JSON is too cumbersome and error prone. IMHO simplest solution is to format the JSON in the |
MylesBorins
force-pushed
the
update-v8
branch
from
75d84e7 to
de67a5e
Compare
|
ci: https://ci.nodejs.org/job/node-test-pull-request/11327/ @nodejs/v8 do you see any reasons to not include this windows fix for gyp? I've opened upstream issues regarding the failures issue to backport arm fix to 6.2 and 6.3: https://bugs.chromium.org/p/v8/issues/detail?id=7060 |
|
Backport was approved, I'm going to start making the merge requests I've sent the windows fix upstream in https://chromium-review.googlesource.com/c/v8/v8/+/761477 @refack I kept you as the author :P |
|
Backport requests for the arm fix in 6.3: https://chromium-review.googlesource.com/c/v8/v8/+/761499 |
|
Backports and upstream windows fix have landed. Submitted backport request for the windows fix: https://bugs.chromium.org/p/v8/issues/detail?id=7062 |
MylesBorins
force-pushed
the
update-v8
branch
from
de67a5e to
83fefc5
Compare
MylesBorins
changed the title
MylesBorins
changed the title
|
I've gone ahead and rolled forward to 6.2.414.42 which includes the arm fix and floated the patch that landed upstream to fix windows. Will update again once the window patch has landed on 6.2 and 6.3 and run the tests one more time. please do not land yet |
|
@MylesBorins I'm not comfortable merging gyp for Windows fix to stable (62). Is there a problem floating it here? Edit: We should wait for at least 3 days of Canary coverage upstream before we merge, i.e., at least 2 more days. |
|
I merged the "[gyp] Fix string escaping for GYP on Windows" fix to 6.2 (765954). We can remove the floating patch. |
MylesBorins
force-pushed
the
update-v8
branch
from
83fefc5 to
f88e2e7
Compare
MylesBorins
changed the title
|
All the patches we were floating have been merged upstream new CI: https://ci.nodejs.org/job/node-test-pull-request/11396/ Will land if green |
|
landed in 14d24cc |
Refs: v8/v8@6.2.414.32...6.2.414.44 PR-URL: #16848 Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Refs: v8/v8@6.2.414.32...6.2.414.44 PR-URL: #16848 Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Refs: v8/v8@6.2.414.32...6.2.414.44
If CI + V8-CI are green I think we should fast track to include this in tomorrow's 9.x release