[node-api] Add new port
closes #26769
Writing C++ modules for NodeJS is a little bit messy at the moment of writing this. You have two options:
- gyp (outdated, required python)
- cmake-js, requires npm on host system, requires writing
cmake-jscommand instead ofcmake <source_dir>
My goal is to add node-api port to make possible to write native modules for NodeJS in every environment supported by vcpkg (in Visual Studio projects, CMake projects, etc)
Making a draft for now, I may return to this PR from time to time. I'd love to see your feedback and any tips.
Example project: https://github.com/Pospelove/napi-test/tree/main
- [x] node-api:x64-windows builds
- [x] node-api:x86-windows builds
- [x] should not pollute user filesystem
- [x] nodejs tool should have the same triplet as target has
- [x] support find_package
- [x] fix TARGET_SOURCES (add to package interface)
- [x] create test node addon, confirm that x86 and x64 work (at least for me, on Windows)
- [x] ensure linux and mac triplets are buildable
@JackBoosY @FrankXie05 could you please take a look?
CMake Error at ports/node-api/portfile.cmake:5 (message):
node not found! Please install it via your system package manager!
two way to fix this:
- Add new tool
nodetovcpkg_find_acquire_program - Add this tool installation to our pipeline script.
- Add new tool
nodetovcpkg_find_acquire_program
Which would be a world rebuild...let's not do that
- Add new tool
nodetovcpkg_find_acquire_programWhich would be a world rebuild...let's not do that
Can you please provide the official doc for install node?
- Add new tool
nodetovcpkg_find_acquire_program
Again, please no new tools in vcpkg_find_acquire_program.
Again, please no new tools in
vcpkg_find_acquire_program.
As @BillyONeal said, we plan to migrate all tools to vcpkg_find_acquire_program and use vcpkg instead of cmake to download and unpack them in the future.
Again, please no new tools in
vcpkg_find_acquire_program.As @BillyONeal said, we plan to migrate all tools to
vcpkg_find_acquire_programand use vcpkg instead of cmake to download and unpack them in the future.
I don't know where this is was said. But I think in order to do so, you must decouple the tool data from the downloads script. We cannot trigger even more world rebuilds from this script. The script simply doesn't scale in its current form (factors: number of tools, number of host platforms, tool versions),
I'll take a look at CI failures a little bit later. Looks fixable
@JackBoosY @FrankXie05 I think it's ready could you please take a look?
@JackBoosY Any progress on this?
Sorry for late, I will review this PR again tomorrow.
@BillyONeal Could you please take a look?
Hey @BillyONeal Could you please take a quick look?
Hey @JackBoosY
We will have some discussion internal this week.
Glad to hear. looking forward to it. Just gave this thread multiple pings to make sure that the RP isn't abandoned. Thank you very much!
and what about the pr lololol @Cheney-W
Thanks for your contribution, sorry it's been a bit of a runaround :)
2. cmake-js, requires npm on host system, requires writing
cmake-jscommand instead ofcmake <source_dir>
I mean, possibly, but given that this port appears to be a thin wrapper around npm, I'm not sure what the user experience is intended to be of this port? Can you show like a "before and after"? Because this seems just actively worse than invoking npm directly. (To be clear, I'm not saying that it is worse than invoking NPM directly, only that it isn't obvious to me reading this right now.) What is a vcpkg customer who got this install expected to do with the results?
Other concerns:
- This isn't going to work with artifact caching because the download is done inside npm's guts, and unlikely to play nicely with binary caching because npm tends to be full of symlink stuff.
- This is "installing" things into the node.js instance vcpkg provides sometimes for other ports to play with, but appears to be attempting to provide services for users of vcpkg, who can't depend on that node.js instance being durable.
===============
New port checklist:
- Review the code ❗
- Check the name against https://repology.org/ No match ⚠️
- Check the name against Bing/Google. (If the first results aren't C++ related, try "name C++") The first hit may be related, but I don't see how what's installed here is connected? ⚠️
- Check the source code for optional
find_packages ✅ - Check that the versioning scheme matches what upstream says This is installing [email protected] but is claiming to be 14.17.4 ⚠️
- Check that the license in vcpkg.json matches what upstream says ✅ MIT == https://github.com/cmake-js/cmake-js/blob/master/LICENSE
- Check that the license installed by the port matches what upstream says file(INSTALL "${NODEJS_DIR}/LICENSE" DESTINATION "${CURRENT_PACKAGES_DIR}/share/${PORT}" RENAME copyright) A node.js LICENSE has a lot of stuff that isn't MIT ❗
- Check that the source code comes from the upstream project's authoritative source. I don't even know how to evaluate this. It certainly isn't pointed at cmake-js's github repo...
- Check that the generated usage is accurate. (Didn't try yet)
- Check for issues that are resolved. Fixes https://github.com/microsoft/vcpkg/issues/26769 .
Hey @BillyONeal and thank you for your review
I mean, possibly, but given that this port appears to be a thin wrapper around npm, I'm not sure what the user experience is intended to be of this port? Can you show like a "before and after"? Because this seems just actively worse than invoking npm directly. (To be clear, I'm not saying that it is worse than invoking NPM directly, only that it isn't obvious to me reading this right now.) What is a vcpkg customer who got this install expected to do with the results?
I left a comment at the top of portfile.cmake. I think it can be useful in the future.
# NodeJS has native modules API called node-api.
# This port uses cmake-js to download and build node-api for you.
# NPM is required to run cmake-js.
# Nor NPM output nor cmake-js output is left after portfile.cmake execution like npm packages or any other files.
# ${DOWNLOADS} and ${NODEJS_BIN_DIR} folders are used in process, but they are cleaned at the end.
# As a normal port, this port leaves some includes and libraries in ${CURRENT_PACKAGES_DIR}.
# So it doesn't break binary caching or any other vcpkg features
following the checklist:
- review the code, ok, it's up to you
- Check the name against https://repology.org/: Not sure about naming, didn't want to name it
nodeornodejssince it's too common. Maybe worth tryingnode-n-api. There is also C++ Embedder API of NodeJS which is out of scope of this port. So. I'd accept any name you suggest - Check the name against Bing/Google. For both the first result is https://nodejs.org/api/n-api.html which is totally expected.
- Check the source code for optional find_packages Yeah it works
- Check that the versioning scheme matches what upstream says. This is installing [email protected] but is claiming to be 14.17.4. We're installing node-api, not cmake-js. So it's the version of nodejs. cmake-js is just a helper tool. It gets wiped at the end of portfile.cmake
- Check that the license in vcpkg.json matches what upstream says. I see. removing MIT for now, please let me know what do I type in vcpkg.json license field
- Check that the source code comes from the upstream project's authoritative source. npm would throw "checksum failed while installing cmake-js" in case of checksums mismatch. btw cmake-js verifies file checksums too: https://github.com/cmake-js/cmake-js/blob/afdaaf203376303abd4dbd78379c0883e6faa6c2/lib/downloader.js#L89
- Check that the generated usage is accurate. It is, there is example project, but yeah feel free to check
I'm sorry I'm still confused. Can you show what, for example, the shortest possible vcpkg.json + CMakeLists.txt + .cpp file that would consume this port would look like?
If it's too much for a comment you can put it on gist.github.com.
Here's an example of what I mean for zlib:
C:\Dev\test>type CMakeLists.txt
cmake_minimum_required(VERSION 3.24)
project(demo)
find_package(ZLIB REQUIRED)
add_executable(test test.cpp)
target_link_libraries(test PRIVATE ZLIB::ZLIB)
C:\Dev\test>type test.cpp
#include <stdio.h>
#include <zlib.h>
int main() {
puts(ZLIB_VERSION);
}
C:\Dev\test>type vcpkg.json
{
"dependencies": ["zlib"]
}
C:\Dev\test>cmake -G Ninja -DCMAKE_BUILD_TYPE=Debug -DCMAKE_TOOLCHAIN_FILE=C:\Dev\vcpkg2\scripts\buildsystems\vcpkg.cmake -S . -B out
-- Running vcpkg install
Detecting compiler hash for triplet x64-windows...
The following packages will be built and installed:
* vcpkg-cmake[core]:x64-windows -> 2022-09-26
zlib[core]:x64-windows -> 1.2.12#2
Additional packages (*) will be modified to complete this operation.
Restored 2 package(s) from C:\Users\bion\AppData\Local\vcpkg\archives in 50.87 ms. Use --debug to see more details.
Installing 1/2 vcpkg-cmake:x64-windows...
Elapsed time to handle vcpkg-cmake:x64-windows: 9.379 ms
Installing 2/2 zlib:x64-windows...
Elapsed time to handle zlib:x64-windows: 17.61 ms
Total elapsed time: 2.153 s
The package zlib is compatible with built-in CMake targets:
find_package(ZLIB REQUIRED)
target_link_libraries(main PRIVATE ZLIB::ZLIB)
-- Running vcpkg install - done
-- The C compiler identification is MSVC 19.34.31931.0
-- The CXX compiler identification is MSVC 19.34.31931.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/Program Files/Microsoft Visual Studio/2022/Preview/VC/Tools/MSVC/14.34.31931/bin/Hostx64/x64/cl.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: C:/Program Files/Microsoft Visual Studio/2022/Preview/VC/Tools/MSVC/14.34.31931/bin/Hostx64/x64/cl.exe - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found ZLIB: optimized;C:/Dev/test/out/vcpkg_installed/x64-windows/lib/zlib.lib;debug;C:/Dev/test/out/vcpkg_installed/x64-windows/debug/lib/zlibd.lib (found version "1.2.12")
-- Configuring done
-- Generating done
-- Build files have been written to: C:/Dev/test/out
C:\Dev\test>ninja -C out
ninja: Entering directory `out'
[2/2] Linking CXX executable test.exe
C:\Dev\test>.\out\test.exe
1.2.12
C:\Dev\test>
Hey @BillyONeal I think it works now, no npm stuff at all
@dan-shaw heyy let's go merge
@vicroms