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

[node-api] Add new port #26768

Merged
merged 112 commits into from
Jan 4, 2023
Merged

[node-api] Add new port #26768

merged 112 commits into from
Jan 4, 2023

Conversation

Pospelove
Copy link
Contributor

@Pospelove Pospelove commented Sep 12, 2022

closes #26769

Writing C++ modules for NodeJS is a little bit messy at the moment of writing this. You have two options:

  1. gyp (outdated, required python)
  2. cmake-js, requires npm on host system, requires writing cmake-js command instead of cmake <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

  • node-api:x64-windows builds
  • node-api:x86-windows builds
  • should not pollute user filesystem
  • nodejs tool should have the same triplet as target has
  • support find_package
  • fix TARGET_SOURCES (add to package interface)
  • create test node addon, confirm that x86 and x64 work (at least for me, on Windows)
  • ensure linux and mac triplets are buildable

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

All manifest files must be formatted

./vcpkg format-manifest ports/*/vcpkg.json

Diff
diff --git a/ports/node-api/vcpkg.json b/ports/node-api/vcpkg.json
index b31d0e3..f17c056 100644
--- a/ports/node-api/vcpkg.json
+++ b/ports/node-api/vcpkg.json
@@ -1,22 +1,22 @@
-{
-  "name": "node-api",
-  "version-semver": "14.17.4",
-  "description": "NodeJS API for writing modules in C++",
-  "homepage": "https://nodejs.org/api/addons.html",
-  "supports": "windows | linux | osx",
-  "license": "MIT",
-  "dependencies": [
-    {
-      "name": "vcpkg-tool-nodejs",
-      "host": true
-    },
-    {
-      "name": "vcpkg-cmake",
-      "host": true
-    },
-    {
-      "name": "vcpkg-cmake-config",
-      "host": true
-    }
-  ]
-}
\ No newline at end of file
+{
+  "name": "node-api",
+  "version-semver": "14.17.4",
+  "description": "NodeJS API for writing modules in C++",
+  "homepage": "https://nodejs.org/api/addons.html",
+  "license": "MIT",
+  "supports": "windows | linux | osx",
+  "dependencies": [
+    {
+      "name": "vcpkg-cmake",
+      "host": true
+    },
+    {
+      "name": "vcpkg-cmake-config",
+      "host": true
+    },
+    {
+      "name": "vcpkg-tool-nodejs",
+      "host": true
+    }
+  ]
+}
After committing all other changes, the version database must be updated
git add -u && git commit
git checkout 23905f6f86ab3e210ac84e4d8c49dd7923fa25be -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index ea4dfd8..7d0e31c 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -5068,6 +5068,10 @@
       "baseline": "2021-02-21",
       "port-version": 3
     },
+    "node-api": {
+      "baseline": "14.17.4",
+      "port-version": 0
+    },
     "nonius": {
       "baseline": "2019-04-20",
       "port-version": 2

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

All manifest files must be formatted

./vcpkg format-manifest ports/*/vcpkg.json

Diff
diff --git a/ports/node-api/vcpkg.json b/ports/node-api/vcpkg.json
index 9700aad..f17c056 100644
--- a/ports/node-api/vcpkg.json
+++ b/ports/node-api/vcpkg.json
@@ -1,22 +1,22 @@
-{
-  "name": "node-api",
-  "version-semver": "14.17.4",
-  "description": "NodeJS API for writing modules in C++",
-  "homepage": "https://nodejs.org/api/addons.html",
-  "supports": "windows | linux | osx",
-  "license": "MIT",
-  "dependencies": [
-    {
-      "name": "vcpkg-tool-nodejs",
-      "host": true
-    },
-    {
-      "name": "vcpkg-cmake",
-      "host": true
-    },
-    {
-      "name": "vcpkg-cmake-config",
-      "host": true
-    }
-  ]
-}
+{
+  "name": "node-api",
+  "version-semver": "14.17.4",
+  "description": "NodeJS API for writing modules in C++",
+  "homepage": "https://nodejs.org/api/addons.html",
+  "license": "MIT",
+  "supports": "windows | linux | osx",
+  "dependencies": [
+    {
+      "name": "vcpkg-cmake",
+      "host": true
+    },
+    {
+      "name": "vcpkg-cmake-config",
+      "host": true
+    },
+    {
+      "name": "vcpkg-tool-nodejs",
+      "host": true
+    }
+  ]
+}
After committing all other changes, the version database must be updated
git add -u && git commit
git checkout 23905f6f86ab3e210ac84e4d8c49dd7923fa25be -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index ea4dfd8..7d0e31c 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -5068,6 +5068,10 @@
       "baseline": "2021-02-21",
       "port-version": 3
     },
+    "node-api": {
+      "baseline": "14.17.4",
+      "port-version": 0
+    },
     "nonius": {
       "baseline": "2019-04-20",
       "port-version": 2

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated
git add -u && git commit
git checkout 23905f6f86ab3e210ac84e4d8c49dd7923fa25be -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index ea4dfd8..7d0e31c 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -5068,6 +5068,10 @@
       "baseline": "2021-02-21",
       "port-version": 3
     },
