RobotWebTools / rclnodejs

Node.js version of ROS 2.0 client
https://docs.ros.org/en/jazzy/Concepts/Basic/About-Client-Libraries.html#community-maintained
Apache License 2.0
335 stars 72 forks source link

appveyor builds occasionally fail for node 16 & 17 #828

Closed wayneparrott closed 2 years ago

wayneparrott commented 2 years ago

appveyor builds occasionally fail for node 16 & 17 I've been successfully running the appveyor build matrix over the past 36 hrs and now after merging a key PR appveyor builds are failing for node 16 & 17. A common error follows:

SET PATH=%PYTHON2%;%PYTHON2%\bin;%PYTHON2%\Scripts;%PATH%
node --version
v16.13.0
npm --version
8.1.0
python --version
Python 2.7.18
npm install
npm ERR! code 1
npm ERR! path C:\proj\rclnodejs\node_modules\@rclnodejs\ref-napi
npm ERR! command failed
npm ERR! command C:\Windows\system32\cmd.exe /d /s /c node-gyp rebuild
npm ERR! gyp:
C:\Users\appveyor\AppData\Local\node-gyp\Cache\16.13.0\common.gypi not
found (cwd: C:\proj\rclnodejs\node_modules\@rclnodejs\ref-napi) while
reading includes of binding.gyp while trying to load binding.gyp
npm ERR! gyp ERR! configure error
npm ERR! gyp ERR! stack Error: `gyp` failed with exit code: 1
npm ERR! gyp ERR! stack     at ChildProcess.onCpExit (C:\Program
Files\nodejs\node_modules\npm\node_modules\node-gyp\lib\configure.js:353:16)
npm ERR! gyp ERR! stack     at ChildProcess.emit (node:events:390:28)
npm ERR! gyp ERR! stack     at Process.ChildProcess._handle.onexit
(node:internal/child_process:290:12)
npm ERR! gyp ERR! System Windows_NT 10.0.17763
npm ERR! gyp ERR! command "C:\\Program Files\\nodejs\\node.exe"
"C:\\Program Files\\nodejs\\node_modules\\npm\\node_modules\\node-gyp\\bin\\node-gyp.js"
"rebuild"
npm ERR! gyp ERR! cwd C:\proj\rclnodejs\node_modules\@rclnodejs\ref-napi
npm ERR! gyp ERR! node -v v16.13.0
npm ERR! gyp ERR! node-gyp -v v8.2.0
npm ERR! gyp ERR! not ok
npm ERR! A complete log of this run can be found in:
...

appveyor.yml - reversed the order of node versions to be in decending order, e.g., 17, 16, ... Doing this will speed up identifying build failures and debug-fix iterations.

coveralls commented 2 years ago

Coverage Status

Coverage remained the same at 90.471% when pulling ed0e20fbd371240dbd6646dd70d659306bcf676e on wayneparrott:develop into a18f204f143a34228d7894b5bb1ec6ed58d645b7 on RobotWebTools:develop.

wayneparrott commented 2 years ago

This PR should be against the galactic branch where the build errors occur. Closing here. Will reopen against the galactic branch.

minggangw commented 2 years ago

As nodejs v17 is not a LTS version yet, maybe we can drop this configuration for Windows platform (because it's not stable and appveyor fails sometimes)? image

wayneparrott commented 2 years ago

Even though window builds upto node16 are succeeding on appveyor I've identified an error. While testing on my window11 machine with nvm and node16 I encounter a repeatable error when running node-gyp rebuild:

C:\dev\test\rclnodejs\node_modules\nan\nan.h(57,10): fatal error C1083: Cannot open include file: 'uv.h': No such file
 or directory [C:\dev\tmp\x\rclnodejs\build\rclnodejs.vcxproj]

As an experiment, I uninstalled nvm and installed node 16 directly. This resulted in successful build. We need to identify why uv.h is not on the MS build include path when using nvm node 16.

wayneparrott commented 2 years ago

update: Note-0: After removing nvm, installing node, rclnodejs builds and installs cleanly. Then uninstalling nodejs, installing nvm, installing node16 no longer produces the error of the missing uv.h header. At this time rclnode#develop builds cleanly on node 12, 14 & 16 on my windows 11 dev machine.

Note-1 While continuing to try and isolate the node-gyp/nvm/node16+ issue I observed a similar issue where when installing dependencies int64-napi failed to local a system header file as it ran node-gyp. I looked up the path to the include file and it was present leading me to wondering if there are race conditions arising where a compile process is running ahead of a download process? Just thinking out loud.

Need to get a dev machine back in the failing state and analyze why uv.h is not being located.

wayneparrott commented 2 years ago

Based on @minggangw feedback I chose to remove node17 from the release scope. See commit 29e33f0

minggangw commented 2 years ago

I think switching different versions of nodejs may have some problems on appveyor, considering you can build successfully on your local dev, we can move forward to push the new tag and release v0.21.0, this is really a big step 🎉