Skip to content
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

windows build start failed on main caused by MSVC update #43092

Open
gengjiawen opened this issue May 14, 2022 · 14 comments
Open

windows build start failed on main caused by MSVC update #43092

gengjiawen opened this issue May 14, 2022 · 14 comments
Labels
build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform.

Comments

@gengjiawen
Copy link
Member

gengjiawen commented May 14, 2022

** TLDR **
MSVC don't care about this issue. nothing to do on our side

**TLDR: Please upvote this MSVC issue to help us resolve this issue.**

https://developercommunity.visualstudio.com/t/build-failed-on-Nodejs-introduced-in-17/10201632

Details

works good:

https://github.com/nodejs/node/runs/6376797643?check_suite_focus=true
Visual Studio: 17.1.32421.90 (Visual Studio Enterprise 2022)

failed:
https://github.com/nodejs/node/runs/6428804600?check_suite_focus=true

Visual Studio: 17.2.32505.173 (Visual Studio Enterprise 2022)

I don't know it's a new bug or MSVC introduced a braking change.

cc @nodejs/build @nodejs/actions

also we have #42560 pending on MSVC side to fix.

@gengjiawen gengjiawen changed the title windows build start failed on master windows build start failed on master caused by MSVC update May 14, 2022
@gengjiawen
Copy link
Member Author

Looks like we need to do something similar https://sourcegraph.com/github.com/nodejs/node/-/blob/deps/v8/src/objects/fixed-array.h?L137 for get method.

@himself65 himself65 added the build Issues and PRs related to build files or the CI. label May 14, 2022
@richardlau richardlau added the windows Issues and PRs related to the Windows platform. label Jun 1, 2022
@targos targos changed the title windows build start failed on master caused by MSVC update windows build start failed on main caused by MSVC update Jul 20, 2022
@dennisameling
Copy link
Contributor

The fix reported in #42560 fixes the VS2022 build for Node 14.x and 16.x (MS just released the fix for that in VS 2022 17.3), but the issue with the main branch remains:

  mksnapshot.cc
  snapshot-empty.cc
F:\node\deps\v8\src\base\bits.h(340,31): warning C4146: unary minus operator applied to unsigned type, result still unsigned [F:\node\tools\v8_gypfiles\mksnapshot.vcxproj]
F:\node\deps\v8\src\base\bits.h(340,31): warning C4146: unary minus operator applied to unsigned type, result still unsigned [F:\node\tools\v8_gypfiles\mksnapshot.vcxproj]
F:\node\deps\v8\src\base\bits.h(340,31): warning C4146: unary minus operator applied to unsigned type, result still unsigned [F:\node\tools\v8_gypfiles\mksnapshot.vcxproj]
F:\node\deps\v8\src\base\bits.h(340,31): warning C4146: unary minus operator applied to unsigned type, result still unsigned [F:\node\tools\v8_gypfiles\mksnapshot.vcxproj]
F:\node\deps\v8\src\base\bits.h(340,31): warning C4146: unary minus operator applied to unsigned type, result still unsigned [F:\node\tools\v8_gypfiles\mksnapshot.vcxproj]
F:\node\deps\v8\src\base\bits.h(340,31): warning C4146: unary minus operator applied to unsigned type, result still unsigned [F:\node\tools\v8_gypfiles\mksnapshot.vcxproj]
     Creating library ..\..\out\Release\mksnapshot.lib and object ..\..\out\Release\mksnapshot.exp
mksnapshot.obj : error LNK2001: unresolved external symbol "public: class v8::internal::Object __cdecl v8::internal::FixedArray::get(int)const " (?get@FixedArray@internal@v8@@QEBA?AVObject@23@H@Z) [F:\node\tools\v8_gypfiles\mksnapshot.vcxproj]
platform-embedded-file-writer-aix.obj : error LNK2001: unresolved external symbol "public: class v8::internal::Object __cdecl v8::internal::FixedArray::get(int)const " (?get@FixedArray@internal@v8@@QEBA?AVObject@23@H@Z) [F:\node\tools\v8_gypfiles\mksna
pshot.vcxproj]
platform-embedded-file-writer-generic.obj : error LNK2001: unresolved external symbol "public: class v8::internal::Object __cdecl v8::internal::FixedArray::get(int)const " (?get@FixedArray@internal@v8@@QEBA?AVObject@23@H@Z) [F:\node\tools\v8_gypfiles\m
ksnapshot.vcxproj]
platform-embedded-file-writer-mac.obj : error LNK2001: unresolved external symbol "public: class v8::internal::Object __cdecl v8::internal::FixedArray::get(int)const " (?get@FixedArray@internal@v8@@QEBA?AVObject@23@H@Z) [F:\node\tools\v8_gypfiles\mksna
pshot.vcxproj]
platform-embedded-file-writer-win.obj : error LNK2001: unresolved external symbol "public: class v8::internal::Object __cdecl v8::internal::FixedArray::get(int)const " (?get@FixedArray@internal@v8@@QEBA?AVObject@23@H@Z) [F:\node\tools\v8_gypfiles\mksna
pshot.vcxproj]
..\..\out\Release\mksnapshot.exe : fatal error LNK1120: 1 unresolved externals [F:\node\tools\v8_gypfiles\mksnapshot.vcxproj]

As @gengjiawen already mentioned above, a similar fix is most likely needed for the FixedArray::get method. I'm happy to help adding a fix, but a total newbie to C++ so not sure which part in fixed-array.h to fix.

@gengjiawen
Copy link
Member Author

As @gengjiawen #43092 (comment), a similar fix is most likely needed for the FixedArray::get method. I'm happy to help adding a fix, but a total newbie to C++ so not sure which part in fixed-array.h to fix.

I did more research, it's actually a MSVC optimize flag regression. The debug build works. But I have no clue which optimize flag cause this.

@targos
Copy link
Member

targos commented Nov 14, 2022

Bug still present in Visual Studio 17.4.0.

@targos
Copy link
Member

targos commented Nov 14, 2022

@gengjiawen did you create an MSVC ticket for it?

@gengjiawen
Copy link
Member Author

gengjiawen commented Nov 14, 2022

@gengjiawen did you create an MSVC ticket for it?

Nope. Feel free to create one. Have to be on windows (The entry is on VS IIRC, don't has a PC now).

@targos
Copy link
Member

targos commented Nov 14, 2022

I'd gladly do it but I don't know how to write an actionable ticket with this bug.

@gengjiawen
Copy link
Member Author

I'd gladly do it but I don't know how to write an actionable ticket with this bug.

Maybe similar to https://developercommunity.visualstudio.com/t/Failed-to-compile-nodejs-16140-with-la/1682115?space=62&q=nodejs. Provide the good MSVC version and broken version.

@gengjiawen

This comment was marked as outdated.

@gengjiawen
Copy link
Member Author

Upstream bugreport: https://developercommunity.visualstudio.com/t/build-failed-on-Nodejs-introduced-in-17/10201632

@phr34k
Copy link

phr34k commented Nov 16, 2022

@gengjiawen I'm getting the same unresolved v8::internal::FixedArray::get external symbols when I use vercel/pkg and target node 18.5 on x86. It seems to compile node from source, understandly. Is there any fix for this error?

@gengjiawen
Copy link
Member Author

@gengjiawen I'm getting the same unresolved v8::internal::FixedArray::get external symbols when I use vercel/pkg and target node 18.5 on x86. It seems to compile node from source, understandly. Is there any fix for this error?

Use visual studio 2019 before MSVC fix this issues.

@phr34k
Copy link

phr34k commented Nov 16, 2022

@gengjiawen I'm getting the same unresolved v8::internal::FixedArray::get external symbols when I use vercel/pkg and target node 18.5 on x86. It seems to compile node from source, understandly. Is there any fix for this error?

Use visual studio 2019 before MSVC fix this issues.

I'll give that a shot, just my two cents. It'd be great if we had a way to override the PLATFORM_TOOLSET, and didn't overwrite it from vcbuild.bat based on the visual studio version. I don't have a full visual studio 2019, but I do have the 2019 compiler and build tools installed on 2022.

Edit:

Never mind, I figured out that you can set SET msbuild_args=/p:PlatformToolset=v142 pass in the additional msbuild parameters, with that the project compiles as expected.

@gengjiawen
Copy link
Member Author

Clearly MSVC don't want to admit this regression and disabled comment.

But still I want to leave this open for a while.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

No branches or pull requests

6 participants