[11.x] use NativeModuleLoader to compile all the bootstrappers #25446

joyeecheung · 2019-01-11T15:45:45Z

This is backported by first reverting out-of-order commits on v11.x-staging and then correcting the order of the commits for the changed files. See #24775 (comment)

cc @BridgeAR

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

Instead use `require('internal/options').getOptionValue('--pending-deprecation')` PR-URL: nodejs#24962 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>

In places of process.binding('inspector'), migrate code to adapt internalBinding. PR-URL: nodejs#24931 Refs: nodejs#22160 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>

Inspecting an ArrayBuffer now also shows their binary contents. PR-URL: nodejs#25006 Reviewed-By: Anna Henningsen <[email protected]>

This removes all internal calls to the deprecated `_extends()` function. It is slower than `Object.assign()` and the object spread notation since V8 6.8 and using the spread notation often also results in shorter code. PR-URL: nodejs#25105 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>

Combine all type checks on the internal types module and do not use the types binding anywhere else anymore. This makes sure all of those checks exist when required. PR-URL: nodejs#25149 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]>

This makes sure extra properties on buffers are not ignored anymore when inspecting the buffer. PR-URL: nodejs#25150 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>

This is a broken commit: it's here so that git interpret this as a file move and preserve most of the history of the Console constructor. PR-URL: nodejs#24709 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>

Since we do not actually use the Console constructor to instantiate the global console, move the two piece of code into two different JS files for clarity, and make console.js a mere re-export of the global console. The hope is to make the global console, a namespace, more web-compatible while keeping the Console constructor available for backwards compatibility. PR-URL: nodejs#24709 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>

Move the wrapping of the inspector console in a separate file for clarity. In addition, save the original console from the VM explicitly via an exported property `require('internal/console/inspector').consoleFromVM` that `require('inspector').console` can alias to it later, instead of hanging the original console onto `per_thread.js` during bootstrap and counting on that `per_thread.js` only gets evaluated once and gets cached. PR-URL: nodejs#24709 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>

Add an `inspectOptions` option to the `console` constructor. That way it's possible to define all inspection defaults for each `console` instance instead of relying on the `inspect()` defaults. PR-URL: nodejs#24978 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]>

This commit adds stricter type checking to the inspectOptions option to the Console constructor. PR-URL: nodejs#25090 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Anto Aravinth <[email protected]> Reviewed-By: James M Snell <[email protected]>

The only known condition where we could not provide appropriate stdio streams so far were non-console Windows applications. Since this issue has come up a few times in our issue tracker now, switch to providing dummy streams for these cases instead. If there are other valid cases in which `uv_guess_handle` fails, and where there is a more sensible way to provide stdio, we’ll probably still find out because the streams don’t work properly either way. Refs: nodejs/help#1251 PR-URL: nodejs#20640 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>

This reverts commit e97acf7.

This reverts commit c168672.

This reverts commit 617bfe4.

This reverts commit 55d185f.

This reverts commit 0cde1a4.

This reverts commit 88a5449.

This reverts commit 1ec4f8d.

Refs: nodejs#22160 PR-URL: nodejs#22478 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Denys Otrishko <[email protected]>

This patch moves all the bootstrapper compilation to use NativeModuleLoader::CompileAndCall(). With this we no longer need to mess with the error decoration and handling any more - there is no point in handling the JS error occurred during bootstrapping by ourselves, we should just crash or let the VM handle it. PR-URL: nodejs#24775 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

PR-URL: nodejs#24770 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Weijia Wang <[email protected]>

PR-URL: nodejs#23400 Refs: nodejs#22160 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>

We now compile the native modules in C++ so these are no longer used. PR-URL: nodejs#24904 Refs:https://github.com/joyeecheung/node/commit/ bd765d6 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>

In places of process.binding('inspector'), migrate code to adapt internalBinding. PR-URL: nodejs#24931 Refs: nodejs#22160 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>

PR-URL: nodejs#25087 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>

Instead use `require('internal/options').getOptionValue('--pending-deprecation')` PR-URL: nodejs#24962 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>

Combine all type checks on the internal types module and do not use the types binding anywhere else anymore. This makes sure all of those checks exist when required. PR-URL: nodejs#25149 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]>

