deps: V8: extend fix for inline methods to Win64 by targos · Pull Request #40000 · nodejs/node

targos · 2021-09-05T10:45:42Z

No description provided.

nodejs-github-bot · 2021-09-05T11:55:12Z

CI: https://ci.nodejs.org/job/node-test-pull-request/39793/

github-actions · 2021-09-05T14:09:50Z

Fast-track has been requested by @jasnell. Please 👍 to approve.

targos · 2021-09-05T15:01:49Z

Still broken on 32-bit

14:12:12 platform-embedded-file-writer-aix.obj : error LNK2019: unresolved external symbol "public: void __thiscall v8::internal::FixedArray::set(int,class v8::internal::Smi)" (?set@FixedArray@internal@v8@@QAEXHVSmi@23@@Z) referenced in function "protected: void __thiscall v8::internal::OrderedHashTable<class v8::internal::OrderedHashMap,2>::SetNumberOfBuckets(int)" (?SetNumberOfBuckets@?$OrderedHashTable@VOrderedHashMap@internal@v8@@$01@internal@v8@@IAEXH@Z) [C:\workspace\node-compile-windows\node\tools\v8_gypfiles\mksnapshot.vcxproj]
14:12:13 platform-embedded-file-writer-generic.obj : error LNK2001: unresolved external symbol "public: void __thiscall v8::internal::FixedArray::set(int,class v8::internal::Smi)" (?set@FixedArray@internal@v8@@QAEXHVSmi@23@@Z) [C:\workspace\node-compile-windows\node\tools\v8_gypfiles\mksnapshot.vcxproj]
14:12:13 platform-embedded-file-writer-mac.obj : error LNK2001: unresolved external symbol "public: void __thiscall v8::internal::FixedArray::set(int,class v8::internal::Smi)" (?set@FixedArray@internal@v8@@QAEXHVSmi@23@@Z) [C:\workspace\node-compile-windows\node\tools\v8_gypfiles\mksnapshot.vcxproj]
14:12:13 mksnapshot.obj : error LNK2001: unresolved external symbol "public: void __thiscall v8::internal::FixedArray::set(int,class v8::internal::Smi)" (?set@FixedArray@internal@v8@@QAEXHVSmi@23@@Z) [C:\workspace\node-compile-windows\node\tools\v8_gypfiles\mksnapshot.vcxproj]
14:12:13 ..\..\out\Release\mksnapshot.exe : fatal error LNK1120: 1 unresolved externals [C:\workspace\node-compile-windows\node\tools\v8_gypfiles\mksnapshot.vcxproj]

We need to find a long-term solution to this. The failure happens on x64 too with V8 9.3

targos · 2021-09-05T15:22:54Z

I got the condition backwards 🤦🏻 . We actually need to keep the patch and extend it to x64...

It is necessary for V8 9.3 and Visual Studio 2022.

This reverts commit 95dffe7.

nodejs-github-bot · 2021-09-05T15:40:40Z

CI: https://ci.nodejs.org/job/node-test-pull-request/39799/

targos · 2021-09-05T17:51:12Z

And now it fails in debug mode 😭

17:46:37 C:\workspace\node-compile-windows-debug\node\deps\v8\src\objects\fixed-array.h(139,5): error C2027: use of undefined type 'v8::internal::Map' [C:\workspace\node-compile-windows-debug\node\tools\v8_gypfiles\v8_init.vcxproj]
17:46:37 C:\workspace\node-compile-windows-debug\node\deps\v8\src\roots\roots.h(26): message : see declaration of 'v8::internal::Map' [C:\workspace\node-compile-windows-debug\node\tools\v8_gypfiles\v8_init.vcxproj]
17:46:37 v8_pch.cc
17:46:37 C:\workspace\node-compile-windows-debug\node\deps\v8\src\objects\fixed-array.h(139,5): error C2027: use of undefined type 'v8::internal::Map' [C:\workspace\node-compile-windows-debug\node\tools\v8_gypfiles\v8_compiler.vcxproj]
17:46:37 C:\workspace\node-compile-windows-debug\node\deps\v8\src\roots\roots.h(26): message : see declaration of 'v8::internal::Map' [C:\workspace\node-compile-windows-debug\node\tools\v8_gypfiles\v8_compiler.vcxproj]
17:46:40 v8_pch.cc
17:46:40 C:\workspace\node-compile-windows-debug\node\deps\v8\src\objects\fixed-array.h(139,5): error C2027: use of undefined type 'v8::internal::Map' [C:\workspace\node-compile-windows-debug\node\tools\v8_gypfiles\v8_base_without_compiler.vcxproj]
17:46:40 C:\workspace\node-compile-windows-debug\node\deps\v8\src\roots\roots.h(26): message : see declaration of 'v8::internal::Map' [C:\workspace\node-compile-windows-debug\node\tools\v8_gypfiles\v8_base_without_compiler.vcxproj]
17:46:40

addaleax · 2021-09-05T21:32:06Z

@targos You could just comment out the DCHECK?

gengjiawen · 2021-09-06T03:16:26Z

 return get(isolate, index).IsTheHole(isolate);
 }

-#if !defined(_WIN32) || defined(_WIN64)


_WIN32 actually is defined in windows 64bit mode. So this is windows ...

Context: https://stackoverflow.com/a/6679822/1713757.

My another desperate try: #40010.

gengjiawen · 2021-09-06T03:29:05Z

@targos You could just comment out the DCHECK?

Mostly likely won't help. It's a weird linker bug in vs.

targos · 2021-09-06T06:26:03Z

Closing in favor of #40010

addaleax · 2021-09-06T07:58:20Z

@targos You could just comment out the DCHECK?
Mostly likely won't help. It's a weird linker bug in vs.

@gengjiawen It’s actually not and I generally do know what I’m talking about.

gengjiawen · 2021-09-06T08:04:10Z

@gengjiawen It’s actually not and I generally do know what I’m talking about.

You are probably right. I'm just speculating from past experience handling v8 build issue :)

nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency. labels Sep 5, 2021

targos added windows Issues and PRs related to the Windows platform. dont-land-on-v12.x request-ci Add this label to start a Jenkins CI on a PR. labels Sep 5, 2021

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 5, 2021

jasnell approved these changes Sep 5, 2021
View reviewed changes

jasnell added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fast-track PRs that do not need to wait for 48 hours to land. labels Sep 5, 2021

targos removed the fast-track PRs that do not need to wait for 48 hours to land. label Sep 5, 2021

targos added the blocked PRs that are blocked by other issues or PRs. label Sep 5, 2021

targos added 2 commits September 5, 2021 17:25

deps: V8: extend fix for inline methods to Win64
72e21b7
It is necessary for V8 9.3 and Visual Studio 2022.

Revert "Revert "build: add windows-2022 to GitHub test matrix""
010fdac
This reverts commit 95dffe7.

targos force-pushed the revert-inline-fix branch from a5dbea3 to 010fdacCompare September 5, 2021 15:29

targos changed the title ~~Revert "deps: fix V8 build issue with inline methods"~~deps: V8: extend fix for inline methods to Win64Sep 5, 2021

targos added request-ci Add this label to start a Jenkins CI on a PR. and removed blocked PRs that are blocked by other issues or PRs. labels Sep 5, 2021

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 5, 2021

cjihrig approved these changes Sep 5, 2021
View reviewed changes

targos added blocked PRs that are blocked by other issues or PRs. and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Sep 5, 2021

mscdex mentioned this pull request Sep 6, 2021
fix: patch for v8 on windows #40010
Closed

gengjiawen reviewed Sep 6, 2021
View reviewed changes

targos closed this Sep 6, 2021

richardlau mentioned this pull request Sep 30, 2021
build: compile with --gnu++17 #38807
Closed

Uh oh!

Uh oh!

Conversation

targos commented Sep 5, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Sep 5, 2021

Uh oh!

github-actionsBot commented Sep 5, 2021

Uh oh!

targos commented Sep 5, 2021

Uh oh!

targos commented Sep 5, 2021

Uh oh!

nodejs-github-bot commented Sep 5, 2021

Uh oh!

targos commented Sep 5, 2021

Uh oh!

addaleax commented Sep 5, 2021

Uh oh!

gengjiawenSep 6, 2021

Choose a reason for hiding this comment

Uh oh!

gengjiawenSep 6, 2021

Choose a reason for hiding this comment

Uh oh!

gengjiawenSep 6, 2021

Choose a reason for hiding this comment

Uh oh!

gengjiawen commented Sep 6, 2021

Uh oh!

targos commented Sep 6, 2021

Uh oh!

addaleax commented Sep 6, 2021

Uh oh!

gengjiawen commented Sep 6, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

targos commented Sep 5, 2021•
edited
Loading