search

nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
106.43k stars 29.01k forks source link

[V8] bring msvc build to upstream v8 #36906

Open gengjiawen opened 3 years ago

gengjiawen commented 3 years ago

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.

In the end, the msvc issues has been fixed, right ?

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

targos commented 3 years ago

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.

targos commented 3 years ago

Noting:

https://github.com/v8/v8/blob/1bd5755bba33303edf128a6e01dfaab9b6c552d0/BUILD.gn#L983-L985

gengjiawen commented 3 years ago

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 https://github.com/nodejs/node/pull/36139#issuecomment-759062695 (debug build failure) and https://github.com/nodejs/node/pull/34337#issuecomment-678791255 (Msvc has strong checks).

Is it because they lacks debug build and arm64 for windows in CI ?

Another confused me is that https://github.com/nodejs/node/pull/34337#issuecomment-678791255 just won't build in msvc. if so, how do this passing the CI.

gengjiawen commented 3 years ago

Another showcase https://github.com/bnoordhuis/v8-cmake/pull/40. I am pretty positive upstream v8 msvc support has flaws. @nodejs/tsc Maybe you can push this forward ?

gengjiawen commented 3 years ago

Given the upcoming V8 8.9 windows build failed in https://github.com/nodejs/node-v8 and https://github.com/bnoordhuis/v8-cmake/pull/45. I am pretty positive upstream V8 CI not working.

gengjiawen commented 3 years ago

Given the upcoming V8 8.9 windows build failed in nodejs/node-v8 and bnoordhuis/v8-cmake#45. I am pretty positive upstream V8 CI not working.

Fixed in https://github.com/nodejs/node/pull/37330#issuecomment-783000812.

ping @nodejs/tsc @Trott @mhdawson Can you reach out V8 team for this. This is the best way V8 builds works consistently on msvc.

mhdawson commented 3 years ago

@hashseed is this something you can help with? @targos do you know who are best/most responsive contact is these days?

targos commented 3 years ago

Unfortunately no.

mmarchini commented 3 years ago

cc @nodejs/v8 @verwaest

gengjiawen commented 2 years ago

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.

joyeecheung commented 2 years ago

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.

gengjiawen commented 2 years ago

We don't, strictly speaking, support MSVC, so keeping that build working is more best effort and community-led; we don't want to block any of our engineers by making them fix MSVC issues. That said, it'd maybe still be useful to have an FYI bot with MSVC so that we at least see when it's broken -- I'll chat to our infra team about it.

https://chromium-review.googlesource.com/c/v8/v8/+/3199856/comments/49f62820_b7e38917

Let's hope some good news.

bnoordhuis commented 2 months ago

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) https://groups.google.com/g/v8-dev/c/9WLui7QgBI8 (gcc)

Some contingency planning is probably in order.

RedYetiDev commented 2 months ago

For reference:

https://groups.google.com/g/v8-dev/c/8mSxb2UriSU (msvc)

"V8 will follow Chromium's lead and will discontinue support for MSVC. To give projects time to adjust, we will stop support after V8 version 13.0 in September 2024. After that, we will remove the infrastructure bots and MSVC compatibility hacks in the code base."

https://groups.google.com/g/v8-dev/c/9WLui7QgBI8 (gcc)

"V8 will follow Chromium's lead and will stop providing infrastructure for GCC builds. We will still accept patches from the community but we will not invest in keeping this build functioning. To give projects time to adjust, we will perform this change after V8 version 13.0 in September 2024."

joyeecheung commented 2 months ago

Latest clangcl switch work in tracked in https://github.com/nodejs/node/issues/52809

bnoordhuis commented 2 months ago

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.