[Tarantool-patches] [PATCH v2] build: enable cmake in curl build
Igor Munkin
imun at tarantool.org
Wed Sep 16 23:15:21 MSK 2020
Sasha,
Thanks for the patch! Please consider my comments below.
On 10.07.20, Alexander V. Tikhonov wrote:
> Tryed to chang autoconf tools in Curl build to cmake use for builds
Typo: s/Tryed/Tried/.
Typo: s/chang/change/.
> where cmake major starts from 3. It was made so because cmake in curl
> build works only with 3.0 version and above, the following issue
> found on:
> CentOS 6,7
> Ubuntu 14.04
>
> Issue with curl cmake based build with cmake before 3.0 version:
> CMake Error at CMakeLists.txt:41 (cmake_minimum_required):
> CMake 3.0 or higher is required. You are running version 2.8.12.2
>
> For curl cmake build all autoconf options were ported to cmake
> configuration call.
>
> Also found that CURL cmake configuration file:
> third_party/curl/lib/CMakeLists.txt
>
> has installation part for built libcurl.a library:
> install(TARGETS ${LIB_NAME}
> EXPORT ${TARGETS_EXPORT_NAME}
> ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
> LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
>
> where it changes CMAKE_INSTALL_LIBDIR to appropriate name with
> suffix of the architecture, like:
> lib
> lib64
> x86_64
>
> Found that find_library routine from the file:
> cmake/FindLibCURL.cmake
> returns only 'lib' value and it breaks the building of the depends
> binaries. To avoid of it the CMAKE_INSTALL_LIBDIR option was set to
> cmake call:
> -DCMAKE_INSTALL_LIBDIR=lib
>
> Completely changed autoconf to cmake in curl build. After curl
> sources were changed to be able to be build since 2.8 version
> the old OS like CentOS 6/7 and Ubuntu 14.04 became available
> for curl build using cmake.
OK, so the issue with old CMake is gone, isn't it? Let's reword the
part at the beginning:
| Replaced autoconf tools in CURL build with CMake. Since vanilla CURL
| CMakeLists.txt requires CMake 3.0 or higher, our fork has been patched
| in c4c5dda4774a51670d6ea9857750c74816a14759 (build: accept cmake 2.8
| version for build) to support the distros providing the older CMake by
| default:
| * CentOS 6
| * CentOS 7
| * Ubuntu 14.04
|
| All autoconf build options were respectively converted to CMake flags.
>
> Autoconf part completely removed and code cleaned up for cmake.
>
> 1. Found issue with building on CentOS 6:
>
> Linking C executable curl
> build/ares/dest/lib/libcares.a(ares__timeval.c.o): In function `ares__tvnow':
> ares__timeval.c:(.text+0x15): undefined reference to `clock_gettime'
> collect2: error: ld returned 1 exit status
>
> It was fixed with added "-lrt" flag to CMAKE_C_FLAGS and
> CMAKE_CXX_FLAGS build flags, when cmake version is lower
> than 3.0 and RT library had needed function.
>
> 2. Found issue with building FreeBSD 12: app/socket test failed.
>
> [035] --- app/socket.result Tue May 26 03:06:32 2020
> [035] +++ app/socket.reject Fri May 8 08:26:16 2020
> [035] @@ -836,7 +836,7 @@
> [035] ...
> [035] s:recv()
> [035] ---
> [035] -- Hello, world
> [035] +-
> [035] ...
> [035] sc:sendto('127.0.0.1', s:name().port, 'Hello, world, 2')
> [035] ---
>
> It was fixed with added "-DLDFLAGS=" flag to cmake call.
This define was set before, so it doesn't look like an issue (IMHO).
>
> 3. Found issue with static build using CentOS 7, where SSL cmake rule
> failed. Building the image got the issue:
>
> [ 1%] Performing configure step for 'bundled-libcurl-project'
> CMake Warning at CMakeLists.txt:50 (message):
> the curl cmake build system is poorly maintained. Be aware
>
> -- curl version=[7.66.0-DEV]
> -- Found c-ares: /tarantool/build/ares/dest/lib/libcares.a
> Found *nroff option: -- -man
> CMake Error at /usr/share/cmake/Modules/FindOpenSSL.cmake:278 (list):
> list GET given empty list
> Call Stack (most recent call first):
> CMakeLists.txt:347 (find_package)
>
> Root cause of the issue that Dockerfile uses globaly
> installed openSSL with:
>
> cmake ... -DOPENSSL_ROOT_DIR=/usr/local ...
>
> Its cmake build file:
>
> /usr/share/cmake/Modules/FindOpenSSL.cmake
>
> fails on parsing the SSL version:
>
> it has:
> REGEX "^#define[\t ]+OPENSSL_VERSION_NUMBER[\t ]+0x([0-9a-fA-F])+.*")
>
> but it should to use:
> REGEX "^#[\t ]*define[\t ]+OPENSSL_VERSION_NUMBER[\t ]+0x([0-9a-fA-F])+.*")
>
> Anyway we want to use the same OpenSSL library for libcurl, as is used
> for Tarantool itself. So the path to it set for cURL build:
>
> list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_MODULE_PATH=${PROJECT_SOURCE_DIR}/cmake")
>
> Closes #5019
> Closes #4968
> Closes #5020
Minor: Please sort the issues above.
> ---
By the way, I don't see this commit on this branch:
| commit c4c5dda4774a51670d6ea9857750c74816a14759
| Author: Alexander V. Tikhonov <avtikhon at tarantool.org>
| Date: Sat May 23 13:55:04 2020 +0300
|
| build: accept cmake 2.8 version for build
|
| Changed cmake minimal accepted version for curl build from 3.0 to 2.8.
>
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4874-out-of-source-build-full-ci
> Issue: https://github.com/tarantool/tarantool/issues/4968
> Issue: https://github.com/tarantool/tarantool/issues/5019
> Issue: https://github.com/tarantool/tarantool/issues/5020
> V2: moved fix for #5019 from standalone commit to current
>
> cmake/BuildLibCURL.cmake | 202 ++++++++++++++-------------------------
> 1 file changed, 74 insertions(+), 128 deletions(-)
>
> diff --git a/cmake/BuildLibCURL.cmake b/cmake/BuildLibCURL.cmake
> index 5f8b15a63..022dd90cd 100644
> --- a/cmake/BuildLibCURL.cmake
> +++ b/cmake/BuildLibCURL.cmake
<snipped>
> @@ -14,39 +15,80 @@ macro(curl_build)
> message(FATAL_ERROR "Unable to find zlib")
> endif()
>
> - # Use the same OpenSSL library for libcurl as is used for
> - # tarantool itself.
> + # add librt for clock_gettime function definition
> + if(${CMAKE_MAJOR_VERSION} VERSION_LESS "3")
> + CHECK_LIBRARY_EXISTS (rt clock_gettime "" HAVE_LIBRT)
> + if (HAVE_LIBRT)
Typo: whitespace differs from the code nearby.
> + list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_CXX_FLAGS=-lrt")
> + list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_C_FLAGS=-lrt")
> + endif()
> + endif()
> +
<snipped>
> + # switch off the shared build
Minor: Build for shared libs was enabled in autoconf version. If you
disabled it intentionally, please mention it within commit message.
> + list(APPEND LIBCURL_CMAKE_FLAGS "-DBUILD_SHARED_LIBS=OFF")
<snipped>
> + # found that on FreeBSD 12 this setup is needed for app/socket.test.lua
I see the related comment below. Does it make sense to leave it here?
> + list(APPEND LIBCURL_CMAKE_FLAGS "-DLDFLAGS=")
> +
> + # In hardened mode, which enables -fPIE by default,
> + # the cmake checks don't work without -fPIC.
> + list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_REQUIRED_FLAGS=-fPIC")
Sorry, but I don't get this line. What does it solve?
>
> include(ExternalProject)
> ExternalProject_Add(
> @@ -56,100 +98,11 @@ macro(curl_build)
> DOWNLOAD_DIR ${LIBCURL_BINARY_DIR}
> TMP_DIR ${LIBCURL_BINARY_DIR}/tmp
> STAMP_DIR ${LIBCURL_BINARY_DIR}/stamp
<snipped>
> - # Pass empty LDFLAGS to discard using of
> - # corresponding environment variable.
> - # It is possible that a linker flag assumes that
> - # some compilation flag is set. We don't pass
> - # CFLAGS from environment, so we should not do it
> - # for LDFLAGS too.
I guess this is a valuable comment to be saved in the patch.
> - LDFLAGS=
<snipped>
> - --without-brotli
I totally agree with Sasha Tu.[1] regarding explicitly disabling the
options that are disabled by default in CURL CMake.
> - --without-gnutls
<snipped>
> - BUILD_COMMAND cd <BINARY_DIR> && $(MAKE)
> + cd <BINARY_DIR> && cmake <SOURCE_DIR>
> + ${LIBCURL_CMAKE_FLAGS}
> + BUILD_COMMAND cd <BINARY_DIR> && $(MAKE) -j
Minor: -j is OK, but it's better to use -j value from the parent scope.
> INSTALL_COMMAND cd <BINARY_DIR> && $(MAKE) install)
<snipped>
> --
> 2.17.1
>
[1]: https://github.com/tarantool/tarantool/issues/4968#issuecomment-692420653
--
Best regards,
IM
More information about the Tarantool-patches
mailing list