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

["Alexander V. Tikhonov" <avtikhon@tarantool.org>]

Hi Igor, I've made all corrections as you suggested and checked all
the new options set in cmake. Founf 3 new options which I've blocked
in curl build:

+    list(APPEND LIBCURL_CMAKE_FLAGS "-DPICKY_COMPILER=OFF")
+    list(APPEND LIBCURL_CMAKE_FLAGS "-DBUILD_CURL_EXE=OFF")
+    list(APPEND LIBCURL_CMAKE_FLAGS "-DCURL_ZLIB=OFF")

On Thu, Oct 15, 2020 at 05:17:13PM +0300, Igor Munkin wrote:
> 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