+    "node-api": {
+      "baseline": "14.17.4",
+      "port-version": 0
+    },
     "nonius": {
       "baseline": "2019-04-20",
       "port-version": 2

github-actions[bot]
github-actions bot previously approved these changes Sep 12, 2022
@FrankXie05 FrankXie05 added category:new-port The issue is requesting a new library to be added; consider making a PR! category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly labels Sep 13, 2022
@JackBoosY JackBoosY self-assigned this Sep 13, 2022
github-actions[bot]
github-actions bot previously approved these changes Sep 13, 2022
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

error: checked-in files for node-api have changed but the version was not updated
version: 14.17.4
old SHA: c7f7248a9cbc46b1328a30a3b06a3ed19ea4ad04
new SHA: 37a510df295a6495ef0925d8c0c9a022bf0c2ccb
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

All manifest files must be formatted

./vcpkg format-manifest ports/*/vcpkg.json

Diff
diff --git a/ports/node-api/vcpkg.json b/ports/node-api/vcpkg.json
index 5ad5fad..f2b89f2 100644
--- a/ports/node-api/vcpkg.json
+++ b/ports/node-api/vcpkg.json
@@ -6,7 +6,6 @@
   "license": "MIT",
   "supports": "windows | linux | osx",
   "dependencies": [
-    "vcpkg-tool-nodejs",
     {
       "name": "vcpkg-cmake",
       "host": true
@@ -14,6 +13,7 @@
     {
       "name": "vcpkg-cmake-config",
       "host": true
-    }
+    },
+    "vcpkg-tool-nodejs"
   ]
-}
\ No newline at end of file
+}
PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

error: checked-in files for vcpkg-tool-nodejs have changed but the version was not updated
version: 14.17.4
old SHA: 91407db5ac964382618c11765caafc444e684a19
new SHA: 55c9b4d0a8e82801876a9dd14be36bef2319aaac
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***
error: checked-in files for node-api have changed but the version was not updated
version: 14.17.4
old SHA: c7f7248a9cbc46b1328a30a3b06a3ed19ea4ad04
new SHA: fce11127665883039401b8fdb7733e4372b25d2b
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

You have modified or added at least one vcpkg.json where you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/vcpkg-tool-nodejs/vcpkg.json

Valid values for the license field can be found in the documentation

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

All manifest files must be formatted

./vcpkg format-manifest ports/*/vcpkg.json

Diff
diff --git a/ports/node-api/vcpkg.json b/ports/node-api/vcpkg.json
index 5ad5fad..f2b89f2 100644
--- a/ports/node-api/vcpkg.json
+++ b/ports/node-api/vcpkg.json
@@ -6,7 +6,6 @@
   "license": "MIT",
   "supports": "windows | linux | osx",
   "dependencies": [
-    "vcpkg-tool-nodejs",
     {
       "name": "vcpkg-cmake",
       "host": true
@@ -14,6 +13,7 @@
     {
       "name": "vcpkg-cmake-config",
       "host": true
-    }
+    },
+    "vcpkg-tool-nodejs"
   ]
-}
\ No newline at end of file
+}
PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

error: checked-in files for vcpkg-tool-nodejs have changed but the version was not updated
version: 14.17.4
old SHA: 91407db5ac964382618c11765caafc444e684a19
new SHA: 55c9b4d0a8e82801876a9dd14be36bef2319aaac
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***
error: checked-in files for node-api have changed but the version was not updated
version: 14.17.4
old SHA: c7f7248a9cbc46b1328a30a3b06a3ed19ea4ad04
new SHA: fddf2a0684148e2aafacb4e46e271ada933b103a
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

You have modified or added at least one vcpkg.json where you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/vcpkg-tool-nodejs/vcpkg.json

Valid values for the license field can be found in the documentation

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

All manifest files must be formatted

./vcpkg format-manifest ports/*/vcpkg.json

Diff
diff --git a/ports/node-api/vcpkg.json b/ports/node-api/vcpkg.json
index 5ad5fad..f2b89f2 100644
--- a/ports/node-api/vcpkg.json
+++ b/ports/node-api/vcpkg.json
@@ -6,7 +6,6 @@
   "license": "MIT",
   "supports": "windows | linux | osx",
   "dependencies": [
-    "vcpkg-tool-nodejs",
     {
       "name": "vcpkg-cmake",
       "host": true
@@ -14,6 +13,7 @@
     {
       "name": "vcpkg-cmake-config",
       "host": true
-    }
+    },
+    "vcpkg-tool-nodejs"
   ]
-}
\ No newline at end of file
+}
PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

error: checked-in files for vcpkg-tool-nodejs have changed but the version was not updated
version: 14.17.4
old SHA: 91407db5ac964382618c11765caafc444e684a19
new SHA: 55c9b4d0a8e82801876a9dd14be36bef2319aaac
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***
error: checked-in files for node-api have changed but the version was not updated
version: 14.17.4
old SHA: c7f7248a9cbc46b1328a30a3b06a3ed19ea4ad04
new SHA: 6a3b634a8228bb848513d8bb5f065bbb7ed3a577
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

You have modified or added at least one vcpkg.json where you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/vcpkg-tool-nodejs/vcpkg.json

Valid values for the license field can be found in the documentation

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

All manifest files must be formatted

./vcpkg format-manifest ports/*/vcpkg.json

