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.

Sign up for GitHub

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

add nodejs package for linux #4554

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions packages/n/nodejs/xmake.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package("nodejs")

set_homepage("https://nodejs.org/")
set_description("Cross-platform JavaScript runtime environment.")

on_fetch("linux", function (package, opt)
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.

xmake-repo/packages/l/libiconv/xmake.lua

Line 28 in 1230151

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 •
edited
Loading

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 •
edited
Loading

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 •
edited
Loading

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

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.

if headers_path then
return {includedirs = {headers_path}}
end
end
end)
Loading