[Tarantool-patches] [PATCH v2] build: fix unit tests build with lrt

[Tarantool-patches] [PATCH v2] build: fix unit tests build with lrt

[Alexander V. Tikhonov]

After the commit 77fa45bd05f8cdd4c0f9bad85185ef5b61528d49
('lua: add fiber.top() listing fiber cpu consumption')
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

Found that fiber.cc is using now clock_gettime(), which requires -lrt
with glibc. To fix it added librt dependency for core library for glibc.
Due to glibc requires for -lrt for clock_gettime() only for some
versions, check 'man clock_gettime.2':
  'Link with -lrt (only for glibc versions before 2.17).'
the check whether is able to use clock_gettime() w/o librt library is
added.

Close #4639
---

Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4639-lrt-suggested-full-ci
Issue: https://github.com/tarantool/tarantool/issues/4639

 CMakeLists.txt              | 6 ++++++
 src/lib/core/CMakeLists.txt | 8 ++++++++
 2 files changed, 14 insertions(+)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 7d6c0846e..ca968eb8d 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 e60b5199e..8623aa0de 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()
-- 
2.17.1

Re: [Tarantool-patches] [PATCH v2] build: fix unit tests build with lrt

[Alexander Turenko]

LGTM.

I'm okay with the approach I proposed, of course :) My review would be
insufficient.

Serge, can you do the second review?

WBR, Alexander Turenko.

> build: fix unit tests build with lrt

Nit: librt or -ltr

Or just 'on CentOS 6'. All those details about librt are described in
the commit message below, while the header would be better if it will
give us an idea why the commit was made.

> After the commit 77fa45bd05f8cdd4c0f9bad85185ef5b61528d49
> ('lua: add fiber.top() listing fiber cpu consumption')
> 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
> 
> Found that fiber.cc is using now clock_gettime(), which requires -lrt
> with glibc. To fix it added librt dependency for core library for glibc.
> Due to glibc requires for -lrt for clock_gettime() only for some
> versions, check 'man clock_gettime.2':
>   'Link with -lrt (only for glibc versions before 2.17).'
> the check whether is able to use clock_gettime() w/o librt library is
> added.
> 
> Close #4639
> ---
> 
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4639-lrt-suggested-full-ci
> Issue: https://github.com/tarantool/tarantool/issues/4639
> 
>  CMakeLists.txt              | 6 ++++++
>  src/lib/core/CMakeLists.txt | 8 ++++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index 7d6c0846e..ca968eb8d 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 e60b5199e..8623aa0de 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()
> -- 
> 2.17.1
> 

Re: [Tarantool-patches] [PATCH v2] build: fix unit tests build with lrt

[Alexander Tikhonov]

[-- Attachment #1: Type: text/plain, Size: 3569 bytes --]

Alexander,

Thanks for the review, updated commit message as you suggested.

>Четверг,  5 декабря 2019, 4:49 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>:
>
>LGTM.
>
>I'm okay with the approach I proposed, of course :) My review would be
>insufficient.
>
>Serge, can you do the second review?
>
>WBR, Alexander Turenko.
>
>> build: fix unit tests build with lrt
>
>Nit: librt or -ltr
>
>Or just 'on CentOS 6'. All those details about librt are described in
>the commit message below, while the header would be better if it will
>give us an idea why the commit was made.
Corrected commit message added 'on CentOS 6' add '-lrt', (use of 'librt' exceeded the limit of the commit message length)
>
>> After the commit 77fa45bd05f8cdd4c0f9bad85185ef5b61528d49
>> ('lua: add fiber.top() listing fiber cpu consumption')
>> 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
>> 
>> Found that fiber.cc is using now clock_gettime(), which requires -lrt
>> with glibc. To fix it added librt dependency for core library for glibc.
>> Due to glibc requires for -lrt for clock_gettime() only for some
>> versions, check 'man clock_gettime.2':
>>   'Link with -lrt (only for glibc versions before 2.17).'
>> the check whether is able to use clock_gettime() w/o librt library is
>> added.
>> 
>> Close #4639
>> ---
>> 
>> Github:  https://github.com/tarantool/tarantool/tree/avtikhon/gh-4639-lrt-suggested-full-ci
>> Issue:  https://github.com/tarantool/tarantool/issues/4639
>> 
>>  CMakeLists.txt              | 6 ++++++
>>  src/lib/core/CMakeLists.txt | 8 ++++++++
>>  2 files changed, 14 insertions(+)
>> 
>> diff --git a/CMakeLists.txt b/CMakeLists.txt
>> index 7d6c0846e..ca968eb8d 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 e60b5199e..8623aa0de 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()
>> -- 
>> 2.17.1
>> 


-- 
Alexander Tikhonov

[-- Attachment #2: Type: text/html, Size: 4746 bytes --]

Re: [Tarantool-patches] [PATCH v2] build: fix unit tests build with lrt

[Serge Petrenko]

[-- Attachment #1: Type: text/plain, Size: 4069 bytes --]

Hi! Thanks for the patch.

Looks good to me.


> 6 дек. 2019 г., в 7:50, Alexander Tikhonov <avtikhon@tarantool.org> написал(а):
> 
> Alexander,
> 
> Thanks for the review, updated commit message as you suggested.
> 
> Четверг, 5 декабря 2019, 4:49 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>:
> 
> LGTM.
> 
> I'm okay with the approach I proposed, of course :) My review would be
> insufficient.
> 
> Serge, can you do the second review?
> 
> WBR, Alexander Turenko.
> 
> > build: fix unit tests build with lrt
> 
> Nit: librt or -ltr
> 
> Or just 'on CentOS 6'. All those details about librt are described in
> the commit message below, while the header would be better if it will
> give us an idea why the commit was made.
> Corrected commit message added 'on CentOS 6' add '-lrt', (use of 'librt' exceeded the limit of the commit message length)
> 
> > After the commit 77fa45bd05f8cdd4c0f9bad85185ef5b61528d49
> > ('lua: add fiber.top() listing fiber cpu consumption')
> > 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
> > 
> > Found that fiber.cc is using now clock_gettime(), which requires -lrt
> > with glibc. To fix it added librt dependency for core library for glibc.
> > Due to glibc requires for -lrt for clock_gettime() only for some
> > versions, check 'man clock_gettime.2':
> > 'Link with -lrt (only for glibc versions before 2.17).'
> > the check whether is able to use clock_gettime() w/o librt library is
> > added.
> > 
> > Close #4639
> > ---
> > 
> > Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4639-lrt-suggested-full-ci <https://github.com/tarantool/tarantool/tree/avtikhon/gh-4639-lrt-suggested-full-ci>
> > Issue: https://github.com/tarantool/tarantool/issues/4639 <https://github.com/tarantool/tarantool/issues/4639>
> > 
> > CMakeLists.txt | 6 ++++++
> > src/lib/core/CMakeLists.txt | 8 ++++++++
> > 2 files changed, 14 insertions(+)
> > 
> > diff --git a/CMakeLists.txt b/CMakeLists.txt
> > index 7d6c0846e..ca968eb8d 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 e60b5199e..8623aa0de 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()
> > -- 
> > 2.17.1
> > 
> 
> 
> -- 
> Alexander Tikhonov


--
Serge Petrenko
sergepetrenko@tarantool.org



[-- Attachment #2: Type: text/html, Size: 6895 bytes --]

Re: [Tarantool-patches] [PATCH v2] build: fix unit tests build with lrt

[Kirill Yukhin]

Hello,

On 29 ноя 10:06, Alexander V. Tikhonov wrote:
> After the commit 77fa45bd05f8cdd4c0f9bad85185ef5b61528d49
> ('lua: add fiber.top() listing fiber cpu consumption')
> 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
> 
> Found that fiber.cc is using now clock_gettime(), which requires -lrt
> with glibc. To fix it added librt dependency for core library for glibc.
> Due to glibc requires for -lrt for clock_gettime() only for some
> versions, check 'man clock_gettime.2':
>   'Link with -lrt (only for glibc versions before 2.17).'
> the check whether is able to use clock_gettime() w/o librt library is
> added.
> 
> Close #4639
> ---
> 
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4639-lrt-suggested-full-ci
> Issue: https://github.com/tarantool/tarantool/issues/4639

I've checked your patch into master.

--
Regards, Kirill Yukhin