-
Notifications
You must be signed in to change notification settings - Fork 114
Fix GitHub Actions CI failing on Windows runners (prebuild failing to build binaries) #165
Fix GitHub Actions CI failing on Windows runners (prebuild failing to build binaries) #165
Conversation
…tudio is utilized Part of #164
}, | ||
"devDependencies": { | ||
"chai": "^4.1.2", | ||
"chalk": "^2.4.1", | ||
"eslint": "^4.19.1", | ||
"jasmine": "^3.1.0", | ||
"node-abi": "^3.1.0", | ||
"node-gyp": "^8.2.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it trying to use an old [email protected]
in the actual build when we install [email protected]
for the project?
Must be what's installed with the Node.js version, C:\\hostedtoolcache\\windows\\node\\14.19.0\\x64\\node_modules\\npm\\node_modules\\node-gyp\\bin\\node-gyp.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like node-gyp
is might be connected to the npm
version,
- https://github.com/nodejs/node-gyp/blob/245cd5bbe4441d4f05e88f2fa20a86425419b6af/docs/Updating-npm-bundled-node-gyp.md
- https://github.com/nodejs/node-gyp/blob/245cd5bbe4441d4f05e88f2fa20a86425419b6af/docs/Force-npm-to-use-global-node-gyp.md
But the instructions to update npm
are custom to each OS and npm
version 😫
Default npm
versions:
- Node.js v12 ->
[email protected]
- Node.js v14 ->
[email protected]
- Node.js v16 ->
[email protected]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on those documents, I tried many different ways to update node-gyp
and while it works for most platforms, I couldn't get it to work on Windows with npm@6
. It always used [email protected]
no matter what (expected [email protected]
).
# Make sure the build is using the latest version of node-gyp (which also
# matches our package.json) to ensure maximum build compatibility and
# avoid random build errors.
#
# See
# - https://github.com/nodejs/node-gyp/blob/245cd5bbe4441d4f05e88f2fa20a86425419b6af/docs/Updating-npm-bundled-node-gyp.md
# - https://github.com/nodejs/node-gyp/blob/245cd5bbe4441d4f05e88f2fa20a86425419b6af/docs/Force-npm-to-use-global-node-gyp.md
#
# For macOS/Linux and npm <= 6
- name: 'Install the latest version of node-gyp@^8'
if: ${{ matrix.os != 'Windows' && (matrix.node == '12' || matrix.node == '14') }}
run: npm explore npm/node_modules/npm-lifecycle -g -- npm install node-gyp@^8
# For macOS/Linux and npm >= 7
- name: 'Install the latest version of node-gyp@^8'
if: ${{ matrix.os != 'Windows' && (matrix.node == '16') }}
run: npm explore npm/node_modules/@npmcli/run-script -g -- npm_config_global=false npm install node-gyp@^8
# For Windows and npm <= 6
- name: 'Install the latest version of node-gyp@^8'
if: ${{ matrix.os == 'Windows' && (matrix.node == '12' || matrix.node == '14') }}
run: |
"@FOR /f \"tokens=*\" %i IN ('where node') DO cd/d \"%~dpi\""
cd node_modules\\npm\\node_modules\\npm-lifecycle
npm install node-gyp@^8
npm install --global node-gyp@^8
for /f "delims=" %P in ('npm prefix -g') do npm config set node_gyp "%P\node_modules\node-gyp\bin\node-gyp.js"
shell: cmd
# For Windows and npm >= 7
- name: 'Install the latest version of node-gyp@^8'
if: ${{ matrix.os == 'Windows' && (matrix.node == '16') }}
run: |
"@FOR /f \"tokens=*\" %i IN ('where node') DO cd/d \"%~dpi\""
cd node_modules\\npm\\node_modules\\@npmcli\\run-script
npm install node-gyp@^8
shell: cmd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I resorted to just install npm@^8
for all versions which has a newer bundled node-gyp
. Still unclear how that's different from above though 🤷♀️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like someone saw this PR and solved it a different way: https://github.com/snowtrack/snowfs/pull/258
.github/workflows/ci.yml
Outdated
- name: 'Install the latest version of node-gyp' | ||
if: ${{ matrix.os == 'windows' && (matrix.node == '16') }} | ||
run: \@FOR /f "tokens=*" %i IN ('where node') DO cd/d "%~dpi" && cd node_modules\npm\node_modules\@npmcli\run-script && npm install node-gyp@latest | ||
shell: cmd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe could use powershell although prefer not I think. Here is the start:
cd (Split-Path((get-command node).Path))
"prebuild": "^11.0.0" | ||
"node-abi": "^3.8.0", | ||
"node-gyp": "^8.4.0", | ||
"prebuild": "^11.0.3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we try to Prebuild binaries for all ABIs
, the following error occurs:
https://github.com/MadLittleMods/node-usb-detection/runs/5400425223?check_suite_focus=true
/tmp/prebuild/node/17.0.0/include/node/v8-persistent-handle.h:10:10: fatal error: v8-weak-callback-info.h: No such file or directory
This is tracked on the prebuild
side in prebuild/prebuild#284 and was fixed in Node.js v17.0.1, see nodejs/node#40526
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can follow serialport/node-serialport#2356 and drop v17 support for now ⏩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Followed-up in #165 (comment)
Side-effect of not many prebuilds because only building for 3 versions instead of all of the in between versions 😢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might be able to try the alternative workaround in prebuild/prebuild#284 (comment) to get all of the versions supported 😮
.github/workflows/ci.yml
Outdated
# See https://github.com/nodejs/node-gyp/tree/245cd5bbe4441d4f05e88f2fa20a86425419b6af/docs#issues-finding-the-installed-visual-studio | ||
- name: 'Set msvs_version so node-gyp can find the Visual Studio install' | ||
if: ${{ matrix.os == 'Windows' }} | ||
run: npm config set msvs_version 2022 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't fix the Error: Could not find any Visual Studio installation to use
, see https://github.com/MadLittleMods/node-usb-detection/runs/5400636691
But our npm run node-gyp -- configure --verbose
command does seem to find it 🤔:
gyp verb find VS - looking for Visual Studio version 2022
gyp verb find VS VCINSTALLDIR not set, not running in VS Command Prompt
gyp verb find VS checking VS2022 (17.1.32210.238) found at:
gyp verb find VS "C:\Program Files\Microsoft Visual Studio\2022\Enterprise"
gyp verb find VS - found "Visual Studio C++ core features"
gyp verb find VS - found VC++ toolset: v1[43](https://github.com/MadLittleMods/node-usb-detection/runs/5400636691?check_suite_focus=true#step:11:43)
gyp verb find VS - found Windows SDK: 10.0.20348.0
gyp info find VS using VS2022 (17.1.32210.238) found at:
gyp info find VS "C:\Program Files\Microsoft Visual Studio\2022\Enterprise"
Full output:
$ npm run node-gyp -- configure --verbose
> usb-detection@[4](https://github.com/MadLittleMods/node-usb-detection/runs/5400636691?check_suite_focus=true#step:11:4).13.0 node-gyp
> node-gyp "configure" "--verbose"
gyp info it worked if it ends with ok
gyp verb cli [
gyp verb cli 'C:\\hostedtoolcache\\windows\\node\\14.19.0\\x[6](https://github.com/MadLittleMods/node-usb-detection/runs/5400636691?check_suite_focus=true#step:11:6)4\\node.exe',
gyp verb cli 'D:\\a\\node-usb-detection\\node-usb-detection\\node_modules\\node-gyp\\bin\\node-gyp.js',
gyp verb cli 'configure',
gyp verb cli '--verbose'
gyp verb cli ]
gyp info using [email protected]
gyp info using [email protected] | win32 | x64
gyp verb command configure []
gyp verb find Python Python is not set from command line or npm configuration
gyp verb find Python Python is not set from environment variable PYTHON
gyp verb find Python checking if "python3" can be used
gyp verb find Python - executing "python3" to get executable path
gyp verb find Python - executable path is "C:\hostedtoolcache\windows\Python\3.9.10\x64\python3.exe"
gyp verb find Python - executing "C:\hostedtoolcache\windows\Python\3.9.10\x64\python3.exe" to get version
gyp verb find Python - version is "3.9.10"
gyp info find Python using Python version 3.9.10 found at "C:\hostedtoolcache\windows\Python\3.9.10\x64\python3.exe"
gyp verb get node dir no --target version specified, falling back to host node version: 14.19.0
gyp verb command install [ '14.19.0' ]
gyp verb install input version string "14.19.0"
gyp verb install installing version: 14.19.0
gyp verb install --ensure was passed, so won't reinstall if already installed
gyp verb install version is already installed, need to check "installVersion"
gyp verb got "installVersion" 9
gyp verb needs "installVersion" 9
gyp verb install version is good
gyp verb get node dir target node version installed: 14.19.0
gyp verb build dir attempting to create "build" dir: D:\a\node-usb-detection\node-usb-detection\build
gyp verb build dir "build" dir needed to be created? No
gyp verb find VS msvs_version was set from command line or npm config
gyp verb find VS - looking for Visual Studio version 2022
gyp verb find VS VCINSTALLDIR not set, not running in VS Command Prompt
gyp verb find VS checking VS2022 (1[7](https://github.com/MadLittleMods/node-usb-detection/runs/5400636691?check_suite_focus=true#step:11:7).1.32210.23[8](https://github.com/MadLittleMods/node-usb-detection/runs/5400636691?check_suite_focus=true#step:11:8)) found at:
gyp verb find VS "C:\Program Files\Microsoft Visual Studio\2022\Enterprise"
gyp verb find VS - found "Visual Studio C++ core features"
gyp verb find VS - found VC++ toolset: v143
gyp verb find VS - found Windows SDK: 10.0.20348.0
gyp info find VS using VS2022 (17.1.32210.238) found at:
gyp info find VS "C:\Program Files\Microsoft Visual Studio\2022\Enterprise"
gyp info find VS run with --verbose for detailed information
gyp verb build/config.gypi creating config file
gyp verb build/config.gypi writing out config file: D:\a\node-usb-detection\node-usb-detection\build\config.gypi
gyp verb config.gypi checking for gypi file: D:\a\node-usb-detection\node-usb-detection\config.gypi
gyp verb common.gypi checking for gypi file: D:\a\node-usb-detection\node-usb-detection\common.gypi
gyp verb gyp gyp format was not specified; forcing "msvs"
gyp info spawn C:\hostedtoolcache\windows\Python\3.[9](https://github.com/MadLittleMods/node-usb-detection/runs/5400636691?check_suite_focus=true#step:11:9).[10](https://github.com/MadLittleMods/node-usb-detection/runs/5400636691?check_suite_focus=true#step:11:10)\x64\python3.exe
gyp info spawn args [
gyp info spawn args 'D:\\a\\node-usb-detection\\node-usb-detection\\node_modules\\node-gyp\\gyp\\gyp_main.py',
gyp info spawn args 'binding.gyp',
gyp info spawn args '-f',
gyp info spawn args 'msvs',
gyp info spawn args '-I',
gyp info spawn args 'D:\\a\\node-usb-detection\\node-usb-detection\\build\\config.gypi',
gyp info spawn args '-I',
gyp info spawn args 'D:\\a\\node-usb-detection\\node-usb-detection\\node_modules\\node-gyp\\addon.gypi',
gyp info spawn args '-I',
gyp info spawn args 'C:\\Users\\runneradmin\\AppData\\Local\\node-gyp\\Cache\\[14](https://github.com/MadLittleMods/node-usb-detection/runs/5400636691?check_suite_focus=true#step:11:14).[19](https://github.com/MadLittleMods/node-usb-detection/runs/5400636691?check_suite_focus=true#step:11:19).0\\include\\node\\common.gypi',
gyp info spawn args '-Dlibrary=shared_library',
gyp info spawn args '-Dvisibility=default',
gyp info spawn args '-Dnode_root_dir=C:\\Users\\runneradmin\\AppData\\Local\\node-gyp\\Cache\\14.19.0',
gyp info spawn args '-Dnode_gyp_dir=D:\\a\\node-usb-detection\\node-usb-detection\\node_modules\\node-gyp',
gyp info spawn args '-Dnode_lib_file=C:\\\\Users\\\\runneradmin\\\\AppData\\\\Local\\\\node-gyp\\\\Cache\\\\14.19.0\\\\<(target_arch)\\\\node.lib',
gyp info spawn args '-Dmodule_root_dir=D:\\a\\node-usb-detection\\node-usb-detection',
gyp info spawn args '-Dnode_engine=v8',
gyp info spawn args '--depth=.',
gyp info spawn args '--no-parallel',
gyp info spawn args '--generator-output',
gyp info spawn args 'D:\\a\\node-usb-detection\\node-usb-detection\\build',
gyp info spawn args '-Goutput_dir=.'
gyp info spawn args ]
gyp info ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like someone else is having trouble using prebuild
in GitHub Actions:
When you will update dependencies?
It NOT works with visual studio 2022 on github actions.-- Update dependencies: node-gyp stills using v6 it is currently in v8, prebuild/prebuild#286 (comment)
And we just have to wait for prebuild/prebuild#286 to be solved first
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resorted to just reverting back to the windows-2019
runners which work ✅
"prebuild": "prebuild --all --strip --verbose", | ||
"prebuild": "npm run prebuild-node && npm run prebuild-electron", | ||
"prebuild-node": "prebuild --force --strip --verbose -t 12.0.0 -t 14.0.0 -t 16.0.0", | ||
"prebuild-electron": "prebuild --force --strip --verbose -r electron -t 13.0.0 -t 14.0.0 -t 15.0.0 -t 16.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following up from #165 (comment) where we skipped Node.js 17.0.0 builds (fix from serialport/node-serialport#2356), this results in not many prebuilds. 26 vs >100 becaues it's only building for 12.0.0
14.0.0
and 16.0.0
directly and not the in between versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix GitHub Actions CI failing on Windows runners:
Fix #164
Before
windows-latest
->windows-2022
:After
windows-2019
✅Dev notes
Default
npm
versions:[email protected]
->[email protected]
[email protected]
->[email protected]
[email protected]
->[email protected]