build: introduce configure --shared #6994

stefanmb · 2016-05-26T14:48:27Z

Checklist

tests and code linting passes
documentation is changed or added
the commit message follows commit guidelines

Affected core subsystem(s)

build, configure

Description of change

This PR is the continuation of #5918 where the originator (@indutny) agreed that I can take over this task in order to land the changes as soon as possible.

The PR consists of two commits, the original one from #5918 which adds "[a] configure flag for building shared library that could be used to embed Node.js in some application (like Electron)" and a second commit that modifies the following aspects:

Use the "--shared" configure flag to build a shared library.
Use the "--no-bundled-v8" flag to exclude V8 headers from the deps/ folder, this option is useful to Electron which provides its own copy of V8.
Use the "--no-v8-platform" flag to avoid initializing the V8 platform inside Node (useful for some embedders).
Insure that "./configure --shared && make" works and produces a useable libnode.so for anyone (i.e. decouple --shared from Electron's build system).

I intend to land both commits in the PR at the same time (the original one from @indutny and my changes on top of it) to maintain a clear history/ownership of the work. If anyone feels strongly about this approach either way, please let me know.

All these options are provided as-is for embedders are not officially supported at this time, as indicated in the documentation.

Future changes:

This PR is concerned with Linux only - once this PR is landed a separate PR will be made to resolve issues related to building a DLL on Windows.

stefanmb · 2016-05-26T14:50:41Z

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

MylesBorins · 2016-05-31T04:23:14Z

@stefanmb it looks like this needs a rebase. Let me know if there is anything I can help with on this

rvagg · 2016-05-31T04:33:08Z

@nodejs/build we should probably reach out to distro packagers to get feedback on this, if it's not useful to them all then it's probably not worth it as it is. Who do we have? @kapouer is the only one I can think of off the top of my head.

kapouer · 2016-05-31T06:00:18Z

Yes, why not. I'll try to use that option later today and report back.

kapouer · 2016-05-31T07:30:20Z

When building a shared library, it is a common and good practice to give it a soname.
v8 contains an example of how to do so with gyp, just grep soname there.
I think the right soname for node is libnode.so.${process.versions.modules}.

bnoordhuis · 2016-05-31T07:46:17Z

/cc @sgallagher - see #6994 (comment).

sgallagher · 2016-05-31T13:53:27Z

Passing the buck over to @thughes

mhdawson · 2016-05-31T15:01:02Z

We are interested in it because we have internal product teams that what to include Node.js in a product without having a separate process. So I don't see that it should be tied to the distro packages. If they say it would be useful to have a shared library but this does not address the requirement then we need that input and need to adjust. If on the other hand they just don't need a shared library I don't think that invalidates its usefulness as we have examples like electron and our internal team which do want it as a shared library.

kapouer · 2016-05-31T16:40:00Z

@mhdawson on the debian side i'm pretty sure it's going to be useful, one way or another.

mhdawson · 2016-05-31T16:57:59Z

@kapouer good to hear.

stefanmb · 2016-05-31T18:54:48Z

@thealphanerd Rebased onto master, thanks.

@rvagg @mhdawson In addition to Michael's comments, I think the need for this feature was strongly expressed in #5918, nodejs/node-v0.x-archive#7336, and nodejs/roadmap#9. It is my intention to get this feature working on all platforms starting with Linux in this PR.

@kapouer Thank you for the soname suggestion, I've implemented it (and fixed the install.py script which I'd previously broken due to some bad merging, sorry!). It required a few hacks to get the information in the right place but the end product will now be "libnode.so.${process.versions.modules}" as you indicated. This may be a bit counter-intuitive to regular users since the module version does not map to the Node release version in an obvious way, but I believe it makes sense to provide this naming convention for embedders.

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

stefanmb · 2016-05-31T19:56:21Z

One unrelated failure (socket timeout) on ARM: https://ci.nodejs.org/job/node-test-binary-arm/RUN_SUBSET=0,label=pi1-raspbian-wheezy/2368/console

jbergstroem · 2016-06-01T07:12:05Z

@kapouer do you know of any packages that would use this?

kapouer · 2016-06-01T08:04:31Z

