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

Activation fails on newly released stable VSCode v1.95.0 with electron v32.2.1 (no binding found) #593

Closed
rotemdan opened this issue Oct 29, 2024 · 7 comments

Comments

@rotemdan
Copy link
Contributor

rotemdan commented Oct 29, 2024

Debug messages (I enabled SPELLRIGHT_DEBUG_OUTPUT in the code):

[Extension Host] [spellright] Found no bindings among these files:
workbench.desktop.main.js:sourcemap:613 [Extension Host] bin\spellchecker-win32-30.1.2-x64.node
workbench.desktop.main.js:sourcemap:613 [Extension Host] bin\spellchecker-win32-29.4.0-x64.node
workbench.desktop.main.js:sourcemap:613 [Extension Host] bin\spellchecker-win32-28.2.10-x64.node
workbench.desktop.main.js:sourcemap:613 [Extension Host] bin\spellchecker-macos-30.1.2-x64.node
workbench.desktop.main.js:sourcemap:613 [Extension Host] bin\spellchecker-macos-29.4.0-x64.node
workbench.desktop.main.js:sourcemap:613 [Extension Host] bin\spellchecker-macos-28.2.10-x64.node
workbench.desktop.main.js:sourcemap:613 [Extension Host] bin\spellchecker-linux-30.1.2-x64.node
workbench.desktop.main.js:sourcemap:613 [Extension Host] bin\spellchecker-linux-29.4.0-x64.node
workbench.desktop.main.js:sourcemap:613 [Extension Host] bin\spellchecker-linux-28.2.10-x64.node
workbench.desktop.main.js:sourcemap:627 Activating extension 'ban.spellright' failed: Cannot read properties of null (reading 'Spellchecker').

Looks like:

  nodeFiles.forEach((file) => {
    try {
      if (binding == null) {
        binding = require(path.join(__dirname, file));
        if (SPELLRIGHT_DEBUG_OUTPUT) {
          console.log('[spellright] Bindings: \"' + path.basename(file) + '\".');
        }
      }
    } catch (e) {
    }
  });

Goes and tries to require each addon in turn, and selects the last one that succeeds?

So no addon succeeds initialization now, is it because of the new Electron version?

VSCode info:

Version: 1.95.0 (user setup)
Commit: 912bb683695358a54ae0c670461738984cbb5b95
Date: 2024-10-28T20:16:24.561Z
Electron: 32.2.1
ElectronBuildId: 10427718
Chromium: 128.0.6613.186
Node.js: 20.18.0
V8: 12.8.374.38-electron.0
OS: Windows_NT x64 10.0.22631

