[Tarantool-patches] [PATCH v2 1/2] build: link bundled libcurl with c-ares
Alexander Turenko
alexander.turenko at tarantool.org
Sat Feb 8 22:42:55 MSK 2020
I mostly okay with the patch, but expect several actions like filing
some issues against c-ares and curl and minor tweaks of the patch.
Re blocking in threaded resolver
--------------------------------
Let's file an issue to curl, because it is inappropriate to block an
application that expect libcurl to give a control if something would
block. An error like 'DNS resolver thread pool is exhausted' is better,
because it can be handled on an application side somehow:
* do other work / be responsible until free resolver threads will be
available;
* give a user an error for further requests;
* dynamically increase threads count (automatically or upon a user
request).
Re ExternalProject
------------------
In brief: let's file another issue as described below and postpone
further activities.
There is the way to link c-ares into libcurl w/o installing c-ares using
a pkgconfig file, because it allows to set different include and library
directories and is supported by libcurl's configure script (see [1]).
This however cannot be used with CMake (I didn't find a way). I propose
to file an issue (or even a PR) to curl to support different directories
for include and library files for c-ares (in CMake build). The
motivation is to use libcurl + c-ares as part of a parent build process
(like we going to do). It seems we just need to add two hints (one for
an include directory and one for a library directory) to
${CURL}/CMake/FindCARES.cmake (see [2]) like we do in
tarantool/modulekit (see [3]).
Anyway, I'm okay with ExternalProject() and so installation of c-ares as
a part of build process for now. Hopefully we'll change it in the future
and so we'll not need to adjust flags like -isysroot in our cmake files.
Re c-ares dependencies
----------------------
See [4].
Other
-----
BTW, maybe file another issue to lower minimal required CMake version is
c-ares to 2.8?
[1]: https://github.com/curl/curl/issues/2203
[2]: https://github.com/curl/curl/blob/5ce7102ceae250e2d31b54aad2f33b3bc35f243a/CMake/FindCARES.cmake
[3]: https://github.com/tarantool/modulekit/blob/0994d76d57cc42dd107bd2a9ddcd04ddc91f52da/FindTarantool.cmake#L12
[4]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-February/014024.html
----
> + set(ARES_CMAKE_FLAGS "-DCARES_STATIC=ON")
> + list(APPEND ARES_CMAKE_FLAGS "-DCARES_SHARED=OFF")
> + list(APPEND ARES_CMAKE_FLAGS "-DCARES_BUILD_TOOLS=OFF")
> + # We buid both static and shared versions of curl, so ares
Typo: buid.
> diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
> index e12de6005..bdeec5f89 100644
> --- a/src/CMakeLists.txt
> +++ b/src/CMakeLists.txt
> @@ -227,7 +227,8 @@ if(BUILD_STATIC)
> list(APPEND EXPORT_LIST ${SYMBOLS_LIB})
> # set variable to allow rescan (CMake depended)
> set(SYMBOLS_LIB "SYMBOLS_LIB-NOTFOUND")
> - elseif (${libstatic} STREQUAL bundled-libcurl)
> + elseif (${libstatic} STREQUAL bundled-libcurl OR
> + ${libstatic} STREQUAL bundled-ares)
> message("We don't need to export symbols from statically linked libcurl, skipped")
It would be good to adjust the message using ${libstatic} (because not
it is not only about libcurl).
More information about the Tarantool-patches
mailing list