nodejs-github-bot · 2019-01-11T15:45:47Z

@joyeecheung build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/2233/pipeline

This reverts commit 1ec4f8d. PR-URL: #25446 Reviewed-By: Anna Henningsen <[email protected]>

Refs: #22160 PR-URL: #22478 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Backport-PR-URL: #25446

This patch moves all the bootstrapper compilation to use NativeModuleLoader::CompileAndCall(). With this we no longer need to mess with the error decoration and handling any more - there is no point in handling the JS error occurred during bootstrapping by ourselves, we should just crash or let the VM handle it. PR-URL: #24775 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Backport-PR-URL: #25446

PR-URL: #24770 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Weijia Wang <[email protected]> Backport-PR-URL: #25446

PR-URL: #23400 Refs: #22160 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Backport-PR-URL: #25446 Reviewed-By: Anna Henningsen <[email protected]>

We now compile the native modules in C++ so these are no longer used. PR-URL: #24904 Refs:https://github.com/joyeecheung/node/commit/ bd765d6 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Backport-PR-URL: #25446

In places of process.binding('inspector'), migrate code to adapt internalBinding. PR-URL: #24931 Refs: #22160 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Backport-PR-URL: #25446

PR-URL: #25087 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Backport-PR-URL: #25446

Instead use `require('internal/options').getOptionValue('--pending-deprecation')` PR-URL: #24962 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Backport-PR-URL: #25446

Combine all type checks on the internal types module and do not use the types binding anywhere else anymore. This makes sure all of those checks exist when required. PR-URL: #25149 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Backport-PR-URL: #25446

addaleax · 2019-01-14T13:14:03Z

I’ve squashed what I could, but only two of the reverts would actually cleanly cancel out with the previous commits.

For the rest of this PR: Landed in 1805236...6528ce6

This reverts commit c168672. PR-URL: nodejs#25446 Reviewed-By: Anna Henningsen <[email protected]>

This reverts commit 55d185f. PR-URL: nodejs#25446 Reviewed-By: Anna Henningsen <[email protected]>

This reverts commit 0cde1a4. PR-URL: nodejs#25446 Reviewed-By: Anna Henningsen <[email protected]>

This reverts commit 88a5449. PR-URL: nodejs#25446 Reviewed-By: Anna Henningsen <[email protected]>

This reverts commit 1ec4f8d. PR-URL: nodejs#25446 Reviewed-By: Anna Henningsen <[email protected]>

Refs: nodejs#22160 PR-URL: nodejs#22478 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Backport-PR-URL: nodejs#25446

This patch moves all the bootstrapper compilation to use NativeModuleLoader::CompileAndCall(). With this we no longer need to mess with the error decoration and handling any more - there is no point in handling the JS error occurred during bootstrapping by ourselves, we should just crash or let the VM handle it. PR-URL: nodejs#24775 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Backport-PR-URL: nodejs#25446

PR-URL: nodejs#24770 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Weijia Wang <[email protected]> Backport-PR-URL: nodejs#25446

PR-URL: nodejs#23400 Refs: nodejs#22160 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Backport-PR-URL: nodejs#25446 Reviewed-By: Anna Henningsen <[email protected]>

We now compile the native modules in C++ so these are no longer used. PR-URL: nodejs#24904 Refs:https://github.com/joyeecheung/node/commit/ bd765d6 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Backport-PR-URL: nodejs#25446

In places of process.binding('inspector'), migrate code to adapt internalBinding. PR-URL: nodejs#24931 Refs: nodejs#22160 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Backport-PR-URL: nodejs#25446

PR-URL: nodejs#25087 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Backport-PR-URL: nodejs#25446

Instead use `require('internal/options').getOptionValue('--pending-deprecation')` PR-URL: nodejs#24962 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Backport-PR-URL: nodejs#25446

Combine all type checks on the internal types module and do not use the types binding anywhere else anymore. This makes sure all of those checks exist when required. PR-URL: nodejs#25149 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Backport-PR-URL: nodejs#25446

MylesBorins · 2019-01-18T06:34:56Z

just noticing this now... generally we don't treat staging branches as "holy"... we generally rebase out commits that shouldn't have landed for LTS

edit: also apologies if I misunderstood, but it seems like we could have rebased out commits from staging rather than revert

joyeecheung and others added 28 commits January 10, 2019 01:28

util: inspect ArrayBuffers contents as well

dc4e5c5

Inspecting an ArrayBuffer now also shows their binary contents. PR-URL: nodejs#25006 Reviewed-By: Anna Henningsen <[email protected]>

buffer: inspect extra properties

95fa3c7

This makes sure extra properties on buffers are not ignored anymore when inspecting the buffer. PR-URL: nodejs#25150 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>

Revert "lib: expose all type checks from the internal types module"

c219b8f

This reverts commit e97acf7.

Revert "inspector: move process.binding to internalBinding"

f4dbe57

This reverts commit c168672.

Revert "lib: remove internalBinding('config').pendingDeprecation"

7aeec97

This reverts commit 617bfe4.

Revert "os: move process.binding('os') to internalBinding"

48dfda7

This reverts commit 55d185f.

Revert "lib: remove unused NativeModule/NativeModule wraps"

abfde34

This reverts commit 0cde1a4.

Revert "src,lib: make process.binding('config') internal"

c5b2c31

This reverts commit 88a5449.

Revert "lib: remove duplicated noop function"

fb6f2f2

This reverts commit 1ec4f8d.

src,lib: make process.binding('config') internal

b6afa46

PR-URL: nodejs#23400 Refs: nodejs#22160 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>

os: move process.binding('os') to internalBinding

181d03a

PR-URL: nodejs#25087 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>

nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Jan 11, 2019

addaleax pushed a commit that referenced this pull request Jan 14, 2019

Revert "lib: remove duplicated noop function"

d7d772b

This reverts commit 1ec4f8d. PR-URL: #25446 Reviewed-By: Anna Henningsen <[email protected]>

addaleax closed this Jan 14, 2019

BridgeAR mentioned this pull request Jan 16, 2019

v11.7.0 proposal #25537

Merged

BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 16, 2019

Revert "inspector: move process.binding to internalBinding"

07a6949

This reverts commit c168672. PR-URL: nodejs#25446 Reviewed-By: Anna Henningsen <[email protected]>

BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 16, 2019

Revert "os: move process.binding('os') to internalBinding"

fd8e02c

This reverts commit 55d185f. PR-URL: nodejs#25446 Reviewed-By: Anna Henningsen <[email protected]>

BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 16, 2019

Revert "lib: remove unused NativeModule/NativeModule wraps"

252716e

This reverts commit 0cde1a4. PR-URL: nodejs#25446 Reviewed-By: Anna Henningsen <[email protected]>

BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 16, 2019

Revert "src,lib: make process.binding('config') internal"

55d4d13

This reverts commit 88a5449. PR-URL: nodejs#25446 Reviewed-By: Anna Henningsen <[email protected]>

BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 16, 2019

Revert "lib: remove duplicated noop function"

468ccac

This reverts commit 1ec4f8d. PR-URL: nodejs#25446 Reviewed-By: Anna Henningsen <[email protected]>

MylesBorins mentioned this pull request Jan 24, 2019

v11.8.0 proposal #25687

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[11.x] use NativeModuleLoader to compile all the bootstrappers #25446

[11.x] use NativeModuleLoader to compile all the bootstrappers #25446

joyeecheung commented Jan 11, 2019 •

edited

Loading

nodejs-github-bot commented Jan 11, 2019

addaleax commented Jan 14, 2019

MylesBorins commented Jan 18, 2019 •

edited

Loading

[11.x] use NativeModuleLoader to compile all the bootstrappers #25446

[11.x] use NativeModuleLoader to compile all the bootstrappers #25446

Conversation

joyeecheung commented Jan 11, 2019 • edited Loading

Checklist

nodejs-github-bot commented Jan 11, 2019

addaleax commented Jan 14, 2019

MylesBorins commented Jan 18, 2019 • edited Loading

joyeecheung commented Jan 11, 2019 •

edited

Loading

MylesBorins commented Jan 18, 2019 •

edited

Loading