From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp52.i.mail.ru (smtp52.i.mail.ru [94.100.177.112]) (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 67A0F46970E for ; Sat, 8 Feb 2020 22:28:38 +0300 (MSK) Date: Sat, 8 Feb 2020 22:28:53 +0300 From: Alexander Turenko Message-ID: <20200208192852.qlg3pqmycu7ig7t6@tkn_work_nb> References: <20191125135347.5105-1-sergepetrenko@tarantool.org> <20191218023335.6olxxxqeyj7qhyhb@tkn_work_nb> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Subject: Re: [Tarantool-patches] [PATCH] build: link bundled libcurl with c-ares List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Serge Petrenko Cc: tarantool-patches@dev.tarantool.org Oh, so weird world of build systems. I tried to understand how everything should work, what is flawed on c-ares build system side and on our side. This description becomes large, so I'll summarize what actions I expect from you: * File an issue against c-ares that CMake script should not add unnecessary libraries (./configure does not do that). * Add empty CARES_LIBRARIES (with proper FIXME comment) and add it to CURL_LIBRARIES. ---- > > Is c-ares depends on some libraries? > > CMakeLists.txt extract: Having dependent libraries opens several questions that we should verify for each of those libs: 1. Is it possible that the library exists on a build system, but may be missed on a target system. A build system may be one where we build a package (Deb, RPM, brew bottle, FreeBSD binary package), a target system is where tarantool is installed from the package. 2. If a library on which our static library (c-ares) depends is not an integral part of a system, then we should add it to dependencies of packages and link the transitive dependency statically when doing a static build of tarantool. > # Tell C-Ares about libraries to depend on > IF (HAVE_LIBRESOLV) > LIST (APPEND CARES_DEPENDENT_LIBS resolv) > ENDIF () It is for iPhone targets (see [1]), so we can don't bother with that. [1]: https://github.com/c-ares/c-ares/commit/affc63cba875df11daade6f6767cb06e013ac6c3 > IF (HAVE_LIBNSL) > LIST (APPEND CARES_DEPENDENT_LIBS nsl) > ENDIF () It just adds -lnsl at linking (there are no ifdefs that depends on this flag). I guess it is only to borrow gethostbyname() and maybe other network related functions, which is part of libc. Points: * Old glibc ships gethostbyname() in libnsl.so, but now it is right in libc.so. * There is modern separate libnsl (see [2]), which supports IPv6. * -lnsl is not added by ./configure if gethostbyname() is available w/o it. * gethostbyname() is used only in adig.c and we anyway disable tools with -DCARES_BUILD_TOOLS=OFF. Don't sure about other functions from this library. I would work around CMake logic to stick with ./configure logic: add -lnsl only when gethostbyname() is not availiable w/o it. After this we'll build c-ares with -lnsl only when libnsl.so is the integral part of a distro and so both build and install system should have it. (Update: tarantool executable is linked w/o libnsl.so, so this may be ignored.) Let's file an issue to c-ares, which will show different default behaviour of cmake and configure. BTW, grpc even disabled this unconditionally, see [3] and [4]. [2]: https://github.com/thkukuk/libnsl [3]: https://github.com/grpc/grpc/issues/17255 [4]: https://github.com/grpc/grpc/blob/e83426b741d21b744c111109dbac899d6e99b4d7/cmake/cares.cmake#L23 > IF (HAVE_LIBSOCKET) > LIST (APPEND CARES_DEPENDENT_LIBS socket) > ENDIF () It seems it is mostly the same as libnsl: usually don't needed and should be enable only when necessary and is a part of a system. There are no however separate library with the same name (I didn't find any) and so enabling it when it exists looks more or less safe. So no actions is necessary from our side, however I would anyway mention in the issue (which I proposed to send above) that it would be better to enable it only when there are no getaddrinfo() right inside libc.so. > IF (HAVE_LIBRT) > LIST (APPEND CARES_DEPENDENT_LIBS rt) > ENDIF () c-ares cmake file doing the same bad thing here: it should check whether clock_gettime() is available in libc.so (w/o extra libs) and only then check for librt.so. See `man 2 clock_gettime` (cited below) and [5]. Anyway, it should be hurtless in the case, because librt.so either exists on both build and install systems or doesn't. >From `man 2 clock_gettime`: | Link with -lrt (only for glibc versions before 2.17). [5]: https://github.com/tarantool/tarantool/commit/960e9c0c7ab92155842deaafb40bf240501c8145 ---- Since all those libraries are system ones (except new libnsl, but we should avoid it), static build should be fine: we don't need to add any static libs at linking of tarantool executable. It would be good to add all those librt.so, libsocket.so and others to link command of tarantool executable (and possibly some unit tests) when they're used to create libcares.a archive. This is what we doing using CURL_LIBRARIES variable for libcurl. Everything works now, but just because libraries that are added to libcares.so are not actually needed on systems we have in CI. Let's compare c-ares default libs when compiling with the configure script and CMake on my system (I have the new libnsl installed): Configure: | $ ./buildconf && ./configure && make -j | $ ldd .libs/libcares.so | linux-vdso.so.1 (0x00007ffeaa77b000) | libc.so.6 => /lib64/libc.so.6 (0x00007fc0ae12c000) | /lib64/ld-linux-x86-64.so.2 (0x00007fc0ae34a000) CMake: | $ cmake . && make -j | $ ldd lib/libcares.so | linux-vdso.so.1 (0x00007ffceebd6000) | libnsl.so.2 => /usr/lib64/libnsl.so.2 (0x00007f98321cf000) | librt.so.1 => /lib64/librt.so.1 (0x00007f98321c5000) | libc.so.6 => /lib64/libc.so.6 (0x00007f9831ff3000) | libtirpc.so.3 => /lib64/libtirpc.so.3 (0x00007f9831dc8000) | libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f9831da5000) | /lib64/ld-linux-x86-64.so.2 (0x00007f983243a000) NB: I didn't look why libtirpc and libpthread were added. I guess they are not necessary too. But tarantool executable does not have, say, libnsl.so.2 and libtirpc.so.3 in deps (verified on commit 60d67cf38872e012f83410591dfa39140fc679e0): | $ ldd src/tarantool | linux-vdso.so.1 (0x00007ffeb2796000) | libicui18n.so.65 => /usr/lib64/libicui18n.so.65 (0x00007fe4f3053000) | libicuuc.so.65 => /usr/lib64/libicuuc.so.65 (0x00007fe4f2e74000) | libicudata.so.65 => /usr/lib64/libicudata.so.65 (0x00007fe4f13c1000) | libreadline.so.8 => /lib64/libreadline.so.8 (0x00007fe4f136e000) | libncurses.so.6 => /lib64/libncurses.so.6 (0x00007fe4f1310000) | libform.so.6 => /usr/lib64/libform.so.6 (0x00007fe4f12fd000) | libz.so.1 => /lib64/libz.so.1 (0x00007fe4f12e1000) | librt.so.1 => /lib64/librt.so.1 (0x00007fe4f12d7000) | libssl.so.1.1 => /usr/lib64/libssl.so.1.1 (0x00007fe4f126a000) | libcrypto.so.1.1 => /usr/lib64/libcrypto.so.1.1 (0x00007fe4f0ffd000) | libdl.so.2 => /lib64/libdl.so.2 (0x00007fe4f0ff7000) | libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fe4f0fd4000) | libgcc_s.so.1 => /usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/libgcc_s.so.1 (0x00007fe4f0fb8000) | libunwind-x86_64.so.8 => /usr/lib64/libunwind-x86_64.so.8 (0x00007fe4f0f96000) | libunwind.so.8 => /usr/lib64/libunwind.so.8 (0x00007fe4f0f79000) | libgomp.so.1 => /usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/libgomp.so.1 (0x00007fe4f0f41000) | libstdc++.so.6 => /usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/libstdc++.so.6 (0x00007fe4f0cc9000) | libm.so.6 => /lib64/libm.so.6 (0x00007fe4f0b8b000) | libc.so.6 => /lib64/libc.so.6 (0x00007fe4f09b7000) | /lib64/ld-linux-x86-64.so.2 (0x00007fe4f39d9000) | liblzma.so.5 => /lib64/liblzma.so.5 (0x00007fe4f098e000) So, in short: * It is good to provide CARES_LIBRARIES, which will be added to CURL_LIBRARIES. Even if we'll left it empty for now, it will show proper place where to add a library if we'll need to fix a build on some specific platform, say, Solaris. * It would be good to get rid of all unnecessary libraries during c-ares build, but since we ignore them anyway at tarantool build and everything occasionally works okay on all platforms in CI, then maybe we can leave everything as is for now.