Skip to content
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

deps: reintroduce supporting shared c-ares builds #5775

Merged

Conversation

@jbergstroem
Copy link
Member

@jbergstroem jbergstroem commented Mar 18, 2016

Pull Request check-list

  • Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
    this change (including linting)?
  • Is the commit message formatted according to CONTRIBUTING.md?
  • If this change fixes a bug (or a performance problem), is a regression
    test (or a benchmark) included?
  • Is a documentation update included (if this change modifies
    existing APIs, or introduces new ones)?

Affected core subsystem(s)

build, deps

Description of change

As of cc192f0 we've now in sync with upstream which means that we also can allow our users to build against a shared version of c-ares.

Note: It is still up to users to make sure that the library version is on par with what Node.js bundles.

This "reverts" commit 25fa5c4.

Tested on gentoo with a shared version c-ares. Tests pass.

/CC @indutny, @bnoordhuis

@jbergstroem
Copy link
Member Author

@jbergstroem jbergstroem commented Mar 18, 2016

@indutny
Copy link
Member

@indutny indutny commented Mar 18, 2016

LGTM if it works. Thank you!

@jbergstroem
Copy link
Member Author

@jbergstroem jbergstroem commented Mar 18, 2016

(@nodejs/build should probably be CC:ed as well)

@@ -8,6 +8,7 @@
'node_has_winsdk%': 'false',
'node_shared_zlib%': 'false',
'node_shared_http_parser%': 'false',
'node_shared_libcares%': 'false',
Copy link
Member

@bnoordhuis bnoordhuis Mar 18, 2016

We previously called it node_shared_cares and it might be nice to keep that name in the (admittedly unlikely) case that people check for process.config.variables.node_shared_cares.

Copy link
Member Author

@jbergstroem jbergstroem Mar 21, 2016

The reason it's changed is that we now use the shared configure_library so I have to map it to what the .pc file contains. Should be fixable though.

@jasnell
Copy link
Member

@jasnell jasnell commented Mar 18, 2016

LGTM

@jbergstroem
Copy link
Member Author

@jbergstroem jbergstroem commented Mar 21, 2016

@bnoordhuis just pushed a version that uses the old name.

To test this "properly" you likely have to build c-ares with ./configure -enable-symbol-hiding; otherwise c-ares will suffix the exposed api. In short: the homebrew version won't work.

@jbergstroem
Copy link
Member Author

@jbergstroem jbergstroem commented Mar 21, 2016

Here's some output from a gentoo-system, (built with: ./configure --shared-cares --shared-zlib --shared-openssl --shared-http-parser --shared-libuv):

(staging) ~/node-v5.9.0 $ ldd node
    linux-vdso.so.1 (0x00007ffef2ba9000)
    libz.so.1 => /lib64/libz.so.1 (0x00007fe083064000)
    libhttp_parser.so.2.6.2 => /usr/lib64/libhttp_parser.so.2.6.2 (0x00007fe082e5c000)
    libuv.so.1 => /usr/lib64/libuv.so.1 (0x00007fe082c38000)
    librt.so.1 => /lib64/librt.so.1 (0x00007fe082a30000)
    libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fe082814000)
    libnsl.so.1 => /lib64/libnsl.so.1 (0x00007fe0825fc000)
    libdl.so.2 => /lib64/libdl.so.2 (0x00007fe0823f8000)
    libcares.so.2 => /usr/lib64/libcares.so.2 (0x00007fe0821e6000)
    libssl.so.1.0.0 => /usr/lib64/libssl.so.1.0.0 (0x00007fe081f70000)
    libcrypto.so.1.0.0 => /usr/lib64/libcrypto.so.1.0.0 (0x00007fe081b34000)
    libstdc++.so.6 => /usr/lib/gcc/x86_64-pc-linux-gnu/5.3.0/libstdc++.so.6 (0x00007fe0817b2000)
    libm.so.6 => /lib64/libm.so.6 (0x00007fe0814bc000)
    libgcc_s.so.1 => /usr/lib/gcc/x86_64-pc-linux-gnu/5.3.0/libgcc_s.so.1 (0x00007fe0812a5000)
    libc.so.6 => /lib64/libc.so.6 (0x00007fe080f09000)
    /lib64/ld-linux-x86-64.so.2 (0x00007fe08327a000)
(staging) ~/node-v5.9.0 $ make check
make -C out BUILDTYPE=Release V=1
make[1]: Entering directory '/home/jbergstroem/node-v5.9.0/out'
make[1]: Nothing to be done for 'all'.
make[1]: Leaving directory '/home/jbergstroem/node-v5.9.0/out'
ln -fs out/Release/node node
Running main() from gtest_main.cc
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from UtilTest
[ RUN      ] UtilTest.ListHead
[       OK ] UtilTest.ListHead (0 ms)
[----------] 1 test from UtilTest (0 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (0 ms total)
[  PASSED  ] 1 test.
/usr/lib/python-exec/python2.7/python tools/test.py --mode=release message parallel sequential -J
[01:39|% 100|+ 1046|-   0]: Done

configure_library('libcares', output)
# stay backwards compatible with shared cares builds
output['variables']['node_shared_cares'] = \
output['variables'].pop('node_shared_libcares')
Copy link
Member

@bnoordhuis bnoordhuis Mar 21, 2016

Four space indent.

Copy link
Member Author

@jbergstroem jbergstroem Mar 21, 2016

We have a lot of stuff doing 2. Should we perhaps pep8 configure in a separate PR?

Copy link
Member

@bnoordhuis bnoordhuis Mar 21, 2016

I'm no fan of lint commits, but I like new code to have proper indentation.

@jbergstroem
Copy link
Member Author

@jbergstroem jbergstroem commented Mar 27, 2016

@bnoordhuis ok with the latest revision?

@saghul
Copy link
Member

@saghul saghul commented Mar 29, 2016

FWIW, upstream does not include this patch: 72c5458

@jbergstroem
Copy link
Member Author

@jbergstroem jbergstroem commented Mar 29, 2016

@saghul yeah, its known. Thanks for the notice though.

Edit: the reason at least I don't care much is because we don't build shared libraries on Windows.

@jbergstroem
Copy link
Member Author

@jbergstroem jbergstroem commented Apr 4, 2016

ping @bnoordhuis.

@bnoordhuis
Copy link
Member

@bnoordhuis bnoordhuis commented Apr 5, 2016

LGTM. Sorry for the delay.

@jbergstroem jbergstroem force-pushed the feature/revert-shared-cares-25fa5c4c branch from 63a23fa to c89a49d Apr 5, 2016
@jbergstroem
Copy link
Member Author

@jbergstroem jbergstroem commented Apr 5, 2016

As of cc192f0 we've now in sync with upstream which means that we
also can allow our users to build against a shared version of c-ares.

Note: It is still up to users to make sure that the library version
is on par with what Node.js bundles.

This "reverts" commit 25fa5c4.

PR-URL: nodejs#5775
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@jbergstroem jbergstroem force-pushed the feature/revert-shared-cares-25fa5c4c branch from c89a49d to 2253be9 Apr 5, 2016
@jbergstroem jbergstroem merged commit 2253be9 into nodejs:master Apr 5, 2016
@jasnell jasnell mentioned this pull request Apr 19, 2016
jasnell added a commit that referenced this issue Apr 26, 2016
The following significant (semver-major) changes have been made since the
previous Node v5.0.0 release.

* Buffer
  * New Buffer constructors have been added
    [#4682](#4682)
  * Previously deprecated Buffer APIs are removed
    [#5048](#5048),
    [#4594](#4594)
  * Improved error handling [#4514](#4514)
* Cluster
  * Worker emitted as first argument in 'message' event
    [#5361](#5361).
* Crypto
  * Improved error handling [#3100](#3100),
    [#5611](#5611)
  * Simplified Certificate class bindings
    [#5382](#5382)
  * Improved control over FIPS mode
    [#5181](#5181)
  * pbkdf2 digest overloading is deprecated
    [#4047](#4047)
* Dependencies
  * Reintroduce shared c-ares build support
    [#5775](#5775).
  * V8 updated to 5.0.71.31 [#6111](#6111).
* DNS
  * Add resolvePtr API to query plain DNS PTR records
    [#4921](#4921).
* Domains
  * Clear stack when no error handler
  [#4659](#4659).
* File System
  * The `fs.realpath()` and `fs.realpathSync()` methods have been updated
    to use a more efficient libuv implementation. This change includes the
    removal of the `cache` argument and the method can throw new errors
    [#3594](#3594)
  * FS apis can now accept and return paths as Buffers
    [#5616](#5616).
  * Error handling and type checking improvements
    [#5616](#5616),
    [#5590](#5590),
    [#4518](#4518),
    [#3917](#3917).
  * fs.read's string interface is deprecated
    [#4525](#4525)
* HTTP
  * 'clientError' can now be used to return custom errors from an
    HTTP server [#4557](#4557).
* Modules
  * Current directory is now prioritized for local lookups
    [#5689](#5689)
  * Symbolic links are preserved when requiring modules
    [#5950](#5950)
* Net
  * DNS hints no longer implicitly set
    [#6021](#6021).
  * Improved error handling and type checking
    [#5981](#5981),
    [#5733](#5733),
    [#2904](#2904)
* Path
  * Improved type checking [#5348](#5348).
* Process
  * Introduce process warnings API
    [#4782](#4782).
  * Throw exception when non-function passed to nextTick
    [#3860](#3860).
* Readline
  * Emit key info unconditionally
    [#6024](#6024)
* REPL
  * Assignment to `_` will emit a warning.
    [#5535](#5535)
* Timers
  * Fail early when callback is not a function
    [#4362](#4362)
* TLS
  * Rename 'clientError' to 'tlsClientError'
    [#4557](#4557)
  * SHA1 used for sessionIdContext
    [#3866](#3866)
* TTY
  * Previously deprecated setRawMode wrapper is removed
    [#2528](#2528).
* Util
  * Changes to Error object formatting
    [#4582](#4582).
* Windows
  * Windows XP and Vista are no longer supported
    [#5167](#5167),
    [#5167](#5167).
jasnell added a commit that referenced this issue Apr 26, 2016
The following significant (semver-major) changes have been made since the
previous Node v5.0.0 release.

* Buffer
  * New Buffer constructors have been added
    [#4682](#4682)
  * Previously deprecated Buffer APIs are removed
    [#5048](#5048),
    [#4594](#4594)
  * Improved error handling [#4514](#4514)
* Cluster
  * Worker emitted as first argument in 'message' event
    [#5361](#5361).
* Crypto
  * Improved error handling [#3100](#3100),
    [#5611](#5611)
  * Simplified Certificate class bindings
    [#5382](#5382)
  * Improved control over FIPS mode
    [#5181](#5181)
  * pbkdf2 digest overloading is deprecated
    [#4047](#4047)
* Dependencies
  * Reintroduce shared c-ares build support
    [#5775](#5775).
  * V8 updated to 5.0.71.31 [#6111](#6111).
* DNS
  * Add resolvePtr API to query plain DNS PTR records
    [#4921](#4921).
* Domains
  * Clear stack when no error handler
  [#4659](#4659).
* File System
  * The `fs.realpath()` and `fs.realpathSync()` methods have been updated
    to use a more efficient libuv implementation. This change includes the
    removal of the `cache` argument and the method can throw new errors
    [#3594](#3594)
  * FS apis can now accept and return paths as Buffers
    [#5616](#5616).
  * Error handling and type checking improvements
    [#5616](#5616),
    [#5590](#5590),
    [#4518](#4518),
    [#3917](#3917).
  * fs.read's string interface is deprecated
    [#4525](#4525)
* HTTP
  * 'clientError' can now be used to return custom errors from an
    HTTP server [#4557](#4557).
* Modules
  * Current directory is now prioritized for local lookups
    [#5689](#5689)
  * Symbolic links are preserved when requiring modules
    [#5950](#5950)
* Net
  * DNS hints no longer implicitly set
    [#6021](#6021).
  * Improved error handling and type checking
    [#5981](#5981),
    [#5733](#5733),
    [#2904](#2904)
* Path
  * Improved type checking [#5348](#5348).
* Process
  * Introduce process warnings API
    [#4782](#4782).
  * Throw exception when non-function passed to nextTick
    [#3860](#3860).
* Readline
  * Emit key info unconditionally
    [#6024](#6024)
* REPL
  * Assignment to `_` will emit a warning.
    [#5535](#5535)
* Timers
  * Fail early when callback is not a function
    [#4362](#4362)
* TLS
  * Rename 'clientError' to 'tlsClientError'
    [#4557](#4557)
  * SHA1 used for sessionIdContext
    [#3866](#3866)
* TTY
  * Previously deprecated setRawMode wrapper is removed
    [#2528](#2528).
* Util
  * Changes to Error object formatting
    [#4582](#4582).
* Windows
  * Windows XP and Vista are no longer supported
    [#5167](#5167),
    [#5167](#5167).
jasnell added a commit that referenced this issue Apr 26, 2016
The following significant (semver-major) changes have been made since the
previous Node v5.0.0 release.

* Buffer
  * New Buffer constructors have been added
    [#4682](#4682)
  * Previously deprecated Buffer APIs are removed
    [#5048](#5048),
    [#4594](#4594)
  * Improved error handling [#4514](#4514)
* Cluster
  * Worker emitted as first argument in 'message' event
    [#5361](#5361).
* Crypto
  * Improved error handling [#3100](#3100),
    [#5611](#5611)
  * Simplified Certificate class bindings
    [#5382](#5382)
  * Improved control over FIPS mode
    [#5181](#5181)
  * pbkdf2 digest overloading is deprecated
    [#4047](#4047)
* Dependencies
  * Reintroduce shared c-ares build support
    [#5775](#5775).
  * V8 updated to 5.0.71.31 [#6111](#6111).
* DNS
  * Add resolvePtr API to query plain DNS PTR records
    [#4921](#4921).
* Domains
  * Clear stack when no error handler
  [#4659](#4659).
* File System
  * The `fs.realpath()` and `fs.realpathSync()` methods have been updated
    to use a more efficient libuv implementation. This change includes the
    removal of the `cache` argument and the method can throw new errors
    [#3594](#3594)
  * FS apis can now accept and return paths as Buffers
    [#5616](#5616).
  * Error handling and type checking improvements
    [#5616](#5616),
    [#5590](#5590),
    [#4518](#4518),
    [#3917](#3917).
  * fs.read's string interface is deprecated
    [#4525](#4525)
* HTTP
  * 'clientError' can now be used to return custom errors from an
    HTTP server [#4557](#4557).
* Modules
  * Current directory is now prioritized for local lookups
    [#5689](#5689)
  * Symbolic links are preserved when requiring modules
    [#5950](#5950)
* Net
  * DNS hints no longer implicitly set
    [#6021](#6021).
  * Improved error handling and type checking
    [#5981](#5981),
    [#5733](#5733),
    [#2904](#2904)
* OS X
  * MACOSX_DEPLOYMENT_TARGET has been bumped up to 10.7
    [#6402](#6402).
* Path
  * Improved type checking [#5348](#5348).
* Process
  * Introduce process warnings API
    [#4782](#4782).
  * Throw exception when non-function passed to nextTick
    [#3860](#3860).
* Readline
  * Emit key info unconditionally
    [#6024](#6024)
* REPL
  * Assignment to `_` will emit a warning.
    [#5535](#5535)
* Timers
  * Fail early when callback is not a function
    [#4362](#4362)
* TLS
  * Rename 'clientError' to 'tlsClientError'
    [#4557](#4557)
  * SHA1 used for sessionIdContext
    [#3866](#3866)
* TTY
  * Previously deprecated setRawMode wrapper is removed
    [#2528](#2528).
* Util
  * Changes to Error object formatting
    [#4582](#4582).
* Windows
  * Windows XP and Vista are no longer supported
    [#5167](#5167),
    [#5167](#5167).
jasnell added a commit that referenced this issue Apr 26, 2016
The following significant (semver-major) changes have been made since the
previous Node v5.0.0 release.

* Buffer
  * New Buffer constructors have been added
    [#4682](#4682)
  * Previously deprecated Buffer APIs are removed
    [#5048](#5048),
    [#4594](#4594)
  * Improved error handling [#4514](#4514)
* Cluster
  * Worker emitted as first argument in 'message' event
    [#5361](#5361).
* Crypto
  * Improved error handling [#3100](#3100),
    [#5611](#5611)
  * Simplified Certificate class bindings
    [#5382](#5382)
  * Improved control over FIPS mode
    [#5181](#5181)
  * pbkdf2 digest overloading is deprecated
    [#4047](#4047)
* Dependencies
  * Reintroduce shared c-ares build support
    [#5775](#5775).
  * V8 updated to 5.0.71.31 [#6111](#6111).
* DNS
  * Add resolvePtr API to query plain DNS PTR records
    [#4921](#4921).
* Domains
  * Clear stack when no error handler
  [#4659](#4659).
* File System
  * The `fs.realpath()` and `fs.realpathSync()` methods have been updated
    to use a more efficient libuv implementation. This change includes the
    removal of the `cache` argument and the method can throw new errors
    [#3594](#3594)
  * FS apis can now accept and return paths as Buffers
    [#5616](#5616).
  * Error handling and type checking improvements
    [#5616](#5616),
    [#5590](#5590),
    [#4518](#4518),
    [#3917](#3917).
  * fs.read's string interface is deprecated
    [#4525](#4525)
* HTTP
  * 'clientError' can now be used to return custom errors from an
    HTTP server [#4557](#4557).
* Modules
  * Current directory is now prioritized for local lookups
    [#5689](#5689)
  * Symbolic links are preserved when requiring modules
    [#5950](#5950)
* Net
  * DNS hints no longer implicitly set
    [#6021](#6021).
  * Improved error handling and type checking
    [#5981](#5981),
    [#5733](#5733),
    [#2904](#2904)
* OS X
  * MACOSX_DEPLOYMENT_TARGET has been bumped up to 10.7
    [#6402](#6402).
* Path
  * Improved type checking [#5348](#5348).
* Process
  * Introduce process warnings API
    [#4782](#4782).
  * Throw exception when non-function passed to nextTick
    [#3860](#3860).
* Readline
  * Emit key info unconditionally
    [#6024](#6024)
* REPL
  * Assignment to `_` will emit a warning.
    [#5535](#5535)
* Timers
  * Fail early when callback is not a function
    [#4362](#4362)
* TLS
  * Rename 'clientError' to 'tlsClientError'
    [#4557](#4557)
  * SHA1 used for sessionIdContext
    [#3866](#3866)
* TTY
  * Previously deprecated setRawMode wrapper is removed
    [#2528](#2528).
* Util
  * Changes to Error object formatting
    [#4582](#4582).
* Windows
  * Windows XP and Vista are no longer supported
    [#5167](#5167),
    [#5167](#5167).
jasnell added a commit that referenced this issue Apr 26, 2016
The following significant (semver-major) changes have been made since the
previous Node v5.0.0 release.

* Buffer
  * New Buffer constructors have been added
    [#4682](#4682)
  * Previously deprecated Buffer APIs are removed
    [#5048](#5048),
    [#4594](#4594)
  * Improved error handling [#4514](#4514)
* Cluster
  * Worker emitted as first argument in 'message' event
    [#5361](#5361).
* Crypto
  * Improved error handling [#3100](#3100),
    [#5611](#5611)
  * Simplified Certificate class bindings
    [#5382](#5382)
  * Improved control over FIPS mode
    [#5181](#5181)
  * pbkdf2 digest overloading is deprecated
    [#4047](#4047)
* Dependencies
  * Reintroduce shared c-ares build support
    [#5775](#5775).
  * V8 updated to 5.0.71.31 [#6111](#6111).
* DNS
  * Add resolvePtr API to query plain DNS PTR records
    [#4921](#4921).
* Domains
  * Clear stack when no error handler
  [#4659](#4659).
* File System
  * The `fs.realpath()` and `fs.realpathSync()` methods have been updated
    to use a more efficient libuv implementation. This change includes the
    removal of the `cache` argument and the method can throw new errors
    [#3594](#3594)
  * FS apis can now accept and return paths as Buffers
    [#5616](#5616).
  * Error handling and type checking improvements
    [#5616](#5616),
    [#5590](#5590),
    [#4518](#4518),
    [#3917](#3917).
  * fs.read's string interface is deprecated
    [#4525](#4525)
* HTTP
  * 'clientError' can now be used to return custom errors from an
    HTTP server [#4557](#4557).
* Modules
  * Current directory is now prioritized for local lookups
    [#5689](#5689)
  * Symbolic links are preserved when requiring modules
    [#5950](#5950)
* Net
  * DNS hints no longer implicitly set
    [#6021](#6021).
  * Improved error handling and type checking
    [#5981](#5981),
    [#5733](#5733),
    [#2904](#2904)
* OS X
  * MACOSX_DEPLOYMENT_TARGET has been bumped up to 10.7
    [#6402](#6402).
* Path
  * Improved type checking [#5348](#5348).
* Process
  * Introduce process warnings API
    [#4782](#4782).
  * Throw exception when non-function passed to nextTick
    [#3860](#3860).
* Readline
  * Emit key info unconditionally
    [#6024](#6024)
* REPL
  * Assignment to `_` will emit a warning.
    [#5535](#5535)
* Timers
  * Fail early when callback is not a function
    [#4362](#4362)
* TLS
  * Rename 'clientError' to 'tlsClientError'
    [#4557](#4557)
  * SHA1 used for sessionIdContext
    [#3866](#3866)
* TTY
  * Previously deprecated setRawMode wrapper is removed
    [#2528](#2528).
* Util
  * Changes to Error object formatting
    [#4582](#4582).
* Windows
  * Windows XP and Vista are no longer supported
    [#5167](#5167),
    [#5167](#5167).
@chrmoritz chrmoritz mentioned this pull request Apr 27, 2016
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants