process: allow StartExecution() to take a main script ID - #25474
Closed
joyeecheung wants to merge 1 commit into
Closed
process: allow StartExecution() to take a main script ID#25474joyeecheung wants to merge 1 commit into
joyeecheung wants to merge 1 commit into
Conversation
Copy link
Copy Markdown
Member
Sorry, something went wrong.
The idea is to allow the C++ layer to run arbitrary scripts as the main script. This paves the way for - cctest of the execution of Node.js instances - Earlier handling of per-process CLI options that affect execution modes (those usually do not make sense for the embedders). - Targets like mkcodecache or mksnapshot. Also moves the handling of `_third_party_main.js` into C++.
Copy link
Copy Markdown
Collaborator
|
@joyeecheung sadly an error occured when I tried to trigger a build :( |
Sorry, something went wrong.
2 tasks
addaleax
approved these changes
Jan 13, 2019
Comment thread
src/node.cc
| Local<Value> main_script_v; | ||
| if (main_script_id == nullptr) { | ||
| // TODO(joyeecheung): make this mandatory - we may also create an overload | ||
| // for main_script that is a Local<Function>. |
Copy link
Copy Markdown
Member
There was a problem hiding this comment.
Just to make sure I understand this correctly, overload means an overload of StartExecution() that takes a Local<Function>? I like that idea. :)
Sorry, something went wrong.
Copy link
Copy Markdown
Member
Author
There was a problem hiding this comment.
@addaleax Yeah, and maybe we could also use an overload with a C++ callback that takes some useful arguments (maybe Environment?)
Sorry, something went wrong.
joyeecheung
commented
Jan 13, 2019
Comment thread
lib/internal/bootstrap/node.js
| // directory which will be executed instead of Node's normal loading. | ||
| if (NativeModule.exists('_third_party_main')) { | ||
| function startMainThreadExecution(mainScript) { | ||
| if (mainScript) { |
Copy link
Copy Markdown
Member
Author
There was a problem hiding this comment.
This bit here is temporary - I think it would be better to do the branching in C++ and directly compileAndCall the main scripts instead. For per-process CLI options like --help we could branch in node::Start() since they do not make a lot of sense for either embedders or the workers anyway. Although unfortunately for _third_party_main we have to keep a driver that wrap it into a process.nextTick() (or maybe there is a cleverer way out?)
Sorry, something went wrong.
JungMinu
approved these changes
Jan 14, 2019
Copy link
Copy Markdown
Member
Author
Sorry, something went wrong.
pull Bot
pushed a commit
to shakir-abdo/node
that referenced
this pull request
Jan 15, 2019
The idea is to allow the C++ layer to run arbitrary scripts as the main script. This paves the way for - cctest of the execution of Node.js instances - Earlier handling of per-process CLI options that affect execution modes (those usually do not make sense for the embedders). - Targets like mkcodecache or mksnapshot. Also moves the handling of `_third_party_main.js` into C++. PR-URL: nodejs#25474 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
BridgeAR
pushed a commit
that referenced
this pull request
Jan 16, 2019
The idea is to allow the C++ layer to run arbitrary scripts as the main script. This paves the way for - cctest of the execution of Node.js instances - Earlier handling of per-process CLI options that affect execution modes (those usually do not make sense for the embedders). - Targets like mkcodecache or mksnapshot. Also moves the handling of `_third_party_main.js` into C++. PR-URL: #25474 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Merged
BridgeAR
pushed a commit
to BridgeAR/node
that referenced
this pull request
Jan 16, 2019
The idea is to allow the C++ layer to run arbitrary scripts as the main script. This paves the way for - cctest of the execution of Node.js instances - Earlier handling of per-process CLI options that affect execution modes (those usually do not make sense for the embedders). - Targets like mkcodecache or mksnapshot. Also moves the handling of `_third_party_main.js` into C++. PR-URL: nodejs#25474 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
BridgeAR
pushed a commit
to BridgeAR/node
that referenced
this pull request
Jan 17, 2019
The idea is to allow the C++ layer to run arbitrary scripts as the main script. This paves the way for - cctest of the execution of Node.js instances - Earlier handling of per-process CLI options that affect execution modes (those usually do not make sense for the embedders). - Targets like mkcodecache or mksnapshot. Also moves the handling of `_third_party_main.js` into C++. PR-URL: nodejs#25474 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The idea is to allow the C++ layer to run arbitrary scripts
as the main script. This paves the way for
execution modes (those usually do not make sense for the
embedders).
Also moves the handling of _third_party_main.js into C++.
Checklist