-
Notifications
You must be signed in to change notification settings - Fork 29.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[V8] bring msvc build to upstream v8 #36906
Comments
V8 already has an MSVC builder in their CI, which runs on every CL: https://ci.chromium.org/p/v8/builders/try/v8_win64_msvc_compile_rel Honestly I don't understand how we have so many issues with V8 and MSVC. We probably need to look into the GYP configuration to make it closer to what they have. |
But many msvc issue seems not strong connection with gyp. Like #36139 (comment) (debug build failure) and #34337 (comment) (Msvc has strong checks). Is it because they lacks debug build and arm64 for windows in CI ? Another confused me is that #34337 (comment) just won't build in msvc. if so, how do this passing the CI. |
Another showcase bnoordhuis/v8-cmake#40. I am pretty positive upstream v8 msvc support has flaws. @nodejs/tsc Maybe you can push this forward ? |
Given the upcoming V8 8.9 windows build failed in https://github.com/nodejs/node-v8 and bnoordhuis/v8-cmake#45. I am pretty positive upstream V8 CI not working. |
Fixed in #37330 (comment). ping @nodejs/tsc @Trott @mhdawson Can you reach out V8 team for this. This is the best way V8 builds works consistently on msvc. |
Unfortunately no. |
cc @nodejs/v8 @verwaest |
ping @nodejs/tsc , this keep dragging us, anyone from google side can helps ?. https://chromium-review.googlesource.com/c/v8/v8/+/3199856 takes really long time to close. In the long run, if it's not on v8 CI system, we have to spend more time on this. |
I'd suggest opening an issue in the v8 issue tracker or asking in v8-dev regarding the state of their MSVC build bot. I vaguely remember that they switched to clang+llvm on Windows for consistent builds, so not sure why there is still a MSVC bot, but my memory could be wrong. |
https://chromium-review.googlesource.com/c/v8/v8/+/3199856/comments/49f62820_b7e38917 Let's hope some good news. |
The V8 team just announced they'll be dropping msvc (and gcc) support wholesale in September, i.e., they'll remove all the hacks and tweaks needed for compilers that aren't clang. Refs: https://groups.google.com/g/v8-dev/c/8mSxb2UriSU (msvc) Some contingency planning is probably in order. |
For reference:
|
Latest clangcl switch work in tracked in #52809 |
One thing that's important to stress - because it affects add-ons - is that msvcrt is going to be replaced with libc++. Big, fundamental change. Node.js somehow has to ensure add-ons link to that libc++, otherwise many things will go boom in many different and colorful ways. |
This will never happen. V8 officially dropped support for MSVC. |
For multi versions, we have special issue on msvc which is hard to trace and fix.
Build v8 with every commit in upstream will help us find root commit soon.
Currently we have daily build on https://github.com/nodejs/node-v8, but it's not good enough. Mainly it can be broken due to gyp issue or other issue. Then we lost track of which commit start to broken.
Yes. But the prices comes very high. You know how about cpp, it's very hard to find the exact error. On windows, it's worse. I have to take 40 minites or longer to verify my fix, even a single line. Windows doesn't support ninja or ccache for now. @targos and I spent weeks on v8 8.5 and 8.6 just to fix msvc issues.
So, if in upstream v8 doesn't has msvc build in every commit (don't has to be success, just to track which starts to broken will help a lot). This will super helpful for us maintaining v8 update.
cc @nodejs/v8-update
The text was updated successfully, but these errors were encountered: