build: add --enable-shared and --enable-static

[bnoordhuis]

Add configure switches for building a shared or static library.

The switches are mutually exclusive for now; building both a shared and
a static library at the same time requires a fair amount of overhaul.

Caveat emptor: the static library (libnode.a) builds but is not useful
yet, it doesn't find built-in modules like contextify at run-time.

The prime suspect is commit 76b9846 ("node: register modules from DSO
constructors"); it looks like the constructors aren't linked into the
final binary.

R=@indutny

[Nodejs-Jenkins]

Thank you for contributing this pull request! Here are a few pointers to make sure your submission will be considered for inclusion.

The following commiters were not found in the CLA:

• Ben Noordhuis

You can fix all these things without opening another issue.

Please see CONTRIBUTING.md for more information

[bnoordhuis]

In case you're wondering, three people asked me about this in the last two weeks (where this == how to build a library and the run-time error with the static library) so I figured I'd just write the patch and point people at it. :-)

[tjfontaine]

I am very interested in having this as an option for node, but right now we have perhaps the worst concept about what is and isn't our public api -- so I'm not quite ready to make this easier for people until we can solidify that.

[bnoordhuis]

I feel that but if it helps: two out of the three people that approached me just wanted a way to start node in-process - the only public API they're interested in is node::Start().

If it helps, I can hide the configure switches from the output of --help and add 'here be dragons' comments.

[zcbenz]

Caveat emptor: the static library (libnode.a) builds but is not useful
yet, it doesn't find built-in modules like contextify at run-time.

The prime suspect is commit 76b9846 ("node: register modules from DSO
constructors"); it looks like the constructors aren't linked into the
final binary.

I encountered this when upgrading the node fork in atom/atom-shell, I checked the final binary and some modules' DSO constructors were not found in it, the found ones were only:

0000000000088f00 T __register_buffer
0000000000103790 T __register_crypto
00000000000ac8e0 T __register_pipe_wrap
00000000000b1c50 T __register_smalloc
00000000000ca840 T __register_tcp_wrap
000000000012a110 T __register_tls_wrap
00000000000d1e50 T __register_udp_wrap

The DSO constructors' symbols were correct in each module's object file, so it should a linker bug, and my investigation has stopped here.

[zcbenz]

Found this post when investigating on this, so this is indeed an expected behavior of linker when building node as static library.

[zcbenz]

I have got a workaround for the linking problem when building node as static library, first we need to export the DSO constructors, then reference the DSO constructors](electron/electron@69adff1) somewhere else to force the linker to include them.

[bnoordhuis]

@zcbenz I don't think that's a general solution; it will neatly address the issue with built-in modules but making constructors public is bound to result in symbol clashes between third-party modules. Perhaps you can add the static keyword when BUILDING_NODE_EXTENSION is defined?

[indutny]

@bnoordhuis still relevant?

[bnoordhuis]

Well, it won't merge cleanly if that is what you mean and the constructor issue still exists but I do believe it's a useful feature. I can dust it off if people feel the same way.

[trevnorris]

@bnoordhuis heck, why not. if it's not much effort don't see why it shouldn't be included.

[bnoordhuis]

@trevnorris Updated. The workaround for the module registration issue is somewhat pervasive but it seemed cleanest to me.

[zcbenz]

👍

[trevnorris]

@indutny I'm reviewing/testing it now, but would also like your eyes as a sanity check.

[trevnorris]

@bnoordhuis I ran ./configure --enable-static; make -j8 and when the build was complete it had still created a symlink node -> out/Release/node which was broken.

Edit: Same thing happens w/ --enable-shared.

[tjfontaine]

I really don't want to drop the way the modules are being loaded here, we had to do similar work arounds for windows to not drop the initializers. We should try and figure out a way around that.

Initialization of modules through the constructor mechanism provides us the opportunity for people to include binary addons during build without code modification, but can be passed as a configuration option that defines the path to the object that should be linked in

We still don't really have a good conversation about what our supportable interface is for Node, beyond Start(argc, argv)

[bnoordhuis]

I ran ./configure --enable-static; make -j8 and when the build was complete it had still created a symlink node -> out/Release/node which was broken.

I'm not sure how to stop that from happening without a nasty hack to the Makefile (write build type to config.mk, then wrap ln in ifdefs in the Makefile.) It looks pretty gnarly because that section already ifdefs on USE_NINJA but I can make that change if you want. Moving it into a Makefile function would also work but I don't think it'll look any prettier.

Alternatively, I can add a documentation note somewhere explaining that you should build the library with make -C out BUILDTYPE=<type>.

I really don't want to drop the way the modules are being loaded here, we had to do similar work arounds for windows to not drop the initializers. We should try and figure out a way around that.

I'm open to suggestions. The only way I know of to make it work is linking the final executable with -Wl,--whole-archive,libnode.a,--no-whole-archive but that seems like a heavy-handed approach.

It also pushes the responsibility to the consumer of the archive, making it harder to use. That's something I'd like to avoid if possible.

We still don't really have a good conversation about what our supportable interface is for Node, beyond Start(argc, argv)

Commits da30c00 and ba09409 by @deanm export node::Init() and node::CreateEnvironment() and makes the latter take a Local<Context> argument. I think that's enough to get a usable node instance up and running.

[deanm]

Just a note on the original problem, if you are looking at solving the constructor problem without explicitly referencing the symbols, it should just be a matter of using the archive with --whole-archive (gcc) / -force_load (osx). Not sure what's required on Windows but likely something similar.

[ghost]

If your linker so requires (it seems like the illumos linker does not, as I cannot reproduce this behaviour), then --whole-archive or -zallextract, or whatever equivalent your linker offers, is not heavy-handed; it is in fact the only correct solution. The alternative is to teach your linker about init and fini sections, but even if you so modified every linker in the world there would still be older ones in use. This is not a bug in the DSO loader, it is a basic shortcoming of static libraries, which is why most modern systems no longer make use of static libraries. Use a shared library, or tell the linker what you want it to do. Please don't change the DSO loader to work around buggy linkers and/or incorrect linker flags in consumers.

[trevnorris]

@bnoordhuis Eh, I'm not worried about it. Maybe you can just conditionally rm the symlink at the end of the build. ;-P

[bnoordhuis]

I'm going to close this PR because it's well and truly stale by now.