[tarantool-patches] Re: [PATCH] build: fix OpenSSL linking problems on FreeBSD

[Igor Munkin]

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.

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.
> 
> 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 <SOURCE_DIR> && ./buildconf &&
>              cd <BINARY_DIR> && <SOURCE_DIR>/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.

>                  --prefix <INSTALL_DIR>
>                  --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).

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