Subject: Re: [bug#62401] [PATCH] home: Add home-syncthing-service-type.

From: Sergey Trofimov <[email-masked]>
Date: Tue, 28 Mar 2023 21:29:15 +0200

Hi Ludovic,

Ludovic Courtès <[email-masked]> writes:

> Hi,
>
> Sergey Trofimov <[email-masked]> skribis:
>
>> * gnu/home/services/syncthing.scm
>> (home-syncthing-service-type): New variable.
>> (home-syncthing-configuration): New procedure.
>> * gnu/services/syncthing.scm
>> (syncthing-shepherd-service): Adapt to be used as a home 
>> service.
>> * doc/guix.texi: Ditto.
>
> Nice!
>
> Note: You can just say “New file” for a new file; however please 
> specify
> the name of the node added to the manual, for clarity.

So it looks like this?
--8<---------------cut here---------------start------------->8---
* gnu/home/services/syncthing.scm: New file.
* doc/guix.texi: Document home-syncthing-service-type and
home-syncthing-configuration.
--8<---------------cut here---------------end--------------->8---

>> +(define syncthing-home-shepherd-service
>> +  (lambda (configuration)
>> +    (map (lambda (srv) (shepherd-service (inherit srv) 
>> (requirement '())))
>> +         ((@@ (gnu services syncthing) 
>> syncthing-shepherd-service) configuration))))
>
> You could do that without resorting to @@, by using ‘find’ on 
> the
> extensions of ‘syncthing-service-type’.

Not sure I get you. I use @@ because `syncthing-shepherd-service` 
is not exported and I want to leave that as is.

>> diff --git a/gnu/services/syncthing.scm 
>> b/gnu/services/syncthing.scm
>> index 7c3d5b027d..960f0a1db8 100644
>> --- a/gnu/services/syncthing.scm
>> +++ b/gnu/services/syncthing.scm
>> @@ -43,8 +43,7 @@ (define-record-type* 
>> <syncthing-configuration>
>>               (default '()))
>>    (logflags  syncthing-configuration-logflags  ;number
>>               (default 0))
>> -  (user      syncthing-configuration-user      ;string
>> -             (default #f))
>> +  (user      syncthing-configuration-user)     ;string
>
> This means that users of ‘syncthing-service-type’ will now have 
> to
> provide a value for ‘user’, which wasn’t the case until now.  Is 
> this
> change necessary?
>

The existing code fails on this line, when the user is not 
specified:
--8<---------------cut here---------------start------------->8---
(provision (list (string->symbol (string-append "syncthing-" 
user))))
--8<---------------cut here---------------end--------------->8---

string-append doesn't accept nil args

>
> The rest looks reasonable to me.  My main concern is whether it 
> would
> make sense to automate system->home service translation.  I’ll 
> expand on
> that separately.

Does guix really need a system syncthing service? It runs under 
`user`, looks for configuration in `/home/user` and therefore 
looks out of place in the system services list.