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

Port cppgc config to gyp #161

Closed
targos opened this issue Jun 22, 2020 · 43 comments
Closed

Port cppgc config to gyp #161

targos opened this issue Jun 22, 2020 · 43 comments

Comments

@targos
Copy link
Member

targos commented Jun 22, 2020

V8 now depends on it. We need to port the config to our GYP files

@targos targos changed the title cppgc Port cppgc config to gyp Jun 22, 2020
targos added a commit to nodejs/node that referenced this issue Jun 24, 2020
targos added a commit to nodejs/node that referenced this issue Jun 24, 2020
@targos
Copy link
Member Author

targos commented Jun 24, 2020

I fixed the build at least on Linux with nodejs/node@4ba0b28 and nodejs/node@ac8fb3f.
Let's keep this issue open because cppgc is still a moving target and I did not look into implementing the build options that it provides.

nodejs-ci pushed a commit that referenced this issue Jun 24, 2020
@targos
Copy link
Member Author

targos commented Jun 24, 2020

CI: https://ci.nodejs.org/job/node-test-commit-node-v8/546/

Need to fix cross-compilation:

19:41:52 python ./configure --verbose  --dest-cpu=arm
19:41:55 gyp: Dependency '/home/iojs/build/workspace/node-cross-compile/tools/v8_gypfiles/v8.gyp:cppgc_base#host' not found while trying to load target /home/iojs/build/workspace/node-cross-compile/tools/v8_gypfiles/v8.gyp:v8_base_without_compiler#host

And macOS:

19:41:43 running: 
19:41:43     python tools/gyp_node.py --no-parallel -Dconfiguring_node=1 -f make-mac
19:41:43 static library v8_base_without_compiler has several files with the same basename:
19:41:43   allocation: ../../deps/v8/src/utils/allocation.cc ../../deps/v8/src/heap/cppgc/allocation.cc
19:41:43   free-list: ../../deps/v8/src/heap/free-list.cc ../../deps/v8/src/heap/cppgc/free-list.cc
19:41:43   sweeper: ../../deps/v8/src/heap/sweeper.cc ../../deps/v8/src/heap/cppgc/sweeper.cc
19:41:43   heap: ../../deps/v8/src/heap/heap.cc ../../deps/v8/src/heap/cppgc/heap.cc
19:41:43 libtool on OS X will generate warnings for them.
19:41:43 Error running GYP
19:41:43 make: *** [build-ci] Error 1

And Windows:

19:46:05   cpp-heap.cc
19:46:05 C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.26.28801\include\xutility(3778,16): error C2280: 'std::unique_ptr<cppgc::internal::BaseSpace,std::default_delete<cppgc::internal::BaseSpace>> &std::unique_ptr<cppgc::internal::BaseSpace,std::default_delete<cppgc::internal::BaseSpace>>::operator =(const std::unique_ptr<cppgc::internal::BaseSpace,std::default_delete<cppgc::internal::BaseSpace>> &)': attempting to reference a deleted function [C:\workspace\node-compile-windows\node\tools\v8_gypfiles\v8_base_without_compiler.vcxproj]
19:46:05 C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.26.28801\include\memory(1917): message : see declaration of 'std::unique_ptr<cppgc::internal::BaseSpace,std::default_delete<cppgc::internal::BaseSpace>>::operator =' [C:\workspace\node-compile-windows\node\tools\v8_gypfiles\v8_base_without_compiler.vcxproj]
19:46:05 C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.26.28801\include\memory(1917,17): message : 'std::unique_ptr<cppgc::internal::BaseSpace,std::default_delete<cppgc::internal::BaseSpace>> &std::unique_ptr<cppgc::internal::BaseSpace,std::default_delete<cppgc::internal::BaseSpace>>::operator =(const std::unique_ptr<cppgc::internal::BaseSpace,std::default_delete<cppgc::internal::BaseSpace>> &)': function was explicitly deleted [C:\workspace\node-compile-windows\node\tools\v8_gypfiles\v8_base_without_compiler.vcxproj]
19:46:05 C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.26.28801\include\vector(1127): message : see reference to function template instantiation '_OutIt *std::_Copy_unchecked<_Iter,std::unique_ptr<cppgc::internal::BaseSpace,std::default_delete<cppgc::internal::BaseSpace>>*>(_InIt,_InIt,_OutIt)' being compiled [C:\workspace\node-compile-windows\node\tools\v8_gypfiles\v8_base_without_compiler.vcxproj]

/cc @nodejs/v8

@ryzokuken
Copy link

The ARM issue doesn't look like a platform-specific error?

nodejs-ci pushed a commit that referenced this issue Jun 25, 2020
nodejs-ci pushed a commit that referenced this issue Jun 26, 2020
@gengjiawen

This comment has been minimized.

nodejs-ci pushed a commit that referenced this issue Jun 27, 2020
nodejs-ci pushed a commit that referenced this issue Jun 27, 2020
targos added a commit to nodejs/node that referenced this issue Jun 27, 2020
targos added a commit to nodejs/node that referenced this issue Jun 27, 2020
nodejs-ci pushed a commit that referenced this issue Jun 27, 2020
@targos
Copy link
Member Author

targos commented Jun 27, 2020

I pushed a few fixes. New CI: https://ci.nodejs.org/job/node-test-commit-node-v8/551/

@gengjiawen
Copy link
Member

gengjiawen commented Jun 27, 2020

Windows still failed on latest vs2019 (works on vs2017), not sure is this a regression.

