<HTML><BODY><p>Alexander,<br><br>Thanks a lot for the review and suggested patch, I've used it for the current fix and checked it - it completely passed. I've updated the commit message according the new patch and corrected it as you suggested.</p><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">
Четверг, 28 ноября 2019, 19:41 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>:<br><br><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15749593100386253886_BODY">fiber.cc now uses clock_getttime(), which requires -lrt with glibc prior<br>
2.1. Let's add librt dependency for core library for glibc of those<br>
versions.<br><br>
We should check for glibc version (using __GLIBC__ and __GLIBC_MINOR__<br>
macros) or perform a check whether we able to use clock_gettime() w/o<br>
librt.<br></div></div></div></div></blockquote>Ok, I've red the man page to know it as you wrote in your patch comments (you misspelled - just not 2.1 but 2.17).<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15749593100386253886_BODY"><br>
So the patch should look like this:<br><br>
| diff --git a/CMakeLists.txt b/CMakeLists.txt<br>
| index 7d6c084..ca968eb 100644<br>
| --- a/CMakeLists.txt<br>
| +++ b/CMakeLists.txt<br>
| @@ -116,6 +116,12 @@ else ()<br>
| set(HAVE_CLOCK_GETTIME ${HAVE_CLOCK_GETTIME_DECL})<br>
| endif ()<br>
| set(CMAKE_REQUIRED_LIBRARIES "")<br>
| +# According to `man 2 clockgettime` the glibc wrapper requires<br>
| +# -lrt on glibc versions before 2.17. We need to check whether<br>
| +# the function is available without -lrt to set this linker option<br>
| +# conditionally.<br>
| +check_function_exists(clock_gettime HAVE_CLOCK_GETTIME_WITHOUT_RT)<br>
| +<br>
| check_symbol_exists(__get_cpuid cpuid.h HAVE_CPUID)<br>
| <br>
| # Checks for libev<br>
| diff --git a/src/lib/core/CMakeLists.txt b/src/lib/core/CMakeLists.txt<br>
| index e60b519..72e631b 100644<br>
| --- a/src/lib/core/CMakeLists.txt<br>
| +++ b/src/lib/core/CMakeLists.txt<br>
| @@ -46,3 +46,11 @@ target_link_libraries(core salad small uri decNumber ${LIBEV_LIBRARIES}<br>
| if (ENABLE_BACKTRACE AND NOT TARGET_OS_DARWIN)<br>
| target_link_libraries(core gcc_s ${UNWIND_LIBRARIES})<br>
| endif()<br>
| +<br>
| +# Since fiber.top() introduction, fiber.cc, which is part of core<br>
| +# library, depends on clock_gettime() syscall, so we should set<br>
| +# -lrt when it is appropriate. See a comment for<br>
| +# HAVE_CLOCK_GETTIME_WITHOUT_RT in ${REPO}/CMakeLists.txt.<br>
| +if ("${HAVE_CLOCK_GETTIME}" AND NOT "${HAVE_CLOCK_GETTIME_WITHOUT_RT}")<br>
| + target_link_libraries(core rt)<br>
| +endif()<br></div></div></div></div></blockquote>I've used your patch as a fix.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15749593100386253886_BODY"><br>
CCed Serge as author of the patch that introduces the regression. Serge,<br>
it seems we should disable fiber.top() or give explicit error when<br>
HAVE_CLOCK_GETTIME is false.<br><br>
WBR, Alexander Turenko.<br><br>
On Fri, Nov 22, 2019 at 04:51:38PM +0300, Alexander V. Tikhonov wrote:<br>
> After the commit 77fa45bd05f8cdd4c0f9bad85185ef5b61528d49<br><br>
Please, use format commit_hash ('commit message header').<br></div></div></div></div></blockquote>Fixed, also I've checked as you did it and made the same.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15749593100386253886_BODY"><br>
> the unit tests builds failed like:<br>
> <br>
> /opt/rh/devtoolset-6/root/usr/libexec/gcc/x86_64-redhat-linux/6.3.1/ld:<br>
> ../../src/lib/core/libcore.a(fiber.c.o): undefined reference to symbol<br>
> 'clock_gettime@@GLIBC_2.2.5'<br>
> //lib64/librt.so.1: error adding symbols: DSO missing from command line<br>
> collect2: error: ld returned 1 exit status<br>
> test/unit/CMakeFiles/cbus.test.dir/build.make:108: recipe for target<br>
> 'test/unit/cbus.test' failed<br>
> make[2]: *** [test/unit/cbus.test] Error 1<br>
> <br>
> To fix the issue the 'rt' library was added to test unit cmake file only<br>
> for linux and freebsd targets.<br><br>
It is glibc flaw, no need to change something on FreeBSD.<br></div></div></div></div></blockquote>Ok.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15749593100386253886_BODY"><br>
> <br>
> Close #4639<br>
> ---<br>
> <br>
> Github: <a href="https://github.com/tarantool/tarantool/tree/avtikhon/gh-4639-lrt-unit-tests-full-ci" target="_blank">https://github.com/tarantool/tarantool/tree/avtikhon/gh-4639-lrt-unit-tests-full-ci</a><br>
> Issue: <a href="https://github.com/tarantool/tarantool/issues/4639" target="_blank">https://github.com/tarantool/tarantool/issues/4639</a><br></div></div></div></div></blockquote>
<br>
<br>-- <br>Alexander Tikhonov<br></BODY></HTML>