Skip to content

Commit

Permalink
Port to NAPI (microsoft#644)
Browse files Browse the repository at this point in the history
* Port to NAPI

The "5th pty bug" in microsoft#432 fixed also.

* Fix help message in pty.cc

* Move NAPI deps to devDependencies in package.json

* Apply most of deepak1556's suggestions

* Fix winpty

* Fix conpty missing CloseHandle

* Use unique_ptr to avoid `goto`s

* Why macos failed?

* fix: ci and minor cleanups

* fix build failed on windows

---------

Co-authored-by: deepak1556 <hop2deep@gmail.com>
  • Loading branch information
kkocdko and deepak1556 authored Jan 26, 2024
1 parent d6ce76a commit b1fdda4
Show file tree
Hide file tree
Showing 10 changed files with 543 additions and 651 deletions.
7 changes: 5 additions & 2 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ jobs:
inputs:
versionSpec: $(node_version)
displayName: 'Install Node.js'
- script: |
python3 -m pip install setuptools
displayName: Install setuptools (macOS)
- script: |
npm i
displayName: 'Install dependencies and build'
Expand All @@ -54,7 +57,7 @@ jobs:
- job: Windows
pool:
vmImage: 'windows-2019'
vmImage: 'windows-latest'
strategy:
matrix:
node_16_x:
Expand Down Expand Up @@ -87,7 +90,7 @@ jobs:
steps:
- task: NodeTool@0
inputs:
versionSpec: '16.x'
versionSpec: '18.x'
displayName: 'Install Node.js'
- script: |
npm i
Expand Down
19 changes: 4 additions & 15 deletions binding.gyp
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
{
'target_defaults': {
'dependencies': [
"<!(node -p \"require('node-addon-api').targets\"):node_addon_api_except",
],
'conditions': [
['OS=="win"', {
'msvs_configuration_attributes': {
Expand Down Expand Up @@ -28,9 +31,6 @@
'targets': [
{
'target_name': 'conpty',
'include_dirs' : [
'<!(node -e "require(\'nan\')")'
],
'sources' : [
'src/win/conpty.cc',
'src/win/path_util.cc'
Expand All @@ -41,17 +41,14 @@
},
{
'target_name': 'conpty_console_list',
'include_dirs' : [
'<!(node -e "require(\'nan\')")'
],
'sources' : [
'src/win/conpty_console_list.cc'
],
},
{
'target_name': 'pty',
'include_dirs' : [
'<!(node -e "require(\'nan\')")',
'<!(node -p "require(\'node-addon-api\').include_dir")',
'deps/winpty/src/include',
],
# Disabled due to winpty
Expand All @@ -73,9 +70,6 @@
'targets': [
{
'target_name': 'pty',
'include_dirs' : [
'<!(node -e "require(\'nan\')")'
],
'sources': [
'src/unix/pty.cc',
],
Expand All @@ -91,11 +85,6 @@
'libraries!': [
'-lutil'
]
}],
['OS=="mac"', {
"xcode_settings": {
"MACOSX_DEPLOYMENT_TARGET":"10.7"
}
}]
]
}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
"prepublishOnly": "npm run build"
},
"dependencies": {
"nan": "^2.17.0"
"node-addon-api": "^7.1.0"
},
"devDependencies": {
"@types/mocha": "^7.0.2",
Expand Down
Loading

0 comments on commit b1fdda4

Please sign in to comment.