Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: fix preload timing #1694

Closed
wants to merge 1 commit into from
Closed

src: fix preload timing #1694

wants to merge 1 commit into from

Conversation

yosuke-furukawa
Copy link
Member

related: #881
related: #1691

I investigate the issue.
it should load modules after path.resolve(process.argv[1]);

Reviewer: @ofrobots

@@ -857,6 +850,16 @@
};
};

// Load preload modules
startup.preloadModule = function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

preloadModules (plural) may be better :)

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

@mscdex mscdex added the module Issues and PRs related to the module subsystem. label May 13, 2015
@ofrobots
Copy link
Contributor

LGTM.

@yosuke-furukawa
Copy link
Member Author

PTAL. @rvagg @chrisdickinson @trevnorris

@trevnorris
Copy link
Contributor

LGTM

@Fishrock123
Copy link
Contributor

@Fishrock123
Copy link
Contributor

Errors are either jenkins or recently fixed, other than

https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/679/nodes=win2012r2/tapTestReport/test.tap-130/

But it seems unrelated.

yosuke-furukawa added a commit that referenced this pull request May 15, 2015
Fixes: #1691
PR-URL: #1694
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
@Fishrock123
Copy link
Contributor

Thanks, landed in 0a461e5 for 2.0.2

Fishrock123 added a commit to Fishrock123/node that referenced this pull request May 15, 2015
PR-URL: nodejs#1679

Notable Changes:

* win,node-gyp: the delay-load hook for windows addons has now been
correctly enabled by default, it had wrongly defaulted to off in the
release version of 2.0.0 (Bert Belder) nodejs#1433
* os: tmpdir()'s trailing slash stripping has been refined to fix an
issue when the temp directory is at '/'. Also considers which slash is
used by the operating system. (cjihrig) nodejs#1673
* tls: default ciphers have been updated to use gcm and aes128 (Mike
MacCana) nodejs#1660
* build: v8 snapshots have been re-enabled by default as suggested by
the v8 team, since prior security issues have been resolved. This
should give some perf improvements to both startup and vm context
creation. (Trevor Norris) nodejs#1663
* src: fixed preload modules not working when other flags were used
before --require (Yosuke Furukawa) nodejs#1694
* dgram: fixed send()'s callback not being asynchronous (Yosuke
Furukawa) nodejs#1313
* readline: emitKeys now keeps buffering data until it has enough to
parse. This fixes an issue with parsing split escapes. (Alex Kocharin)
* cluster: works now properly emit 'disconnect' to cluser.worker (Oleg
Elifantiev) nodejs#1386
events: uncaught errors now provide some context (Evan Lucas) nodejs#1654
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request May 19, 2015
Fixes: nodejs#1691
PR-URL: nodejs#1694
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
andrewdeandrade pushed a commit to andrewdeandrade/node that referenced this pull request Jun 3, 2015
Fixes: nodejs/node#1691
PR-URL: nodejs/node#1694
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module Issues and PRs related to the module subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants