GNU bug report logs - #74819
[PATCH] services: elogind: Support Hook Directories

Previous Next

Full log

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 45mg <45mg.writes <at> gmail.com>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 74819 <at> debbugs.gnu.org
Subject: Re: [bug#74819] [PATCH v4] services: elogind: Support Hook Directories
Date: Sat, 28 Dec 2024 14:46:33 +0900

Hi,

45mg <45mg.writes <at> gmail.com> writes:

> Allow the user to specify scripts to be added into Elogind's hook
> directories. These scripts will be run before/after
> suspend/hibernate/poweroff/reboot.
>
> Also allow setting the associated config options.
>
> * gnu/services/desktop.scm (elogind-configuration): add
> `system-sleep-hook-files`, `system-shutdown-hook-files`,
> and 4 new config options.
> (elogind-configuration-file): Add entries for the new config options
> under the `[Sleep]` section.
> (/etc/elogind): New function, to generate /etc/elogind directory.
> (elogind-service-type): Extend `etc-service-type` using `/etc/elogind`.
> * doc/guix.texi: Document the new options.

Soods good.

[...]

> +(define (elogind-etc-directory config)
> +  "Return the /etc/elogind directory for CONFIG."
> +  (with-imported-modules (source-module-closure '((guix build utils)))
> +    (computed-file
> +     "etc-elogind"
> +
> +     #~(begin
> +         (use-modules (guix build utils))
> +
> +         (define sleep-directory (string-append #$output "/system-sleep/"))
> +         (define shutdown-directory (string-append #$output "/system-shutdown/"))
> +
> +         (define (copy-script file directory)
> +           "Copy FILE into DIRECTORY, giving rx (500) permissions."
> +           (let ((dest (string-append directory "/" (basename file))))
> +             (mkdir-p directory)
> +             (copy-file file dest)
> +             (chmod dest #o500)))
> +
> +         (mkdir-p #$output) ; in case neither directory gets created

Unimportant nitpick: please keep more than 2 spaces between the code and
a comment (I use M-; in Emacs to do so); also prefer to avoid the space
between ';' and the comment for in-line comment only (that's a 'may',
not a 'must' according to our referenced style guide [0] though).

[0]  https://mumble.net/~campbell/scheme/style.txt

> +         (for-each
> +          (lambda (f) (copy-script f sleep-directory))

I've move the copy-script on its own line under the lambda, which is
more conventional.

> +          '#$(elogind-system-sleep-hook-files config))j
> +         (for-each
> +          (lambda (f) (copy-script f shutdown-directory))

Likewise.

Other than these tiny cosmetic things, it LGTM:

Reviewed-by: Maxim Cournoyer <maxim.cournoyer <at> gmail>

-- 
Thanks,
Maxim

Previous Next

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