Skip to content

allow node-gyp to use open source toolchains on windows #3166

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Kreijstal
Copy link

@Kreijstal Kreijstal commented Jun 3, 2025

Checklist
  • npm install && npm run lint && npm test passes
  • tests are included
  • documentation is changed or added
  • commit message follows commit guidelines
Description of change

It is a bit too biased towards propietary toolchains, this will diversify the supported toolchains of node-gyp
fixes #3120
fixes #3079
fixes #1240
fixes #194

It is a bit too biased towards propietary toolchains, this will diversify the supported toolchains of node-gyp
@Kreijstal
Copy link
Author

ok ruff lint is failing but all other tests pass

@cclauss
Copy link
Contributor

cclauss commented Jun 3, 2025

This might significantly increase our support issues.

These changes should probably land first on https://github.com/nodejs/gyp-next

Please install https://docs.astral.sh/ruff locally because those errors cannot be ignored.

ruff should pass
@Kreijstal
Copy link
Author

Kreijstal commented Jun 3, 2025

@cclauss
Copy link
Contributor

cclauss commented Jun 3, 2025

I mean that it will open more https://github.com/nodejs/node-gyp/issues for a volunteer group of maintainers to triage.

@Kreijstal
Copy link
Author

I mean that it will open more https://github.com/nodejs/node-gyp/issues for a volunteer group of maintainers to triage.

because more people will use node-gyp since it will fix something that was broken? That might be true. Is that the reason why it was not fixed?
But it will also close at least 2 issues, and will prevent the same issue about not allowing msys2 from coming up on node-gyp. About gyp-next, unfortunately it is way too different for this patchset to work. And more repos depend on node-gyp and not gyp-next.

I will try submitting a patch for gyp-next but it wont be anytime soon, since it is basically a rewrite.

@cclauss cclauss added the Windows label Jun 3, 2025
@Kreijstal
Copy link
Author

also with this, you can work with a fork of https://github.com/Kreijstal/node-ffi-napi/tree/master ffi-napi.. since of course it is msvc biased..

@cclauss cclauss requested a review from StefanStojanovic June 3, 2025 12:07
@Kreijstal
Copy link
Author

currently it is the only option on msys2, since broooklyn will never update napi-sys. And koffi.rs does not work in msys2 either

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants