GNU bug report logs - #77288
[PATCH 0/6] Rootless guix-daemon on Guix System

Previous Next

Full log

Message #136 received at 77288 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 77288 <at> debbugs.gnu.org
Subject: Re: [bug#77288] [PATCH v2 7/8] services: guix: Allow
 ‘guix-daemon’
 to run without root privileges.
Date: Mon, 21 Apr 2025 10:14:31 +0900

Hi Ludovic,

Ludovic Courtès <ludo <at> gnu.org> writes:

> Hello,
>
> Maxim Cournoyer <maxim.cournoyer <at> gmail.com> 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.

OK.  Perhaps it should?  We could report to bug-guile and here add a
comment pointing to the issue to remind us to export it in Guile and use
it when it's there in the future.

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

It's problematic that it's not documented though.  Grepping the guile
source shouldn't be needed to understand the code, ideally.  The
'in-vicinity' name also suggests it might do something cleverer than
just concatenating strings.

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

Uh.  Are there plans to change this in the future?  It seems we're well
into the Unicode age :-).

>>> +                                ;; 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.

That surprises me, because I thought I recently observed some problem
with the use of 'start-service' in our test suite.  Perhaps the issue
was that simply asserting against 'start-service', without checking that
the returned service object 'running' slot is set to #t doesn't guard
against when the service fails to start, which is what is currently done
in many places.  Right?

-- 
Thanks,
Maxim

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.