GNU bug report logs - #75270
[PATCH 0/3] services: greetd: Improve greeter configurations.

Previous Next

Full log

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: muradm <mail <at> muradm.net>
Cc: ludo <at> gnu.org, 75270 <at> debbugs.gnu.org, pelzflorian <at> pelzflorian.de
Subject: Re: [bug#75270] [PATCH v4 3/3] services: greetd: Add new gtkgreet
 greeter.
Date: Wed, 29 Jan 2025 00:56:25 +0900

Hi,

muradm <mail <at> muradm.net> writes:

> * gnu/services/base.scm (<greetd-gtkgreet-sway-session>): New record,
> represents `gtkgreet` greeter session configuration.
> * doc/guix.texi (Base Services): Document new `gtkgreet` greeter.

OK.

> Change-Id: I10395a742fa7dee2c8ee2f92e80fe5c56a2e4fb6
> ---
>  doc/guix.texi         | 48 +++++++++++++++++++++++++++++++++++++++++--
>  gnu/services/base.scm | 33 +++++++++++++++++++++++++++++
>  2 files changed, 79 insertions(+), 2 deletions(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index d74a8938a6d..b1a3558e8ff 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -20594,8 +20594,9 @@ Base Services
>  The user to use for running the greeter.
>  
>  @item @code{default-session-command} (default: @code{(greetd-agreety-session)})
> -Can be either @code{greetd-agreety-session}, @code{greetd-wlgreet-sway-session} or
> -@code{gexp->script} like object to use as greeter.
> +Can be either @code{greetd-agreety-session}, @code{greetd-wlgreet-sway-session},
> +@code{greetd-gtkgreet-sway-session} or @code{gexp->script} like object to use
> +as greeter.

I think you mean file-like object here also (which means just a G-Exp
that lowers to a file name IIUC).  Your fill-column value or editor's
equivalent is set wrong (doesn't fold under 80 columns).

>  @end table
>  @end deftp
> @@ -20732,6 +20733,49 @@ Base Services
>  @end lisp
>  @end deftp
>  
> +@deftp {Data Type} greetd-gtkgreet-sway-session
> +Configuration record for the gtkgreet greetd greeter.  Can be used as
> +following:
> +
> +@lisp
> +  (greetd-configuration
> +   ;; Graphical greeter require additional group membership.

Like earlier, missing 'The' article, as well as 's' to require.  groups
should be plural too now that I think about it, since we're registering
more than one.

> +   (greeter-supplementary-groups (list "video" "input" "seat"))
> +   (terminals
> +    (list (greetd-terminal-configuration
> +           (terminal-vt "1")
> +           (terminal-switch #t)
> +           (default-session-command
> +            (greetd-gtkgreet-sway-session
> +             ;; optionally use Adwaita:dark configured version
> +             (gtkgreet gtkgreet-adwaita-dark)

As mentioned in another mail, I think we need to refine how changing a
theme works.  Rebuilding the package just for that is inelegant and
wasteful.

> +             (command
> +              (greetd-user-session
> +               ;; signal to our .bashrc that we want wayland compositor

Please use full sentences for line comments (including proper
punctuation).

> +               (xdg-session-type "wayland")))))))))
> +@end lisp
> +
> +@table @asis
> +@item @code{sway} (default: @code{sway})
> +The package with @command{/bin/sway} and @command{/bin/swaymsg} commands.

I'd write 'The package providing the @command{sway} and @command{swaymsg}
commands'.

> +@item @code{sway-configuration} (default: @code{(plain-file "greetd-gtkgreet-sway-config" "")})
> +Extra configuration for sway to be included before executing greeter.

*the* greeter.

> +
> +@item @code{gtkgreet} (default: @code{gtkgreet})
> +The package with @command{/bin/gtkgreet} command.

The package with the @command{gtkgreet} command.

> +@item @code{gtkgreet-style} (default: @code{(plain-file "greetd-gtkgreet-sway-gtkgreet-style.css" "")})
> +Extra CSS stylesheet to customize GTK look.

[...] customize *the* GTK look.

> +
> +@item @code{command} (default: @code{(greetd-user-session)})
> +Command to be started by @command{/bin/agreety} on successful login.

@command{agreety}

> +Normally should be a variation of @code{greetd-user-session}, but could
> +be any @code{gexp->script} like object.

Same as in the previous patch reviewed; s/variation/instance of the
@code{<greetd-user-session>} record/

also s/@code{gexp->script} like/file-like/

> +
> +@end table
> +@end deftp
> +
>  @node Scheduled Job Execution
>  @subsection Scheduled Job Execution
>  
> diff --git a/gnu/services/base.scm b/gnu/services/base.scm
> index c6098af8f4f..1b46bf76a25 100644
> --- a/gnu/services/base.scm
> +++ b/gnu/services/base.scm
> @@ -279,6 +279,7 @@ (define-module (gnu services base)
>              greetd-wlgreet-color
>              greetd-wlgreet-configuration
>              greetd-wlgreet-sway-session
> +            greetd-gtkgreet-sway-session
>  
>              %base-services))
>  
> @@ -3572,6 +3573,38 @@ (define-gexp-compiler (greetd-wlgreet-sway-session-compiler
>        sway
>        (make-greetd-wlgreet-sway-session-sway-config session)))))
>  
> +(define-record-type* <greetd-gtkgreet-sway-session>
> +  greetd-gtkgreet-sway-session make-greetd-gtkgreet-sway-session
> +  greetd-gtkgreet-sway-session?
> +  (sway greetd-gtkgreet-sway-session-sway (default sway))
> +  (sway-configuration greetd-wlgreet-sway-session-sway-configuration
> +                      (default (plain-file "greetd-gtkgreet-sway-config" "")))
> +  (gtkgreet greetd-gtkgreet-sway-session-gtkgreet (default gtkgreet))
> +  (gtkgreet-style greetd-gtkgreet-sway-session-gtkgreet-style
> +                  (default (plain-file "greetd-gtkgreet-sway-gtkgreet-style.css" "")))
> +  (command greetd-gtkgreet-sway-session-command (default (greetd-user-session))))
> +
> +(define make-greetd-gtkgreet-sway-session-sway-config
> +  (match-lambda
> +    (($ <greetd-gtkgreet-sway-session> sway sway-config gtkgreet gtkgreet-style command)
> +     (let ((gtkgreet-bin (file-append gtkgreet "/bin/gtkgreet"))
> +           (swaymsg-bin (file-append sway "/bin/swaymsg")))
> +       (mixed-text-file
> +        "gtkgreet-sway-config"
> +        "include " sway-config "\n"
> +        "xwayland disable\n"
> +        "exec \"" gtkgreet-bin " -l -s " gtkgreet-style " -c " command "; " swaymsg-bin " exit\"\n")))))

Please break all these long lines extending past the 80 columns limit.

> +(define-gexp-compiler (greetd-gtkgreet-sway-session-compiler
> +                       (session <greetd-gtkgreet-sway-session>)
> +                       system target)
> +  (match-record session <greetd-gtkgreet-sway-session>
> +    (sway)
> +    (lower-object
> +     (make-greetd-sway-greeter-command
> +      sway
> +      (make-greetd-gtkgreet-sway-session-sway-config session)))))
> +

I wonder if we could use define-configuration and avoid the lower-level
define-gexp-compiler glue code.

Otherwise, LGTM.

Assuming you address my above comments,

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.