Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alexander Turenko <alexander.turenko@tarantool.org>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: Alexander Turenko <alexander.turenko@tarantool.org>,
	tarantool-patches@freelists.org
Subject: [tarantool-patches] [PATCH] build: fix linking with static openssl library
Date: Fri, 23 Aug 2019 05:08:00 +0300	[thread overview]
Message-ID: <3124ad309d395d9fe9581499198bcd2de9c83b3c.1566525651.git.alexander.turenko@tarantool.org> (raw)

System-wide dynamic libraries usually (always?) have NEEDED and RUNPATH
tags in a dynamic section (as `readelf -d /usr/lib/lib<...>.so` shows),
so when we link, say, with libssl.so, which depends on libz.so, a linker
does not complain against unresolved symbols that can be found in Z
library (if it is installed within a system).

Things are different when we linking with a static library. Say, when we
linking with libssl.a, which contains an unresolved symbol from Z
library, a linker reports an error. It is not possible to store an
information where to find unresolved symbols (NEEDED / RUNPATH) in a
static library (AFAIK).

We depend on three libraries that are depend on Z library: libcurl,
libssl and libcrypto (two latter are part of OpenSSL). When one of those
libraries is linked statically we should link with libz.so or libz.a
(depending on BUILD_STATIC flag). The patch doing exactly this.

Before this patch we add Z library to OPENSSL_LIBRARIES when
BUILD_STATIC is enabled. It is not quite correct: we should do that when
OPENSSL_USE_STATIC_LIBS is enabled. However we didn't experienced
problems with static linking of libcurl (when BUILD_STATIC is enabled),
because OPENSSL_LIBRARIES is added to CURL_LIBRARIES. This is a kind of
side effect of dependency libcurl on OpenSSL and it is better to add
libz explicitly to CURL_LIBRARIES (and OPENSSL_LIBRARIES) when
appropriate.

Fixes #4437.
---

https://github.com/tarantool/tarantool/issues/4437
https://github.com/tarantool/tarantool/tree/Totktonada/gh-4437-fix-static-openssl-build-full-ci

I more or less understood what is going on with all that linking stuff,
but it would be glad if you'll check my wording around this topic.

I doubt that the ability to link OpenSSL statically, but other libs
dynamically is really needed, but the commit at least holds the approach
when to add transitive dependencies to ours and when don't.

 CMakeLists.txt       | 10 +++++++---
 cmake/FindCURL.cmake |  6 ++++++
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 9b3950bdf..ddca8db85 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -324,10 +324,14 @@ endif()
 
 #
 # OpenSSL can require Z library (depending on build time options), so we add
-# it to libraries list in case of static build.
+# it to libraries list in case of static openssl linking.
 #
-if(BUILD_STATIC)
-    find_library(Z_LIBRARY libz.a)
+if(OPENSSL_USE_STATIC_LIBS)
+    if(BUILD_STATIC)
+        find_library(Z_LIBRARY libz.a)
+    else()
+        find_library(Z_LIBRARY z)
+    endif()
     set(OPENSSL_LIBRARIES ${OPENSSL_LIBRARIES} ${Z_LIBRARY})
 endif()
 
diff --git a/cmake/FindCURL.cmake b/cmake/FindCURL.cmake
index e13b6cde4..6193a58ad 100644
--- a/cmake/FindCURL.cmake
+++ b/cmake/FindCURL.cmake
@@ -18,9 +18,11 @@
 
 if(BUILD_STATIC)
     set(CURL_LIB_NAME libcurl.a)
+    set(Z_LIB_NAME libz.a)
     set(NGHTTP2_LIB_NAME libnghttp2.a)
 else()
     set(CURL_LIB_NAME curl)
+    set(Z_LIB_NAME z)
     set(NGHTTP2_LIB_NAME nghttp2)
 endif()
 
@@ -30,6 +32,7 @@ set(DL_LIB_NAME dl)
 
 # Curl may be linked with optional or target-dependent libraries,
 # search for them and add to dependicies if found.
+find_library(Z_LIBRARY NAMES ${Z_LIB_NAME})
 find_library(NGHTTP2_LIBRARY NAMES ${NGHTTP2_LIB_NAME})
 find_library(PTHREAD_LIBRARY NAMES ${PTHREAD_LIB_NAME})
 find_library(DL_LIBRARY NAMES ${DL_LIB_NAME})
@@ -88,6 +91,9 @@ if(CURL_FOUND)
   set(CURL_LIBRARIES ${CURL_LIBRARIES} ${OPENSSL_LIBRARIES})
   if(BUILD_STATIC)
     # In case of a static build we have to add curl dependencies.
+    if(NOT "${Z_LIBRARY}" STREQUAL "Z_LIBRARY-NOTFOUND")
+      set(CURL_LIBRARIES ${CURL_LIBRARIES} ${Z_LIBRARY})
+    endif()
     if(NOT "${NGHTTP2_LIBRARY}" STREQUAL "NGHTTP2_LIBRARY-NOTFOUND")
       set(CURL_LIBRARIES ${CURL_LIBRARIES} ${NGHTTP2_LIBRARY})
     endif()
-- 
2.22.0

             reply	other threads:[~2019-08-23  2:08 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-23  2:08 Alexander Turenko [this message]
2019-08-23 21:07 ` [tarantool-patches] " Vladislav Shpilevoy
2019-08-25 11:15   ` Alexander Turenko
2019-08-26 23:02     ` Alexander Turenko
2019-08-27 22:23       ` Vladislav Shpilevoy
2019-08-27 23:38 ` Kirill Yukhin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3124ad309d395d9fe9581499198bcd2de9c83b3c.1566525651.git.alexander.turenko@tarantool.org \
    --to=alexander.turenko@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [tarantool-patches] [PATCH] build: fix linking with static openssl library' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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