deps: update V8 to 13.8 #58491
deps: update V8 to 13.8 #58491
Conversation
|
Review requested:
|
nodejs-github-bot
added
build
targos
added
the
semver-major
targos
force-pushed
the
v8-138
branch
2 times, most recently
from
aeefb42 to
f0e6449
Compare
|
I removed the reverts. It's not sustainable to keep them. Let's wait for nodejs/build#4083 to be done. |
targos
force-pushed
the
v8-138
branch
2 times, most recently
from
d5baa5d to
eeee442
Compare
targos
added
request-ci
|
@danmcd the illumos patch doesn't apply anymore so I had to skip it, but SmartOS still fails to build: https://ci.nodejs.org/job/node-test-commit-smartos/60903/nodes=smartos23-x64/ |
|
@nodejs/platform-aix PTAL at https://ci.nodejs.org/job/node-test-commit-aix/57505/nodes=aix72-ppc64/console |
Yeah... this is because of the AIX patch to V8 that's now in. (I referred to this earlier). I'll have an updated one that can drop here very soon, based on this v8-138 branch. Because of Google's overhead for direct contributions to V8, and the centrifuge of mergers-and-acquisitions my $DAYJOB has been through, I cannot engage them directly just yet. Also, what's the status on #58237 ? I want THAT also landing upstream in V8 too later, but it really helps folks running modern illumos (most everyone these days save a few Triton Data Center VMs/components, which is why Brie set up the Jenkins agents for you the way she did). |
|
@targos -> This commit is supposed to re-fix the VA48 in V8 post-IBM-fix: but I'm getting coredumps from |
|
Yeah my first post-IBM patch was broken badly. This one: has both compiled and shown only this for Which resembles the last my-up-to-date-illumos-environment. |
Looks like 13.8 includes the leaptiering fix for AIX / IBM i. Can we try to remove the changes we added here (this change disabled leaptiering for AIX and IBM i). Then rebuild to see if that resolves the error above? |
IF AIX leaptier gets removed, the old illumos VA48 fix needs to return. I've not had cycles or corporate clearance to put that into V8 directly. |
|
My suggestion above was to just turn off the switch that disables building with leaptiering (added back in 13.4). Now that the leaptiering changes are upstreamed and included in 13.8 we can turn turn that switch off. AIX leaptiering changes will remain the same and not be removed. |
How urgent is the above issue? |
OH MY GOODNESS, I'm so sorry for not reading that clearly. My new post-IBM-leaptiering illumos VA48 commit stands, then, and it'd be great to have it here (and in V8, but that's not a here problem). |
|
I created a branch to test enabling leaptiering with V8 13.8 for AIX: https://github.com/abmusse/node/tree/v8-138-aix Ran a test build and things are building again: https://ci.nodejs.org/job/node-test-commit-aix/nodes=aix72-ppc64/57652/console 🎉 |
Major V8 updates are usually API/ABI incompatible with previous versions. This commit adapts NODE_MODULE_VERSION for V8 13.8. Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md
PR-URL: nodejs#54077 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Paolo Insogna <[email protected]>
PR-URL: nodejs#53134 Refs: nodejs#52809 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Paolo Insogna <[email protected]>
It's causing linker errors with node.lib in node-gyp and potentially breaks other 3rd party tools PR-URL: nodejs#56238 Refs: nodejs#55784 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Paolo Insogna <[email protected]>
GCC emits warnings because of the trailing backslashes. PR-URL: nodejs#58070 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
PR-URL: nodejs#58070 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
|
The illumos patch 96b370f is not the correct one to apply after the IBM commit. (EDIT/AMEND): Uggh, let's try this again. See here: https://github.com/danmcd/node/commits/v8-138/ There's a revert-commit of the bad one, and a new-commit of the good one. |
illumos pointers are VA48, can allocate from the top of the 64-bit range as well.
Original commit message:
[explicit-resource-management] Fix parsing for (using of=null;;)
Apparently `using of` is allowed in the initializer position of C-style
for loops.
See tc39/proposal-explicit-resource-management#248
Bug: 42203506
Change-Id: Ia056b161f4ea28a0f3ba4e3e420f1718195274a4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6594471
Commit-Queue: Shu-yu Guo <[email protected]>
Reviewed-by: Rezvan Mahdavi Hezaveh <[email protected]>
Cr-Commit-Position: refs/heads/main@{#100531}
Refs: v8/v8@249de88
PR-URL: nodejs#58561
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
No description provided.