Re: [Tarantool-patches] [PATCH] build: don't re-export libcurl.so/dylib symbols

["Alexander V. Tikhonov" <avtikhon@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
>