Re: [Tarantool-patches] [PATCH 1/3] build: export libCURL symbols

[Alexander Turenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

On Fri, Mar 19, 2021 at 05:13:06PM +0300, Roman Khabibov wrote:
> Export the symbols to tarantool executable in the case of libCURL
> included as bundled library.
> 
> This patch is just 1.10 adaptation of the original commit 9fc57c4
> performed by Yaroslav Dynnikov <yaroslav.dynnikov@gmail.com>.

The commit hash you reference is not reachable from the master branch.

Aside of this, some logic arrives from
47c19eeb4f67cb7257ce32542443c09410b6e752 ('build: don't re-export
libcurl.so/dylib symbols').

> diff --git a/extra/curl_symbols b/extra/curl_symbols
> new file mode 100755
> index 000000000..89e247a00
> --- /dev/null
> +++ b/extra/curl_symbols
> @@ -0,0 +1,81 @@
> +curl_easy_cleanup
> +curl_easy_duphandle
> +curl_easy_escape

It is the part of extra/exports, so I would name it
extra/exports_libcurl.

I would also keep Yaroslav's comment at the start:

 | # The following list was obtained by parsing libcurl.a static library:
 | # nm libcurl.a | grep -oP 'T \K(curl_.+)$' | sort
 |
 | curl_easy_cleanup
 | curl_easy_duphandle
 | <...>

> diff --git a/extra/mkexports b/extra/mkexports
> index 145e5b8ce..c10f20ae4 100755
> --- a/extra/mkexports
> +++ b/extra/mkexports
> @@ -2,22 +2,30 @@
>  # $1 - in  file
>  # $2 - out file
>  # $3 - os
> -# $4 - export templates
> +# $4 - is bundled curl on/off flag
> +# $5 - curl symbols
> +# $6 - export templates

I would just pass one or two files (extra/exports,
extra/exports_libcurl) in $1: it'll be easier to read.

My diff (from 1.10):

 | diff --git a/extra/mkexports b/extra/mkexports
 | index 145e5b8ce..95c3f8eed 100755
 | --- a/extra/mkexports
 | +++ b/extra/mkexports
 | @@ -1,5 +1,5 @@
 |  #! /bin/sh
 | -# $1 - in  file
 | +# $1 - in file(s)
 |  # $2 - out file
 |  # $3 - os
 |  # $4 - export templates
 | diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
 | index 1e840aab9..db771cce9 100644
 | --- a/src/CMakeLists.txt
 | +++ b/src/CMakeLists.txt
 | @@ -301,6 +301,13 @@ if(BUILD_STATIC)
 |      endif()
 |  endif()
 |  
 | +set(exports_file_sources ${CMAKE_SOURCE_DIR}/extra/exports)
 | +if (EXPORT_LIBCURL_SYMBOLS)
 | +    set(exports_file_sources ${exports_file_sources}
 | +        ${CMAKE_SOURCE_DIR}/extra/exports_libcurl)
 | +endif()
 | +string(REPLACE ";" " " exports_file_sources "${exports_file_sources}")
 | +
 |  # Exports syntax is toolchain-dependent, preprocessing is necessary
 |  set(exports_file ${CMAKE_BINARY_DIR}/extra/exports.${CMAKE_SYSTEM_NAME})
 |  add_custom_target(preprocess_exports
 | @@ -309,7 +316,7 @@ add_custom_command(
 |      OUTPUT  ${exports_file}
 |      DEPENDS ${CMAKE_SOURCE_DIR}/extra/exports
 |      COMMAND ${CMAKE_SOURCE_DIR}/extra/mkexports
 | -            ${CMAKE_SOURCE_DIR}/extra/exports
 | +            ${exports_file_sources}
 |              ${exports_file} ${CMAKE_SYSTEM_NAME}
 |              ${EXPORT_LIST}
 |  )

----

Just for the record: I also tried another implementation variant:

 | diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
 | index 1e840aab9..6804ba301 100644
 | --- a/src/CMakeLists.txt
 | +++ b/src/CMakeLists.txt
 | @@ -284,8 +284,9 @@ 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 OR
 | -                ${libstatic} STREQUAL bundled-ares)
 | +        elseif (${libstatic} STREQUAL bundled-libcurl)
 | +            # Handled below.
 | +        elseif (${libstatic} STREQUAL bundled-ares)
 |              message("We don't need to export symbols from statically linked ${libstatic}, skipped")
 |          else()
 |              message(WARNING "${libstatic} should be a static")
 | @@ -301,10 +302,24 @@ if(BUILD_STATIC)
 |      endif()
 |  endif()
 |  
 | +# Expose libcurl symbols.
 | +if(ENABLE_BUNDLED_LIBCURL OR BUILD_STATIC)
 | +    set(reexport_libraries ${reexport_libraries} ${CURL_LIBRARIES})
 | +
 | +    if (ENABLE_BUNDLED_LIBCURL)
 | +        get_target_property(libstatic bundled-libcurl IMPORTED_LOCATION)
 | +        string(REGEX REPLACE "libcurl.a" "libcurl.so" libdynamic ${libstatic})
 | +    else()
 | +        find_library(libdynamic NAMES curl)
 | +    endif()
 | +
 | +    set(EXPORT_LIST ${EXPORT_LIST} ${libdynamic})
 | +endif()
 | +
 |  # Exports syntax is toolchain-dependent, preprocessing is necessary
 |  set(exports_file ${CMAKE_BINARY_DIR}/extra/exports.${CMAKE_SYSTEM_NAME})
 |  add_custom_target(preprocess_exports
 | -                  DEPENDS ${exports_file})
 | +                  DEPENDS ${exports_file} build_bundled_libs)
 |  add_custom_command(
 |      OUTPUT  ${exports_file}
 |      DEPENDS ${CMAKE_SOURCE_DIR}/extra/exports

(At least it works for libcurl bundling, but I didn't verified it with static
build.)

However explicit listing of public libcurl symbols looks easier to understand
and in tune with master.