From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id AF326E8094D; Fri, 13 Dec 2024 16:47:49 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org AF326E8094D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1734097669; bh=l+N8L1wWCnRYRUNO9OO6qA7wy3312zST1h3bNVbnOJ4=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=XO3yTU1aS7KQcCkdW1Z8qkgL3renQ+XP8h5E1kglTx7AHrnrFOHbZSUihw6zvlkz8 Ld4MZfj/UGidHZWeuZfAYTTeIocxdPEdbKUZD5REsXL4gT4pJJ9pmS4wRKasRA1HpA 2i9PoT9dPR1AMiJaJhbO1pkCDZJqOMAUcyyA2tOk= Received: from send37.i.mail.ru (send37.i.mail.ru [89.221.237.132]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id A4CF9E80941 for ; Fri, 13 Dec 2024 16:47:48 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A4CF9E80941 Received: by exim-smtp-bf6fb4755-p9cjx with esmtpa (envelope-from ) id 1tM61D-00000000Kp4-2VT3; Fri, 13 Dec 2024 16:47:48 +0300 Content-Type: multipart/alternative; boundary="------------p0aB5ICaKVjl0BqKXT3dLctT" Message-ID: Date: Fri, 13 Dec 2024 16:47:47 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: Sergey Kaplun , Maxim Kokryashkin Cc: tarantool-patches@dev.tarantool.org References: <20241213125929.22051-1-skaplun@tarantool.org> In-Reply-To: <20241213125929.22051-1-skaplun@tarantool.org> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD911C40A3FA7A658832F1729972165F0B373B65DBF156EFFA8182A05F538085040BFA5ABC398117AD73DE06ABAFEAF67055C1C997495D6A0A43F6DF0D90471C9035E5D4A160AC272F4 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7A33E1178EA603666EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063719899BAB9B61B3948638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8BE7A644F7C947BD779E80BCBEE1F826152297C7A12BAA243CC7F00164DA146DAFE8445B8C89999728AA50765F790063793270F7220657A0A389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC8B861051D4BA689FCF6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA73AA81AA40904B5D9A18204E546F3947C4E7D9683544204AF6E0066C2D8992A164AD6D5ED66289B523666184CF4C3C14F6136E347CC761E07725E5C173C3A84C3B74263D4D5690889BA3038C0950A5D36B5C8C57E37DE458B330BD67F2E7D9AF16D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE7AEA1580DED4E70E3731C566533BA786AA5CC5B56E945C8DA X-C1DE0DAB: 0D63561A33F958A562AE7B1D308E5B095002B1117B3ED6961AAE293577448E4203803A57F48E4E5A823CB91A9FED034534781492E4B8EEAD17AEC49845D0B908 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF4229EF0BC69CA538663BFD370DD27395C3F81FFE6F7B6250513C819320EDBEB4CFC48FC8A09AC8558E708AA2021A9CC244637A7F5563C6C7BCDE0F1B1CB61518D70EC858CF8EF77C5F4332CA8FE04980913E6812662D5F2AB9AF64DB4688768036DF5FE9C0001AF333F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojgFHDrhqZ0P/25a2FMT7pvw== X-Mailru-Sender: 520A125C2F17F0B1E52FEF5D219D61401808D09CE13ABF4D15C513EAE8123D4DE5819D52AAD0E83F0152A3D17938EB451EB5A0BCEC6A560B3DDE9B364B0DF289BE2DA36745F2EEB5CEBA01FB949A1F1EEAB4BC95F72C04283CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] cmake: fix build for Alpine X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" This is a multi-part message in MIME format. --------------p0aB5ICaKVjl0BqKXT3dLctT Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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 > 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: 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: 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. > + 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() --------------p0aB5ICaKVjl0BqKXT3dLctT Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit

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