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 47D19E8864B; Fri, 13 Dec 2024 18:19:54 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 47D19E8864B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1734103194; bh=O0xYIYRFO8gF7m69GbMKoZnrXBeI7x1tJYyqvCW1TIM=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=i8fJm5kliAQS7Dl6GS3juX7LzS/DnlQ2fq5p4/qGcJmfPCwwKCsTsQQn+oEDENCAx WliX+ETJpw+3WT8G43U0+diQIEkeTxZg4wGnndGjY+XzJDhh5P+3h6eLJ4YlQ5ixG2 8jRH5BU8PXPLGYGtb7e+nqtjnpFme2UXvnYtSqUY= Received: from send36.i.mail.ru (send36.i.mail.ru [89.221.237.131]) (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 E528846049D for ; Fri, 13 Dec 2024 18:19:52 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org E528846049D Received: by exim-smtp-76d484c77c-nls7s with esmtpa (envelope-from ) id 1tM7SJ-00000000SEL-3e82; Fri, 13 Dec 2024 18:19:52 +0300 Content-Type: multipart/alternative; boundary="------------9ZgZxrpbiNPDKn5g57a8fF9E" Message-ID: <0e54ee72-c1ad-4001-a741-cb342d2c9a7c@tarantool.org> Date: Fri, 13 Dec 2024 18:19:51 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Sergey Kaplun References: <20241213125929.22051-1-skaplun@tarantool.org> Content-Language: en-US In-Reply-To: X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD977EAE61095F37AC5114D756849604E62314B8F7F8370BE1A182A05F538085040123D37A83B7BB2113DE06ABAFEAF67058443012F70765058C12375E8F19520CA48DBE7C8E2014941 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7F544D30F1A6FA191EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006375448D590B04CE87D8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D81B97E55B510D7309AD8F4DA33376D17DCAA7EDD3E329FBE6CC7F00164DA146DAFE8445B8C89999728AA50765F7900637F3E38EE449E3E2AE389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC8D2DCF9CF1F528DBCF6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA73AA81AA40904B5D9A18204E546F3947C4E7D9683544204AFC0837EA9F3D197644AD6D5ED66289B523666184CF4C3C14F6136E347CC761E07725E5C173C3A84C3F6D1C8D476B9D508BA3038C0950A5D36B5C8C57E37DE458B330BD67F2E7D9AF16D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE7AEA1580DED4E70E3731C566533BA786AA5CC5B56E945C8DA X-C1DE0DAB: 0D63561A33F958A54BF77851CE3F299B5002B1117B3ED696AF7B10D81FE74FBFE99897350C7C491E823CB91A9FED034534781492E4B8EEADB8BDF568CC942472BDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CFF1067A7DD048CE0A2465A2A1BD61D0E7B2152DF5AB7692849AA36ECD6B81AC7BA54F0D539447C58B8E708AA2021A9CC27F13BBA8848989BE6FC8CBAE6066A447469727F9F7AE49305F4332CA8FE04980913E6812662D5F2AB9AF64DB4688768036DF5FE9C0001AF333F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojQCcRYJ72L2RFuv4Zo7CQpg== X-Mailru-Sender: 520A125C2F17F0B1E52FEF5D219D61402881425C1024D155EDF1CDDB2C83CE56B362D114887D1E420152A3D17938EB451EB5A0BCEC6A560B3DDE9B364B0DF289BE2DA36745F2EEB5CEBA01FB949A1F1EEAB4BC95F72C04283CDA0F3B3F5B9367 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 Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" This is a multi-part message in MIME format. --------------9ZgZxrpbiNPDKn5g57a8fF9E Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 > | [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. >>> > > >>> +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: uses `strtod()`, old versions of > =================================================================== > >> [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. > 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() --------------9ZgZxrpbiNPDKn5g57a8fF9E Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit

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


        

      

      

    

  
  


--------------9ZgZxrpbiNPDKn5g57a8fF9E--