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

[briete]

see: #22160

Checklist

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

[jdalton]

Holding removals until migration is ironed out

[jasnell]

Actual code change looks good. process.binding('os') is one that we definitely need to be careful about.

[jasnell]

@briete .. when you get a moment, can you rebase this from master then apply the following patch, which will be necessary for this to proceed

Author: James M Snell <[email protected]>
Date:   Tue Aug 14 19:36:25 2018 -0700

    [Squash] add `process.binding('os')` to fallthrough whitelist

diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
index 08daeb1915..ffae6e4cd9 100644
--- a/lib/internal/bootstrap/node.js
+++ b/lib/internal/bootstrap/node.js
@@ -327,7 +327,7 @@
     // that are whitelisted for access via process.binding()... this is used
     // to provide a transition path for modules that are being moved over to
     // internalBinding.
-    const internalBindingWhitelist = new SafeSet(['uv']);
+    const internalBindingWhitelist = new SafeSet(['uv', 'os']);
     process.binding = function binding(name) {
       return internalBindingWhitelist.has(name) ?
         internalBinding(name) :
diff --git a/test/parallel/test-process-binding-internalbinding-whitelist.js b/test/parallel/test-process-binding-internalbinding-whitelist.js
index ece967a0b7..cc0119917d 100644
--- a/test/parallel/test-process-binding-internalbinding-whitelist.js
+++ b/test/parallel/test-process-binding-internalbinding-whitelist.js
@@ -7,3 +7,4 @@ const assert = require('assert');
 // Assert that whitelisted internalBinding modules are accessible via
 // process.binding().
 assert(process.binding('uv'));
+assert(process.binding('os'));

[lundibundi]

LGTM with a nit.

[lundibundi]

Missing newline at the end.

[briete]

Thank you. Fixed

[lundibundi]

@briete could you please rebase this on master and resolve the conflicts? I'll then run the CI and we will hopefully get your PR landed 👍.

[briete]

@lundibundi I resolved the conflict by rebasing

[lundibundi]

@briete could you try to remove all of the non-yours commits from this PR? You will have to either:

• use git rebase --interactive and remove unnecessary commits and then rebase cleanly on master
• close this PR and open a new one that will be based on up to date master branch (you can also use git cherry-pick to get your commits from this branch)

Also, I can do the first option for you and just push to your branch (briete:os-internal-binding) if that's fine with you.

Unfortunately, I cannot start the CI until this is resolved with whichever method you prefer.

[briete]

@lundibundi sorry. I am confused by my first OSS contribution.:sweat:
May I ask you the first option?

[lundibundi]

@briete I've updated your branch.

BE AWARE that the following will remove all your local changes to os-internal-binding branch (those not in github), if you want to keep them you have to use git stash or some other method.
If you didn't make any changes there is nothing to worry about.
If you want to pull the changes locally you will have to use

git checkout os-internal-binding
git fetch
git reset --hard origin/os-internal-binding

Refs: https://stackoverflow.com/a/9813888/3923380

[lundibundi]

@jdalton (or someone else) could you please take a look at ca7dad0 (second commit in this branch) as I'm not sure if I did it correctly.

[lundibundi]

CI doesn't seem very healty right now, I'll start it later.

CI: https://ci.nodejs.org/job/node-test-pull-request/16866/

[briete]

@lundibundi Thank you. informative.:+1:

[starkwang]

@briete Are you still interested in this PR ?

[briete]

@starkwang I'm sorry. Since time has passed since the first PR, close this and make a new PR.

[briete]

See #25087