GNU bug report logs - #75959
[PATCH] (home-)syncthing-service: added support for config serialization

Previous Next

Full log

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

From: Leo Famulari <leo <at> famulari.name>
To: Zacchaeus Scheffer <eikcaz <at> zacchae.us>
Cc: Bruno Victal <mirai <at> makinata.eu>, 75959 <at> debbugs.gnu.org
Subject: Re: [PATCH v8] services: syncthing: Add support for config file
 generation.
Date: Fri, 14 Feb 2025 03:09:46 -0500

[Message part 1 (text/plain, inline)]

On Tue, Feb 11, 2025 at 12:31:23AM -0500, Zacchaeus Scheffer wrote:
> Subject: [PATCH v8] services: syncthing: Add support for config file
>  generation.
> 
> * gnu/services/syncthing.scm: (syncthing-config-file,
> syncthing-folder, syncthing-device, syncthing-folder-device): New
> records;  (syncthing-service-type): Add special-files-service-type
> extension for the config file; (syncthing-files-service): Add service
> to create config file.
> * gnu/home/services/syncthing.scm: (home-syncthing-service-type):
> Extend home-files-services-type and re-exported more things from
> gnu/services/syncthing.scm.
> * doc/guix.texi: (syncthing-service-type): Document changes.

Great! I read through the documentation and made numerous stylistic
edits, and also added some references, links, and markup. See the
attached diff. I did not change the structure of the docs. Please let me
know what you think.

> +      (start #~(lambda _
> +                 ;; if we are managing the config, and it's not a home
> +                 ;; service, then exepect the config file at
> +                 ;; /var/lib/syncthing-<user>.  This makes sure the ownership
> +                 ;; is correct
> +                 (unless (or #$(not config-file) #$home-service?)
> +                   (system* "chown" #$user (string-append "/var/lib/syncthing-" #$user))
> +                   (system* "chmod" "700" (string-append "/var/lib/syncthing-" #$user)))

Please use the Guile procedures chown and chmod here, rather than
shelling out. Also, I think the folder's group should be set as well
using the chown procedure; right now it remains owned by root. There are
examples in 'gnu/services'.

https://www.gnu.org/software/guile/manual/html_node/File-System.html#index-chmod
https://www.gnu.org/software/guile/manual/html_node/File-System.html#index-chown

> +                 (make-forkexec-constructor
> +                  (append (list (string-append #$syncthing "/bin/syncthing")
> +                                "--no-browser"
> +                                "--no-restart"

It will be helpful to add brief Scheme code comments explaining what
these options do.

[guix.texi.diff (text/plain, attachment)]

Previous Next

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