From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp54.i.mail.ru (smtp54.i.mail.ru [217.69.128.34]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 936744765E0 for ; Wed, 30 Dec 2020 08:23:59 +0300 (MSK) Date: Wed, 30 Dec 2020 08:23:57 +0300 From: "Alexander V. Tikhonov" Message-ID: <20201230052357.GA94078@hpalx> References: <6a0b559505240475037444d82c2345a66852b8ba.1609302497.git.alexander.turenko@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <6a0b559505240475037444d82c2345a66852b8ba.1609302497.git.alexander.turenko@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH] build: don't re-export libcurl.so/dylib symbols List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Turenko Cc: tarantool-patches@dev.tarantool.org Hi Alexander, thanks for the patch, as I see no new degradation found in gitlab-ci testing commit criteria pipeline [1], patch LGTM. [1] - https://gitlab.com/tarantool/tarantool/-/pipelines/235694227 On Wed, Dec 30, 2020 at 07:53:42AM +0300, Alexander Turenko wrote: > Export libcurl's symbols only when they are provided by tarantool > itself: when the library is linked statically into the tarantool's > executable. There is no much sense to export the symbols when we link > against the library dynamically. > > Regarding motivation of the change. Since 2.6.0-36-g29ec62891 ('Ensure > all curl symbols are exported') the curl_multi_poll() function is > exported from the tarantool executable. It leads to a failure in > Homebrew's build, because there we link (dynamically) with a system > libcurl. On Mac OS 10.15 it is libcurl 7.64.1, while the function > appears since libcurl 7.66.0. So a linker reports the undefined symbol: > `curl_multi_poll`. > > Now the symbols are not exported at dynamic linking with libcurl, so the > linker is happy. > > This commit relaxes bounds for dynamic linking, but an attempt to link > with libcurl older than 7.66.0 statically still leads to a linking > failure. The box-tap/gh-5223-curl-exports.test.lua test still fails when > tarantool is linked (dynamically) against an old libcurl. > > It looks as the good compromise. When libcurl functionality is provided > by tarantool itself, *all* functions listed in the test are present > (otherwise a linker will complain). But tarantool does not enforce a > newer libcurl version, when it just *uses* this functionality and don't > provide it for modules and stored procedured. It is not tarantool's > responsibility in the case. > > We possibly should skip the box-tap/gh-5223-curl-exports.test.lua test > when tarantool is built against libcurl dynamically or revisit the > described approach. I'll leave it as possible follow up activity. > > Fixes #5542 > --- > > https://github.com/tarantool/tarantool/issues/5542 > https://github.com/tarantool/tarantool/tree/Totktonada/gh-5542-dont-export-libcurl-so-symbols > Totktonada/gh-5542-dont-export-libcurl-so-symbols > > CMakeLists.txt | 10 ++++++++++ > src/exports.c | 2 ++ > src/exports.h | 2 ++ > src/trivia/config.h.cmake | 2 ++ > 4 files changed, 16 insertions(+) > > diff --git a/CMakeLists.txt b/CMakeLists.txt > index f0e1f5090..4fbd19558 100644 > --- a/CMakeLists.txt > +++ b/CMakeLists.txt > @@ -397,6 +397,16 @@ else() > find_package(CURL) > endif() > > +# > +# Export libcurl symbols if the library is linked statically. > +# > +if (ENABLE_BUNDLED_LIBCURL OR BUILD_STATIC) > + set(EXPORT_LIBCURL_SYMBOLS ON) > +else() > + set(EXPORT_LIBCURL_SYMBOLS OFF) > +endif() > +message(STATUS "EXPORT_LIBCURL_SYMBOLS: ${EXPORT_LIBCURL_SYMBOLS}") > + > # > # ReadLine > # > diff --git a/src/exports.c b/src/exports.c > index a3c27143e..2a9c296a8 100644 > --- a/src/exports.c > +++ b/src/exports.c > @@ -88,6 +88,8 @@ > * below. But automatically. > */ > > +#include "trivia/config.h" > + > /** > * Symbol is just an address. No need to know its definition or > * even type to get that address. Even an integer global variable > diff --git a/src/exports.h b/src/exports.h > index 2116036fa..1b95e75f6 100644 > --- a/src/exports.h > +++ b/src/exports.h > @@ -130,6 +130,7 @@ EXPORT(csv_feed) > EXPORT(csv_iterator_create) > EXPORT(csv_next) > EXPORT(csv_setopt) > +#ifdef EXPORT_LIBCURL_SYMBOLS > EXPORT(curl_easy_cleanup) > EXPORT(curl_easy_duphandle) > EXPORT(curl_easy_escape) > @@ -211,6 +212,7 @@ EXPORT(curl_url_get) > EXPORT(curl_url_set) > EXPORT(curl_version) > EXPORT(curl_version_info) > +#endif /* EXPORT_LIBCURL_SYMBOLS */ > EXPORT(decimal_unpack) > EXPORT(error_ref) > EXPORT(error_set_prev) > diff --git a/src/trivia/config.h.cmake b/src/trivia/config.h.cmake > index 21efb978e..38f8eb4dc 100644 > --- a/src/trivia/config.h.cmake > +++ b/src/trivia/config.h.cmake > @@ -258,6 +258,8 @@ > > #cmakedefine ENABLE_FEEDBACK_DAEMON 1 > > +#cmakedefine EXPORT_LIBCURL_SYMBOLS 1 > + > /* > * vim: syntax=c > */ > -- > 2.25.0 >