Repository navigation
V8 update script broken #166
Description
https://ci.nodejs.org/view/Node.js%20Daily/job/node-update-v8-canary/1435/console
13:55:51 + git-node v8 major --branch=lkgr --base-dir=/home/iojs/build/workspace/node-update-v8-canary --node-dir=/home/iojs/build/workspace/node-update-v8-canary/node --no-version-bump
13:55:52 [05:55:52] Update local V8 clone [started]
13:55:52 [05:55:52] Fetch V8 [started]
13:56:03 [05:56:03] Fetch V8 [completed]
13:56:03 [05:56:03] Update local V8 clone [completed]
13:56:03 [05:56:03] Major V8 update [started]
13:56:03 [05:56:03] Get current V8 version [started]
13:56:03 [05:56:03] Get current V8 version [completed]
13:56:03 [05:56:03] Checkout V8 branch [started]
13:56:06 [05:56:06] Checkout V8 branch [completed]
13:56:06 [05:56:06] Remove deps/v8 [started]
13:56:07 [05:56:06] Remove deps/v8 [completed]
13:56:07 [05:56:06] Clone branch to deps/v8 [started]
13:56:10 [05:56:10] Clone branch to deps/v8 [completed]
13:56:10 [05:56:10] Remove deps/v8/.git [started]
13:56:10 [05:56:10] Remove deps/v8/.git [completed]
13:56:10 [05:56:10] Update V8 DEPS [started]
13:56:10 [05:56:10] Update V8 DEPS [failed]
13:56:10 [05:56:10] → Cannot read property 'split' of undefined
13:56:10 [05:56:10] Major V8 update [failed]
13:56:10 [05:56:10] → Cannot read property 'split' of undefined
13:56:10 ✖ Cannot read property 'split' of undefined
Activity
gengjiawen commentedon Aug 6, 2020
Also cc @nodejs/node-core-utils An edge case in
node-core-utils?This jenkins job runs latest node-core-utils from github master branch, start broken on July 24.
mmarchini commentedon Aug 6, 2020
Did V8 add a new dependency or remove an old dependency? That could break things I think.
mmarchini commentedon Aug 6, 2020
On a second thought (and after looking at V8 git history for today and yesterday), that's probably not the issue.
codebytere commentedon Aug 6, 2020
hmm 🤔 not much has changed in that section of the codebase lately that i'm aware of but i can go have a poke
gengjiawen commentedon Aug 6, 2020
It starts broken on July 24. I checked the node-core-utils v8 part, looks not touched recently.
More likely from V8. but I don't know
13:56:10 [05:56:10] → Cannot read property 'split' of undefinedcomes from since it lacks stacktrace.mmarchini commentedon Aug 6, 2020
Probably something changed on V8 which broke spectations we have on ncu.
codebytere commentedon Aug 6, 2020
If i had to guess it's https://github.com/nodejs/node-core-utils/blob/88d9c618c3ccf60aed8c6832eb6a4b47eb5dd9e6/lib/update-v8/majorUpdate.js#L133 but i'll test locally
Edit: confirmed - it dies on that line
codebytere commentedon Aug 6, 2020
Got it: nodejs/node-core-utils#465
targos commentedon Aug 6, 2020
I worked on a fix two days ago but didn't have time to test it and open a PR yet:targos/node-core-utils@15bc3b0
codebytere commentedon Aug 6, 2020
I'm happy to close mine out in favor of yours, or we can merge mine in the short term and then update to yours once we've verified things are shipshape? Up to yall :)
codebytere commentedon Aug 6, 2020
@ryzokuken implicit there would be addressing small review feedback to ensure that's the case :)
ryzokuken commentedon Aug 6, 2020
Oops, I just deleted my comment because it was obvious. Thanks.
mmarchini commentedon Aug 6, 2020
@ryzokuken the changed code is only used on major bumps, so I think it would be safe to land @codebytere PR and make a release (to unbreak the daily v8 updated), and then follow up integrating @targos changes to allow for relative paths.
ryzokuken commentedon Aug 6, 2020
Right. I feel it's highly unlikely that we would ever bump up to a version without those files, right?
mmarchini commentedon Aug 6, 2020
There's the ongoing V8 8.5 bump (nodejs/node#34337) which might also be backported to Node.js v14. Either way, for the ongoing PR it's still possible to use a previous ncu version, and we should get a patch with a long-term fix landed before we start to backport to 8.5 v14.
codebytere commentedon Aug 6, 2020
Should i ensure mine is backwards compatible then or is it fine as-is as a stopgap do we think? happy to try and make it a little cleaner if needed
gengjiawen commentedon Aug 6, 2020
If you can make it in a day I thinks it's okay. But longer may cause some issue, since node.js daily build broken more than 10 days. We got more time on V8 8.5 bump.
mmarchini commentedon Aug 6, 2020
Let's keep this open until the long-term fix is applied :)
vsemozhetbyt commentedon Aug 7, 2020
Is it normal that
v15.0.0-v8-canary20200723fa696aa4d8had"v8":"8.6.228.0"while the lastv15.0.0-v8-canary202008071df3ac3d89has"v8":"8.6.0.0"as per https://nodejs.org/download/v8-canary/index.json?targos commentedon Aug 10, 2020
That happens sometimes, when the HEAD commit of V8's
lkgrbranch wasn't tagged to a new version number yet.fix(v8): support non-relative paths in V8 DEPS
fix(v8): support non-relative paths in V8 DEPS (#471)
fix(v8): support non-relative paths in V8 DEPS (#471)
fix(v8): support non-relative paths in V8 DEPS (#471)
fix(v8): support non-relative paths in V8 DEPS (#471)
fix(v8): support non-relative paths in V8 DEPS (#471)
fix(v8): support non-relative paths in V8 DEPS (#471)