Re: [Tarantool-patches] [PATCH v4 4/5] build: enable cmake in curl build

[Igor Munkin <imun@tarantool.org>]

Sasha,

Thanks for the patch! There is one question and several nits below,
please consider them.

On 14.10.20, Alexander V. Tikhonov wrote:
> Initially tried to change autoconf tools in Curl build to cmake and
> found the following build issue on:
> 
>   CentOS 6,7

Minor (again): IMHO, it's better to split these lines as you did in the
commit you're referring below.

>   Debian 8
>   Ubuntu 14.04
> 
>   Issue found:
> 

<snipped>

> 
> 3. Found issues with building Tarantool statically on Debian 9 and
>    its package build on CentOS 8.
> 
>    Building statically got the issues with openssl linking, like [2]:
> 
>      static-build/openssl-prefix/lib/libcrypto.a(threads_pthread.o): In function `CRYPTO_THREAD_lock_new':
>      threads_pthread.c:(.text+0x45): undefined reference to `pthread_rwlock_init'
> 
>    It happened because in openssl radically changed how threads-related
>    things were handled before 1.1.0 version. It required the application
>    to provide us with the locking functionality in form of callbacks.
>    Since 1.1.0, these matters are entirely internal, so libcrypto
>    requires the presence of the platform specific thread implementation
>    of our choosing, which is pthread on everything.
> 
>    After '-pthread' added to C compile flags package build on CentOS 8
>    failed with the issue [3]:
> 
>      /build/usr/src/debug/tarantool-2.6.0.141/third_party/curl/lib/warnless.c:101:4: error: #error "SIZEOF_CURL_OFF_T not defined"
>       #  error "SIZEOF_CURL_OFF_T not defined"
>          ^~~~~
>      /build/usr/src/debug/tarantool-2.6.0.141/third_party/curl/lib/warnless.c: In function 'curlx_uztoso':
>      /build/usr/src/debug/tarantool-2.6.0.141/third_party/curl/lib/warnless.c:192:40: error: 'CURL_MASK_SCOFFT' undeclared (first use in this function); did you mean 'CURL_MASK_SINT'?
>       return (curl_off_t)(uznum & (size_t) CURL_MASK_SCOFFT);
>                                            ^~~~~~~~~~~~~~~~
>                                            CURL_MASK_SINT
> 
>    To avoid of the issue decided to use '-pthread' flag only for openssl
>    when it had not this flag in openssl compilation:
> 
>      # check '-pthread' in openssl compile options
>      execute_process(COMMAND openssl version -f
>                      OUTPUT_VARIABLE OPENSSL_COMPILE_OPTIONS)
>      # add pthread library for openssl static library linking
>      if(NOT OPENSSL_COMPILE_OPTIONS MATCHES ".* -pthread .*")
>          list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_C_FLAGS=-pthread")
>      endif()

Minor: The wording is nice and quite clear, feel free to drop the code
snipped above.

> 

<snipped>

> 
> 5. Found that on Gentoo need to switch off use of libSSH2:

Minor: This doesn't relate to Gentoo per se. As I mentioned here[1], the
issue occurred as a result of moving curl build from autotools to CMake:
CMAKE_USE_LIBSSH2 flag is enabled by default, but --with-libssh2 is not.

> 
>      list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_USE_LIBSSH2=OFF")
> 
>    to avoid of linking issues, like:
> 
>      ld: libssh2.c:(.text+0x4d8): undefined reference to `libssh2_*...
> 
>    this issue exists in curl issues [4].
> 
> Closes #4968
> Closes #5019
> Closes #5396
> 
> [1] - https://github.com/tarantool/tarantool/issues/4968#issue-617183031
> [2] - https://gitlab.com/tarantool/tarantool/-/jobs/779176133#L6021
> [3] - https://gitlab.com/tarantool/tarantool/-/jobs/778309145#L3060
> [4] - https://github.com/curl/curl/issues/1146
> ---
> 
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4874-out-of-source-build-full-ci
> Issue: https://github.com/tarantool/tarantool/issues/4968
> 
> V4: Changes:
> - Disabled libSSH2.
> - Added disable symbols hiding flag.
> 
>  cmake/BuildLibCURL.cmake | 250 +++++++++++++++++++--------------------
>  1 file changed, 121 insertions(+), 129 deletions(-)
> 
> diff --git a/cmake/BuildLibCURL.cmake b/cmake/BuildLibCURL.cmake
> index 86fec39e9..0c3b2fda4 100644
> --- a/cmake/BuildLibCURL.cmake
> +++ b/cmake/BuildLibCURL.cmake

<snipped>

> @@ -19,40 +20,126 @@ macro(curl_build)
>          message(FATAL_ERROR "Unable to find zlib")
>      endif()
>      get_filename_component(FOUND_ZLIB_ROOT_DIR ${ZLIB_INCLUDE_DIR} DIRECTORY)
> +    list(APPEND LIBCURL_CMAKE_FLAGS "-DZLIB_ROOT=${FOUND_ZLIB_ROOT_DIR}")
> +
> +    # check '-pthread' in openssl compile options

Typo: s/check/Check/.
Typo: s/options/options./.

> +    execute_process(COMMAND openssl version -f
> +                    OUTPUT_VARIABLE OPENSSL_COMPILE_OPTIONS)
> +    # add pthread library for openssl static library linking

Typo: s/add/Add/.
Typo: s/linking/linking./.

> +    if(NOT OPENSSL_COMPILE_OPTIONS MATCHES ".* -pthread .*")
> +        list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_C_FLAGS=-pthread")
> +    endif()
> +
> +    # add librt for clock_gettime function definition

Typo: s/add/Add/.
Typo: s/definition/definition./.

> +    if(${CMAKE_MAJOR_VERSION} VERSION_LESS "3")
> +        CHECK_LIBRARY_EXISTS (rt clock_gettime "" HAVE_LIBRT)
> +        if (HAVE_LIBRT)
> +            list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_C_FLAGS=-lrt")
> +            list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_CXX_FLAGS=-lrt")
> +        endif()
> +    endif()
> +
> +    # switch on the static build

Typo: s/switch/Switch/.
Typo: s/build/build./.

> +    list(APPEND LIBCURL_CMAKE_FLAGS "-DCURL_STATICLIB=ON")
> +
> +    # switch off the shared build

Typo: s/switch/Switch/.
Typo: s/build/build./.

> +    list(APPEND LIBCURL_CMAKE_FLAGS "-DBUILD_SHARED_LIBS=OFF")

<snipped>

> +    # let's disable testing for curl to save build time

Typo: s/let/Let/.
Typo: s/time/time./.

> +    list(APPEND LIBCURL_CMAKE_FLAGS "-DBUILD_TESTING=OFF")

<snipped>

> +    # Found that on Gentoo need to switch off use of libSSH2 to avoid of issues, like:
> +    #   ld: libssh2.c:(.text+0x4d8): undefined reference to `libssh2_*...

Minor: This doesn't relate to Gentoo per se. As I mentioned here[1], the
issue occurred as a result of moving curl build from autotools to CMake:
CMAKE_USE_LIBSSH2 flag is enabled by default, but --with-libssh2 is not.

Anyway, I see no additions related to this inconsistency. Do other CMake
defaults match the corresponding autotools values?

> +    list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_USE_LIBSSH2=OFF")
> +
> +    # Switch off the group of protocols with special flag HTTP_ONLY:
> +    #   ftp, file, ldap, ldaps, rtsp, dict, telnet, tftp, pop3, imap, smtp

Typo: s/smtp/smtp./.

> +    list(APPEND LIBCURL_CMAKE_FLAGS "-DHTTP_ONLY=ON")
> +
> +    # additionaly disable some more protocols

Typo: s/additionaly/Additionally/.
Typo: s/protocols/protocols./.

> +    list(APPEND LIBCURL_CMAKE_FLAGS "-DCURL_DISABLE_SMB=ON")
> +    list(APPEND LIBCURL_CMAKE_FLAGS "-DCURL_DISABLE_GOPHER=ON")
> +    list(APPEND LIBCURL_CMAKE_FLAGS "-DCURL_DISABLE_CRYPTO_AUTH=ON")
> +
> +    # switch on ca-fallback feature

Typo: s/switch/Switch/.
Typo: s/feature/feature./.

> +    list(APPEND LIBCURL_CMAKE_FLAGS "-DCURL_CA_FALLBACK=ON")

<snipped>

> +    # should be already set by "-DHTTP_ONLY=ON" above

Typo: s/should/Should/.
Typo: s/above/above./.

> +    list(APPEND LIBCURL_CMAKE_FLAGS "-DCURL_DISABLE_FTP=ON")

<snipped>

> +        BUILD_COMMAND cd <BINARY_DIR> && $(MAKE) -j

Side note: This change is reverted on the branch, as discussed offline.

>          INSTALL_COMMAND cd <BINARY_DIR> && $(MAKE) install)

<snipped>

> @@ -168,6 +165,7 @@ macro(curl_build)
>      endif()
>      add_dependencies(bundled-libcurl bundled-libcurl-project)
> 
> +    # setup CURL_INCLUDE_DIRS & CURL_LIBRARIES for global use

Typo: s/setup/Setup/.
Typo: s/use/use./.

>      set(CURL_INCLUDE_DIRS ${LIBCURL_INSTALL_DIR}/include)
>      set(CURL_LIBRARIES bundled-libcurl ${LIBZ_LIBRARY})
>      if (BUNDLED_LIBCURL_USE_ARES)

<snipped>

> -- 
> 2.25.1
> 

-- 
Best regards,
IM