image

@targos
Copy link
Member Author

targos commented Jun 27, 2020

I did not do anything for Windows. I don't know if it's a bug in the compiler or in V8

@targos
Copy link
Member Author

targos commented Jun 27, 2020

@nodejs/platform-windows PTAL: https://ci.nodejs.org/job/node-compile-windows/34716/

@gengjiawen
Copy link
Member

I did not do anything for Windows. I don't know if it's a bug in the compiler or in V8

I tend to a compiler bug, but I simulate related code but still failed to create a repro :(

nodejs-ci pushed a commit that referenced this issue Aug 26, 2020
nodejs-ci pushed a commit that referenced this issue Aug 27, 2020
nodejs-ci pushed a commit that referenced this issue Aug 28, 2020
nodejs-ci pushed a commit that referenced this issue Aug 29, 2020
nodejs-ci pushed a commit that referenced this issue Aug 30, 2020
nodejs-ci pushed a commit that referenced this issue Aug 31, 2020
nodejs-ci pushed a commit that referenced this issue Sep 1, 2020
nodejs-ci pushed a commit that referenced this issue Sep 2, 2020
nodejs-ci pushed a commit that referenced this issue Sep 3, 2020
nodejs-ci pushed a commit that referenced this issue Sep 4, 2020
nodejs-ci pushed a commit that referenced this issue Sep 5, 2020
nodejs-ci pushed a commit that referenced this issue Sep 6, 2020
@vsemozhetbyt
Copy link

vsemozhetbyt commented Sep 6, 2020

Sorry to bother, but is it worth to mention the Windows collaborators team here? We have not had Windows canary for three months...

@gengjiawen
Copy link
Member

Sorry to bother, but is it worth to mention the Windows collaborators team here? We have not had Windows canary for three months...

This is more like a gyp issue: nodejs/gyp-next#60

nodejs-ci pushed a commit that referenced this issue Sep 7, 2020
@mhdawson
Copy link
Member

mhdawson commented Sep 9, 2020

@miladfarca could you take a look at cppgc so we understand how it might impact power/s390 in the future?

@vsemozhetbyt
Copy link

nodejs/gyp-next#60 seems fixed but there are no canaries at all for last days.

@targos
Copy link
Member Author

targos commented Sep 11, 2020

nodejs/gyp-next#60 seems fixed but there are no canaries at all for last days.

Canary needed an update in v8.gyp. I'm testing it locally.

Edit: should be fine tomorrow if V8 doesn't introduce another breaking change.

@miladfarca
Copy link

miladfarca commented Sep 12, 2020

@mhdawson It's a new mark and sweep GC called Oilpan. It does scanning of C++/JS objects (treats them as one heap). It's actively being worked on and any port to other platforms will also get ported to p/z linux and AIX:
https://chromium-review.googlesource.com/c/v8/v8/+/2144691

This also eliminates the use of a clang plugin: nodejs/node#27257 (comment)
V8 issue for tracing: https://bugs.chromium.org/p/chromium/issues/detail?id=1056170

@vsemozhetbyt
Copy link

Something seems break all builds again. IIUC, #167

@vsemozhetbyt
Copy link

Still no Windows in today's build(

@targos
Copy link
Member Author

targos commented Sep 14, 2020

@vsemozhetbyt there's a build failure in release CI, but it seems unrelated to this issue. I opened #172

@mhdawson
Copy link
Member

@miladfarca thanks for the clarification :)

@gengjiawen
Copy link
Member

Still no Windows in today's build(

@vsemozhetbyt windows fix got merged, binary should be on the way 🎁

@vsemozhetbyt
Copy link

Thank you all! Win-x64 is back.

Now I can publish my translation about Intl.Segmenter (shipped in V8 87) and link to the full binaries set for readers to test the feature)

Are Win-x86 binaries not built anymore or is this a build issue? There were ones in the last successful build.

@targos
Copy link
Member Author

targos commented Sep 17, 2020

x86 build still fails:

05:31:36      Creating library ..\..\out\Release\mksnapshot.lib and object ..\..\out\Release\mksnapshot.exp
05:31:37 mksnapshot.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:\ws\tools\v8_gypfiles\mksnapshot.vcxproj]
05:31:37 v8_base_without_compiler.lib(stack.obj) : error LNK2019: unresolved external symbol _PushAllRegistersAndIterateStack referenced in function "public: void __thiscall heap::base::Stack::IteratePointers(class heap::base::StackVisitor *)const " (?IteratePointers@Stack@base@heap@@QBEXPAVStackVisitor@23@@Z) [c:\ws\tools\v8_gypfiles\mksnapshot.vcxproj]
05:31:37 ..\..\out\Release\mksnapshot.exe : fatal error LNK1120: 2 unresolved externals [c:\ws\tools\v8_gypfiles\mksnapshot.vcxproj]

@gengjiawen
Copy link
Member

gengjiawen commented Sep 17, 2020

x86 build still fails:

05:31:36      Creating library ..\..\out\Release\mksnapshot.lib and object ..\..\out\Release\mksnapshot.exp
05:31:37 mksnapshot.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@@$

@targos looks like you forget the patch x32 to ia32 in v8.gyp from v8 8.5.

@targos
Copy link
Member Author

targos commented Sep 18, 2020

@gengjiawen right, good catch! I pushed the change to canary-base. Build should go well tomorrow.

@gengjiawen
Copy link
Member

gengjiawen commented Oct 20, 2020

V8 8.6 merged into core 🎁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants