Support Electron 13+ on Windows

[verhovsky]

Closes #90, follow up to #95

[verhovsky]

@selfint as you pointed out

Both _MSC_VER and NODE_RUNTIME_ELECTRON were not set, and I couldn't figure out how to set them.

NODE_RUNTIME_ELECTRON was not being set so the code in the #if never ran, and it had some typos and whatever so it didn't work even after fixing the #if.

[verhovsky]

For reference: I saw the changes in native/cocos/bindings/jswrapper/v8/Object.cpp in cocos/cocos-engine#13394 which I found with https://sourcegraph.com/search?q=context:global+lang:C%2B%2B+node::Buffer::New&patternType=standard&sm=1&groupBy=repo

[verhovsky]

The CI is able to prebuild node-tree-sitter for all platforms finally, but I don't know how to actually test if what it builds works. I don't have a Windows computer and for @selfint's test repo https://github.com/selfint/vscode-tree-sitter/ the CI fails to run yarn add -D windows-build-tools so it fails before it starts actually compiling or testing the code

[selfint]

Should defined(_MSC_VER) be here?

When using the library from Electron on a Mac (or Linux) machine, this is still the correct thing to do.

[selfint]

failed ci example.

The step is marked as successful, but the tests do crash. Loading the backtrace with lldb on my local machine shows it's the same old missing_symbol error for ArrayBuffer:

(lldb) bt
* thread #1
  * frame #0: 0x0000000196c1d118 dyld`__abort_with_payload + 8
    frame #1: 0x0000000196c28d7c dyld`abort_with_payload_wrapper_internal + 104
    frame #2: 0x0000000196c28db0 dyld`abort_with_payload + 16
    frame #3: 0x0000000196bb48a8 dyld`dyld4::halt(char const*) + 328
    frame #4: 0x0000000196bea770 dyld`dyld4::APIs::_dyld_missing_symbol_abort() + 44
    frame #5: 0x0000000105574274 tree_sitter_runtime_binding.node`node_tree_sitter::InitConversions(exports=(val_ = 0x0000011c003c4038)) at conversions.cc:41:37 [opt]
    frame #6: 0x0000000105573f68 tree_sitter_runtime_binding.node`node_tree_sitter::InitAll(exports=(val_ = 0x0000011c003c4038)) at binding.cc:16:3 [opt]

[verhovsky]

I'm able to build successfully on my laptop (macOS 13.4.1, M1 macbook air).

~/code/third-party/vscode-tree-sitter$ yarn test
yarn run v1.22.19
$ yarn run rebuild:dev && yarn run compile && yarn run lint
$ electron-rebuild -v 22.0.0 --types prod,dev
✔ Rebuild Complete
$ tsc -p ./
$ eslint src --ext ts
$ mocha ./out/test/runTest.js


  0 passing (0ms)

Downloading VS Code 1.79.2 from https://update.code.visualstudio.com/1.79.2/darwin-arm64/stable
Downloading VS Code [==============================] 100%
Downloaded VS Code into /Users/space/code/third-party/vscode-tree-sitter/.vscode-test/vscode-darwin-arm64-1.79.2

Downloaded VS Code into /Users/space/code/third-party/vscode-tree-sitter/.vscode-test/vscode-darwin-arm64-1.79.2
[main 2023-06-25T10:39:28.707Z] update#ctor - updates are disabled by the environment
2023-06-25 11:39:29.391 Code Helper (Renderer)[45587:366530] CoreText note: Client requested name ".NewYork-Regular", it will get TimesNewRomanPSMT rather than the intended font. All system UI font access should be through proper APIs such as CTFontCreateUIFontForLanguage() or +[NSFont systemFontOfSize:].
2023-06-25 11:39:29.391 Code Helper (Renderer)[45587:366530] CoreText note: Set a breakpoint on CTFontLogSystemFontNameRequest to debug.
Started initializing default profile extensions in extensions installation folder. file:///Users/space/code/third-party/vscode-tree-sitter/.vscode-test/extensions
Completed initializing default profile extensions in extensions installation folder. file:///Users/space/code/third-party/vscode-tree-sitter/.vscode-test/extensions
Loading development extension at /Users/space/code/third-party/vscode-tree-sitter
[vscode.php]: One or more snippets from the extension 'php' very likely confuse snippet-variables and snippet-placeholders (see https://code.visualstudio.com/docs/editor/userdefinedsnippets#_snippet-syntax for more details)
Unexpected token '�', "�JuU�����"... is not valid JSON

  FileTree Test Suite
    ✔ Test Rust
    ✔ Test Typescript
  Installer Test Suite
    ✔ Parsers compatibility (35245ms)
  vendor/node-tree-sitter Test Suite
    ✔ Test Rust parser
    ✔ Test TSNode methods
  5 passing (35s)
[main 2023-06-25T10:40:06.175Z] Extension host with pid 45619 exited with code: 0, signal: unknown.
Exit code:   0
Done

✨  Done in 166.16s.

One thing that I ran into is that there's a submodule within a submodule in vscode-tree-sitter/vendor/node-tree-sitter/vendor/tree-sitter/ so I had to run

cd vendor/node-tree-sitter/
git submodule init
git submodule update

I don't know, you might be right. For context defined(_MSC_VER) comes from the original PR #95, which comes from this suggestion electron/electron#29893 (comment) and also the code I mentioned earlier

https://github.com/cocos/cocos-engine/pull/13394/files#diff-20bf293d344678d153d4509b5cf76d8472d032743bea1c0cc7348831b352b7e1R245

only does it on Windows

    #if CC_EDITOR && CC_PLATFORM == CC_PLATFORM_WINDOWS

I noticed that they also do

#if CC_EDITOR && CC_PLATFORM == CC_PLATFORM_WINDOWS
    #include <node_buffer.h>
#endif

maybe we need that too.

@kjvalencik or @dogeFu could you comment and help us out here?

[selfint]

I have a windows machine I can try to run this on today, will update.

Also, I wrote a simpler test repo, I copied all the tests from node-tree-sitter and ran them from vscode.

The Parser.setLogger.when the given callback throws an exception it logs the error to the console test fails, so I skip it for now, but otherwise they all pass. I think it fails because vscode does something funny with the console during testing (maybe?), I'll also try to fix it.

[selfint]

Just tested on Windows (using the test-node-tree-sitter repo, it doesn't work.

I tried once using the npm library (v0.20.4), the tests crashed with this backtrace (from lldb):

* thread #1, stop reason = Exception 0xc0000005 encountered at address 0x7ff76bd7328a
  * frame #0: 0x00007ff76bd7328a Code.exe`void node::FreeArrayBufferAllocator(class node::ArrayBufferAllocator *) + 20170
    frame #1: 0x00007ff770256ed8 Code.exe`void v8::internal::WebSnapshotDeserializer::UpdatePointersCallback(class v8::Isolate *, enum v8::GCType, enum v8::GCCallbackFlags, void *) + 984
    frame #2: 0x00007ff76c53230b Code.exe`class std::Cr::unique_ptr<class v8::BackingStore, struct std::Cr::default_delete<class v8::BackingStore>> v8::ArrayBuffer::NewBackingStore(void *, unsigned __int64, void (__cdecl *)(void *, unsigned __int64, void *), void *) + 171
    frame #3: 0x00007ff76d8459dd Code.exe`class v8::MaybeLocal<class v8::Object> node::Buffer::New(class v8::Isolate *, char *, unsigned __int64, void (__cdecl *)(char *, void *), void *) + 717
    frame #4: 0x00007ff76d84578d Code.exe`class v8::MaybeLocal<class v8::Object> node::Buffer::New(class v8::Isolate *, char *, unsigned __int64, void (__cdecl *)(char *, void *), void *) + 125
    frame #5: 0x00007ffab71eebe9 tree_sitter_runtime_binding.node
    frame #6: 0x00007ffab71ee7b4 tree_sitter_runtime_binding.node
    frame #7: 0x00007ff76d844396 Code.exe`struct node::node_module * node::binding::get_linked_module(char const *) + 4854

And again using selfint/node-tree-sitter, remove-msc-ver branch, also crashed but with a slightly different backtrace:

* thread #1, stop reason = Exception 0xc0000005 encountered at address 0x7ff76bd7328a
  * frame #0: 0x00007ff76bd7328a Code.exe`void node::FreeArrayBufferAllocator(class node::ArrayBufferAllocator *) + 20170
    frame #1: 0x00007ff770256ed8 Code.exe`void v8::internal::WebSnapshotDeserializer::UpdatePointersCallback(class v8::Isolate *, enum v8::GCType, enum v8::GCCallbackFlags, void *) + 984
    frame #2: 0x00007ff76c53230b Code.exe`class std::Cr::unique_ptr<class v8::BackingStore, struct std::Cr::default_delete<class v8::BackingStore>> v8::ArrayBuffer::NewBackingStore(void *, unsigned __int64, void (__cdecl *)(void *, unsigned __int64, void *), void *) + 171
    frame #3: 0x00007ff76d8459dd Code.exe`class v8::MaybeLocal<class v8::Object> node::Buffer::New(class v8::Isolate *, char *, unsigned __int64, void (__cdecl *)(char *, void *), void *) + 717
    frame #4: 0x00007ff76d84578d Code.exe`class v8::MaybeLocal<class v8::Object> node::Buffer::New(class v8::Isolate *, char *, unsigned __int64, void (__cdecl *)(char *, void *), void *) + 125

A bit of searching for 0xc0000005 errors shows it's probably a memory access violation error.

[selfint]

@verhovsky is switching from NaN to n-api (https://github.com/tree-sitter/node-tree-sitter/tree/napi) planned? Maybe then it'll "just work"?

[verhovsky]

There's no plan. If someone does it, it'll definitely get merged. I think #129 is almost finished, I didn't understand @MichaelBelousov's question about Language so idk how far away it is.

It seems though that even with NAPI there might still be an issue with this exact API for Electron 21+ electron/electron#35801 https://www.electronjs.org/blog/v8-memory-cage

[selfint]

The CI is able to prebuild node-tree-sitter for all platforms finally, but I don't know how to actually test if what it builds works. I don't have a Windows computer and for @selfint's test repo https://github.com/selfint/vscode-tree-sitter/ the CI fails to run yarn add -D windows-build-tools so it fails before it starts actually compiling or testing the code

Ci on windows now works (still crashes, but the build steps work).

New & simpler test repo: https://github.com/selfint/test-node-tree-sitter