<!DOCTYPE html>
<html data-lt-installed="true">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body style="padding-bottom: 1px;">
<p>Hello again, Sergey,</p>
<p>thanks for the patch!</p>
<p>LGTM with minor comments, see below<br>
</p>
<div class="moz-cite-prefix">On 13.12.2024 15:59, Sergey Kaplun
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:20241213125929.22051-1-skaplun@tarantool.org">
<pre class="moz-quote-pre" wrap="">Since Alpine uses musl [1] as its C standard library, the build for it</pre>
</blockquote>
s/musl/Musl/<br>
<blockquote type="cite"
cite="mid:20241213125929.22051-1-skaplun@tarantool.org">
<pre class="moz-quote-pre" wrap="">
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]: <a class="moz-txt-link-freetext" href="https://wiki.alpinelinux.org/wiki/Musl">https://wiki.alpinelinux.org/wiki/Musl</a>
[2]: <a class="moz-txt-link-freetext" href="https://www.linux.org/docs/man5/os-release.html">https://www.linux.org/docs/man5/os-release.html</a></pre>
</blockquote>
<p>There is also a documentation about os-release on Freedesktop
[1].</p>
<p>I would add this link as well. Feel free to ignore.<br>
</p>
<p><br>
</p>
<p>[1]:
<a class="moz-txt-link-freetext" href="https://www.freedesktop.org/software/systemd/man/latest/os-release.html">https://www.freedesktop.org/software/systemd/man/latest/os-release.html</a><br>
</p>
<blockquote type="cite"
cite="mid:20241213125929.22051-1-skaplun@tarantool.org">
<pre class="moz-quote-pre" wrap="">
---
Branch: <a class="moz-txt-link-freetext" href="https://github.com/tarantool/luajit/tree/skaplun/gh-noticket-fix-alpine-build">https://github.com/tarantool/luajit/tree/skaplun/gh-noticket-fix-alpine-build</a>
PR for tests on master: <a class="moz-txt-link-freetext" href="https://github.com/tarantool/tarantool/pull/10824">https://github.com/tarantool/tarantool/pull/10824</a>
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]: <a class="moz-txt-link-freetext" href="https://sourceware.org/bugzilla/show_bug.cgi?id=16151">https://sourceware.org/bugzilla/show_bug.cgi?id=16151</a>
- list(APPEND LUAJIT_TEST_TAGS_EXTRA +libc=${LIBC_VERSION})
+if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
+ GetLinuxDistro(LINUX_DISTRO)
+ # Alpine uses musl instead of GLibC.</pre>
</blockquote>
<p>s/musl/Musl/</p>
<p>s/GLibC/glibc/ or s/GLibC/GNU C Library/, see [2].<br>
</p>
<p>[2]: <a class="moz-txt-link-freetext" href="https://www.gnu.org/software/libc/">https://www.gnu.org/software/libc/</a><br>
</p>
<blockquote type="cite"
cite="mid:20241213125929.22051-1-skaplun@tarantool.org">
<pre class="moz-quote-pre" wrap="">
+ 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]: <a class="moz-txt-link-freetext" href="https://sourceware.org/bugzilla/show_bug.cgi?id=16151">https://sourceware.org/bugzilla/show_bug.cgi?id=16151</a>
+ 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 <a class="moz-txt-link-freetext" href="https://www.linux.org/docs/man5/os-release.html">https://www.linux.org/docs/man5/os-release.html</a>
+# 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)</pre>
</blockquote>
<p>Fallback to /usr/lib/os-release as suggested in doc:</p>
<p>> 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.<br>
</p>
<blockquote type="cite"
cite="mid:20241213125929.22051-1-skaplun@tarantool.org">
<pre class="moz-quote-pre" wrap="">
+ 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()
</pre>
</blockquote>
</body>
<lt-container></lt-container>
</html>