Hello again, Sergey,
thanks for the patch!
LGTM with minor comments, see below
s/musl/Musl/Since Alpine uses musl [1] as its C standard library, the build for it
failed after the commit af0f59da76292f30ff75be1ab01458d47b226995 ("test: fix LuaJIT-tests for old libc version"), since `GetLibCVersion()` raises an error. This patch adds the check of the ID in the /etc/os-release [2] of the Linux distribution and avoids setting the GLibC version if the distro is "alpine". [1]: https://wiki.alpinelinux.org/wiki/Musl [2]: https://www.linux.org/docs/man5/os-release.html
There is also a documentation about os-release on Freedesktop [1].
I would add this link as well. Feel free to ignore.
[1]:
https://www.freedesktop.org/software/systemd/man/latest/os-release.html
--- Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-noticket-fix-alpine-build PR for tests on master: https://github.com/tarantool/tarantool/pull/10824 Side note: build locally in Docker for test for Alpine. test/CMakeLists.txt | 1 + test/LuaJIT-tests/CMakeLists.txt | 18 +++++++++++------- test/cmake/GetLinuxDistro.cmake | 18 ++++++++++++++++++ 3 files changed, 30 insertions(+), 7 deletions(-) create mode 100644 test/cmake/GetLinuxDistro.cmake diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index be02d2e4..86de9ed1 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -74,6 +74,7 @@ separate_arguments(LUAJIT_TEST_COMMAND) set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake") include(AddTestLib) include(GetLibCVersion) +include(GetLinuxDistro) include(LibRealPath) # CTEST_FLAGS is used by CMake targets in test suites. diff --git a/test/LuaJIT-tests/CMakeLists.txt b/test/LuaJIT-tests/CMakeLists.txt index 0df3f4ea..adc630f0 100644 --- a/test/LuaJIT-tests/CMakeLists.txt +++ b/test/LuaJIT-tests/CMakeLists.txt @@ -62,13 +62,17 @@ if(CMAKE_C_FLAGS MATCHES "-march=skylake-avx512") list(APPEND LUAJIT_TEST_TAGS_EXTRA +avx512) endif() -if(NOT CMAKE_SYSTEM_NAME STREQUAL "Darwin") - GetLibCVersion(LIBC_VERSION) - # XXX: <tonumber_scan.lua> uses `strtod()`, old versions of - # which have the bug [1] for "0x1p-2075" parsing. Add the skip - # check for it. - # [1]: https://sourceware.org/bugzilla/show_bug.cgi?id=16151 - list(APPEND LUAJIT_TEST_TAGS_EXTRA +libc=${LIBC_VERSION}) +if(CMAKE_SYSTEM_NAME STREQUAL "Linux") + GetLinuxDistro(LINUX_DISTRO) + # Alpine uses musl instead of GLibC.
s/musl/Musl/
s/GLibC/glibc/ or s/GLibC/GNU C Library/, see [2].
[2]: https://www.gnu.org/software/libc/
+ if(NOT LINUX_DISTRO MATCHES "alpine") + GetLibCVersion(LIBC_VERSION) + # XXX: <tonumber_scan.lua> uses `strtod()`, old versions of + # which have the bug [1] for "0x1p-2075" parsing. Add the skip + # check for it. + # [1]: https://sourceware.org/bugzilla/show_bug.cgi?id=16151 + list(APPEND LUAJIT_TEST_TAGS_EXTRA +libc=${LIBC_VERSION}) + endif() endif() if(LUAJIT_ENABLE_TABLE_BUMP) diff --git a/test/cmake/GetLinuxDistro.cmake b/test/cmake/GetLinuxDistro.cmake new file mode 100644 index 00000000..027368be --- /dev/null +++ b/test/cmake/GetLinuxDistro.cmake @@ -0,0 +1,18 @@ +# Determine the Linux distro id and return it in the `output` +# variable. See https://www.linux.org/docs/man5/os-release.html +# for details. +macro(GetLinuxDistro output) + if(NOT CMAKE_SYSTEM_NAME STREQUAL "Linux") + message(FATAL_ERROR "GetLinuxDistro macro must be used only on Linux") + endif() + file(READ /etc/os-release OS_RELEASE)
Fallback to /usr/lib/os-release as suggested in doc:
> Applications should check for the former, and exclusively
use its data if it exists, and only fall back to
/usr/lib/os-release if it is missing.
+ string(REGEX MATCH "ID=([0-9a-z._-]+)" MATCH ${OS_RELEASE}) + if(MATCH) + set(${output} ${CMAKE_MATCH_1}) + else() + set(${output} linux) + endif() + unset(OS_RELEASE) + unset(MATCH) + unset(CMAKE_MATCH_1) +endmacro()