Subject: Re: [bug#77288] [PATCH v2 7/8] services: guix: Allow‘guix-daemon’to run without root privileges.

From: Ludovic Courtès <[email-masked]>
Date: Sun, 20 Apr 2025 18:27:03 +0200

Hello,

Maxim Cournoyer <[email-masked]> writes:

>> +  (program-file "validate-guix-ownership"
>> +                (with-imported-modules (source-module-closure
>> +                                        '((guix build utils)))
>> +                  #~(begin
>> +                      (use-modules (guix build utils)
>> +                                   (ice-9 ftw)
>> +                                   (ice-9 match))
>> +
>> +                      (define (lchown file uid gid)
>> +                        (let ((parent (open (dirname file) O_DIRECTORY)))
>> +                          (chown-at parent (basename file) uid gid
>> +                                    AT_SYMLINK_NOFOLLOW)
>
> Why do we need an atomic variant only for symlinks?  Perhaps worth a
> comment.

This procedure emulates lchown(2), for which Guile does not provide
bindings.

>> +                                          (lambda (file stat errno result)
>> +                                            (format (current-error-port) "i/o error: ~a: ~a~%"
>
> That's too wide for our 80 columns maximum width convention :-).  Easy
> to fix by breaking the line either after program-file or
> file-system-fold.

Will do!

>> +                                      (change-ownership (in-vicinity "/var/guix" directory)
>
> Likewise.  Also, I never remember why `in-vicinity' is useful, and it's
> not documented anywhere.

It’s more concise and more accurate than (string-append a "/" b).
I’ve come to use it more.

>> +                         (setlocale LC_ALL "C.UTF-8") ;for file name decoding
>
> Isn't C.UTF-8 the default locale used in Guile?  Or is there a reason
> why it shouldn't be?  I'm still surprised as to why this is needed.

C.UTF-8 is now always available (embedded in our libc), but the default
is always C.

>> +                                ;; XXX: Do it a second time to work around
>> +                                ;; <https://issues.guix.gnu.org/77274> and its
>> +                                ;; effect on the 'guix-ownership' service.
>> +                                ;; TODO: Remove when Shepherd 1.0.4
>> is out.
>
> Shepherd 1.0.4 is out!

Oh right.  :-)  I’ll adjust accordingly.

>>                                  (start-service 'guix-daemon))
>
> Are you sure this translates to 'wait for X to be up?'

Yes, and many system tests use this idiom.  You can experience it,
assuming you have a system that takes a long enough to start, by running
‘herd start X & herd start X’: one client will just wait for the other.

> Reviewed-by: Maxim Cournoyer <[email-masked]>

Thank you!

Ludo’.