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

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

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()

-- 
Best regards,
Sergey Kaplun