@jbergstroem besides addons (it's more natural to link them to libnode) no, not yet. Electron, eventually...

jbergstroem · 2016-06-01T15:15:54Z

@kapouer ok, thanks. How about libv8 bindings? Would any of those deps overflow/consider switching?

kapouer · 2016-06-01T15:23:27Z

Could you be a little more specific ? I don't understand the question.

jbergstroem · 2016-06-02T06:24:32Z

@kapouer sorry; was just wondering if packages that builds against a shared v8 would/could be interested in this.

kapouer · 2016-06-02T06:37:32Z

@jbergstroem i'm not sure it would be a good idea to link to libnode for a software that just needs v8. Maybe one day libnode will expose only nan-like symbols ?

stefanmb · 2016-06-02T21:53:07Z

Based on some comments from @mhdawson I've made the following changes:

Rebase again onto master.
Fixed NODE_CTOR_PREFIX to be guarded under the correct NODE_SHARED_MODE define. This define will remove static from the DSO constructors for the modules when in shared library mode. It's not strictly needed to do so in order to have the constructor symbols exported, but Electron uses them so I've kept compatibility, see build: add --enable-shared and --enable-static node-v0.x-archive#7336 (comment) for details.
Remove superfluous dependencies in node.gyp.
Fixed some rules in node.gyp to check for no_bundled_v8 instead of node_shared

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

mhdawson · 2016-06-08T16:35:52Z

LGTM with one question I'd like to have confirmation for. I assume that post-mortem and vtune support is disabled for node_no_bundled_v8 because we need the specific v8 version/code to support those which we don't have when that option is supported, right ?

stefanmb · 2016-06-08T17:33:20Z

@mhdawson Those options both introduce dependencies from the bundled V8 folder:
deps/v8/src/third_party/vtune/v8vtune.gyp:v8_vtune and deps/v8/tools/gyp/v8.gyp:postmortem-metadata respectively. The node_no_bundled_v8 option is meant to exclude dependencies on the bundled V8.

Updates to build the shared library version of node on AIX. Adds the same functionality to AIX that was added on Linux under this: Ref: nodejs#6994

Updates to build the shared library version of node on AIX. Adds the same functionality to AIX that was added on Linux under this: Ref: #6994 PR-URL: #9675 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michael Dawson <[email protected]>

Updates to build the shared library version of node on AIX. Adds the same functionality to AIX that was added on Linux under this: Ref: nodejs#6994 PR-URL: nodejs#9675 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michael Dawson <[email protected]>

Required to support the shared library builds on AIX - this sets the shared library suffix within GYP to .a instead of .so on AIX My patch: https://codereview.chromium.org/2492233002/ was landed as as part of this one which fixed some other (not required, but included for completeness of the backport) changes: Ref: https://codereview.chromium.org/2511733005/

Updates to build the shared library version of node on AIX. Adds the same functionality to AIX that was added on Linux under this: Ref: #6994 PR-URL: #9675 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michael Dawson <[email protected]>

This LTS release comes with 108 commits. This includes 30 which are doc related, 28 which are test related, 16 which are build / tool related, and 4 commits which are updates to dependencies. Notable Changes: The SEMVER-MINOR changes include: * build: - export openssl symbols on Windows making it possible to build addons linked against the bundled version of openssl (Alex Hultman) #7576 * debugger: - make listen address configurable in the debugger server (Ben Noordhuis) #3316 * dgram: - generalized send queue to handle close fixing a potential throw when dgram socket is closed in the listening event handler. (Matteo Collina) #7066 * http: - Introduce the 451 status code "Unavailable For Legal Reasons" (Max Barinov) #4377 * tls: - introduce `secureContext` for `tls.connect` which is useful for caching client certificates, key, and CA certificates. (Fedor Indutny) #4246 Notable SEMVER-PATCH changes include: * build: - introduce the configure --shared option for embedders (sxa555) #6994 * gtest: - the test reporter now outputs tap comments as yamlish (Johan Bergström) #9262 * src: - node no longer aborts when c-ares initialization fails (Ben Noordhuis) #8710 * tls: - fix memory leak when writing data to TLSWrap instance during handshake (Fedor Indutny) #9586 PR-URL: #9736

This LTS release comes with 108 commits. This includes 30 which are doc related, 28 which are test related, 16 which are build / tool related, and 4 commits which are updates to dependencies. Notable Changes: The SEMVER-MINOR changes include: * build: - export openssl symbols on Windows making it possible to build addons linked against the bundled version of openssl (Alex Hultman) nodejs/node#7576 * debugger: - make listen address configurable in the debugger server (Ben Noordhuis) nodejs/node#3316 * dgram: - generalized send queue to handle close fixing a potential throw when dgram socket is closed in the listening event handler. (Matteo Collina) nodejs/node#7066 * http: - Introduce the 451 status code "Unavailable For Legal Reasons" (Max Barinov) nodejs/node#4377 * tls: - introduce `secureContext` for `tls.connect` which is useful for caching client certificates, key, and CA certificates. (Fedor Indutny) nodejs/node#4246 Notable SEMVER-PATCH changes include: * build: - introduce the configure --shared option for embedders (sxa555) nodejs/node#6994 * gtest: - the test reporter now outputs tap comments as yamlish (Johan Bergstrom) nodejs/node#9262 * src: - node no longer aborts when c-ares initialization fails (Ben Noordhuis) nodejs/node#8710 * tls: - fix memory leak when writing data to TLSWrap instance during handshake (Fedor Indutny) nodejs/node#9586 PR-URL: nodejs/node#9736 Signed-off-by: Ilkka Myller <[email protected]>

Updates to build the shared library version of node on AIX. Adds the same functionality to AIX that was added on Linux under this: Ref: #6994 PR-URL: #9675 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michael Dawson <[email protected]>

dominictarr · 2016-12-19T16:46:39Z

My understanding is also that this would be essential for running on a unrooted andoid. you get one process, which must be java, but you can link a .so which could be libnode.so. That is how jxcore (RIP) did it.

MylesBorins · 2016-12-19T16:51:46Z

@dominictarr you should find this commit now in the latest v4, v6, and v7

I believe there are still other edges regarding android... let me know if you try and get it working

Updates to build the shared library version of node on AIX. Adds the same functionality to AIX that was added on Linux under this: Ref: #6994 PR-URL: #9675 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michael Dawson <[email protected]>

stefanmb added c++ Issues and PRs that require attention from people who are familiar with C++. install Issues and PRs related to the installers. build Issues and PRs related to build files or the CI. labels May 26, 2016

stefanmb self-assigned this May 26, 2016

stefanmb mentioned this pull request May 26, 2016

configure: introduce --embedded #5918

Closed

4 tasks

mhdawson mentioned this pull request May 30, 2016

Building node as a shared library instead of an executable nodejs/roadmap#9

Closed

stefanmb force-pushed the sharedlib-pr1 branch from 33c0e4c to b3bf28a Compare May 31, 2016 18:53

stefanmb force-pushed the sharedlib-pr1 branch 2 times, most recently from 16b3dbf to 69fca3e Compare June 2, 2016 21:43

sxa pushed a commit to sxa/node that referenced this pull request Nov 21, 2016

build: add shared library support to AIX build

63143f3

Updates to build the shared library version of node on AIX. Adds the same functionality to AIX that was added on Linux under this: Ref: nodejs#6994

MylesBorins mentioned this pull request Nov 22, 2016

v4.7.0 proposal #9736

Merged

eljefedelrodeodeljefe mentioned this pull request Dec 19, 2016

build, api, docs: android support #10341

Closed

6 tasks

empyrical mentioned this pull request Nov 4, 2018

N-API: An api for embedding Node in applications #23265

Open

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: introduce configure --shared #6994

build: introduce configure --shared #6994

stefanmb commented May 26, 2016

stefanmb commented May 26, 2016

MylesBorins commented May 31, 2016

rvagg commented May 31, 2016

kapouer commented May 31, 2016

kapouer commented May 31, 2016

bnoordhuis commented May 31, 2016

sgallagher commented May 31, 2016

mhdawson commented May 31, 2016

kapouer commented May 31, 2016

mhdawson commented May 31, 2016

stefanmb commented May 31, 2016

stefanmb commented May 31, 2016

jbergstroem commented Jun 1, 2016

kapouer commented Jun 1, 2016

jbergstroem commented Jun 1, 2016

kapouer commented Jun 1, 2016

jbergstroem commented Jun 2, 2016

kapouer commented Jun 2, 2016

stefanmb commented Jun 2, 2016

mhdawson commented Jun 8, 2016

stefanmb commented Jun 8, 2016

dominictarr commented Dec 19, 2016

MylesBorins commented Dec 19, 2016

build: introduce configure --shared #6994

build: introduce configure --shared #6994

Conversation

stefanmb commented May 26, 2016

Checklist

Affected core subsystem(s)

Description of change

stefanmb commented May 26, 2016

MylesBorins commented May 31, 2016

rvagg commented May 31, 2016

kapouer commented May 31, 2016

kapouer commented May 31, 2016

bnoordhuis commented May 31, 2016

sgallagher commented May 31, 2016

mhdawson commented May 31, 2016

kapouer commented May 31, 2016

mhdawson commented May 31, 2016

stefanmb commented May 31, 2016

stefanmb commented May 31, 2016

jbergstroem commented Jun 1, 2016

kapouer commented Jun 1, 2016

jbergstroem commented Jun 1, 2016

kapouer commented Jun 1, 2016

jbergstroem commented Jun 2, 2016

kapouer commented Jun 2, 2016

stefanmb commented Jun 2, 2016

mhdawson commented Jun 8, 2016

stefanmb commented Jun 8, 2016

dominictarr commented Dec 19, 2016

MylesBorins commented Dec 19, 2016