-
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
node rollup does not work #20407
Comments
Alternatively, it seems webpack has no trouble with it. |
it is not about npm |
going through the dependency trail now attempting to make it work with napi-rs. napi-rs/napi-rs#2001
So, it builds fine, it gives you a hello world, but not sure what is exactly wrong here. |
because it donwloads the prebuilt msvc-based rollup modules
we need to build |
yeah, but for that we need to fix upstream dependencies, like napi-rs |
just created this small check to check if nodejs can't be loaded but it can..
returns
|
I managed to reduce the bug surface a bit in case anyone is good with rust
The problem is libloading, it doesn't seem to want to load the functions but I'm not 100% sure either how Libloading is supposed to work, in case anyone is curious about how this magic works, Bindgen prelude exposes a dll function |
I am puzzled trying to understand how the rust version works, it attempts to cast
this works
apparently this does runtime linking but not in the way I wanted, this still works, but does not help me understand why they try to call GetProcAddress |
so I recreated the toy example using GetProcAddress, and in mingw it still works, what I do not see is how they are loading the dll. #include <assert.h>
#include <node/node_api.h>
#include <windows.h>
#include <stdio.h>
HMODULE nodeModule = NULL;
typedef napi_status (*napi_create_string_utf8_func)(napi_env env,
const char* str,
size_t length,
napi_value* result);
typedef napi_status (*napi_define_properties_func)(napi_env env,
napi_value object,
size_t property_count,
const napi_property_descriptor* properties);
napi_create_string_utf8_func napi_create_string_utf8dyn =NULL;
napi_define_properties_func napi_define_propertiesdyn =NULL;
bool LoadNodeFunctions() {
nodeModule = LoadLibraryA("libnode.dll");
if (!nodeModule) {
printf("Failed to load Node.js DLL\n");
return false;
}
napi_create_string_utf8dyn = (napi_create_string_utf8_func)GetProcAddress(nodeModule, "napi_create_string_utf8");
napi_define_propertiesdyn = (napi_define_properties_func)GetProcAddress(nodeModule, "napi_define_properties");
if (!napi_create_string_utf8 || !napi_define_properties) {
printf("Failed to get function addresses\n");
FreeLibrary(nodeModule);
return false;
}
return true;
}
napi_value Method(napi_env env, napi_callback_info info) {
napi_status status;
napi_value world;
status = napi_create_string_utf8dyn(env, "world", NAPI_AUTO_LENGTH, &world);
assert(status == napi_ok);
return world;
}
#define DECLARE_NAPI_METHOD(name, func) \
{ name, 0, func, 0, 0, 0, napi_default, 0 }
napi_value napi_register_module_v1(napi_env env, napi_value exports) {
if (!nodeModule) {
if (!LoadNodeFunctions()) {
return exports;
}
}
napi_status status;
napi_property_descriptor desc = DECLARE_NAPI_METHOD("hello", Method);
status = napi_define_propertiesdyn(env, exports, 1, &desc);
assert(status == napi_ok);
return exports;
} This will compile without linking to libnode.dll explizitly, but it links on runtime, (I imagine they do that so it works on many nodejs versions) |
someone might have a similar issue here nodejs/node-gyp#2834 |
and I manage to reproduce the mistake of napi-rs #include <assert.h>
#include <node/node_api.h>
#include <windows.h>
#include <stdio.h>
#include <tlhelp32.h>
#include <psapi.h>
void printMyProcess() {
// Get the current process ID
DWORD processId = GetCurrentProcessId();
// Get a handle to the current process
HANDLE hProcess = OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, processId);
if (hProcess == NULL) {
printf("Failed to open process\n");
return;
}
// Get the process name
char processName[MAX_PATH];
if (!GetModuleFileNameEx(hProcess, NULL, processName, MAX_PATH)) {
printf("Failed to get process name\n");
CloseHandle(hProcess);
return;
}
// Print the process name
printf("Current process name: %s\n", processName);
// Close the handle to the process
CloseHandle(hProcess);
}
typedef enum {
GET_MODULE_HANDLE_EX_W_UNKNOWN,
GET_MODULE_HANDLE_EX_W_ERROR
} GetModuleHandleExWError;
GetModuleHandleExWError GetModuleHandleExWErrorFromLastError() {
DWORD lastError = GetLastError();
if (lastError != 0) {
return GET_MODULE_HANDLE_EX_W_ERROR;
}
return GET_MODULE_HANDLE_EX_W_UNKNOWN;
}
HMODULE GetCurrentModuleHandle() {
HMODULE handle = NULL;
BOOL result = GetModuleHandleExW(0,NULL,
&handle
);
if (result == 0) {
GetModuleHandleExWError error = GetModuleHandleExWErrorFromLastError();
if (error == GET_MODULE_HANDLE_EX_W_ERROR) {
printf("GetModuleHandleExW error: %lu\n", GetLastError());
} else {
printf("Unknown GetModuleHandleExW error\n");
}
return NULL;
}
return handle;
}
HMODULE nodeModule = NULL;
typedef napi_status (*napi_create_string_utf8_func)(napi_env env,
const char* str,
size_t length,
napi_value* result);
typedef napi_status (*napi_define_properties_func)(napi_env env,
napi_value object,
size_t property_count,
const napi_property_descriptor* properties);
napi_create_string_utf8_func napi_create_string_utf8dyn =NULL;
napi_define_properties_func napi_define_propertiesdyn =NULL;
bool LoadNodeFunctions() {
#ifdef LOAD_FROM_PROCESS
nodeModule = GetCurrentModuleHandle();
#else
nodeModule = LoadLibraryA("libnode.dll");
#endif
if (!nodeModule) {
printf("Failed to load Node.js DLL\n");
return false;
}
napi_create_string_utf8dyn = (napi_create_string_utf8_func)GetProcAddress(nodeModule, "napi_create_string_utf8");
napi_define_propertiesdyn = (napi_define_properties_func)GetProcAddress(nodeModule, "napi_define_properties");
if (!napi_create_string_utf8dyn || !napi_define_propertiesdyn) {
printf("Failed to get function addresses\n");
#ifndef LOAD_FROM_PROCESS
FreeLibrary(nodeModule);
#endif
return false;
}
return true;
}
napi_value Method(napi_env env, napi_callback_info info) {
napi_status status;
napi_value world;
status = napi_create_string_utf8dyn(env, "world", NAPI_AUTO_LENGTH, &world);
assert(status == napi_ok);
return world;
}
#define DECLARE_NAPI_METHOD(name, func) \
{ name, 0, func, 0, 0, 0, napi_default, 0 }
napi_value napi_register_module_v1(napi_env env, napi_value exports) {
printMyProcess();
if (!nodeModule) {
if (!LoadNodeFunctions()) {
return exports;
}
}
napi_status status;
napi_property_descriptor desc = DECLARE_NAPI_METHOD("hello", Method);
status = napi_define_propertiesdyn(env, exports, 1, &desc);
assert(status == napi_ok);
return exports;
} compile as |
Just noticed that other people solve this problem by just statically linking https://users.rust-lang.org/t/why-is-msvc-the-default-toolchain/44287/7 I guess we can do that, it is much faster as well.. |
patching upstream napi-rs/napi-rs#2026 |
napi-rs has finally been merged, but that's half the battle, now you need to submit pull requests to all napi-rs dependents to use the new napi-rs version |
that is still not enough.. every napi-rs based package must update their support for windows gnu variant.. else, the msvc-based binary would get downloaded and used.. |
Since you are submitting pull request to all depends then you just add the option of using the gnu napi... No? |
napi now works for us, but not for other packages.. npm just check for windows and bitness.. thats all. thats why we still can build those packages, but with msvc binary.. which will fail at runtime.. these only affect binary packages. pure nodejs packages are not affected.. its the same story as python pip.. |
same error . any solution ?? |
well, the new version of napi-rs should support gnu, but you need to update the dependencies so that this is the case. |
Description / Steps to reproduce the issue
require node rollup
use
rollup -c
will give you an esoteric error, but it is clear that it was made with msvc in mind, we can see this, because when wenpm install
rollup
we get: https://www.npmjs.com/package/@rollup/rollup-win32-x64-msvc ,Not sure if either npm would have to be patched, or rollup would be just patched and installed as a msys2 package.. (If that can be figured out).
Anyway, also reporting this for future viewers. And people that happen to come across this tauri-apps/tauri#6685
Expected behavior
gives a meaningful error or it works
Actual behavior
Verification
Windows Version
MINGW64_NT-10.0-22631 ayylmai 3.4.10.x86_64 2024-02-10 08:39 UTC x86_64 Msys
MINGW environments affected
Are you willing to submit a PR?
if I figure out a solution
The text was updated successfully, but these errors were encountered: