<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">This requires a change log request, AFAICS.<div class=""><br class=""></div><div class="">@ChangeLog</div><div class=""> - when building tarantool with bundled libcurl, link it with c-ares by default (gh-4591)</div><div class=""> - add bundled curl and c-ares versions to tarantool version output (gh-4591)</div><div class=""><br class=""><div class="">
<div dir="auto" style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div>--</div><div>Serge Petrenko</div><div><a href="mailto:sergepetrenko@tarantool.org" class="">sergepetrenko@tarantool.org</a></div><div class=""><br class=""></div></div><br class="Apple-interchange-newline"><br class="Apple-interchange-newline">
</div>
<div><br class=""><blockquote type="cite" class=""><div class="">3 марта 2020 г., в 19:46, Serge Petrenko <<a href="mailto:sergepetrenko@tarantool.org" class="">sergepetrenko@tarantool.org</a>> написал(а):</div><br class="Apple-interchange-newline"><div class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Hi! Thanks for the review! I fixed your comments and performed the actions you expected me</span><div class="" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">to perform, except one. More info inline. The diff is below.<br class=""><div class=""><br class=""><blockquote type="cite" class=""><div class="">8 февр. 2020 г., в 22:42, Alexander Turenko <<a href="mailto:alexander.turenko@tarantool.org" class="">alexander.turenko@tarantool.org</a>> написал(а):</div><br class="Apple-interchange-newline"><div class=""><div class="">I mostly okay with the patch, but expect several actions like filing<br class="">some issues against c-ares and curl and minor tweaks of the patch.<br class=""><br class="">Re blocking in threaded resolver<br class="">--------------------------------<br class=""><br class="">Let's file an issue to curl, because it is inappropriate to block an<br class="">application that expect libcurl to give a control if something would<br class="">block. An error like 'DNS resolver thread pool is exhausted' is better,<br class="">because it can be handled on an application side somehow:<br class=""><br class="">* do other work / be responsible until free resolver threads will be<br class=""> available;<br class="">* give a user an error for further requests;<br class="">* dynamically increase threads count (automatically or upon a user<br class=""> request).<br class=""></div></div></blockquote><div class=""><br class=""></div><div class="">There already exist 2 issues: [1] - closed by a bot, unintentionally, I think.</div><div class="">[2] - a duplicate of [1]. </div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class="">Re ExternalProject<br class="">------------------<br class=""><br class="">In brief: let's file another issue as described below and postpone<br class="">further activities.<br class=""><br class="">There is the way to link c-ares into libcurl w/o installing c-ares using<br class="">a pkgconfig file, because it allows to set different include and library<br class="">directories and is supported by libcurl's configure script (see [1]).<br class=""><br class="">This however cannot be used with CMake (I didn't find a way). I propose<br class="">to file an issue (or even a PR) to curl to support different directories<br class="">for include and library files for c-ares (in CMake build). The<br class="">motivation is to use libcurl + c-ares as part of a parent build process<br class="">(like we going to do). It seems we just need to add two hints (one for<br class="">an include directory and one for a library directory) to<br class="">${CURL}/CMake/FindCARES.cmake (see [2]) like we do in<br class="">tarantool/modulekit (see [3]).</div></div></blockquote><div class=""><br class=""></div><div class="">I’ll think some more about the issue you propose, and file it as soon as I</div><div class="">come up with good wording and reasoning. I’m a little bit out of context</div><div class="">right now.</div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class="">Anyway, I'm okay with ExternalProject() and so installation of c-ares as<br class="">a part of build process for now. Hopefully we'll change it in the future<br class="">and so we'll not need to adjust flags like -isysroot in our cmake files.<br class=""><br class="">Re c-ares dependencies<br class="">----------------------<br class=""><br class="">See [4].<br class=""><br class="">Other<br class="">-----<br class=""><br class="">BTW, maybe file another issue to lower minimal required CMake version is<br class="">c-ares to 2.8?<br class=""></div></div></blockquote><div class=""><br class=""></div><div class="">I’ve opened the pull request in c-ares repo [3].</div><div class="">I’ve updated the minimum cmake version to 2.8.12. This is the versrion we tested</div><div class="">on and the version that used to be the minimal requirement before it was raised to 3+.</div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class="">[1]:<span class="Apple-converted-space"> </span><a href="https://github.com/curl/curl/issues/2203" class="">https://github.com/curl/curl/issues/2203</a><br class="">[2]:<span class="Apple-converted-space"> </span><a href="https://github.com/curl/curl/blob/5ce7102ceae250e2d31b54aad2f33b3bc35f243a/CMake/FindCARES.cmake" class="">https://github.com/curl/curl/blob/5ce7102ceae250e2d31b54aad2f33b3bc35f243a/CMake/FindCARES.cmake</a><br class="">[3]:<span class="Apple-converted-space"> </span><a href="https://github.com/tarantool/modulekit/blob/0994d76d57cc42dd107bd2a9ddcd04ddc91f52da/FindTarantool.cmake#L12" class="">https://github.com/tarantool/modulekit/blob/0994d76d57cc42dd107bd2a9ddcd04ddc91f52da/FindTarantool.cmake#L12</a><br class="">[4]:<span class="Apple-converted-space"> </span><a href="https://lists.tarantool.org/pipermail/tarantool-patches/2020-February/014024.html" class="">https://lists.tarantool.org/pipermail/tarantool-patches/2020-February/014024.html</a><br class=""><br class="">----<br class=""><br class=""><blockquote type="cite" class="">+ set(ARES_CMAKE_FLAGS "-DCARES_STATIC=ON")<br class="">+ list(APPEND ARES_CMAKE_FLAGS "-DCARES_SHARED=OFF")<br class="">+ list(APPEND ARES_CMAKE_FLAGS "-DCARES_BUILD_TOOLS=OFF")<br class="">+ # We buid both static and shared versions of curl, so ares<br class=""></blockquote><br class="">Typo: buid.<br class=""></div></div></blockquote><div class=""><br class=""></div>Fixed.</div><div class=""><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class=""><blockquote type="cite" class="">diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt<br class="">index e12de6005..bdeec5f89 100644<br class="">--- a/src/CMakeLists.txt<br class="">+++ b/src/CMakeLists.txt<br class="">@@ -227,7 +227,8 @@ if(BUILD_STATIC)<br class=""> list(APPEND EXPORT_LIST ${SYMBOLS_LIB})<br class=""> # set variable to allow rescan (CMake depended)<br class=""> set(SYMBOLS_LIB "SYMBOLS_LIB-NOTFOUND")<br class="">- elseif (${libstatic} STREQUAL bundled-libcurl)<br class="">+ elseif (${libstatic} STREQUAL bundled-libcurl OR<br class="">+ ${libstatic} STREQUAL bundled-ares)<br class=""> message("We don't need to export symbols from statically linked libcurl, skipped")<br class=""></blockquote><br class="">It would be good to adjust the message using ${libstatic} (because not<br class="">it is not only about libcurl).<br class=""></div></div></blockquote><br class=""></div><div class="">Fixed</div><div class=""><br class=""></div><div class="">I’ve also fixed your comments from [4], added an empty ARES_LIBRARIES var as a placeholder,</div><div class="">and filed an issue to c-ares regarding CMake linking unnecessary libraries [5].</div><div class=""><br class=""></div><div class="">Speaking of linking with unnecessary libraries, as you pointed out in [4], I just left it as-is for now,</div><div class="">since everything works in our CI. We may disable the libs altogether later, if it is needed.</div><div class=""><br class=""></div>[1]: <a href="https://github.com/curl/curl/issues/2975" class="">https://github.com/curl/curl/issues/2975</a></div><div class="" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">[2]: <a href="https://github.com/curl/curl/issues/4852" class="">https://github.com/curl/curl/issues/4852</a></div><div class="" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">[3]: <a href="https://github.com/c-ares/c-ares/pull/306" class="">https://github.com/c-ares/c-ares/pull/306</a></div><div class="" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">[4]:<span class="Apple-converted-space"> </span><a href="https://lists.tarantool.org/pipermail/tarantool-patches/2020-February/014024.html" class="">https://lists.tarantool.org/pipermail/tarantool-patches/2020-February/014024.html</a></div><div class="" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">[5]: <a href="https://github.com/c-ares/c-ares/issues/307" class="">https://github.com/c-ares/c-ares/issues/307</a></div><span class="" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><br class=""><br class="">diff --git a/cmake/BuildAres.cmake b/cmake/BuildAres.cmake<br class="">index 0f9f174ce..a38fa8f30 100644<br class="">--- a/cmake/BuildAres.cmake<br class="">+++ b/cmake/BuildAres.cmake<br class="">@@ -13,7 +13,7 @@ macro(ares_build)<br class=""> set(ARES_CMAKE_FLAGS "-DCARES_STATIC=ON")<br class=""> list(APPEND ARES_CMAKE_FLAGS "-DCARES_SHARED=OFF")<br class=""> list(APPEND ARES_CMAKE_FLAGS "-DCARES_BUILD_TOOLS=OFF")<br class="">- # We buid both static and shared versions of curl, so ares<br class="">+ # We build both static and shared versions of curl, so ares<br class=""> # has to be built with -fPIC for the shared version.<br class=""> list(APPEND ARES_CMAKE_FLAGS "-DCARES_STATIC_PIC=ON")<br class=""> # Even though we set the external project's install dir<br class="">@@ -53,6 +53,7 @@ macro(ares_build)<br class=""> set_target_properties(bundled-ares PROPERTIES IMPORTED_LOCATION<br class=""> ${ARES_INSTALL_DIR}/lib/libcares.a)<br class=""> add_dependencies(bundled-ares bundled-ares-project)<br class="">+ set(ARES_LIBRARIES bundled-ares)<br class=""> <br class=""> unset(ARES_CMAKE_FLAGS)<br class=""> unset(ARES_CFLAGS)<br class="">diff --git a/cmake/BuildLibCURL.cmake b/cmake/BuildLibCURL.cmake<br class="">index 756297878..5f8b15a63 100644<br class="">--- a/cmake/BuildLibCURL.cmake<br class="">+++ b/cmake/BuildLibCURL.cmake<br class="">@@ -164,7 +164,7 @@ macro(curl_build)<br class=""> set(CURL_INCLUDE_DIRS ${LIBCURL_INSTALL_DIR}/include)<br class=""> set(CURL_LIBRARIES bundled-libcurl ${LIBZ_LIBRARY})<br class=""> if (BUNDLED_LIBCURL_USE_ARES)<br class="">- set(CURL_LIBRARIES ${CURL_LIBRARIES} bundled-ares)<br class="">+ set(CURL_LIBRARIES ${CURL_LIBRARIES} ${ARES_LIBRARIES})<br class=""> endif()<br class=""> if (TARGET_OS_LINUX OR TARGET_OS_FREEBSD)<br class=""> set(CURL_LIBRARIES ${CURL_LIBRARIES} rt)<br class="">diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt<br class="">index bdeec5f89..7d865472d 100644<br class="">--- a/src/CMakeLists.txt<br class="">+++ b/src/CMakeLists.txt<br class="">@@ -229,7 +229,7 @@ if(BUILD_STATIC)<br class=""> set(SYMBOLS_LIB "SYMBOLS_LIB-NOTFOUND")<br class=""> elseif (${libstatic} STREQUAL bundled-libcurl OR<br class=""> ${libstatic} STREQUAL bundled-ares)<br class="">- message("We don't need to export symbols from statically linked libcurl, skipped")<br class="">+ message("We don't need to export symbols from statically linked ${libstatic}, skipped")<br class=""> else()<br class=""> message(WARNING "${libstatic} should be a static")<br class=""> endif()<br class="">diff --git a/third_party/c-ares b/third_party/c-ares<br class="">index 56a74c501..bbbffa4da 160000<br class="">--- a/third_party/c-ares<br class="">+++ b/third_party/c-ares<br class="">@@ -1 +1 @@<br class="">-Subproject commit 56a74c501a615c16ca54d608d1796e966f9a503a<br class="">+Subproject commit bbbffa4da8baf35b0e4e1c376e38018f9a8bcb4e<br class=""><br class="">--<br class="">Serge Petrenko<br class=""><a href="mailto:sergepetrenko@tarantool.org" class="">sergepetrenko@tarantool.org</a></span></div></blockquote></div><br class=""></div></body></html>