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