Reduce the number of patches we float on V8 · nodejs/node#45118

(13 comments) (0 reactions) (0 assignees)JavaScript (117,218 stars) (35,535 forks)batch import

help wantedmetav8 engine

Description

We should aim for 0 floating patches from our side (except cherry-picks of commits that landed upstream).

Here's the list of everything that we currently float on top of V8 10.7:

deps: V8: forward declaration of Rtl*FunctionTable I don't remember why we have this one. It's quite old, so I opened a PR trying to revert it.
deps: fix V8 build issue with inline methods This bundles 4 fixes related to inline methods with Windows/MSVC. I think we would actually need a 5th one to fix the MSVC build with Visual Studio 2022 (https://github.com/nodejs/node/issues/43092). Ideally we would upstream the necessary fixes but I don't know enough of C++/Windows to open a CL that the V8 team would accept.
deps: V8: fix v8-cppgc.h for MSVC Not sure why this wasn't submitted upstream. Revert tentative: https://github.com/nodejs/node/pull/45700 Now fixed upstream in https://chromium-review.googlesource.com/c/v8/v8/+/3533019
deps: make V8 compilable with older glibc Can we remove this one now? PR: https://github.com/nodejs/node/pull/45162
deps: silence irrelevant V8 warning I often have to do this kind of patches. The problem is that if we keep the deprecation, V8's own headers trigger it and the build fails because of -Werror. There's probably a better way to handle it but I don't know it.
deps: fix V8 build on SmartOS Still waiting for SmartOS people to upstream the fix. (https://github.com/nodejs/node-v8/issues/239) Fixed upstream in https://chromium-review.googlesource.com/c/v8/v8/+/4026402
deps: fix V8 build on Windows with MSVC Don't know what do to about this one. See discussion in https://github.com/nodejs/node-v8/issues/235

@nodejs/v8-update

Tech stack: ccppnodejs
Domain: backendbuild systemtooling
Issue type: refactor
Difficulty: 5
Estimated time: over 1 week
Activity status: active
Clarity: mostly clear
Prerequisites: C++V8 internalsNode.js build systemWindows/MSVC
Newbie friendliness: 15
Research direction: Investigate each remaining floating patch listed in the issue: the inline methods fix (eac4bb), the MSVC build fix (7a8fa2), and the silencing of V8 warning (7a952e). For each, check upstream V8 commits and see if the issue has been resolved or if a fix can be upstreamed. The issue links to node v8 issues #239 and #235, and PR #45700. Coordinate with nodejs/v8 update team to determine if patches can be reverted or replaced with upstream cherry picks.