-
-
Notifications
You must be signed in to change notification settings - Fork 398
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
add nodejs package for linux #4554
Conversation
if opt.system then | ||
import("lib.detect.find_path") | ||
|
||
local headers_path = find_path("node.h", { "/usr/**", "/usr/local/**"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not return /usr/include
path, it will break for gcc.
please use find_package.
xmake-repo/packages/l/libiconv/xmake.lua
Line 28 in 1230151
return package:find_package("system::iconv", {includes = "iconv.h"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok no problem, can you explain why ? it seems to work fine for me with gcc-13
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh is it because the linux headers have file named node.h
too ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i was only able to find the package with package:find_package("apt::nodejs")
(i'm on ubuntu). system
and pkgconfig
did not work.
the returned table is empty though, even if i pass {includes = "node_api.h"}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm using local headers_path = find_path("node_api.h", { "/usr/**", "/usr/local/**"})
in my local package for now and everything seems to be working fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it will pass -isystem /usr/include
or -isystem /usr/local/include
to gcc, and it will break gcc.
This is a known issue with gcc. If node_api.h
is in /usr/include and /usr/local/include, gcc will find it even if you don't add -isystem /usr/include
.
xmake-io/xmake#4596 (comment)
msys2/MINGW-packages#10761
pcb2gcode/pcb2gcode#587
The package:find_package("system::xx")
automatically handles this and avoids adding unnecessary -isystem /usr/include
. like this
xmake-repo/packages/l/libiconv/xmake.lua
Line 28 in 1230151
return package:find_package("system::iconv", {includes = "iconv.h"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok thank you for the explanation.
The headers are located in /usr/include/node/
so that's why it is working for me.
I still not sure why package:find_package("system:nodejs")
doesn't find anything but I dug a little more and according to nodejs/node-addon-api#882, there is no way to find the path to the headers that come packaged with nodejs
in a platform agnostic way.
However, this issue indirectly links to https://github.com/nodejs/node-api-headers which seems to contain exactly what I need for node-addon-api
. So I think I should probably create a package from this repo instead.
I'll wait for your feedback before creating yet another PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I think I should probably create a package from this repo instead.node-addon-api
you can try.
The headers are located in so that's why it is working for me./usr/include/node/
I still not sure why doesn't find anything but I dug a little more and according to nodejs/node-addon-api#882, there is no way to find the path to the headers that come packaged with in a platform agnostic way.package:find_package("system:nodejs")nodejs
try
package:find_package("system:nodejs", {includes = "node/node_api.h"})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try
package:find_package("system:nodejs", {includes = "node/node_api.h"})
Still nothing :/
I have made a node-api-headers
package that works perfectly for me. It as the advantage of being way lighter as it doesn't contain the whole Node runtime + ecosystem but just the headers that I need for my c++ addon. I'll make a PR right now.
Needed by #4505 (#4505 (comment))