Broken on Windows since libuv 1.22.0 #113

Activity

1. richardlau commented on Jul 30, 2018

   

   richardlau

   on Jul 30, 2018

   Member

   More actions

   IIRC the due (and timeout fields used on non-Windows) are private fields that we ended up using as there wasn't a public way to read the timer information. Long term the correct thing to is probably look at libuv/libuv#1255 again -- In the meantime we should see if we can work around this in node-report.
2. mhdawson commented on Jul 30, 2018

   

   mhdawson

   on Jul 30, 2018

   Member

   More actions

   @richardlau are you investigating how to work around it?
3. richardlau commented on Jul 31, 2018

   

   richardlau

   on Jul 31, 2018

   Member

   More actions

   @mhdawson Yes. My fear is this is only fixable at compile time, which would mean any node-report binaries compiled against Node.js v10 prior to the libuv update (<v10.7.0) would be incompatible with Node.js v10 post libuv update (>v10.7.0) and vice versa.
4. richardlau commented on Jul 31, 2018

   

   richardlau

   on Jul 31, 2018

   Member

   More actions

   I'm also updating https://ci.nodejs.org/job/nodereport-continuous-integration/ to add v10 and update the nightly version (nodejs/build#1421).

   For reference, against Node.js v10, the reported failure on Windows looks like this:

   > node-gyp rebuild
   
   
   C:\workspace\nodereport-continuous-integration\MACHINE\win2012r2\node-report>if not defined npm_config_node_gyp (node "C:\workspace\nodereport-continuous-integration\MACHINE\win2012r2\node-v10.7.0-win-x64\node_modules\npm\node_modules\npm-lifecycle\node-gyp-bin\\..\..\node_modules\node-gyp\bin\node-gyp.js" rebuild )  else (node "C:\workspace\nodereport-continuous-integration\MACHINE\win2012r2\node-v10.7.0-win-x64\node_modules\npm\node_modules\node-gyp\bin\node-gyp.js" rebuild ) 
   Building the projects in this solution one at a time. To enable parallel build, please add the "/m" switch.
     node_report.cc
     module.cc
     utilities.cc
     win_delay_load_hook.cc
   C:\Program Files (x86)\Windows Kits\8.1\Include\um\dbghelp.h(1544): warning C4091: 'typedef ': ignored on left of '' when no variable is declared (compiling source file ..\src\node_report.cc) [C:\workspace\nodereport-continuous-integration\MACHINE\win2012r2\node-report\build\api.vcxproj]
   C:\Program Files (x86)\Windows Kits\8.1\Include\um\dbghelp.h(3190): warning C4091: 'typedef ': ignored on left of '' when no variable is declared (compiling source file ..\src\node_report.cc) [C:\workspace\nodereport-continuous-integration\MACHINE\win2012r2\node-report\build\api.vcxproj]
   ..\src\utilities.cc(421): error C2039: 'due': is not a member of 'uv_timer_s' [C:\workspace\nodereport-continuous-integration\MACHINE\win2012r2\node-report\build\api.vcxproj]
     C:\Users\Administrator\.node-gyp\10.7.0\include\node\uv.h(800): note: see declaration of 'uv_timer_s'
   gyp ERR! build error 
   

   https://ci.nodejs.org/job/nodereport-continuous-integration/215/MACHINE=win2012r2/console
5. richardlau

   self-assigned this

   on Jul 31, 2018
6. mhdawson commented on Aug 2, 2018

   

   mhdawson

   on Aug 2, 2018

   Member

   More actions

   @richardlau the potential incompatibility with the Node binary is another good reason why moving into core is a good idea. Please update if you figure out what the impact is going to be.
7. richardlau

   added a commit that references this issue on Dec 19, 2018

   Fix build on Windows with libuv 1.22 and later

   ...

   785fe0a
8. richardlau commented on Dec 19, 2018

   

   richardlau

   on Dec 19, 2018

   Member

   More actions

   In the end it turns out this is much simpler than anticipated since libuv/libuv@95c5bf8#diff-8eef231986656444fd461ec05249614cL533 preserved the layout of the structs. Fix in #118.
9. richardlau

   added a commit that references this issue on Jan 22, 2019

   Fix build on Windows with libuv 1.22 and later

   ...

   729e284
10. richardlau

    closed this as completedin a696eb0on Jan 22, 2019