diff mbox series

[bug#65486] syscalls: Add support for musl libc

Message ID 3V5VFSTNLLLHJ.3BBPH8V2N2EUI@8pit.net
State New
Headers show
Series [bug#65486] syscalls: Add support for musl libc | expand

Commit Message

Sören Tempel Sept. 15, 2023, 10:57 a.m. UTC
Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> wrote:
> You could call it ‘musl?’ instead, to (hopefully) convey we’re
> interested in the C library specifically.

I used musl-libc? instead to make it more clear that we are interested
in the C library for this case-distinction. This is implemented in the
attached git-format-patch(1). Would that be suitable for inclusion in
Guix?

> No no, I meant something like:
> 
>   (or (false-if-exception (dynamic-func "readdir64" (dynamic-link)))
>       (dynamic-func "readdir" (dynamic-link)))
> 
> Of course, it’s not as simple as this because we’d rather have it
> integrated with ‘syscall->procedure’ (maybe by adding an
> #:alternative-name argument for the Musl name?), but you get the idea.

Also this check doesn't ensure struct layout compatibility, e.g. if
readdir uses 32-bit types so not sure if this is necessarily better
than the musl libc check I proposed above.

Let me know what you think.

Greetings
Sören
From b1d478defc7f3e794974be2b9665cd4a58030569 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=B6ren=20Tempel?= <soeren+git@soeren-tempel.net>
Date: Thu, 14 Sep 2023 12:35:38 +0000
Subject: [PATCH] syscalls: Add support for musl libc

This commit allows using Guix on a foreign distro which uses musl libc,
for example, Alpine Linux. Usage of musl libc is detected via a new
musl-libc? variable using the Guile %host-type.

Using the new musl-libc? variable, we can now implement musl-specific
quirks. The two compatibility problems I encountered in this regard are
that musl dose not export a readdir64 and statfs64 symbol. On musl,
these two functions are implemented as CPP macros that expand to
readdir/statfs. To workaround that, a case-distinction was added.

The existing linux? variable has been modified to return true if the
%host-system contains "linux-" in order to ensure it is true for both
linux-gnu as well as linux-musl host systems.

The patch has been tested on Alpine Linux and is already used for the
downstream Guix package shipped in Alpine Linux's package repository.

* guix/build/syscalls.scm (musl-libc?): New variable.
* guix/build/syscalls.scm (linux?): Truth value on any linux system.
* guix/build/syscalls.scm (readdir-procedure): Support musl libc.
* guix/build/syscalls.scm (statfs): Support musl libc.
---
 guix/build/syscalls.scm | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Ludovic Courtès Sept. 17, 2023, 10:14 a.m. UTC | #1
Hi,

Sören Tempel <soeren@soeren-tempel.net> skribis:

> From b1d478defc7f3e794974be2b9665cd4a58030569 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?S=C3=B6ren=20Tempel?= <soeren+git@soeren-tempel.net>
> Date: Thu, 14 Sep 2023 12:35:38 +0000
> Subject: [PATCH] syscalls: Add support for musl libc
>
> This commit allows using Guix on a foreign distro which uses musl libc,
> for example, Alpine Linux. Usage of musl libc is detected via a new
> musl-libc? variable using the Guile %host-type.
>
> Using the new musl-libc? variable, we can now implement musl-specific
> quirks. The two compatibility problems I encountered in this regard are
> that musl dose not export a readdir64 and statfs64 symbol. On musl,
> these two functions are implemented as CPP macros that expand to
> readdir/statfs. To workaround that, a case-distinction was added.
>
> The existing linux? variable has been modified to return true if the
> %host-system contains "linux-" in order to ensure it is true for both
> linux-gnu as well as linux-musl host systems.
>
> The patch has been tested on Alpine Linux and is already used for the
> downstream Guix package shipped in Alpine Linux's package repository.
>
> * guix/build/syscalls.scm (musl-libc?): New variable.
> * guix/build/syscalls.scm (linux?): Truth value on any linux system.
> * guix/build/syscalls.scm (readdir-procedure): Support musl libc.
> * guix/build/syscalls.scm (statfs): Support musl libc.

This version LGTM.  Applied, thanks!

Ludo’.
Ludovic Courtès Sept. 17, 2023, 10:17 a.m. UTC | #2
Oooh, now I remember why I ended up not applying the previous
syscalls.scm patch: OpenJDK and a bunch of other things depend on it, so
changing syscalls.scm entails lots of rebuilds.  (Really, they shouldn’t
depend on it in the first place, IMO.)

So we need bordeaux.guix to build everything ahead of time.

To do that, can you resend both syscalls.scm patch (with ‘git
send-email’) here?  Then we’ll check qa.guix to ensure it builds things.

Chris, how does that sound?

Ludo’.
Christopher Baines Sept. 17, 2023, 11:38 a.m. UTC | #3
Ludovic Courtès <ludo@gnu.org> writes:

> So we need bordeaux.guix to build everything ahead of time.
>
> To do that, can you resend both syscalls.scm patch (with ‘git
> send-email’) here?  Then we’ll check qa.guix to ensure it builds things.
>
> Chris, how does that sound?

This probably falls under the core team (although ./etc/teams.scm says
it doesn't fall under any team), so I'd suggest putting it on a branch.

That way, we can collect some other similar changes together before
merging.
Sören Tempel Sept. 30, 2023, 10:16 a.m. UTC | #4
Hi,

Ludovic Courtès <ludo@gnu.org> wrote:
> To do that, can you resend both syscalls.scm patch (with ‘git
> send-email’) here?  Then we’ll check qa.guix to ensure it builds things.

Did I resend the patches correctly? I noticed that the QA status is
still unknown on issues.guix.gnu.org. Is it required to open a new
thread instead of replying to the existing one?

Greetings,
Sören
Christopher Baines Oct. 10, 2023, 5:15 p.m. UTC | #5
Sören Tempel <soeren@soeren-tempel.net> writes:

> Hi,
>
> Ludovic Courtès <ludo@gnu.org> wrote:
>> To do that, can you resend both syscalls.scm patch (with ‘git
>> send-email’) here?  Then we’ll check qa.guix to ensure it builds things.
>
> Did I resend the patches correctly? I noticed that the QA status is
> still unknown on issues.guix.gnu.org. Is it required to open a new
> thread instead of replying to the existing one?

The unknown status is because QA doesn't have any information on the
builds. Currently there's a limit of 300 builds per system for patches
which these changes exceed.
Ludovic Courtès Oct. 23, 2023, 10:11 a.m. UTC | #6
Hello,

Ludovic Courtès <ludo@gnu.org> skribis:

> Oooh, now I remember why I ended up not applying the previous
> syscalls.scm patch: OpenJDK and a bunch of other things depend on it, so
> changing syscalls.scm entails lots of rebuilds.  (Really, they shouldn’t
> depend on it in the first place, IMO.)

This is now fixed (see <https://issues.guix.gnu.org/66525>), and I’m
happy to report that your patches have finally been pushed!

Thanks,
Ludo’.
diff mbox series

Patch

diff --git a/guix/build/syscalls.scm b/guix/build/syscalls.scm
index c9c0bf594d..b845b8aab9 100644
--- a/guix/build/syscalls.scm
+++ b/guix/build/syscalls.scm
@@ -836,7 +836,8 @@  (define-record-type <file-system>
 (define-syntax fsword                             ;fsword_t
   (identifier-syntax long))
 
-(define linux? (string-contains %host-type "linux-gnu"))
+(define musl-libc? (string-contains %host-type "linux-musl"))
+(define linux? (string-contains %host-type "linux-"))
 
 (define-syntax define-statfs-flags
   (syntax-rules (linux hurd)
@@ -905,7 +906,7 @@  (define-c-struct %statfs                          ;<bits/statfs.h>
   (spare            (array fsword 4)))
 
 (define statfs
-  (let ((proc (syscall->procedure int "statfs64" '(* *))))
+  (let ((proc (syscall->procedure int (if musl-libc? "statfs" "statfs64") '(* *))))
     (lambda (file)
       "Return a <file-system> data structure describing the file system
 mounted at FILE."
@@ -1232,7 +1233,7 @@  (define closedir*
 
 (define (readdir-procedure name-field-offset sizeof-dirent-header
                            read-dirent-header)
-  (let ((proc (syscall->procedure '* "readdir64" '(*))))
+  (let ((proc (syscall->procedure '* (if musl-libc? "readdir" "readdir64") '(*))))
     (lambda* (directory #:optional (pointer->string pointer->string/utf-8))
       (let ((ptr (proc directory)))
         (and (not (null-pointer? ptr))