Is there some automated process (continuous integration) that automates building the addons? Can you maybe give instructions on how to help with this? (doesn't seem like it can be easily automated locally, due to the different architectures? Especially since I'm using Windows, it's likely going to be hard to cross-build for macOS)

Anyway, thanks for making the best spell checking extension for VSCode! I'm really depending on it, that's why I'm trying to offer some help.

@rotemdan rotemdan changed the title Initialization fails on newly released stable VSCode v1.95.0 with electron v32.2.1 (no binding found) Activation fails on newly released stable VSCode v1.95.0 with electron v32.2.1 (no binding found) Oct 29, 2024
@rotemdan
Copy link
Contributor Author

rotemdan commented Oct 30, 2024

I managed to produce a working build of the addon for Windows x64 (spellchecker-win32-32.2.1-x64.node), but that was not easy and took several hour of work, and required multiple changes to the build script and some dependency updates to figure out all the changes needed to get node-gyp to compile it. (I asked for help from several LLMs: Mistral Large 2, Gemini 1.5 Pro and ChatGPT, but it was still hard - without that help it would likely be near-impossible)

Download the electron v32.2.1 compatible addon for Windows x64

spellchecker-win32-32.2.1-x64.zip

To fix the extension on Windows copy the .node file inside the zip to C:\Users\{USERNAME}\.vscode\extensions\ban.spellright-3.0.136\lib\bin (at least that's the path on my system).

How I got it to build

I tried to build with both Node v20.18.0 and v22.11.0. Both eventually worked.

I upgraded all dependencies to latest versions, using node-check-updates.

Here are the updates:

 @electron/rebuild    ^3.2.9  →   ^3.7.0
 @types/mocha        ^10.0.6  →  ^10.0.9
 @types/node        ^20.12.7  →  ^22.8.4
 electron           ^28.2.10  →  ^33.0.2
 eslint               ^9.0.0  →  ^9.13.0
 glob                 ^9.3.0  →  ^11.0.0
 ignore               ^5.3.1  →   ^6.0.2
 mocha               ^10.4.0  →  ^10.8.1
 nan                 ^2.19.0  →  ^2.22.0
 node-gyp            ^10.1.0  →  ^10.2.0
 opn                  ^5.1.0  →   ^6.0.0
 os-locale            ^5.0.0  →   ^6.0.2

The build was failing because it wanted C++20 to be enabled.

So I changed, in binding.gyp:

    'cflags_cc': ['-std=c++20'],

And that didn't help, so eventually ChatGPT suggested:

'OS=="win"', {
 ...
         "msvs_settings": {
            "VCCLCompilerTool": {
              "AdditionalOptions": [
                "/std:c++20"
              ]
            }
          },

Which helped.

Then I got a repeating cryptic error in various header files:

error C2589: '(': illegal token on right side of '::'

Which after lots of effort turns out to be related to this pull request in the Node tracker.

So it was solved by adding this (suggested by Gemini 1.5 Pro):

  'targets': [
    {
      'target_name': 'spellchecker',
      # ......

      'defines': [
        "NOMINMAX"
      ],	  

Then I got some compiler error about an unsafe type cast that was solved by modifying spellchecker_win.cc from:

std::wstring& wword = ToWString(word);

To:

const std::wstring& wword = ToWString(word);

(suggested by Mistral Large 2)

Then it worked.

Command line used, in lib\bin\node-spellchecker:

node-gyp clean
node-gyp configure
node-gyp rebuild --target=32.2.1 --arch=x64 --dist-url=https://electronjs.org/headers
copy build\Release\spellchecker.node ..\spellchecker-win32-32.2.1-x64.node

Ant that's just for getting the Windows build working.

I could maybe help to get the Linux build working, but not really for macOS (I have a working VMWare VM for macOS 13 but it's extremely slow and has no VSCode installed - might also have issue with the VM 8GB memory limit).

@bartosz-antosik
Copy link
Owner

Hi! Great many thanks for hints on compiling for Windows! Somehow it seems all the other platforms have built without problems, just after updating nan and node-gyp, but Windows did not! You are credited in CHANGELOG and really many thanks!

@rotemdan
Copy link
Contributor Author

I actually didn't even run npm install in lib\bin\node-spellchecker. I think what happened is that npm was resolving the packages from the root package.json. The reason I upgraded all packages is because I wanted the build to succeed as soon as possible.

Now that I successfully built in Linux by just updating the packages in lib\bin\node-spellchecker I realized it may have not been necessary.

I noticed the root package.json had:

    "electron": "^33.0.2",
    "@electron/rebuild": "^3.7.0",

I assumed they were essential for the build of the addons at the time. I'm not sure now.

I will try to redo the the build on Windows but with minimal version updates.

@bartosz-antosik
Copy link
Owner

I think I did minimal package update (as I have said, nan and node-gyp) and it worked.

@rotemdan
Copy link
Contributor Author

I tried again to recompile. Starting by resetting all the modifications I used earlier.

The minimum required changes were:

Updating versions of node-gyp and nan in lib\bin\node-spellchecker

Adding:

          'defines': [
            "NOMINMAX"
          ],	

In the binding.gyp file, for Windows:

        ['OS=="win"', {
          'sources': [
             'src/spellchecker_win.cc',
             'src/transcoder_win.cc',
          ],
          'defines': [
            "NOMINMAX"
          ],		  
        }],

And fixing the type error that's possibly caused by the C++20 compilation:

In lib\bin\node-spellchecker\src\spellchecker_win.cc, line 363, change to:

  const std::wstring& wword = ToWString(word);

I'm not sure why it didn't need to be explicitly told it's C++20, but I can see that the calls to CL.exe, made by node-gyp, do include:

-std:c++20 

Maybe something about the order I did the changes now made it do that automatically.

I compiled for x64 and ia32 this way and they both worked.

I see the new version you released does include an ia32 build for windows, so you compiled yourself. Did you need to do these changes to succeed?

@bartosz-antosik
Copy link
Owner

I did exactly as you say. I have added C++20. Will remove it later then. I assume node-gyp update does the thing! Thanks.

bartosz-antosik pushed a commit that referenced this issue Oct 31, 2024
@rotemdan
Copy link
Contributor Author

Thanks.

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

2 participants