Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH v2] build: enable cmake in curl build
@ 2020-07-10  9:31 Alexander V. Tikhonov
  2020-09-16 20:15 ` Igor Munkin
  0 siblings, 1 reply; 2+ messages in thread
From: Alexander V. Tikhonov @ 2020-07-10  9:31 UTC (permalink / raw)
  To: Kirill Yukhin, Alexander Turenko; +Cc: tarantool-patches

Tryed to chang autoconf tools in Curl build to cmake use for builds
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.

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.

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
---

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
@@ -3,6 +3,7 @@ macro(curl_build)
     set(LIBCURL_SOURCE_DIR ${PROJECT_SOURCE_DIR}/third_party/curl)
     set(LIBCURL_BINARY_DIR ${PROJECT_BINARY_DIR}/build/curl/work)
     set(LIBCURL_INSTALL_DIR ${PROJECT_BINARY_DIR}/build/curl/dest)
+    set(LIBCURL_CMAKE_FLAGS "")
 
     if (BUILD_STATIC)
         set(LIBZ_LIB_NAME libz.a)
@@ -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)
+            list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_CXX_FLAGS=-lrt")
+            list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_C_FLAGS=-lrt")
+        endif()
+    endif()
+
+    # switch on the static build
+    list(APPEND LIBCURL_CMAKE_FLAGS "-DCURL_STATICLIB=ON")
+
+    # switch off the shared build
+    list(APPEND LIBCURL_CMAKE_FLAGS "-DBUILD_SHARED_LIBS=OFF")
+
+    # let's disable testing for curl to save build time
+    list(APPEND LIBCURL_CMAKE_FLAGS "-DBUILD_TESTING=OFF")
+
+    # Setup use of openssl, use the same OpenSSL library
+    # for libcurl as is used for tarantool itself.
     get_filename_component(FOUND_OPENSSL_ROOT_DIR ${OPENSSL_INCLUDE_DIR} DIRECTORY)
-    set(LIBCURL_OPENSSL_OPT "--with-ssl=${FOUND_OPENSSL_ROOT_DIR}")
+    list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_USE_OPENSSL=ON")
+    list(APPEND LIBCURL_CMAKE_FLAGS "-DOPENSSL_ROOT_DIR=${FOUND_OPENSSL_ROOT_DIR}")
+    list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_MODULE_PATH=${PROJECT_SOURCE_DIR}/cmake")
 
-    # Use either c-ares bundled with tarantool or
-    # libcurl-default threaded resolver.
+    # Setup ARES and its library path, use either c-ares bundled
+    # with tarantool or libcurl-default threaded resolver.
     if(BUNDLED_LIBCURL_USE_ARES)
-        set(ASYN_DNS_USED "ares")
-        set(ASYN_DNS_UNUSED "threaded-resolver")
-        set(ASYN_DNS_PATH "=${ARES_INSTALL_DIR}")
+        set(ENABLE_ARES "ON")
+        list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_FIND_ROOT_PATH=${ARES_INSTALL_DIR}")
     else()
-        set(ASYN_DNS_USED "threaded-resolver")
-        set(ASYN_DNS_UNUSED "ares")
-        set(ASYN_DNS_PATH "")
-    endif()
-
-    set(ENABLED_DNS_OPT "--enable-${ASYN_DNS_USED}${ASYN_DNS_PATH}")
-    set(DISABLED_DNS_OPT "--disable-${ASYN_DSN_UNUSED}")
-
-    # Pass -isysroot=<SDK_PATH> option on Mac OS to a preprocessor
-    # and a C compiler to find header files installed with an SDK.
-    #
-    # The idea here is to don't pass all compiler/linker options
-    # that is set for tarantool, but only a subset that is
-    # necessary for choosen toolchain, and let curl's configure
-    # script set options that are appropriate for libcurl.
-    set(LIBCURL_CPPFLAGS "")
-    set(LIBCURL_CFLAGS "")
-    if (TARGET_OS_DARWIN AND NOT "${CMAKE_OSX_SYSROOT}" STREQUAL "")
-        set(LIBCURL_CPPFLAGS "${LIBCURL_CPPFLAGS} ${CMAKE_C_SYSROOT_FLAG} ${CMAKE_OSX_SYSROOT}")
-        set(LIBCURL_CFLAGS "${LIBCURL_CFLAGS} ${CMAKE_C_SYSROOT_FLAG} ${CMAKE_OSX_SYSROOT}")
+        set(ENABLE_ARES "OFF")
     endif()
+    list(APPEND LIBCURL_CMAKE_FLAGS "-DENABLE_ARES=${ENABLE_ARES}")
+
+    # switch off the group of protocols with special flag HTTP_ONLY:
+    #   ftp, file, ldap, ldaps, rtsp, dict, telnet, tftp, pop3, imap, smtp
+    list(APPEND LIBCURL_CMAKE_FLAGS "-DHTTP_ONLY=ON")
+
+    # additionaly disable some more 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
+    list(APPEND LIBCURL_CMAKE_FLAGS "-DCURL_CA_FALLBACK=ON")
+
+    # Even though we set the external project's install dir
+    # below, we still need to pass the corresponding install
+    # prefix via cmake arguments.
+    list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_INSTALL_PREFIX=${LIBCURL_INSTALL_DIR}")
+
+    # The default values for the options below are not always
+    # "./lib", "./bin"  and "./include", while curl expects them
+    # to be.
+    list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_INSTALL_LIBDIR=lib")
+    list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_INSTALL_INCLUDEDIR=include")
+    list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_INSTALL_BINDIR=bin")
+
+    # Pass the same toolchain as is used to build tarantool itself,
+    # because they can be incompatible.
+    list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_C_COMPILER=${CMAKE_C_COMPILER}")
+    list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_LINKER=${CMAKE_LINKER}")
+    list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_AR=${CMAKE_AR}")
+    list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_RANLIB=${CMAKE_RANLIB}")
+    list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_NM=${CMAKE_NM}")
+    list(APPEND LIBCURL_CMAKE_FLAGS "-DCMAKE_STRIP=${CMAKE_STRIP}")
+
+    # found that on FreeBSD 12 this setup is needed for app/socket.test.lua
+    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")
 
     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
-        BINARY_DIR ${LIBCURL_BINARY_DIR}
+        BINARY_DIR ${LIBCURL_BINARY_DIR}/curl
         CONFIGURE_COMMAND
-            cd <SOURCE_DIR> && ./buildconf &&
-            cd <BINARY_DIR> && <SOURCE_DIR>/configure
-                # Pass the same toolchain as is used to build
-                # tarantool itself, because they can be
-                # incompatible.
-                CC=${CMAKE_C_COMPILER}
-                LD=${CMAKE_LINKER}
-                AR=${CMAKE_AR}
-                RANLIB=${CMAKE_RANLIB}
-                NM=${CMAKE_NM}
-                STRIP=${CMAKE_STRIP}
-
-                # Pass -isysroot=<SDK_PATH> option on Mac OS, see
-                # above.
-                # Note: Passing of CPPFLAGS / CFLAGS explicitly
-                # discards using of corresponsing environment
-                # variables.
-                CPPFLAGS=${LIBCURL_CPPFLAGS}
-                CFLAGS=${LIBCURL_CFLAGS}
-
-                # 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.
-                LDFLAGS=
-
-                --prefix <INSTALL_DIR>
-                --enable-static
-                --enable-shared
-
-                --with-zlib
-                ${LIBCURL_OPENSSL_OPT}
-                --with-ca-fallback
-
-                --without-brotli
-                --without-gnutls
-                --without-mbedtls
-                --without-cyassl
-                --without-wolfssl
-                --without-mesalink
-                --without-nss
-                --without-ca-bundle
-                --without-ca-path
-                --without-libpsl
-                --without-libmetalink
-                --without-librtmp
-                --without-winidn
-                --without-libidn2
-                --without-nghttp2
-                --without-ngtcp2
-                --without-nghttp3
-                --without-quiche
-                --without-zsh-functions-dir
-                --without-fish-functions-dir
-
-                ${ENABLED_DNS_OPT}
-                --enable-http
-                --enable-proxy
-                --enable-ipv6
-                --enable-unix-sockets
-                --enable-cookies
-                --enable-http-auth
-                --enable-mime
-                --enable-dateparse
-
-                ${DISABLED_DNS_OPT}
-                --disable-ftp
-                --disable-file
-                --disable-ldap
-                --disable-ldaps
-                --disable-rtsp
-                --disable-dict
-                --disable-telnet
-                --disable-tftp
-                --disable-pop3
-                --disable-imap
-                --disable-smb
-                --disable-smtp
-                --disable-gopher
-                --disable-manual
-                --disable-sspi
-                --disable-crypto-auth
-                --disable-ntlm-wb
-                --disable-tls-srp
-                --disable-doh
-                --disable-netrc
-                --disable-progress-meter
-                --disable-dnsshuffle
-                --disable-alt-svc
-        BUILD_COMMAND cd <BINARY_DIR> && $(MAKE)
+            cd <BINARY_DIR> && cmake <SOURCE_DIR>
+                ${LIBCURL_CMAKE_FLAGS}
+        BUILD_COMMAND cd <BINARY_DIR> && $(MAKE) -j
         INSTALL_COMMAND cd <BINARY_DIR> && $(MAKE) install)
 
     add_library(bundled-libcurl STATIC IMPORTED GLOBAL)
@@ -170,13 +123,6 @@ macro(curl_build)
         set(CURL_LIBRARIES ${CURL_LIBRARIES} rt)
     endif()
 
-    unset(ASYN_DNS_USED)
-    unset(ASYN_DNS_UNUSED)
-    unset(ASYN_DNS_PATH)
-    unset(ENABLED_DNS_OPT)
-    unset(DISABLED_DNS_OPT)
-    unset(LIBCURL_CPPFLAGS)
-    unset(LIBCURL_CFLAGS)
     unset(FOUND_OPENSSL_ROOT_DIR)
     unset(LIBCURL_INSTALL_DIR)
     unset(LIBCURL_BINARY_DIR)
-- 
2.17.1

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [Tarantool-patches] [PATCH v2] build: enable cmake in curl build
  2020-07-10  9:31 [Tarantool-patches] [PATCH v2] build: enable cmake in curl build Alexander V. Tikhonov
@ 2020-09-16 20:15 ` Igor Munkin
  0 siblings, 0 replies; 2+ messages in thread
From: Igor Munkin @ 2020-09-16 20:15 UTC (permalink / raw)
  To: Alexander V. Tikhonov; +Cc: tarantool-patches, Alexander Turenko

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@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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-09-16 20:25 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-10  9:31 [Tarantool-patches] [PATCH v2] build: enable cmake in curl build Alexander V. Tikhonov
2020-09-16 20:15 ` Igor Munkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox