win: fix delay-load hook for electron 4

[adill]

Checklist

• documentation is changed or added
• commit message follows commit guidelines

Description of change

Electron, starting with major version 4, ships a node.lib import library referencing electron.exe (node is statically linked) so it is no longer possible to freely rename electron.exe and have native modules work out of the box.

Is node-gyp interested in a change like this? or is this be something that the Electron team should work around themselves?

[refack]

Is node-gyp interested in a change like this? or is this be something that the Electron team should work around themselves?

@adill seems like a valid use case to me.

If you are up to the challenge, could you parameterize this? It could be something like <(node_core_target_name)<(EXECUTABLE_SUFFIX) in the .gyp files. Hence

  'defines': [ 'CORE_EXE_NAME=electron.exe', ],

and then the change is the .cc file would be:

  if (_stricmp(info->szDll, CORE_EXE_NAME) != 0)
    return NULL;

[adill]

@refack Is this what you had in mind? The idea being that electron's node fork will add node_host_binary: 'electron', to it's common.gypi?

[adill]

note: As implemented anyone still using iojs.exe would be broken by this change.

[refack]

@adill yes. Just need to verify two things:

1. the this works as expected

   node-gyp/addon.gypi

   Line 66 in 7a53da6

   'defines': [ 'HOST_BINARY=\"<(node_host_binary)<(EXECUTABLE_SUFFIX)\"', ],

   
   I'm need to verify GYP will expanad the variable in this case
2. That this in fact solves the binary-was-renamed issue

[adill]

I've tested locally and verified that it works as expected. (I used cld as my test native module.)

[richardlau]

note: As implemented anyone still using iojs.exe would be broken by this change.

I don't think that will be an issue if we release in node-gyp@4 (where we're dropping support for EOL versions of Node.js/io.js).

[refack]

I've tested locally and verified that it works as expected. (I used cld as my test native module.)

@adill Could you write a test for this? I'm thinking something like:

1. There's a sample addon in https://github.com/nodejs/node-gyp/tree/master/test/node_modules/hello_world.
2. Copy process.execPath to a temp directory, then call tmp1exe nodeGyp on it and rename the exe
   

   node-gyp/test/test-addon.js

   Line 8 in 2394a1e

   var nodeGyp = path.resolve(__dirname, '..', 'bin', 'node-gyp.js')

[adill]

@refack Added a test. Seems to work -- passes per usual, fails when I make the delay load hook return nullptr.

[khanhas]

Is this fixed released?
It is quite frustrating right now. Electron 4.0 beta 4 didn't "revert" anything. I have to rename my app to node.exe for it recognize native module.

[refack]

CI: https://ci.nodejs.org/job/nodegyp-test-pull-request/102/
CI after rebase: https://ci.nodejs.org/job/nodegyp-test-pull-request/103/

@khanhas, if this passes CI I'll land this, but a release is not ready yet. Hopefully this will be release soon, and quickly adopted by npm for an update.