Introduce --link-module to ./configure

[bmeck]

• Uses JS2C still, so basename collisions result in a compile error
• Allows specifying a _third_party_main outside of the node repository
• Allows embedders to create custom builtin modules outside of node's repository

[Fishrock123]

cc @nodejs/build

@zcbenz Is this something that would help electron too?

[jasnell]

I'm torn on this. While I can see how it can be useful, allowing third parties to embed their own modules in the binary can open up a number of portability issues.

[bmeck]

@jasnell there is already some history of this with lib/_third_party_main.js. My goal is to introduce a way to keep those extra files outside of node's source tree when building. People currently are building custom node executables (jxcore, electron, noda, etc.), so it somewhat is happening.

[bmeck]

Also, we personally use process._linkedBinding as well (outside of this PR's scope), but all of these changes require altering node.gyp which is pretty fraught with nightmares.

[jasnell]

Yeah, that's largely why I'm torn on it.. I'm not a fan of encouraging such behavior ;-) That said, the fact that this would help bring just a bit more sanity is a good thing so I'm definitely not going to stand in the way. The change itself LGTM but it does make me nervous.

[bmeck]

@jasnell it is the first step in a long road towards having node build single binaries which overwhelms such things I think. I doubt most people will be wanting custom node executables, but rather, wanting their application to be a single binary. Using browserify and this patch, it is possible for JS only apps w/o resorting to messing with node.gyp. First part of a plan to catch up with Go/C++/etc. which are capable of producing single binaries.

[Fishrock123]

I'm in favor of stuff that helps embedders, so.. yeah. :P

[zcbenz]

@zcbenz Is this something that would help electron too?

We override the gyp variables directly instead of calling the configure script, so it doesn't help on Electron's side, but I can see it would be helpful for users that do custom node builds.

[thefourtheye]

Will the modules have access to lib\internal? Basically the question is, will they be treated exactly the same as core modules?

[bmeck]

@thefourtheye yes they have same access as core modules. Though as stated, these generally would not be for creating random things to run 3rd party code.

It could be locked down to avoid this, but would require (drastic) changes to the module system.

[Fishrock123]

yes they have same access as core modules

Hmmm, I'm not so convinced that is good.

[bmeck]

@Fishrock123 @thefourtheye maybe I don't understand, we are talking about people running a CC compiler and handing an executable to others. If the concern is for security... well, thats long out the door at that point. If the concern is for support, we could add actual safety measures to core outside of the simplistic internal mechanism.

[Fishrock123]

If the concern is for security...

It's not. The concern is those modules can directly access internal modules and such.

(I mean, sure you can if you use process.binding(), but then we'll tell you your warranty is void and to not do that.)

If the concern is for support, we could add actual safety measures to core outside of the simplistic internal mechanism.

Not sure what you mean, is that basically the above?

[bmeck]

@Fishrock123 right now NativeModule is able to be punched via techniques like proxyquire/process.binding('natives'), which lets you get access to internal modules. Patching that up is pretty drastic so I was concerned with all the modules that depend on this leaky behavior already.

We could lock down the internal modules to a state where they are pass by reference rather than string lookup (that sometimes goes all the way through user require @_o), this would be much safer but a biig change.

[Fishrock123]

We could lock down the internal modules to a state where they are pass by reference rather than string lookup (that sometimes goes all the way through user require @_o), this would be much safer but a biig change.

If it doesn't affect user modules, why not?

I was concerned with all the modules that depend on this leaky behavior already.

We broke graceful-fs and won, I'm not so concerned. The train is going, there are some terrible things people just need to stop doing. :/

[Fishrock123]

I think this LGTM since as @bmeck explained to me, if you're doing things at compile-time anything's available to do anyways.

I'd like another sign-off though.

[jasnell]

I'm ok with this. One thing that does need to be clarified tho is how this
impacts LTS and the internal core API.
On Aug 24, 2015 7:21 AM, "Jeremiah Senkpiel" [email protected]
wrote:

I think this LGTM since as @bmeck https://github.com/bmeck explained to
me, if you're doing things at compile-time anything's available to do
anyways.

I'd like another sign-off though.

—
Reply to this email directly or view it on GitHub
#2497 (comment).

[thefourtheye]

I may be paranoid but I am worried about non-portable code in userland.

[bmeck]

@thefourtheye maybe you can clarify userland here, since this is during building of an executable, not about code used after that point.

[cjihrig]

@jasnell If someone is essentially rolling their own distribution of Node, our liability should be pretty limited.

[jasnell]

@thefourtheye ... I share the concern over non-portable code. As @bmeck points out tho, there are already people doing this, so while there's definitely a risk, it's likely better to give a consistent hook point for it as opposed to just leaving it up to ad hoc solutions.

[bmeck]

Anything pending except questions about portable code?

[jasnell]

Don't believe so.
On Aug 26, 2015 7:03 AM, "Bradley Meck" [email protected] wrote:

Anything pending except questions about portable code?

—
Reply to this email directly or view it on GitHub
#2497 (comment).

[bmeck]

OS level compatibility portability concerns are kind of moot since this requires an end user to compile node themselves.

As @cjihrig stated concern about support on the foundation side is minimal, especially since any use of this would not be distributed by the foundation distributions.

As for incompatibility when referencing core libraries, this exists today. However, unlike npm installation where the dependency may be running in a version of node it was not made for, this requires full building.

In all, there is a clear point where our responsibility drops off, and there are now warning docs about internal module usage. I think we have majority that it is fine to land this, but don't want to say it when so many people are expressing concern.

[jasnell]

Yep. LGTM
On Aug 26, 2015 9:01 AM, "Bradley Meck" [email protected] wrote:

OS level compatibility portability concerns are kind of mute since this
requires an end user to compile node themselves.

As @cjihrig https://github.com/cjihrig stated concern about support on
the foundation side is minimal, especially since any use of this would not
be distributed by the foundation distributions.

As for incompatibility when referencing core libraries, this exists today.
However, unlike npm installation where the dependency may be running in a
version of node it was not made for, this requires full building.

In all, there is a clear point where our responsibility drops off, and
there are now warning docs about internal module usage. I think we have
majority that it is fine to land this, but don't want to say it when so
many people are expressing concern.

—
Reply to this email directly or view it on GitHub
#2497 (comment).

[thefourtheye]

So users link modules and redistribute as node?

[ezavada]

As an embedder, I'm super happy to see this. For 0.10.38, I had avoided changing node source by duplicated the node::Start function and calling my version which added my modules during startup. When I updated to 4.2.2 (a big jump), I found that was only possible if I changed function in node.cc to be non-static so I could call them from my code.

This --link-module method is much, much better, since now I can take the unaltered node, build it as a library with my modules built in, and still have the single double-clickable app that I need.

[bmeck]

@ezavada would love to see how your workflow goes sometime. Are you using process._linkedBinding as well?

[ezavada]

I’m not familiar with process._linkedBinding — i’ll look into that.

I’ll have my changes (which are still in process) available in github hopefully later today, the project is here: https://github.com/ezavada/pdg https://github.com/ezavada/pdg

Currently it has the 0.10.38 based version.

On Dec 2, 2015, at 7:47 AM, Bradley Meck [email protected] wrote:

@ezavada https://github.com/ezavada would love to see how your workflow goes sometime. Are you using process._linkedBinding as well?

—
Reply to this email directly or view it on GitHub #2497 (comment).

[Fishrock123]

@ezavada Glad to hear. Let us know if there is anything we can do to make it easier to embed Node! :D

[ezavada]

@bmeck I sent you an email about some problems I'm having with --link-module, hope you can help