Skip to content

Add new port for nodejs 26.6 - #53480

Closed
Budi Jap (budij) wants to merge 26 commits into
microsoft:masterfrom
budij:nodejs-vcpkg
Closed

Add new port for nodejs 26.6#53480
Budi Jap (budij) wants to merge 26 commits into
microsoft:masterfrom
budij:nodejs-vcpkg

Conversation

@budij

Copy link
Copy Markdown

Add new port: nodejs
This PR adds a new port for Node.js.
The port builds successfully on:

Windows x64 (tested locally)

macOS arm64 (tested locally)

CI will validate:

macOS x64

Linux x64

New Port Checklist
[x] Changes comply with the maintainer guide.

[x] Node.js is a mature project and meets vcpkg’s stability requirements.

[x] Port name matches upstream project name.

[x] Optional dependencies are controlled through the portfile and manifest.

[x] Versioning scheme matches upstream.

[x] License matches upstream.

[x] Copyright file matches upstream.

[x] Source code comes from authoritative upstream release.

[x] Usage text is correct (or auto‑generated usage is sufficient).

[x] Version database updated via vcpkg x-add-version.

[x] Exactly one version entry added.

Notes
This port uses vcpkg’s OpenSSL (if applicable).

All patches are minimal and required for portability.

No downloads occur during the build.

Architecture handling is triplet‑driven (no hardcoded arm64/x64).

@budij

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

Comment thread ports/vcpkg-tool-rust/vcpkg.json Outdated
@BillyONeal
Billy O'Neal (BillyONeal) marked this pull request as draft August 19, 2026 01:32
@budij
Budi Jap (budij) marked this pull request as ready for review August 19, 2026 03:21
@budij
Budi Jap (budij) marked this pull request as draft August 19, 2026 19:22
@BillyONeal

Copy link
Copy Markdown
Member

This appears to effectively conflict with many existing vcpkg ports like libuv, v8, etc. It does not immediately conflict because those dependencies have been moved to subdirectories, but they will still stomp on the same symbols at link time, and ultimately something like #include <uv.h> can only resolve to one place.

Additionally repeating what GPT 5.6 Sol says:

  • Windows ARM64 support is broken. The supported arm64-windows CI configuration fails for both core and feature builds. The OpenSSL path hard-codes libcrypto-3-x64.dll in portfile.cmake, and the separate core ARM64 build failure also requires resolution. Fix both failures or constrain supports accordingly.
  • The installed headers are not directly consumable. The split include/node, include/v8, and include/libuv layout in portfile.cmake prevents <node/node.h> from resolving v8.h when only the triplet root include directory is supplied; documented includes such as <node.h> also do not exist there. Install the public headers in a layout whose internal includes resolve from the standard include root.

Direct consumption is not an absolute requirement but we really want people to try because it's necessary for build systems that don't support bindings, like MSBuild.

  • The port-authored CMake exports use reserved official-looking names. Replace find_package(node CONFIG) and node::libnode with find_package(unofficial-node CONFIG) and an unofficial::node:: target, following the maintainer guidance. Remove the unnecessary nodejsConfig.cmake compatibility package.

Additionally it's not clear why we are adding a "deprecated" bit on day 0.

  • The exported target is incomplete outside Windows. nodeTargets.cmake.in defines runtime locations only under WIN32 while setting import-library properties on every platform. Define configuration-specific IMPORTED_LOCATION values for the installed .so and .dylib artifacts and validate a consumer on Linux and macOS.
  • The npm feature does not install npm. It only omits --without-npm in portfile.cmake; the installation excludes the Node executable, npm files, and launcher. Either install and validate a usable npm toolchain or remove the feature.
  • Dependency control and vendoring require redesign. OpenSSL is an unconditional dependency even though only the optional openssl feature enables --shared-openssl; move it to that feature or use it consistently in core. Replace bundled libraries with declared vcpkg dependencies where Node supports shared alternatives, explicitly justify unavoidable vendoring such as V8, and reassess the SPDX expression to cover the code incorporated into libnode.
  • The node.gyp modification must be a checked patch. The runtime string(REPLACE) logic in portfile.cmake can report success when its expected source text is absent. Move the modification into PATCHES so source-context drift fails deterministically, document why it is vcpkg-specific, and submit it upstream if generally applicable.

Comment thread ports/node/vcpkg.json Outdated
Comment thread ports/nodejs/link-windows-system-libraries.patch
@budij
Budi Jap (budij) marked this pull request as ready for review August 21, 2026 02:00
@BillyONeal

Copy link
Copy Markdown
Member

There needs to be some explanation of the impact on all the vendored components. Also repeating GPT again:

  • The library conflicts with separately packaged dependencies. The default build embeds V8, libuv, zlib, c-ares, nghttp2, brotli, simdjson, simdutf, SQLite, ada, and other libraries. Its Windows DLL publicly exports their symbols (for example, uv_run and thousands of v8:: symbols), so merely moving headers under include/nodejs does not satisfy the curated registry's simultaneous-install/symbol-ownership policy. Devendor every dependency for which Node offers a shared-system option and resolve unavoidable conflicts such as V8 before adding this port.
  • The CMake integration is unusable. unofficial-nodejs-targets.cmake.in advertises include/node, but the port installs include/nodejs; CMake rejects the imported target because that interface path does not exist. On non-Windows, portfile.cmake never assigns the .so/.dylib name to NODE_RUNTIME_LIB, so the generated imported locations have an empty filename. Correct both paths and validate Release and Debug consumers on each supported OS.
  • The custom usage is wholly out of sync with the installed package. usage documents find_package(node), node::libnode, and root-level <node.h>, <uv.h>, and <v8.h>, while the package supplies find_package(unofficial-nodejs), unofficial::nodejs::libnode, and headers beneath include/nodejs. It also claims an examples directory that is not installed. Replace it with brief, tested instructions.

I'm going to add that the last two points make me want to see a test port before merging this.

  • The openssl feature is not additive. Core builds bundled OpenSSL, while enabling the feature replaces it with vcpkg OpenSSL and additionally forces FIPS on Windows. This is a backend/build alternative, which the feature policy does not permit. Select one dependency model for the port; in the curated registry that should be the separately packaged dependency where upstream supports it.
  • The license metadata is incomplete. vcpkg.json declares only MIT, but the built library incorporates code covered by Apache-2.0, BSD-family, OpenSSL, ICU, and other terms recorded in upstream's composite LICENSE. Supply an accurate SPDX expression after devendoring, or use "license": null when no manageable expression represents the remaining content.
  • The Windows system-library patch has no upstream issue or pull request, and searches found no submission. Because it repairs upstream's supported --shared build rather than vcpkg-specific behavior, follow the upstream-notification policy.

@BillyONeal
Billy O'Neal (BillyONeal) marked this pull request as draft August 22, 2026 00:35
@budij

Copy link
Copy Markdown
Author

Hi, thanks for that review.

I'm going to go away for about a month overseas to visit clients, so I don't think I'll be able to fix the suggestions quickly. So, I think I'll close this pull request later.

But before I close it, I'd like to ask for the first bullet point:

  1. The node library actually builds all into the node.dll itself, including sqlite, openssl, libuv, etc. So, if I just build the default, all I get is just the one node.dll. The issue is that the header files. I do need node.h, v8.h, and uv.h, which may clash if someone installs v8 and libuv. I find --shared-libuv, and I can fix it. But I cannot find --shared-v8, and I won't be able to fix that. I'm not sure if that is a deal breaker or not. If it is a deal breaker, then I'll be happy to close the pull request, and keep it as my vcpkg-overlay only. I was thinking of just removing all dependency, and let it build as core, and create 1 node.dll that bundles everything as part of its build.

And, for the other bullet points, not questions, just comments:
2. I find that both portfile.cmake and the unofficial-nodejs-targets.cmake, both advertises include/node. So, that's the reason it works. I will change both to include/nodejs. Regarding the .dylib, and .so files, for the NODE_RUNTIME_LIB, yes, that is missed. I will fix that. I think the reason it works on my Mac installation, is that I point the RPATH to the folder, and it can search for it automatically. But I found why it's empty in the target.cmake, and I will fix that.
3. When I changed from node to nodejs, I forgot to update the usage file. I will update that.
4. The openssl feature, yes, it is not additive. What I had in mind was that I want an option to either use shared openssl, or to just bundle it with node.dll. For my original project, I was using shared openssl, and I realised that nodejs already links everything in the dll, and I thought if I can just use the openssl that I already built, that's be better. But, yes, it is not additive, so I can remove this "feature".
5. License metadata. I will look into that, and fix that.
6. I will update to follow the upstream notification policy.

But as I said before, I'll probably close this pull request, and create it again later when I'm back. But would you be able to answer the question 1 above? Since it's technically statically link everything into the dll, can I just remove all features, and leave it as compiling 1 node.dll. But again, even if I do that, I still need to work out what to do with the header files, so they don't clash. Or at the very least, include --shared-libuv, so at least, libuv is not going to clash.

@budij

Copy link
Copy Markdown
Author

Going away for a period of time. I won't have time to work on addressing the comments soon. I will close this for now.

@budij
Budi Jap (budij) deleted the nodejs-vcpkg branch August 23, 2026 19:28
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.

3 participants