Skip to content

Commit

Permalink
win: work around __pfnDliNotifyHook2 type change
Browse files Browse the repository at this point in the history
Visual Studio 2015 Update 3 defines __pfnDliNotifyHook2 as const.
The decltype specifier makes the declaration work across all supported
versions of VS. It also requires that the source be compiled as C++.

Fixes: #949

PR-URL: #952
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
orangemocha authored Jun 13, 2016
1 parent 3df8222 commit f31482e
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion addon.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
'conditions': [
[ 'OS=="win"', {
'sources': [
'<(node_gyp_dir)/src/win_delay_load_hook.c',
'<(node_gyp_dir)/src/win_delay_load_hook.cc',
],
'msvs_settings': {
'VCLinkerTool': {
Expand Down
2 changes: 1 addition & 1 deletion src/win_delay_load_hook.c → src/win_delay_load_hook.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ static FARPROC WINAPI load_exe_hook(unsigned int event, DelayLoadInfo* info) {
return (FARPROC) m;
}

PfnDliHook __pfnDliNotifyHook2 = load_exe_hook;
decltype(__pfnDliNotifyHook2) __pfnDliNotifyHook2 = load_exe_hook;

#endif

9 comments on commit f31482e

@Fishrock123
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orangemocha How did you merge this?

@Fishrock123
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"@orangemocha committed on GitHub 3 hours ago"

@orangemocha
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the PR branch with the commit metadata, then I used Github's new merge-squash functionality. Is that not ok? I have to admit it's the first time I tried doing it this way.

@Fishrock123
Copy link
Contributor

@Fishrock123 Fishrock123 commented on f31482e Jun 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orangemocha It should be, maybe it just didn't used to display like that when I did it. Seems slightly confusing, but I don't have an issue with it. (So long as it doesn't make more sense as multiple commits)

@orangemocha
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to remove the PR #, which gets suggested as an append to the original commit message. Other than that, the results look exactly the same in git history. I think it will squash multiple commits, so it's a valid alternative only for single-commit PRs.

@orangemocha
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tasoss
Copy link

@tasoss tasoss commented on f31482e Jun 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for the interruption.i would like to ask that in order to make libxmljs compile(it can't compile node-gyp due to the specific error) what should i do? thanks!
ps:forget it i found where i should put the files :)

@orangemocha
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tasoss if you want a hack until this fix is released with Node, you could try to apply it to your Node installation, under node_modules\npm\node_modules\node-gyp.

@tasoss
Copy link

@tasoss tasoss commented on f31482e Jun 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that what's i have done because i had to continue my work.Thanks!!

Please sign in to comment.