http2: Allow using a shared nghttp2 library

[jer-gentoo]

As nice as it is to bundle several libraries for builders' convenience, it also exposes builders to several kinds of security problems (until you release a new version with the bundled libraries updated) and it duplicates the number of versions of a library present on systems. For instance, with libcurl/curl installed and built against nghttp2, having a bundled (and older) version of libnghttp2 statically linked into /usr/bin/node duplicates the other version already present in /usr/lib. Additionally, the currently bundled version 1.22.0 has several problems that were already fixed in later versions, notably the current 1.24.0 which has been out since early July 2017.

[addaleax]

Additionally, the currently bundled version 1.22.0 has several problems that were already fixed in later versions, notably the current 1.24.0 which has been out since early July 2017.

Just fyi, the version bundled with Node uses the yet-unreleased patch in nghttp2/nghttp2@eb306f4 for performance optimizations, so I think you’ll need to wait for another release for a shared lib to be actually usable.

[jer-gentoo]

Thanks. I guess it would be nice if it turned out the node release that uses nghttp2_rcbuf_is_static() coincides with an nghttp2 release that includes it, and that with this change people can use an unreleased version of nghttp2 with an unreleased version of node. Everybody wins, I guess.

[addaleax]

@jer-gentoo @nodejs/http2 Do you know whether nghttp2 has a specific release cycle? We might just be able to ask for a release upstream

[addaleax]

PR for upgrading nghttp2 to v1.25.0: #14955

[addaleax]

LGTM, can confirm that this compiles & runs.

CI: https://ci.nodejs.org/job/node-test-commit/11926/

[jer-gentoo]

The only puzzling aspect of the way the node target is built is that the shared dependencies are not listed for that target, but for the cctest target instead. This means that if you configure the build system for --shared-*, the bundled dependencies get properly added even if you then don't build the cctest, and even if you are cross-compiling. That should be the subject of another exercise in getting gyp to work as it should.

[addaleax]

@jer-gentoo @nodejs/platform-windows This doesn’t seem to be working on Windows yet: https://ci.nodejs.org/job/node-compile-windows/11266/label=win-vs2017/console

[jer-gentoo]

@addaleax : Yes, it does look like the linker can't find a shared nghttp2 library, and hasn't been asked to:

                 'node_shared_cares': 'false',
                 'node_shared_http_parser': 'false',
                 'node_shared_libuv': 'false',
                 'node_shared_nghttp2': 'false',
                 'node_shared_openssl': 'false',
                 'node_shared_zlib': 'false',

But why does it then succeed in finding those other libraries and not nghttp2?

[refack]

AFAICT this needs to move to node.gypi

[jer-gentoo]

Isn't *.gypi generated by the configure script?

[refack]

Only config.gypi is runtime generated, node.gypi is human generated. AFAICT this is the relevant section

node/node.gypi

Lines 246 to 260 in ffed7b6

	[ 'node_shared_zlib=="false"', {
	'dependencies': [ 'deps/zlib/zlib.gyp:zlib' ],
	}],
	[ 'node_shared_http_parser=="false"', {
	'dependencies': [ 'deps/http_parser/http_parser.gyp:http_parser' ],
	}],
	[ 'node_shared_cares=="false"', {
	'dependencies': [ 'deps/cares/cares.gyp:cares' ],
	}],
	[ 'node_shared_libuv=="false"', {
	'dependencies': [ 'deps/uv/uv.gyp:libuv' ],
	}],

[refack]

You might need the condition for this target as well.

[refack]

@jer-gentoo I tried to look at the file, but it's not easy reasoning "analytically" about GYP files. Best tip I can give you is to try and fix this "empirically", by running ./configure before and after the patch and comparing the resulting .mk files in /out/.

If you have access to Windows compare the *.vcxproj files. If you don't have access to a Windows machine, you can ping me (here or on IRC) and I'll run it for you.

[jer-gentoo]

I don't have access to Windows.

[jasnell]

I will try to test this on windows later this week.

[BridgeAR]

Ping @jasnell

[jer-gentoo]

I opened #15256 which has a single clean commit.