Conversation
Every npm version bump requires a few patches to be floated on node-gyp for io.js compatibility. These patches are found in 03d1992, 5de334c, and da730c7. This commit squashes them into a single commit. PR-URL: nodejs#990 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
On Windows, when node or io.js attempts to dynamically load a compiled
addon, the compiled addon tries to load node.exe or iojs.exe again -
depending on which import library the module used when it was linked.
This causes many compiled addons to break when node.exe or iojs.exe are
renamed, because when the binary has been renamed the addon DLL can't
find the (right) .exe file to load its imports from.
This patch gives compiled addon developers an option to overcome this
restriction by compiling a delay-load hook into their binary. The
delay-load hook ensures that whenever a module tries to load imports
from node.exe/iojs.exe, it'll just look at the process image, thereby
making the addon work regardless of what name the node/iojs binary has.
To enable this feature, the addon developer must set the
'win_delay_load_hook' option to 'true' in their binding.gyp file, like
this:
```
{
'targets': [
{
'target_name': 'ernie',
'win_delay_load_hook': 'true',
...
```
Bug: nodejs#751
Bug: nodejs#965
Upstream PR: nodejs/node-gyp#599
PR-URL: nodejs#1251
Reviewed-By: Rod Vagg <rod@vagg.org>
PR-URL: nodejs#1266
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
othiym23
added
the
npm
There was a problem hiding this comment.
This appears to be causing problems for me locally – I have README.md on disk, so git am fails on this patch. I'll look into this more tomorrow morning! @Fishrock123 mind taking a look to see if you're having trouble with this file as well?
There was a problem hiding this comment.
I had some weird problems with this exact file recently trying to change my main working branch from v1.x to master and it wasn't to do with this patch, I didn't end up working it out but had to do some hackery to make git happy with the switch. Odd.
There was a problem hiding this comment.
oh no not again >_<
Yeah, this patch is also broken...
There was a problem hiding this comment.
Hmm... after last week's goat rodeo, I reviewed my git settings, and I'm not actually doing anything special with case sensitivity when creating the patch. It's probably something to do with removing deps/npm completely before unpacking the new version, but that's important to do to catch all of the deletions. I'll investigate and see what I can come up with. I don't think that creating the patch on a case-sensitive filesystem is going to help when applying the patch on a case-insensitive filesystem.
|
LGTM minus the whole patch-is-broken thing... |
|
Replaced by #1573 |
6 participants
This PR is branched from
master, notv1.x, in keeping with @Fishrock123's feedback.This is a fairly small release, and you can refer to the release notes for details. The most significant fix is the improvement in error-handling in
node-tar, although new users will also appreciate the tweaks that makenpm initmore pleasant to use.r: @Fishrock123
r: @chrisdickinson