-
Notifications
You must be signed in to change notification settings - Fork 29.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bundled zlib is compiled despite the --shared-zlib configure-flag #10649
Comments
addaleax
added
build
Issues and PRs related to build files or the CI.
zlib
Issues and PRs related to the zlib subsystem.
labels
Jan 6, 2017
The bundled zlib is used by the C++ test suite, that is why. The fix is probably this: diff --git a/node.gyp b/node.gyp
index abba4c3..5075c51 100644
--- a/node.gyp
+++ b/node.gyp
@@ -895,29 +895,33 @@
],
'conditions': [
['v8_inspector=="true"', {
'defines': [
'HAVE_INSPECTOR=1',
],
'dependencies': [
- 'deps/zlib/zlib.gyp:zlib',
'v8_inspector_compress_protocol_json#host'
],
'include_dirs': [
'<(SHARED_INTERMEDIATE_DIR)'
],
'sources': [
'src/inspector_socket.cc',
'src/inspector_socket_server.cc',
'test/cctest/test_inspector_socket.cc',
'test/cctest/test_inspector_socket_server.cc'
],
'conditions': [
+ [ 'node_shared_zlib=="false"', {
+ 'dependencies': [
+ 'deps/zlib/zlib.gyp:zlib',
+ ]
+ }],
[ 'node_shared_openssl=="false"', {
'dependencies': [
'deps/openssl/openssl.gyp:openssl'
]
}],
[ 'node_shared_http_parser=="false"', {
'dependencies': [
'deps/http_parser/http_parser.gyp:http_parser' Contributors, feel free to steal. I'm not going to PR it, I'm on holiday! |
Will test/steal ^ |
2 tasks
gibfahn
added a commit
to gibfahn/node
that referenced
this issue
Jan 29, 2017
Even if the --shared-zlib flag was used, the bundled deps/zlib was still being compiled into the binary as it was required by the C++ test suite. PR-URL: nodejs#10657 Fixes: nodejs#10649 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
evanlucas
pushed a commit
that referenced
this issue
Jan 31, 2017
Even if the --shared-zlib flag was used, the bundled deps/zlib was still being compiled into the binary as it was required by the C++ test suite. PR-URL: #10657 Fixes: #10649 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
All operating systems out there -- except, perhaps, Windows -- already include libz. However, when building node with the
--shared-zlib
flag, the bundled version of the library is compiled anyway -- even if thenode
-executable duly uses the OS-provded library (/lib/libz.so.6
in my case).This is both wasteful and dangerous, because some day it may happen, that a bundled header-file will be
#include
-ed, that will somehow disagree with the implementation provided by the OS. It should be possible to build node withdeps/zlib
completely absent -- for safety.Other bundled libraries (cares, uv) do not seem to be mishandled this way -- only zlib.
The text was updated successfully, but these errors were encountered: