src,lib: remove dead `process.binding()` code #25829

addaleax · 2019-01-30T20:13:02Z

There are no non-internal builtin modules left, so this
should be safe to remove to a large degree.

Checklist

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

nodejs-github-bot · 2019-01-30T20:13:03Z

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

addaleax · 2019-01-30T22:28:15Z

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

devsnek · 2019-01-31T06:37:41Z

src/node_binding.h

@@ -33,9 +33,6 @@ enum {
      nullptr};                                                                \
  void _register_##modname() { node_module_register(&_module); }

-#define NODE_BUILTIN_MODULE_CONTEXT_AWARE(modname, regfunc)                    \


bnoordhuis

By the way and apropos nothing, node_binding.h includes itself. Accidental change from 13abc6a?

bnoordhuis · 2019-01-31T12:26:03Z

src/node_binding.cc

-      env->ThrowError("Built-in module self-registered.");
-      return false;
-    }
+    CHECK_EQ(mp->nm_flags & NM_F_BUILTIN, 0);


Maybe invert this to CHECK_EQ(0, mp->nm_flags & ~(NM_F_INTERNAL | NM_F_LINKED))? That way you can get rid of NM_F_BUILTIN and it'll also help catch accidental corruption.

I guess we aren’t introducing new flags too often, but maybe at some point we’ll want to add a new flag in a backwards-compatible way?

addaleax · 2019-01-31T12:38:24Z

By the way and apropos nothing, node_binding.h includes itself.

Yeah, removed that – thanks for noticing 👍

joyeecheung · 2019-02-01T21:57:15Z

ping @addaleax this needs a rebase

There are no non-internal builtin modules left, so this should be safe to remove to a large degree.

addaleax · 2019-02-01T22:04:36Z

@joyeecheung Thanks for the ping!

New CI: https://ci.nodejs.org/job/node-test-pull-request/20526/
FreeBSD rebuild: https://ci.nodejs.org/job/node-test-commit-freebsd/23819/
ARM fanned rebuild: https://ci.nodejs.org/job/node-test-commit-arm-fanned/6182/

addaleax · 2019-02-03T19:52:46Z

Landed in 270ffb0

There are no non-internal builtin modules left, so this should be safe to remove to a large degree. PR-URL: #25829 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

addaleax · 2019-02-03T20:10:20Z

(I’m adding backport-requested-v11.x – this should not be hard to backport, but it would be good to backport other PRs first and have this apply cleanly.)

targos · 2019-02-10T18:54:47Z

This lands cleanly on v11.x-staging and tests pass but is it safe to land?

targos · 2019-02-10T20:08:51Z

Feel free to land it on staging if the answer is yes.

There are no non-internal builtin modules left, so this should be safe to remove to a large degree. PR-URL: #25829 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

NODE_BUILTING_MODULE_CONTEXT_AWARE and process.binding are removed in nodejs/node#25829. This changes uses the alternative available without any functionality change.

…17247) * refactor: load electron builtin modules with process._linkedBinding NODE_BUILTING_MODULE_CONTEXT_AWARE and process.binding are removed in nodejs/node#25829. This changes uses the alternative available without any functionality change. * chore: roll node

* fix: backport boringssl patches for node compat * refactor: prevent node macros from overriding base (#17178) * fix: backport boring ssl patch OPENSSL_clear_free * refactor: load electron builtin modules with process._linkedBinding (#17247) * refactor: load electron builtin modules with process._linkedBinding NODE_BUILTING_MODULE_CONTEXT_AWARE and process.binding are removed in nodejs/node#25829. This changes uses the alternative available without any functionality change. * chore: roll node * fix: add boringssl backport to support node upgrade * fix: Update node_includes.h, add DCHECK macros * fix: Update node Debug Options parser usage * fix: Fix asar setup * fix: using v8Util in isolated context * fix: make "process" available in preload scripts * fix: use proper options parser and remove setting of _breakFirstLine _breakFirstLine was being set on the process, but that has changed in node 12 and so is no longer needed. Node will handle it properly when --inspect-brk is provided * fix: process.binding => _linkedBinding in sandboxed isolated preload * chore: update node dep sha * fix: make original-fs work with streams

…lectron#17247) * refactor: load electron builtin modules with process._linkedBinding NODE_BUILTING_MODULE_CONTEXT_AWARE and process.binding are removed in nodejs/node#25829. This changes uses the alternative available without any functionality change. * chore: roll node

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

joyeecheung approved these changes Jan 30, 2019

View reviewed changes

cjihrig approved these changes Jan 30, 2019

View reviewed changes

addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 30, 2019

antsmartian approved these changes Jan 31, 2019

View reviewed changes

devsnek approved these changes Jan 31, 2019

View reviewed changes

bnoordhuis approved these changes Jan 31, 2019

View reviewed changes

src,lib: remove dead process.binding() code

4602232

There are no non-internal builtin modules left, so this should be safe to remove to a large degree.

addaleax force-pushed the dead-process-binding branch from b14fbcd to 4602232 Compare February 1, 2019 22:01

addaleax closed this Feb 3, 2019

addaleax deleted the dead-process-binding branch February 3, 2019 19:52

addaleax added the backport-requested-v11.x label Feb 3, 2019

targos mentioned this pull request Feb 6, 2019

Help wanted: Backporting to v11.x #25403

Closed

addaleax removed the backport-requested-v11.x label Feb 10, 2019

targos mentioned this pull request Feb 14, 2019

v11.10.0 proposal #26098

Merged

This was referenced Feb 15, 2019

2019-02-14 Version 11.10.0 (Current) @targos beevelop/docker-nodejs#263

Closed

2019-02-14 Version 11.10.0 (Current) @targos beevelop/docker-android-nodejs#286

Closed

deepak1556 mentioned this pull request Mar 6, 2019

refactor: load electron builtin modules with process._linkedBinding electron/electron#17247

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

src,lib: remove dead `process.binding()` code #25829

src,lib: remove dead `process.binding()` code #25829

addaleax commented Jan 30, 2019

nodejs-github-bot commented Jan 30, 2019

addaleax commented Jan 30, 2019

devsnek Jan 31, 2019

bnoordhuis left a comment

bnoordhuis Jan 31, 2019

addaleax Jan 31, 2019

addaleax commented Jan 31, 2019

joyeecheung commented Feb 1, 2019

addaleax commented Feb 1, 2019 •

edited

Loading

addaleax commented Feb 3, 2019

addaleax commented Feb 3, 2019

targos commented Feb 10, 2019

targos commented Feb 10, 2019

src,lib: remove dead process.binding() code #25829

src,lib: remove dead process.binding() code #25829

Conversation

addaleax commented Jan 30, 2019

Checklist

nodejs-github-bot commented Jan 30, 2019

addaleax commented Jan 30, 2019

devsnek Jan 31, 2019

Choose a reason for hiding this comment

bnoordhuis left a comment

Choose a reason for hiding this comment

bnoordhuis Jan 31, 2019

Choose a reason for hiding this comment

addaleax Jan 31, 2019

Choose a reason for hiding this comment

addaleax commented Jan 31, 2019

joyeecheung commented Feb 1, 2019

addaleax commented Feb 1, 2019 • edited Loading

addaleax commented Feb 3, 2019

addaleax commented Feb 3, 2019

targos commented Feb 10, 2019

targos commented Feb 10, 2019

src,lib: remove dead `process.binding()` code #25829

src,lib: remove dead `process.binding()` code #25829

addaleax commented Feb 1, 2019 •

edited

Loading