Skip to content
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

Closed
wants to merge 1 commit into from
Closed

Conversation

enzalito
Copy link
Contributor

@enzalito enzalito commented Jul 3, 2024

Needed by #4505 (#4505 (comment))

if opt.system then
import("lib.detect.find_path")

local headers_path = find_path("node.h", { "/usr/**", "/usr/local/**"})
Copy link
Member

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.

return package:find_package("system::iconv", {includes = "iconv.h"})

Copy link
Contributor Author

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

Copy link
Contributor Author

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 ?

Copy link
Contributor Author

@enzalito enzalito Jul 3, 2024

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"}

Copy link
Contributor Author

@enzalito enzalito Jul 3, 2024

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.

Copy link
Member

@waruqi waruqi Jul 4, 2024

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

return package:find_package("system::iconv", {includes = "iconv.h"})

Copy link
Contributor Author

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

Copy link
Member

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"})

Copy link
Contributor Author

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.

@enzalito enzalito closed this Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants