Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

build: Adds node_lib target. #6744

Closed
wants to merge 2 commits into from
Closed

build: Adds node_lib target. #6744

wants to merge 2 commits into from

Conversation

zcbenz
Copy link

@zcbenz zcbenz commented Dec 20, 2013

Adds a node_lib target that builds everything except the node_main.cc as a static library. And the node target would be depended on node_lib and compiles out the executable as usual.

With this other projects would be possible to include node as a third party library and embed a node engine in their applications.

Adds a node_lib target that builds everything except the node_main.cc as
a static library. And the node target would be depended on node_lib and
compiles out the executable as usual.

With this other projects would be possible to include node as a third
party library and embed a node engine in their applications.
@indutny
Copy link
Member

indutny commented Dec 20, 2013

What projects are in need of using it this way? It seems to be pretty crazy, because node will block the thread where it executes and will assume that it is a single self-sustained process anyway.

@zcbenz
Copy link
Author

zcbenz commented Dec 20, 2013

For example the https://github.com/rogerwang/node-webkit.

Yeah the node::Start() is self-sustained, but the users would usually call node::SetupProcessObject and node::Load() to manually setup the environment and integrate the node message loop with other applications.

@indutny
Copy link
Member

indutny commented Dec 20, 2013

I think that this PR should export this symbols then. Otherwise there're not much point in maintaining separate target.

May I ask you to add this?

@zcbenz
Copy link
Author

zcbenz commented Dec 20, 2013

The functions are exported now. Their declarations were removed by 185c515, so I was not sure if we should take them back.

btw, it doesn't really matter if they are not exported, because they are not defined as static and node_lib is built as static_library, in my private project, I simply put the declarations of node::SetupProcessObject and others in the source files and there is no linking error. But of course exporting them would make things much nicer.

They are useful for third party applications to include node as library
and create custom node environments.
@tjfontaine
Copy link

There are a few PRs like this, and I'm not insensitive to the needs of embedders, but I'm not sure we're going to move forward with blessing a mechanism just yet. It would be something to consider once we have a more stable interface that we're sure we want to export.

@indutny
Copy link
Member

indutny commented Dec 20, 2013

@tjfontaine good point.

@zcbenz
Copy link
Author

zcbenz commented Dec 21, 2013

I can understand if you don't want to export those interfaces before they became stable, but could you accept the build: Adds node_lib target patch? It did nothing on exporting interfaces, but meant a lot for embedders.

@obastemur
Copy link

Although the core team answered the thread, I couldn't keep myself without saying couple of words. @zcbenz I've been playing with the Node.JS sources for a while and I don't think the issue here is the stability in general. Instead the project design may not be proper for embedded usage. For example, any kind of multi threaded access to the library (executing a script on the main context) is a killer. No sandboxing (apart from the wrapping) since there is only one instance. If you are interested with embedding only JS compiler, try v8. In case you are mostly interested with the great Node.JS features, I would prefer RPC like solution.

@zcbenz
Copy link
Author

zcbenz commented Jun 25, 2014

@indutny @tjfontaine

Since the Atom editor had been published for a while, I can now share more information on how we integrate node.js into other applications.

The node integration in atom/atom is done in atom/atom-shell, and the patches of node we relied on can be found in atom/node. The patches were mainly for following purposes:

  • Make node being able to be built as library.
  • Make node's Environemnt more friendly with external isolates and contexts.
  • Add some interfaces for embedding purpose.
  • Some desktop application specific patches.

And I have a presentation on how node integration works in atom-shell, though some details were ignored for simplicity: https://speakerdeck.com/zcbenz/practice-on-embedding-node-dot-js-into-atom-editor. During atom-shell's long time uses, I think integrating node into a normal desktop application or into Chromium browser works very well.

The forked node is still on node v0.11.10, and some patches are dirty and not very suitable for merge into upstream, but I'm very willing to rebase the patches on node's master branch and modify some of the patches to make them suitable for upstream merge, so people who wants to embed node into their apps can benefit from what we have worked out.

So is there any chance to merge patches making node more friendly for embedding like this one to upstream in node 0.11 or 0.12?

@jasnell
Copy link
Member

jasnell commented Aug 3, 2015

Closing due to lack of activity. Can revisit if someone is interested in updating the PR. Given that this is a feature request, however, it would need to be targeted at either https://github.com/nodejs/io.js or https://github.com/nodejs/node

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants