Skip to content

Fix V8 patch

This is a particularly weird fix and I ask that @nodejs/lts @nodejs/ctc @nodejs/v8 all take a look and make sure that I'm not off here.

While auditing a backport we were digging into a V8 change that apparantly landed in v6.x during the upgrade to V8 5.1 in cd77ca39, a backport of https://github.com/ofrobots/node/commit/33c957890db04f4f379b8da99a7a4c0150ae1d96

Unfortunately that commit didn't change anything other than the patch number of V8 as the to change had already landed in v6.x as https://github.com/nodejs/node/commit/92ecbc4edcfe38d22fda5fabf422ca505c7423fa

This wouldn't be an issue, except for the fact that neither of these commits backported the exact changeset from the V8 tracker, specifically the changes to deps/v8/build/standalone.gypi . The change is instead found inside of toolchain.gypi with no explanation as to why this was done.

The original backport https://github.com/ofrobots/node/commit/33c957890db04f4f379b8da99a7a4c0150ae1d96 also references the wrong commit sha from V8, but that is pretty minor compared to missing part of the change set.

Phew.

So this PR attempts to rectify everything, as long as there is anything that actually needs to be rectified. To be honest I am a bit confused by the subtle difference. If I am mistaken and the changeset that we currently have is what we want I will close this, but we should try to avoid landing V8 changes without the proper meta data going forward so we can avoid confusion like this.

Merge request reports

Loading