buffer: move setupBufferJS to internal

[bengl]

Stashing it away in internal/buffer so that it can't be used in userland, but can still be used in internals.

/cc @Fishrock123 @addaleax since we've previously talked about doing this.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• commit message follows commit guidelines

Affected core subsystem(s)

buffer

[bengl]

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

And since this modifies process.binding('buffer'), CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1025/

[refack]

Not blocking, and up for debate:
Since this changes global state, maybe move this to node_bootstap, or explicitly require with a comment. Otherwise AFAICT it will run as a side effect of

node/lib/internal/bootstrap_node.js

Line 295 in 02a5267

global.Buffer = NativeModule.require('buffer').Buffer;

which is not obvius

[bengl]

Hmm, I'd like to keep the caching and removing of setupBufferJS in the same place. Would a comment in bootstrap_node.js clarifying that requiring buffer also removes the setupBufferJS from the binding be sufficient to clarify? Something like:

// Note that this requires `lib/internal/buffer.js`, which removes `setupBufferJS`
// from the buffer binding.
global.Buffer = NativeModule.require('buffer').Buffer;

[refack]

Another viable option is to add this to bootstrap_node.js

// This require as side effect removes `setupBufferJS` from the buffer binding.
NativeModule.require('internal/buffer.js');

[bengl]

Cool, I'll throw a comment and explicit require in. Stay tuned.

[bengl]

I've just realized the commit message is wrong. Fixing it now.

[jasnell]

LGTM with green CI

[bengl]

Alright here's another round of CI and CITGM, since this has changed slightly as per @refack's review.

CI: https://ci.nodejs.org/job/node-test-pull-request/10955/
CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1028/

[jasnell]

CI failures are unrelated

[jasnell]

Landed in 8172f45

[gibfahn]

Should this be backported to v8.x-staging? If yes please follow the guide and raise a backport PR, if no let me know or add the dont-land-on label.

[bengl]

@gibfahn I think in order to avoid the circular dependency issue (https://github.com/nodejs/node/blob/v8.x-staging/lib/buffer.js#L1501-L1503) (not an issue on master/9.x) it may not make sense to backport this commit as it's implemented. That is, putting setupBufferJS into internal/buffer might not make sense on 8.x.