Skip to content
This repository has been archived by the owner on Apr 5, 2018. It is now read-only.

win: Adds delay load hook to allow renaming exe #5

Merged
merged 1 commit into from
May 11, 2015
Merged

win: Adds delay load hook to allow renaming exe #5

merged 1 commit into from
May 11, 2015

Conversation

am11
Copy link
Contributor

@am11 am11 commented Mar 29, 2015

Fix inherited by @piscisaureus's work:

Note: this is a disableable/optional feature and
it is disabled by default (since there are
chances that MSVCR chokes on linking, given if
the module exports data)

Issue URL: #4.

@am11
Fix inherited by @piscisaureus's work:
f13599f 
 * nodejs/node#1251 and
 * nodejs/node#1266

Note: this is a disableable/optional feature and
it is disabled by default (since there are
chances that MSVCR chokes on linking, given if
the module exports data)

Issue URL: #4.
PR URL: rvagg#5.
@am11 am11 mentioned this pull request Apr 30, 2015
Closed
am11 added a commit to am11/node-sass that referenced this pull request May 6, 2015
@am11
build: Adds win_delay_hook so iojs runs with alias.
a351ff5 
Before building, first apply the following patch
to pangyp: rvagg/archived-pangyp#5 (until it is merged).

Issue URL: sass#870.
@am11 am11 mentioned this pull request May 6, 2015
Merged
am11 added a commit to am11/node-sass that referenced this pull request May 6, 2015
@am11
build: Adds win_delay_hook so iojs runs with alias.
579baf3 
Before building, first apply the following patch
to pangyp: rvagg/archived-pangyp#5 (until it is merged).

Issue URL: sass#870.
@am11
Copy link
Contributor Author

am11 commented May 7, 2015

Ping @rvagg.

@kkoopa
Copy link

kkoopa commented May 7, 2015

win_delay_load_hook defaults to true since io.js 2.0

@am11
Copy link
Contributor Author

am11 commented May 8, 2015

@kkoopa, mahtava! this is true. :)
Unfortunately, the issue is we are supposed to provide binaries for all:
node.versions.module: [11, 14, 42, 43, 44] for both ia32 and x64 architectures, targeting multiple platforms. For instance:
sass/node-sass-binaries#65 (win)
sass/node-sass-binaries#66 (freebsd)
sass/node-sass-binaries#67 (linux)
sass/node-sass-binaries#68 (darwin)

To achieve that, pangyp is the one promising build machine which we consume as dependency in node-sass.
Currently, we are facing this situation when I pointed the dependency to my fork: sass/node-sass@579baf3#commitcomment-11075876. Therefore, this PR is highly imperative for node-sass and similar packages. The best part is, there is no side-effect of this change. ♠️

This was referenced May 8, 2015
Closed
Closed
rvagg added a commit that referenced this pull request May 11, 2015
@rvagg
Merge pull request #5 from am11/master
d647697 
win: Adds delay load hook to allow renaming exe
@rvagg rvagg merged commit d647697 into rvagg:master May 11, 2015
@rvagg
Copy link
Owner

rvagg commented May 11, 2015

does defaulting it to true actually cause problems for older versions of Node?

@rvagg
Copy link
Owner

rvagg commented May 11, 2015

v2.2.0

xzyfer added a commit to xzyfer/node-sass that referenced this pull request May 11, 2015
@xzyfer
Switch back to official pangyp
55ac395 
We temporarily switched to a fork until
rvagg/archived-pangyp#5 was resolved.
@xzyfer xzyfer mentioned this pull request May 11, 2015
Merged
@am11
Copy link
Contributor Author

am11 commented May 11, 2015

Thanks! 🎉

I have tested with v0.10 and v0.12. Will try to test will older versions and revert.

am11 added a commit to am11/node-sass that referenced this pull request Jul 11, 2015
@am11
Docs: Removes obsolete note for io.js for Windows.
9f53ee8 
The issue was fixed by rvagg/archived-pangyp#5 and shipped with pangyp v2.0.
@am11 am11 mentioned this pull request Jul 11, 2015
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@am11 @kkoopa @rvagg