Hi, Sergey,

LGTM, thanks!


I've also checked locally in Docker using Dockerfile below:


# docker build --tag alpine_test .
# docker run --network="host"  -ti alpine_test bash

FROM alpine:3.16

RUN apk update && apk upgrade
RUN apk add bash
RUN apk add git
RUN apk add vim
RUN apk add gcc
RUN apk add g++
RUN apk add gdb
RUN apk add make
RUN apk add cmake
RUN apk add linux-headers
RUN apk add libc-dev

RUN git clone https://github.com/tarantool/luajit && \
    cd luajit && \
    git checkout skaplun/gh-noticket-fix-alpine-build && \
    cmake -S . -B build -DLUAJIT_DISABLE_SYSPROF=ON && \
    cmake --build build --parallel && \
    cmake --build build --target test --parallel

On 13.12.2024 17:45, Sergey Kaplun wrote:
Hi, Sergey!
Thanks for your comments!
Fixed them and force-pushed the branch.

On 13.12.24, Sergey Bronnikov wrote:
Hello again, Sergey,

thanks for the patch!

LGTM with minor comments, see below

On 13.12.2024 15:59, Sergey Kaplun wrote:
Since Alpine uses musl [1] as its C standard library, the build for it
s/musl/Musl/
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
Replaced typo: s/GLibC/glibc/.
Thanks!

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.
Added, thanks!
Thanks!

      

[1]: https://www.freedesktop.org/software/systemd/man/latest/os-release.html
The new commit message is the following:

| cmake: fix build for Alpine
|
| 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][3] 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
| [3]: 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.

<snipped>

+if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
+  GetLinuxDistro(LINUX_DISTRO)
+  # Alpine uses musl instead of GLibC.
s/musl/Musl/
I've used as a reference (don't be confused by the links
themselfs) [1][2], looks like it should be "musl", after all.
okay
[1]: https://wiki.alpinelinux.org/wiki/Musl
[2]: https://en.wikipedia.org/wiki/Musl

s/GLibC/glibc/ or s/GLibC/GNU C Library/, see [2].
Replaced with glibc.

===================================================================
diff --git a/test/LuaJIT-tests/CMakeLists.txt b/test/LuaJIT-tests/CMakeLists.txt
index adc630f0..c1dbde35 100644
--- a/test/LuaJIT-tests/CMakeLists.txt
+++ b/test/LuaJIT-tests/CMakeLists.txt
@@ -64,7 +64,7 @@ endif()
 
 if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
   GetLinuxDistro(LINUX_DISTRO)
-  # Alpine uses musl instead of GLibC.
+  # Alpine uses musl instead of glibc.
   if(NOT LINUX_DISTRO MATCHES "alpine")
     GetLibCVersion(LIBC_VERSION)
     # XXX: <tonumber_scan.lua> uses `strtod()`, old versions of
===================================================================

[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. Seehttps://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.
Added the fallback:
===================================================================
diff --git a/test/cmake/GetLinuxDistro.cmake b/test/cmake/GetLinuxDistro.cmake
index 027368be..2636019c 100644
--- a/test/cmake/GetLinuxDistro.cmake
+++ b/test/cmake/GetLinuxDistro.cmake
@@ -5,13 +5,18 @@ 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)
+  set(OS_RELEASE_FILE /etc/os-release)
+  if(NOT EXISTS ${OS_RELEASE_FILE})
+    set(OS_RELEASE_FILE /usr/lib/os-release)
+  endif()
+  file(READ ${OS_RELEASE_FILE} OS_RELEASE)
   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_FILE)
   unset(OS_RELEASE)
   unset(MATCH)
   unset(CMAKE_MATCH_1)
===================================================================

Thanks!

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