Skip to content

win,src: fix GetModuleHandle for libnode.dll #3139

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

Merged
merged 1 commit into from
Apr 1, 2025

Conversation

StefanStojanovic
Copy link
Contributor

Based on the description in the linked issue, this should fix the problem. There is no strong reason for making a change like this (and for example not using GetModuleHandleA or L"libnode.dll"). This is the only GetModuleHandle call in the entire project, so if someone thinks other approaches are better, let me know and I'll change it.

Fixes: #3126
Refs: #2834

@StefanStojanovic
Copy link
Contributor Author

I've tried few different variations of a fix, and even running the action without any changes. Windows test jobs always to fail. Because of that, I do not think failures are caused by my changes.

@lukekarrys
Copy link
Member

I think the failing Windows tests in this PR were fixed by #3145. I think this can land.

@lukekarrys lukekarrys merged commit 3e1cdd4 into nodejs:main Apr 1, 2025
31 of 46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build failure on Windows in [email protected] due to GetModuleHandle LPCWSTR conversion issue
2 participants