From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp59.i.mail.ru (smtp59.i.mail.ru [217.69.128.39]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id E3BA141D0BD for ; Fri, 18 Oct 2019 09:01:36 +0300 (MSK) Date: Fri, 18 Oct 2019 09:01:24 +0300 From: Alexander Turenko Message-ID: <20191018060124.zvu3d5eixvqiuyta@tkn_work_nb> References: <20191016221939.i75hojrafnljgbec@tkn_work_nb> <20191017115624.GA16357@tarantool.org> <1571365805.179773702@f496.i.mail.ru> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1571365805.179773702@f496.i.mail.ru> Subject: Re: [Tarantool-patches] [tarantool-patches] Re: [PATCH] build: fix OpenSSL linking problems on FreeBSD List-Id: Tarantool development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Tikhonov Cc: tarantool-patches@freelists.org, tarantool-patches@dev.tarantool.org Thanks for reviews! CCed Kirill. WBR, Alexander Turenko. On Fri, Oct 18, 2019 at 05:30:05AM +0300, Alexander Tikhonov wrote: > The path seems right and LGTM from my side. > > > >Четверг, 17 октября 2019, 14:57 +03:00 от Igor Munkin : > > > >Sasha, > > > >Thanks, the wording is great and the whole patch LGTM. > > > >As discussed offline I've created a new follow-up ticket #4575. > > > >On 17.10.19, Alexander Turenko wrote: > >> I fixed all known flaws now: see inline answers and the whole new patch > >> at end of the email. > >> > >> WBR, Alexander Turenko. > >> > >> On Tue, Sep 24, 2019 at 04:15:56PM +0300, Igor Munkin wrote: > >> > Sasha, > >> > > >> > Thanks, see no flaws except for the issue with Apple CommandLineTools > >> > compiler on OSX Mojave you're working on. Please consider a sole minor > >> > comment below. > >> > >> Fixed it by passing -isysroot= via CPPFLAGS (PP is a > >> preprocessor) and CFLAGS. > >> > >> Added the following paragraph to the commit message: > >> > >> | When CC is passed to libcurl's configure script, the new problem opens > >> | on Mac OS. CMake chooses XCode toolchain by default (at least on a > >> | particular system where I tried it), which requires -isysroot= > >> | option to be passed to a preprocessor and a compiler in order to find > >> | system headers. See [2] for more information. > >> | > >> | <...> > >> | > >> | [2]: https://developer.apple.com/documentation/xcode_release_notes/xcode_10_release_notes#3035623 > >> > >> > > >> > On 23.09.19, Alexander Turenko wrote: > >> > > FreeBSD has OpenSSL as part of the base system: libraries are located in > >> > > /usr/lib, headers are in /usr/include. However a user may install the > >> > > library into /usr/local/{lib,include} from ports / pkg. In this case > >> > > tarantool did choose /usr/local version, while libcurl will pick up a > >> > > base system library. This is fixed by passing --with-ssl option with an > >> > > argument (/usr/local or /usr if custom -DOPENSSL_ROOT_DIR=<...> is not > >> > > passed). > >> > > > >> > > Now the behaviour is the following. If -DOPENSSL_ROOT_DIR=<...> is > >> > > passed, then try to use OpenSSL from it. Otherwise find the library in > >> > > /usr/local and then in /usr. This is right as for tarantool's crypto > >> > > module as well as for libcurl submodule. > >> > > > >> > > There is a flaw here: a user is unable to choose a base system library > >> > > if a ports / pkg version of OpenSSL is installed. The reason here is > >> > > that tarantool's crypto module depends on other libraries and > >> > > -I/usr/local/include may be added to build options. I have no good > >> > > solution for that, so `cmake . -DOPENSSL_ROOT_DIR=/usr` will give a > >> > > warning on FreeBSD and `gmake` likely will fail if libraries are of > >> > > different versions (see cmake/os.cmake comments for more information). > >> > > See also a [discussion][1] in FreeBSD community about all those /usr and > >> > > /usr/local problems. > >> > > > >> > > There were two other problems that may fail tarantool build on FreeBSD: > >> > > they are fixed in this commit and described below. > >> > > > >> > > First, libcurl's configure script chooses GCC by default if it exists > >> > > (say, installed from ports / pkg). It is unexpected behaviour when > >> > > tarantool sources itself are built with clang. Now it is fixed by > >> > > passing a compiler explicitly to the libcurl's configure script: the > >> > > library will use base system clang by default or one that a user pass to > >> > > tarantool's cmake. > >> > > > >> > > Side note: GCC has /usr/local/include in its default headers search > >> > > paths; libcurl's configure script chooses GCC as a compiler and OpenSSL > >> > > from a base system by default that leads to OpenSSL header / library > >> > > mismatch. It is the primary reason of the build fail that was fixed in > >> > > 1f2338bd809585b0b38fe07fd9f80c31747374c2 ('build: FreeBSD packages > >> > > installation'). It is not much relevant anymore, because we don't try to > >> > > link with a base system OpenSSL if /usr/local one exists (if it is asked > >> > > explicitly with -DOPENSSL_ROOT_DIR=<...> we'll do, but will give a > >> > > warning). Anyway, it is important to know such details if we'll change > >> > > build scripts in a future. > >> > >> Added a note in parentheses: > >> > >> | Side note: GCC has /usr/local/include in its default headers search > >> | paths; libcurl's configure script chooses GCC as a compiler and OpenSSL > >> | from a base system by default (when CC and --with-ssl=<...> are not set) > >> | <...> > >> > >> > > > >> > > Second, backtraces are not supported on FreeBSD, but were enabled if > >> > > libunwind headers is found. This leads to an error on cmake stage, > >> > > because of unability to find a right library (this is the bug). Now we > >> > > disable backtraces on FreeBSD by default even if libunwind is found. See > >> > > #4278 for more information. > >> > > > >> > > [1]: https://wiki.freebsd.org/WarnerLosh/UsrLocal > >> > > > >> > > Follows up #4490. > >> > > --- > >> > > > >> > > https://github.com/tarantool/tarantool/issues/4490 > >> > > https://github.com/tarantool/tarantool/commits/Totktonada/gh-4490-fix-freebsd-openssl-linking-problems-full-ci > >> > > > >> > > This is more request for review of wording rather then of the code: hope > >> > > I verified it carefully enough. > >> > > > >> > > cmake/BuildLibCURL.cmake | 15 +++++++-------- > >> > > cmake/compiler.cmake | 4 +++- > >> > > cmake/os.cmake | 27 +++++++++++++++++++++++++++ > >> > > 3 files changed, 37 insertions(+), 9 deletions(-) > >> > > > >> > > diff --git a/cmake/BuildLibCURL.cmake b/cmake/BuildLibCURL.cmake > >> > > index 866b3c49e..45f5af23e 100644 > >> > > --- a/cmake/BuildLibCURL.cmake > >> > > +++ b/cmake/BuildLibCURL.cmake > >> > > @@ -14,14 +14,10 @@ macro(curl_build) > >> > > message(FATAL_ERROR "Unable to find zlib") > >> > > endif() > >> > > > >> > > - # Set curl option to find OpenSSL library. > >> > > - if ("${OPENSSL_ROOT_DIR}" STREQUAL "") > >> > > - # Linux / FreeBSD. > >> > > - set(LIBCURL_OPENSSL_OPT "--with-ssl") > >> > > - else() > >> > > - # Mac OS. > >> > > - set(LIBCURL_OPENSSL_OPT "--with-ssl=${OPENSSL_ROOT_DIR}") > >> > > - endif() > >> > > + # 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}") > >> > > > >> > > include(ExternalProject) > >> > > ExternalProject_Add( > >> > > @@ -35,6 +31,8 @@ macro(curl_build) > >> > > CONFIGURE_COMMAND > >> > > cd && ./buildconf && > >> > > cd && /configure > >> > > + CC=${CMAKE_C_COMPILER} > >> > > + CXX=${CMAKE_CXX_COMPILER} > >> > > >> > This changeset breaks build for OSX Mojave. The note is left as a > >> > reminder of known problem to be fixed in a while. > >> > >> Yep, fixed (see CPPFLAGS and CFLAGS in the new patch). > >> > >> > > >> > > --prefix > >> > > --enable-static > >> > > --enable-shared > >> > > @@ -112,6 +110,7 @@ macro(curl_build) > >> > > set(CURL_LIBRARIES ${CURL_LIBRARIES} rt) > >> > > endif() > >> > > > >> > > + unset(FOUND_OPENSSL_ROOT_DIR) > >> > > unset(LIBCURL_INSTALL_DIR) > >> > > unset(LIBCURL_BINARY_DIR) > >> > > unset(LIBCURL_SOURCE_DIR) > >> > > diff --git a/cmake/compiler.cmake b/cmake/compiler.cmake > >> > > index 887485c80..c9ad2b092 100644 > >> > > --- a/cmake/compiler.cmake > >> > > +++ b/cmake/compiler.cmake > >> > > @@ -128,8 +128,10 @@ else() > >> > > endif() > >> > > find_library(UNWIND_LIBRARY PATH_SUFFIXES system NAMES ${UNWIND_LIB_NAME}) > >> > > > >> > > +# Disabled backtraces support on FreeBSD by default, because of > >> > > +# gh-4278. > >> > > set(ENABLE_BACKTRACE_DEFAULT OFF) > >> > > -if (UNWIND_LIBRARY AND HAVE_LIBUNWIND_H) > >> > > +if (NOT TARGET_OS_FREEBSD AND UNWIND_LIBRARY AND HAVE_LIBUNWIND_H) > >> > > set(ENABLE_BACKTRACE_DEFAULT ON) > >> > > endif() > >> > > > >> > > diff --git a/cmake/os.cmake b/cmake/os.cmake > >> > > index ea581108b..fe96ce773 100644 > >> > > --- a/cmake/os.cmake > >> > > +++ b/cmake/os.cmake > >> > > @@ -22,6 +22,33 @@ elseif (${CMAKE_SYSTEM_NAME} STREQUAL "kFreeBSD") > >> > > elseif (${CMAKE_SYSTEM_NAME} STREQUAL "FreeBSD") > >> > > set(TARGET_OS_FREEBSD 1) > >> > > find_package_message(PLATFORM "Building for FreeBSD" "${CMAKE_SYSTEM_NAME}") > >> > > + > >> > > + # FreeBSD has OpenSSL library installed in /usr as part of a > >> > > + # base system. A user may install OpenSSL from ports / pkg to > >> > > + # /usr/local. It is tricky to use the library from /usr in the > >> > > + # case, because a compilation unit can also depend on > >> > > + # libraries from /usr/local. When -I/usr/local/include is > >> > > + # passed to a compiler it will find openssl/ssl.h from > >> > > + # /usr/local/include first. > >> > > + # > >> > > + # In theory we can create a directory on the build stage and > >> > > + # fill it with symlinks to choosen headers. However this way > >> > > + # does not look as usual way to pick libraries to build > >> > > + # against. I suspect that this is common problem on FreeBSD > >> > > + # and we should wait for some general resolution from FreeBSD > >> > > + # developers rather then work it around. > >> > > + # > >> > > + # Verify that /usr is not set as a directory to pick OpenSSL > >> > > + # library and header files, because it is likely that a user > >> > > + # set it to use the library from a base system, while the > >> > > + # library is also installed into /usr/local. > >> > > + get_filename_component(REAL_OPENSSL_ROOT_DIR "${OPENSSL_ROOT_DIR}" > >> > > + REALPATH BASE_DIR "${CMAKE_BINARY_DIR}") > >> > > + if ("${REAL_OPENSSL_ROOT_DIR}" STREQUAL "/usr") > >> > > >> > It would be much clearer if you dropped a few words for the warning > >> > level usage instead of fatal one here (I guess I saw the rationale > >> > within a commit message). > >> > >> Added the following paragraph: > >> > >> | It is possible however that a user is aware of the problem, > >> | but want to use -DOPENSSL_ROOT_DIR=<...> CMake option to > >> | choose OpenSSL from /usr anyway. We should not fail the > >> | build and block this ability. Say, a user may know that > >> | there are no OpenSSL libraries in /usr/local, but finds it > >> | convenient to set the CMake option explicitly due to some > >> | external reason. > >> > >> > > >> > > + message(WARNING "Using OPENSSL_ROOT_DIR on FreeBSD to choose base " > >> > > + "system libraries is not supported") > >> > > + endif() > >> > > + unset(REAL_OPENSSL_ROOT_DIR) > >> > > elseif (${CMAKE_SYSTEM_NAME} STREQUAL "NetBSD") > >> > > set(TARGET_OS_NETBSD 1) > >> > > find_package_message(PLATFORM "Building for NetBSD" "${CMAKE_SYSTEM_NAME}") > >> > > -- > >> > > 2.22.0 > >> > > > >> > > >> > -- > >> > Best regards, > >> > IM > >> > >> ---- > >> > >> build: fix OpenSSL linking problems on FreeBSD > >> > >> FreeBSD has OpenSSL as part of the base system: libraries are located in > >> /usr/lib, headers are in /usr/include. However a user may install the > >> library into /usr/local/{lib,include} from ports / pkg. In this case > >> tarantool did choose /usr/local version, while libcurl will pick up a > >> base system library. This is fixed by passing --with-ssl option with an > >> argument (/usr/local or /usr if custom -DOPENSSL_ROOT_DIR=<...> is not > >> passed). > >> > >> Now the behaviour is the following. If -DOPENSSL_ROOT_DIR=<...> is > >> passed, then try to use OpenSSL from it. Otherwise find the library in > >> /usr/local and then in /usr. This is right as for tarantool's crypto > >> module as well as for libcurl submodule. > >> > >> There is a flaw here: a user is unable to choose a base system library > >> if a ports / pkg version of OpenSSL is installed. The reason here is > >> that tarantool's crypto module depends on other libraries and > >> -I/usr/local/include may be added to build options. I have no good > >> solution for that, so `cmake . -DOPENSSL_ROOT_DIR=/usr` will give a > >> warning on FreeBSD and `gmake` likely will fail if libraries are of > >> different versions (see cmake/os.cmake comments for more information). > >> See also a [discussion][1] in FreeBSD community about all those /usr and > >> /usr/local problems. > >> > >> There were two other problems that may fail tarantool build on FreeBSD: > >> they are fixed in this commit and described below. > >> > >> First, libcurl's configure script chooses GCC by default if it exists > >> (say, installed from ports / pkg). It is unexpected behaviour when > >> tarantool sources itself are built with clang. Now it is fixed by > >> passing a compiler explicitly to the libcurl's configure script: the > >> library will use base system clang by default or one that a user pass to > >> tarantool's cmake. > >> > >> Side note: GCC has /usr/local/include in its default headers search > >> paths; libcurl's configure script chooses GCC as a compiler and OpenSSL > >> from a base system by default (when CC and --with-ssl=<...> are not set) > >> that leads to OpenSSL header / library mismatch. It is the primary > >> reason of the build fail that was fixed in > >> 1f2338bd809585b0b38fe07fd9f80c31747374c2 ('build: FreeBSD packages > >> installation'). It is not much relevant anymore, because we don't try to > >> link with a base system OpenSSL if /usr/local one exists (however if it > >> is asked explicitly with -DOPENSSL_ROOT_DIR=<...> we'll do, but will > >> give a warning). Anyway, it is important to know such details if we'll > >> change build scripts in a future. > >> > >> Second, backtraces are not supported on FreeBSD, but were enabled if > >> libunwind headers is found. This leads to an error on cmake stage, > >> because of inability to find a right library (this is a bug). Now we > >> disable backtraces on FreeBSD by default even if libunwind is found. See > >> #4278 for more information. > >> > >> When CC is passed to libcurl's configure script, the new problem opens > >> on Mac OS. CMake chooses XCode toolchain by default (at least on a > >> particular system where I tried it), which requires -isysroot= > >> option to be passed to a preprocessor and a compiler in order to find > >> system headers. See [2] for more information. > >> > >> [1]: https://wiki.freebsd.org/WarnerLosh/UsrLocal > >> [2]: https://developer.apple.com/documentation/xcode_release_notes/xcode_10_release_notes#3035623 > >> > >> Follows up #4490. > >> --- > >> cmake/BuildLibCURL.cmake | 25 ++++++++++++++++++------- > >> cmake/compiler.cmake | 4 +++- > >> cmake/os.cmake | 35 +++++++++++++++++++++++++++++++++++ > >> 3 files changed, 56 insertions(+), 8 deletions(-) > >> > >> diff --git a/cmake/BuildLibCURL.cmake b/cmake/BuildLibCURL.cmake > >> index 866b3c49e..0dc7676d6 100644 > >> --- a/cmake/BuildLibCURL.cmake > >> +++ b/cmake/BuildLibCURL.cmake > >> @@ -14,13 +14,18 @@ macro(curl_build) > >> message(FATAL_ERROR "Unable to find zlib") > >> endif() > >> > >> - # Set curl option to find OpenSSL library. > >> - if ("${OPENSSL_ROOT_DIR}" STREQUAL "") > >> - # Linux / FreeBSD. > >> - set(LIBCURL_OPENSSL_OPT "--with-ssl") > >> - else() > >> - # Mac OS. > >> - set(LIBCURL_OPENSSL_OPT "--with-ssl=${OPENSSL_ROOT_DIR}") > >> + # 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}") > >> + > >> + # Pass -isysroot= option on Mac OS to a preprocessor > >> + # and a C compiler to find header files installed with an SDK. > >> + 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}") > >> endif() > >> > >> include(ExternalProject) > >> @@ -35,6 +40,9 @@ macro(curl_build) > >> CONFIGURE_COMMAND > >> cd && ./buildconf && > >> cd && /configure > >> + CC=${CMAKE_C_COMPILER} > >> + CPPFLAGS=${LIBCURL_CPPFLAGS} > >> + CFLAGS=${LIBCURL_CFLAGS} > >> --prefix > >> --enable-static > >> --enable-shared > >> @@ -112,6 +120,9 @@ macro(curl_build) > >> set(CURL_LIBRARIES ${CURL_LIBRARIES} rt) > >> endif() > >> > >> + unset(LIBCURL_CPPFLAGS) > >> + unset(LIBCURL_CFLAGS) > >> + unset(FOUND_OPENSSL_ROOT_DIR) > >> unset(LIBCURL_INSTALL_DIR) > >> unset(LIBCURL_BINARY_DIR) > >> unset(LIBCURL_SOURCE_DIR) > >> diff --git a/cmake/compiler.cmake b/cmake/compiler.cmake > >> index 887485c80..c9ad2b092 100644 > >> --- a/cmake/compiler.cmake > >> +++ b/cmake/compiler.cmake > >> @@ -128,8 +128,10 @@ else() > >> endif() > >> find_library(UNWIND_LIBRARY PATH_SUFFIXES system NAMES ${UNWIND_LIB_NAME}) > >> > >> +# Disabled backtraces support on FreeBSD by default, because of > >> +# gh-4278. > >> set(ENABLE_BACKTRACE_DEFAULT OFF) > >> -if (UNWIND_LIBRARY AND HAVE_LIBUNWIND_H) > >> +if (NOT TARGET_OS_FREEBSD AND UNWIND_LIBRARY AND HAVE_LIBUNWIND_H) > >> set(ENABLE_BACKTRACE_DEFAULT ON) > >> endif() > >> > >> diff --git a/cmake/os.cmake b/cmake/os.cmake > >> index ea581108b..0ed554b9b 100644 > >> --- a/cmake/os.cmake > >> +++ b/cmake/os.cmake > >> @@ -22,6 +22,41 @@ elseif (${CMAKE_SYSTEM_NAME} STREQUAL "kFreeBSD") > >> elseif (${CMAKE_SYSTEM_NAME} STREQUAL "FreeBSD") > >> set(TARGET_OS_FREEBSD 1) > >> find_package_message(PLATFORM "Building for FreeBSD" "${CMAKE_SYSTEM_NAME}") > >> + > >> + # FreeBSD has OpenSSL library installed in /usr as part of a > >> + # base system. A user may install OpenSSL from ports / pkg to > >> + # /usr/local. It is tricky to use the library from /usr in the > >> + # case, because a compilation unit can also depend on > >> + # libraries from /usr/local. When -I/usr/local/include is > >> + # passed to a compiler it will find openssl/ssl.h from > >> + # /usr/local/include first. > >> + # > >> + # In theory we can create a directory on the build stage and > >> + # fill it with symlinks to choosen headers. However this way > >> + # does not look as usual way to pick libraries to build > >> + # against. I suspect that this is common problem on FreeBSD > >> + # and we should wait for some general resolution from FreeBSD > >> + # developers rather then work it around. > >> + # > >> + # Verify that /usr is not set as a directory to pick OpenSSL > >> + # library and header files, because it is likely that a user > >> + # set it to use the library from a base system, while the > >> + # library is also installed into /usr/local. > >> + # > >> + # It is possible however that a user is aware of the problem, > >> + # but want to use -DOPENSSL_ROOT_DIR=<...> CMake option to > >> + # choose OpenSSL from /usr anyway. We should not fail the > >> + # build and block this ability. Say, a user may know that > >> + # there are no OpenSSL libraries in /usr/local, but finds it > >> + # convenient to set the CMake option explicitly due to some > >> + # external reason. > >> + get_filename_component(REAL_OPENSSL_ROOT_DIR "${OPENSSL_ROOT_DIR}" > >> + REALPATH BASE_DIR "${CMAKE_BINARY_DIR}") > >> + if ("${REAL_OPENSSL_ROOT_DIR}" STREQUAL "/usr") > >> + message(WARNING "Using OPENSSL_ROOT_DIR on FreeBSD to choose base " > >> + "system libraries is not supported") > >> + endif() > >> + unset(REAL_OPENSSL_ROOT_DIR) > >> elseif (${CMAKE_SYSTEM_NAME} STREQUAL "NetBSD") > >> set(TARGET_OS_NETBSD 1) > >> find_package_message(PLATFORM "Building for NetBSD" "${CMAKE_SYSTEM_NAME}") > > > >-- > >Best regards, > >IM > > > > > -- > Alexander Tikhonov