Skip to content

Conversation

moraxh
Copy link

@moraxh moraxh commented Mar 13, 2025

This pull request introduces several modifications to the nodejs-whisper project to configure the shelljs library to utilize the Node.js executable path. The key changes include updates across multiple files to set shell.config.execPath to the Node.js executable path.

Configuration of shelljs:

  • src/autoDownloadModel.ts: Added configuration to set shell.config.execPath to the Node.js executable path.
  • src/downloadModel.ts: Added configuration to set shell.config.execPath to the Node.js executable path.
  • src/utils.ts: Added configuration to set shell.config.execPath to the Node.js executable path.
  • src/whisper.ts: Added configuration to set shell.config.execPath to the Node.js executable path.

Update to the WHISPER_CPP_MAIN_PATH constant:

  • src/constants.ts: Updated the WHISPER_CPP_MAIN_PATH for the Windows platform by removing the Release directory from the path.

Additional Reference:

For more information on shelljs compatibility with Electron and configuring the execPath, please refer to the ShellJS Electron compatibility wiki

@moraxh moraxh changed the title fix: WHISPER_CPP_MAIN_PATH constant in win32 platform fix: WHISPER_CPP_MAIN_PATH constant in win32 platform & define execPath when using electron Mar 13, 2025
@nickdebaise
Copy link

+1 Needed this for electron

Also needed a fix for models directory, which you can see at my fork:

https://github.com/nickdebaise/nodejs-whisper.git

@ChetanXpro
Copy link
Owner

Thanks @moraxh! the Electron shelljs configuration is great and needed. however, the Windows path change conflicts with our recent CMake improvements.

could you please keep the shell.config.execPath changes

and revert the WHISPER_CPP_MAIN_PATH change

@ChetanXpro
Copy link
Owner

Thanks @nickdebaise that path resolution fix look useful for Electron/webpack scenarios. I'll create a separate PR to address this properly.

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

Successfully merging this pull request may close these issues.

3 participants