Diff
diff --git a/ports/node-api/vcpkg.json b/ports/node-api/vcpkg.json
index 5ad5fad..f2b89f2 100644
--- a/ports/node-api/vcpkg.json
+++ b/ports/node-api/vcpkg.json
@@ -6,7 +6,6 @@
   "license": "MIT",
   "supports": "windows | linux | osx",
   "dependencies": [
-    "vcpkg-tool-nodejs",
     {
       "name": "vcpkg-cmake",
       "host": true
@@ -14,6 +13,7 @@
     {
       "name": "vcpkg-cmake-config",
       "host": true
-    }
+    },
+    "vcpkg-tool-nodejs"
   ]
-}
\ No newline at end of file
+}
PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

error: checked-in files for vcpkg-tool-nodejs have changed but the version was not updated
version: 14.17.4
old SHA: 91407db5ac964382618c11765caafc444e684a19
new SHA: 55c9b4d0a8e82801876a9dd14be36bef2319aaac
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***
error: checked-in files for node-api have changed but the version was not updated
version: 14.17.4
old SHA: c7f7248a9cbc46b1328a30a3b06a3ed19ea4ad04
new SHA: 70830f93924bd13566361ac411d6b9ff1e7e9859
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

You have modified or added at least one vcpkg.json where you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/vcpkg-tool-nodejs/vcpkg.json

Valid values for the license field can be found in the documentation

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

All manifest files must be formatted

./vcpkg format-manifest ports/*/vcpkg.json

Diff
diff --git a/ports/node-api/vcpkg.json b/ports/node-api/vcpkg.json
index 5ad5fad..f2b89f2 100644
--- a/ports/node-api/vcpkg.json
+++ b/ports/node-api/vcpkg.json
@@ -6,7 +6,6 @@
   "license": "MIT",
   "supports": "windows | linux | osx",
   "dependencies": [
-    "vcpkg-tool-nodejs",
     {
       "name": "vcpkg-cmake",
       "host": true
@@ -14,6 +13,7 @@
     {
       "name": "vcpkg-cmake-config",
       "host": true
-    }
+    },
+    "vcpkg-tool-nodejs"
   ]
-}
\ No newline at end of file
+}
diff --git a/ports/vcpkg-tool-nodejs/vcpkg.json b/ports/vcpkg-tool-nodejs/vcpkg.json
index f636622..7caddbb 100644
--- a/ports/vcpkg-tool-nodejs/vcpkg.json
+++ b/ports/vcpkg-tool-nodejs/vcpkg.json
@@ -1,6 +1,6 @@
 {
   "name": "vcpkg-tool-nodejs",
   "version-semver": "14.17.4",
-  "supports": "windows | linux | osx",
-  "license": "MIT"
+  "license": "MIT",
+  "supports": "windows | linux | osx"
 }
PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

error: checked-in files for vcpkg-tool-nodejs have changed but the version was not updated
version: 14.17.4
old SHA: 91407db5ac964382618c11765caafc444e684a19
new SHA: 90450320c67a75430c6051e5a156c0d83dab7e8b
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***
error: checked-in files for node-api have changed but the version was not updated
version: 14.17.4
old SHA: c7f7248a9cbc46b1328a30a3b06a3ed19ea4ad04
new SHA: 70830f93924bd13566361ac411d6b9ff1e7e9859
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

github-actions[bot]
github-actions bot previously approved these changes Dec 15, 2022
github-actions[bot]
github-actions bot previously approved these changes Dec 15, 2022
@Pospelove Pospelove requested review from BillyONeal and removed request for JackBoosY December 15, 2022 03:09
@Pospelove
Copy link
Contributor Author

Hey @BillyONeal
I think it works now, no npm stuff at all

github-actions[bot]
github-actions bot previously approved these changes Dec 15, 2022
Pospelove added a commit to Pospelove/skymp-1 that referenced this pull request Dec 15, 2022
@Cheney-W Cheney-W added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Dec 16, 2022
@Pospelove
Copy link
Contributor Author

@dan-shaw heyy let's go merge

@vicroms vicroms removed the info:reviewed Pull Request changes follow basic guidelines label Dec 28, 2022
@Pospelove
Copy link
Contributor Author

@vicroms

@vicroms vicroms merged commit fbc96bc into microsoft:master Jan 4, 2023
@Pospelove Pospelove deleted the node branch January 4, 2023 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[New Port Request] node-api
9 participants