-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[nodejs] crash when importing bcrypt #15048
Comments
@raedrizqie any thoughts? |
this may be a node-gyp error i think.. |
after patching node-gyp, i need to run:
in order to proceed compiling of dlopen module using gcc/clang instead of using msvs (default for windows)
maybe it still searching for installed v8, despite node is using v8 internally? |
@fauxpark found a solution here: https://stackoverflow.com/questions/71027495/i-received-this-code-error-each-time-i-tried-to-deploy-may-app-on-herokucode You can use bcryptjs instead..
then..
|
Yes, I did find bcryptjs which I'm sure works fine for a pure JS implementation, but it'd be preferable to have node-gyp functioning properly in MSYS2, for any other packages that need it. |
here is the errors i got when trying to build dlopen (or any C/C++ native module):
Maybe someone can figure out the solution to these error.. |
Seems like errno.h isn't getting picked up? Which would make sense, maybe some Does the same thing happen in MINGW64, or is the error output different? From what I can tell errno.h should be visible there. |
on mingw64 and ucrt64, it fails more or less the same:
|
@fauxpark Please download from the artifacts here: #15372 install bcrypt module:
bcrypt will download the prebuilt module and use it instead of rebuilding it locally.. it's not compatible with mingw, so we need to rebuild the module;
finally test the module:
|
Tested and working: CLANG64, MINGW64, UCRT64 CLANG32 and MINGW32 fail to rebuild:
Doesn't look like a node-gyp issue, and I'm not sure how much we care about 32-bit these days. Also, is there a way to have this automated on install? |
If we remove
I dont know how to automate a rebuild.. bcrypt shouldn't provide prebuilt module anyway.. unless they also support mingw built module |
Perhaps I should have said "fall back to building from source" instead. Bcrypt is a sufficiently popular package that I think being able to install it without the need for a compilation step on the major platforms is fair and warranted. MSYS2 is not one of those. From looking at the bcrypt package.json, this automation is already done by node-pre-gyp. The problem is that it sees we're running on Windows (since If it's possible to detect MINGW in node ( |
I agree..
The easiest way to rebuild the module while installing it is:
We don't have to further patch node-gyp this way.. |
The patching won't be in node-gyp, that seems to be fine now thanks to your work. I'm talking about node-pre-gyp: https://nodejs.github.io/node-addon-examples/build-tools/node-pre-gyp/#the-code-classlanguage-textscriptscode-properties |
yeah, i understand.. but by patching node-pre-gyp, we need to somehow provide a node-pre-gyp package, no? |
I was more thinking along the lines of opening an issue over there to see what they'd prefer to do, if anything. |
ah.. that's better I think.. |
Description / Steps to reproduce the issue
npm init
and install bcryptnode test.js
This is probably not bcrypt-specific, likely a general issue with how Node loads dynamic libraries.
Expected behavior
Hash should be printed.
Actual behavior
Node crashes with the following:
Verification
Windows Version
MINGW64_NT-10.0-19044
MINGW environments affected
Are you willing to submit a PR?
If I can figure out the root cause :)
The text was updated successfully, but these errors were encountered: