modules: adding load linked modules feature#8
Closed
thlorenz wants to merge 1 commit intonodejs:v0.12from
thlorenz:iojs-v0.12-linked-module-loading
Closed
modules: adding load linked modules feature#8thlorenz wants to merge 1 commit intonodejs:v0.12from thlorenz:iojs-v0.12-linked-module-loading
thlorenz wants to merge 1 commit intonodejs:v0.12from
thlorenz:iojs-v0.12-linked-module-loading
Conversation
- introduced NM_F_LINKED flag to identify linked modules - setting node_is_initialized after calling V8::Initialize in order to make the right decision during initial module registration - introduced modlist_linked in order to track modules that were pre-registered in order to complete it once node is initialized - completing registration of linked module similarly to the way it's done inside DLOpen
Member
|
For the record, I already LGTM'd this but I'd like @trevnorris to sign off on it. |
Contributor
|
LGTM |
Member
|
running full CI on this https://jenkins-node-forward.nodesource.com/job/iojs+any-pr+multi/3/ |
Member
|
@thlorenz would it be difficult to add a simple test for this in test/addons/ and perhaps a super-simple sanity check in test/simple/? seems like something that's ripe for regressions. |
Contributor
Author
|
Ideally yes, but that would require adding something to So I think it'd have to be some integration test, possibly even executed manually, not sure. Not sure when I can get to it. Could we merge this anyhow and I'll think about how to best do this and supply tests in another PR? |
Contributor
|
I'd normally require a test, but since the API is technically "private" (done, IMO, so the API can be hashed out as it's used if necessary) there's no need for a test at the moment. |
Contributor
Author
|
Thanks @trevnorris and as I said it won't be a simple test. |
Member
|
@thlorenz I assumed that the test/addons/ directory would be the place for this--they are a separate test run and are all compiled and loaded to test, perhaps we could do something similar to make this testable? |
Contributor
Author
|
No this doesn't work exactly like an addon. It's a linked module, i.e. it has to be made part of the code base by modifying the |
Member
|
this got landed in joyent/node didn't it? should we land it here or close and wait for the next pull from joyent/node? |
Contributor
Author
|
It did land in joyent/node as well but in different shape since things were adapted to older API, i.e. |
rvagg
pushed a commit
that referenced
this pull request
Dec 4, 2014
- introduced NM_F_LINKED flag to identify linked modules - setting node_is_initialized after calling V8::Initialize in order to make the right decision during initial module registration - introduced modlist_linked in order to track modules that were pre-registered in order to complete it once node is initialized - completing registration of linked module similarly to the way it's done inside DLOpen PR-URL: #8 Reviewed-by: Trevor Norris <trev.norris@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Member
|
landed via f4e058d |
rvagg
pushed a commit
that referenced
this pull request
Dec 4, 2014
- introduced NM_F_LINKED flag to identify linked modules - setting node_is_initialized after calling V8::Initialize in order to make the right decision during initial module registration - introduced modlist_linked in order to track modules that were pre-registered in order to complete it once node is initialized - completing registration of linked module similarly to the way it's done inside DLOpen PR-URL: #8 Reviewed-by: Trevor Norris <trev.norris@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
rvagg
pushed a commit
that referenced
this pull request
Dec 4, 2014
- introduced NM_F_LINKED flag to identify linked modules - setting node_is_initialized after calling V8::Initialize in order to make the right decision during initial module registration - introduced modlist_linked in order to track modules that were pre-registered in order to complete it once node is initialized - completing registration of linked module similarly to the way it's done inside DLOpen PR-URL: #8 Reviewed-by: Trevor Norris <trev.norris@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
rvagg
pushed a commit
that referenced
this pull request
Dec 4, 2014
- introduced NM_F_LINKED flag to identify linked modules - setting node_is_initialized after calling V8::Initialize in order to make the right decision during initial module registration - introduced modlist_linked in order to track modules that were pre-registered in order to complete it once node is initialized - completing registration of linked module similarly to the way it's done inside DLOpen PR-URL: #8 Reviewed-by: Trevor Norris <trev.norris@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Closed
Closed
This was referenced Jun 28, 2020
targos
referenced
this pull request
in targos/node
Apr 17, 2021
Original commit message:
Merged: [compiler] Fix bug in SimplifiedLowering's overflow computation
Revision: e371325bcb03f20a362ebfa48225159702c6fde7
BUG=chromium:1126249
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=tebbi@chromium.org
Change-Id: I411d9233f77992e73da12784cef59c885999b556
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2415988
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/branch-heads/8.6@{#8}
Cr-Branched-From: a64aed2333abf49e494d2a5ce24bbd14fff19f60-refs/heads/8.6.395@{#1}
Cr-Branched-From: a626bc036236c9bf92ac7b87dc40c9e538b087e3-refs/heads/master@{#69472}
Refs: v8/v8@a59e3ac
targos
referenced
this pull request
in targos/node
Apr 27, 2021
Original commit message:
Merged: [compiler] Fix bug in SimplifiedLowering's overflow computation
Revision: e371325bcb03f20a362ebfa48225159702c6fde7
BUG=chromium:1126249
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=tebbi@chromium.org
Change-Id: I411d9233f77992e73da12784cef59c885999b556
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2415988
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/branch-heads/8.6@{#8}
Cr-Branched-From: a64aed2333abf49e494d2a5ce24bbd14fff19f60-refs/heads/8.6.395@{#1}
Cr-Branched-From: a626bc036236c9bf92ac7b87dc40c9e538b087e3-refs/heads/master@{#69472}
Refs: v8/v8@a59e3ac
targos
added a commit
that referenced
this pull request
Apr 30, 2021
Original commit message:
Merged: [compiler] Fix bug in SimplifiedLowering's overflow computation
Revision: e371325bcb03f20a362ebfa48225159702c6fde7
BUG=chromium:1126249
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=tebbi@chromium.org
Change-Id: I411d9233f77992e73da12784cef59c885999b556
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2415988
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/branch-heads/8.6@{#8}
Cr-Branched-From: a64aed2333abf49e494d2a5ce24bbd14fff19f60-refs/heads/8.6.395@{#1}
Cr-Branched-From: a626bc036236c9bf92ac7b87dc40c9e538b087e3-refs/heads/master@{#69472}
Refs: v8/v8@a59e3ac
PR-URL: #38275
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
dumganhar
pushed a commit
to dumganhar/node
that referenced
this pull request
Dec 17, 2024
Update README for v18.12.1
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
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
5 participants
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.
refiling PR from node-fwd
make the right decision during initial module registration
pre-registered in order to complete it once node is initialized
done inside DLOpen