-
Notifications
You must be signed in to change notification settings - Fork 107
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
Electron 13 build failure #90
Comments
Yeah, I also have this problem, which is really exhausting: I am developing a VSCode extension that depends on this native module: https://github.com/Symbolk/somanyconflicts. Previously, I have managed to conquer this problem for Node 14.16.0, Electron 12.0.4, electron-rebuild 2.3.5,node-gyp 8.1.0. However, recently VSCode upgrades its Electron to 13.1.7 (https://www.npmjs.com/package/electron-releases), and my users reported that the extension cannot run so I have to rebuild it. Then the annoying error happens again, just like the previous one about I have given up it, and looking for a WebAssembly solution. |
|
@maxbrunsfeld - could you incorporate the fix? |
Any update on this? |
Hi, I suggest to use WASM version, like what I have done: https://github.com/Symbolk/somanyconflicts |
The WASM version is slower, a real native version like the purpose of this repo is always preferred. The repository just needs more maintenance but unfortunately @maxbrunsfeld seems to not have enough time for it, which is a real shame for such great project. |
Exactly, I chose this repo over the WASM version specifically because I've read it's slower; but then I read somewhere that this is no longer the case. However a native solution is always the best. |
So, I tried to use the WASM version myself; but LSP logs say:
Any idea on what that might mean? |
Can anyone give a hint about prospects for an official fix? |
I don't see any point in opening CR. Maintainer seems to not be checking this repo otherwise he would fix this long ago. "dependencies": {
# ...
"tree-sitter": "github:sergei-dyshel/node-tree-sitter#master-next",
# ...
} |
Yes, that's what I'm doing now, still getting some undefined references but
nothing a good debugging session wouldn't fix. Thanks.
…On Mon, Dec 20, 2021, 8:43 PM Sergei Dyshel ***@***.***> wrote:
@sergei-dyshel <https://github.com/sergei-dyshel> Could you please PR the
fix to this repo?
I also need to use Tree-Sitter in a VSCode extension, and limiting users
to vscode-1.56.x is not wise.
I don't see any point in opening CR. Maintainer seems to not be checking
this repo otherwise he would fix this long ago.
Meanwhile you can create a fixed branch in your own fork and use it as a
dependency in package.json. For example I have:
"dependencies": {
# ...
"tree-sitter": "github:sergei-dyshel/node-tree-sitter#master-next",
# ...
}
—
Reply to this email directly, view it on GitHub
<#90 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AIHAT2DOEQZUZQG5B5ZVVZTUR6BPNANCNFSM5AUV5WCA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
I was getting this also. The error went away after rebuilding the WASM using docker instead of |
Even with this approach (which works) I still get a:
referring to a missing symbol definition for |
Happy new year guys
nm -C ./node_modules/tree-sitter/build/Release/obj.target/tree_sitter_runtime_binding.node | grep ArrayBuffer::New
U v8::ArrayBuffer::NewBackingStore(void*, unsigned long, void (*)(void*, unsigned long, void*), void*)
U v8::ArrayBuffer::New(v8::Isolate*, std::shared_ptr<v8::BackingStore>)
ldd ./node_modules/tree-sitter/build/Release/obj.target/tree_sitter_runtime_binding.node
linux-vdso.so.1 (0x00007ffc2c1ad000)
libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007f59d9b3a000)
libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f59d9b17000)
libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f59d9925000)
libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f59d97d6000)
/lib64/ld-linux-x86-64.so.2 (0x00007f59d9da5000)
libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f59d97b9000) Anyone has experienced this before? Any help is much appreciated as it's driving me crazy 😄 |
I tried this and had a similar experience, but didn't pursue and switched to WASM, which seems to work fine. Since rust and WASM bindings were moved into |
Yes, exactly, plus I confirm there is no performance overhead when using the WASM version; at least for my parser. |
Well it is all good with the WASM build but the real native build provides 5x better performance. A quick benchmark with parsing a html file yields: On MacBook Pro M1 MAX:File Size 25KB:
File Size 200KB:
On Windows 10 Pro with AMD Ryzen 7 3800X 8 Core 3.90 GHz:File Size 25KB:
File Size 200KB:
So as try can see the native tree Sitter is much faster and on smaller files even faster than the native DOMParser in from the browser. So we would really like to have a native version of the node-tree-sitter as well, next to the WASM version However without a good rewrite to support NAPI as started in #52 Starting from Electron 14 all non context aware modules that are not NAPI compatible, cannot be loaded any more as stated in: electron/electron#18397 So please @maxbrunsfeld have an urgent look on this. |
tree-sitter doesn't build alongside the latest version of VSCode there error i get is:
|
Hi, any update on this? |
This comment was marked as off-topic.
This comment was marked as off-topic.
I think I've found the symbol for nm -C node_modules/electron/dist/electron | grep 'v8::ArrayBuffer::New'
0000000002caa680 T v8::ArrayBuffer::NewBackingStore(v8::Isolate*, unsigned long)
0000000002caa790 T v8::ArrayBuffer::NewBackingStore(void*, unsigned long, void (*)(void*, unsigned long, void*), void*)
0000000002caa3c0 T v8::ArrayBuffer::New(v8::Isolate*, unsigned long)
0000000002caa4b0 T v8::ArrayBuffer::New(v8::Isolate*, std::__1::shared_ptr<v8::BackingStore>) The last of these seems to be what is needed. Unfortunately, it seems to be using the LLVM runtime. I guess if you can run prebuild against libc++, it might solve this problem. |
Oh, that it is actually good news; Thanks! I have switched to WASM builds since, and they work - I can't remember if this was the only error but if I find time, I'll probably go back and try the native version
|
Please let me know if you end up with a reasonably clean solution. This blog post contains an interesting deep dive into a similar issue that the author encountered when moving to chromium 90.0.4415.0. It seems Electron 13 was the first version to use a version of chromium greater than or equal to that. Their ultimate suggestion is similar to @gpetrov's (use NAPI). |
This comment was marked as outdated.
This comment was marked as outdated.
This should be fixed with node-tree-sitter v0.20.4. If someone could confirm or re-open the issue if it's still not fixed, that would be great. |
node-tree-sitter is now Node-API and should be compatible with Electron natively. This can be closed. |
Anyone tried to build for Electron 13?
Trying it with
prebuild -r electron -t 13.0.1
leads to errors such astree-sitter\src\node.cc(32,129): error C2661: 'v8::ArrayBuffer::New': no overloaded function takes 3 arguments
tree-sitter\src\node.cc(36,42): error C3536: 'js_transfer_buffer': cannot be used before it is initialized
tree-sitter\src\node.cc(36,69): error C2665: 'v8::Uint32Array::New': none of the 2 overloads could convert all the argument types
I'm aware of #83, and the problems described above happen after changing
binding.gyp
to followingThis does not happen when building for other versions of electron or node.
The text was updated successfully, but these errors were encountered: