Re: [Tarantool-patches] [PATCH v2 luajit 2/2] test: fix LuaJIT-tests for old libc version

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

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

Hi, Sergey!

thanks for fixes! LGTM

On 09.12.2024 17:40, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the review!
>
> On 09.12.24, Sergey Bronnikov wrote:
>> Hi, Sergey!
>>
>> thanks for the patch!
>>
>> LGTM after fix a comment below
> Fixed the comment with a slightly modified patch of yours:
>
> ===================================================================
> diff --git a/test/cmake/GetLibCVersion.cmake b/test/cmake/GetLibCVersion.cmake
> index d87e8b70..521a6e45 100644
> --- a/test/cmake/GetLibCVersion.cmake
> +++ b/test/cmake/GetLibCVersion.cmake
> @@ -1,8 +1,12 @@
>   # Get the libc version installed in the system.
>   macro(GetLibCVersion output)
> +  find_program(ECHO echo)
> +  if(NOT ECHO)
> +    message(FATAL_ERROR "`echo' is not found")
> +  endif()
>     # Try to directly parse the version.
>     execute_process(
> -    COMMAND echo "#include <gnu/libc-version.h>"
> +    COMMAND ${ECHO} "#include <gnu/libc-version.h>"
>       COMMAND ${CMAKE_C_COMPILER} -E -dM -
>       OUTPUT_VARIABLE LIB_C_INFO
>       ERROR_VARIABLE ERROR_MSG
> @@ -31,6 +35,7 @@ macro(GetLibCVersion output)
>   
>     set(${output} "${GLIBC_MAJOR}.${GLIBC_MINOR}")
>   
> +  unset(ECHO)
>     unset(CMAKE_MATCH_1)
>     unset(GLIBC_MAJOR)
>     unset(GLIBC_MINOR)
> ===================================================================
>
> Force-pushed the branch and updated the PR.
>
>> On 09.12.2024 13:16, Sergey Kaplun wrote:
>>> The `strtod parsing` subtest in the <lib/base/tonumber_scan.lua> checks
>>> the results yielded by the `strtod()` via FFI call. In GLibc versions
>>> before 2.19 it returns an incorrect result for "0x1p-2075" [1]. This
>>> patch skips this test for a smaller version of the libc installed.
>>>
>>> [1]:https://sourceware.org/bugzilla/show_bug.cgi?id=16151
>>> ---
> <snipped>
>
>>> diff --git a/test/cmake/GetLibCVersion.cmake b/test/cmake/GetLibCVersion.cmake
>>> new file mode 100644
>>> index 00000000..d87e8b70
>>> --- /dev/null
>>> +++ b/test/cmake/GetLibCVersion.cmake
>>> @@ -0,0 +1,41 @@
>>> +# Get the libc version installed in the system.
>>> +macro(GetLibCVersion output)
>>> +  # Try to directly parse the version.
>>> +  execute_process(
>>> +    COMMAND echo "#include <gnu/libc-version.h>"
>> I propose to replace "echo" with a variable set by `find_program()`:
>>
>>
>> diff --git a/test/cmake/GetLibCVersion.cmake
>> b/test/cmake/GetLibCVersion.cmake
>> index d87e8b70..15a8b440 100644
>> --- a/test/cmake/GetLibCVersion.cmake
>> +++ b/test/cmake/GetLibCVersion.cmake
>> @@ -1,8 +1,14 @@
>>    # Get the libc version installed in the system.
>>    macro(GetLibCVersion output)
>> +  find_program(ECHO echo)
>> +  if(NOT ECHO)
>> +    message(FATAL_ERROR "`echo` is not found'")
>> +    return()
>> +  endif()
>> +
>>      # Try to directly parse the version.
>>      execute_process(
>> -    COMMAND echo "#include <gnu/libc-version.h>"
>> +    COMMAND ${ECHO} "#include <gnu/libc-version.h>"
>>        COMMAND ${CMAKE_C_COMPILER} -E -dM -
>>        OUTPUT_VARIABLE LIB_C_INFO
>>        ERROR_VARIABLE ERROR_MSG
>> @@ -31,6 +37,7 @@ macro(GetLibCVersion output)
>>
>>      set(${output} "${GLIBC_MAJOR}.${GLIBC_MINOR}")
>>
>> +  unset(ECHO)
>>      unset(CMAKE_MATCH_1)
>>      unset(GLIBC_MAJOR)
>>      unset(GLIBC_MINOR)
>>
>>> +    COMMAND ${CMAKE_C_COMPILER} -E -dM -
>>> +    OUTPUT_VARIABLE LIB_C_INFO
>>> +    ERROR_VARIABLE ERROR_MSG
>>> +    OUTPUT_STRIP_TRAILING_WHITESPACE
>>> +    RESULT_VARIABLE RES
>>> +  )
>>> +
> <snipped>
>
>>> +endmacro()

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