[bug#74508] [PATCH 1/2] services: mingetty: Add additional configuration options.

[Maxim Cournoyer <maxim.cournoyer@gmail.com>]

Hello,

Tomas Volf <~@wolfsden.cz> writes:

> Not all aspects of mingetty were configurable, so this commit adds the
> additional configuration fields to support that.
>
> It also renames the accessors for all fields except `tty'.  They were named
> just `mingetty-$FIELD', instead of `mingetty-configuration-$FIELD'.  However
> the renaming *is* backwards compatible, since in the define-module's #:export
> argument the correct (`mingetty-configuration-$FIELD') were used already.
> Hence, until now, there was no way to access anything except the `tty' field.

Fun, good catch!  I find it odd that Guile doesn't warn or error when
exporting a symbol that isn't defined; I was bit by that recently
myself.  Personally I'd have effected that correction in a distinct
commit, to keep your new functionality diff to its bare minimum
(clearer/easier to review).

> * gnu/services/base.scm (<mingetty-configuration>): Add delay, print-issue,
> print-hostname, nice, chdir, chroot fields.  Rename accessors for auto-login,
> login-program, login-pause?, clear-on-logout?.

Are all these expected to be functional on Guix, e.g., the 'chroot' option?

> (mingetty-shepherd-service): Use the new fields.
> (define-module)<#:export>: Export the new accessors.
> * doc/guix.texi (Base Services)<mingetty-configuration>: Document the
> additional field.
>
> Change-Id: I4557a82498805ade0b341feda9d33eccc305690f
> ---
>  doc/guix.texi         | 27 ++++++++++++++++++-
>  gnu/services/base.scm | 62 ++++++++++++++++++++++++++++++++++++-------
>  2 files changed, 79 insertions(+), 10 deletions(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 1c39628ffa..d689711e80 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -19285,7 +19285,32 @@ Base Services
>  will have to press a key before the log-in shell is launched.
>  
>  @item @code{clear-on-logout?} (default: @code{#t})
> -When set to @code{#t}, the screen will be cleared after logout.
> +When set to @code{#t}, the screen will be cleared before showing the
> +login prompt.  The field name is bit unfortunate, since it controls
> +clearing also before the initial login, not just after a logout.
> +
> +@item @code{delay} (default: @code{#f})
> +When set to a number, sleep that many seconds after startup.
> +
> +@item @code{print-issue} (default: @code{#t})
> +When set to @code{#t}, write out a new line and the content of
> +@file{/etc/issue}.  Value of @code{'no-nl} can be used to suppress the
> +new line.

This sounds useful.  Could it be used in place of a 'motd', such as in
our hydra/berlin.scm conf (from the Guix maintenance repo), which is
currently relying on PAM to work (and doesn't currently seem to do
anything, at least on berlin, for unknown reason).

> +@item @code{print-hostname} (default: @code{#t})
> +When set to @code{#t}, print the host name before the login prompt.  The
> +host name is printed up to the first dot.  Can be set to @code{'long} to
> +print the full host name.
> +
> +@item @code{nice} (default: @code{#f})
> +When set to a number, change the process priority using @code{nice}.
> +
> +@item @code{chdir} (default: @code{#f})
> +When set to a string, change into that directory before calling the
> +login program.
> +
> +@item @code{chroot} (default: @code{#f})
> +When set to a string, call @code{chroot} with that directory.
>  
>  @item @code{mingetty} (default: @var{mingetty})
>  The Mingetty package to use.
> diff --git a/gnu/services/base.scm b/gnu/services/base.scm
> index 6473e1a91a..b3d3553091 100644
> --- a/gnu/services/base.scm
> +++ b/gnu/services/base.scm
> @@ -186,7 +186,12 @@ (define-module (gnu services base)
>              mingetty-configuration-login-program
>              mingetty-configuration-login-pause?
>              mingetty-configuration-clear-on-logout?
> -            mingetty-configuration-mingetty

Why is the above accessor removed?

> +            mingetty-configuration-delay
> +            mingetty-configuration-print-issue
> +            mingetty-configuration-print-hostname
> +            mingetty-configuration-nice
> +            mingetty-configuration-chdir
> +            mingetty-configuration-chroot
>              mingetty-configuration?
>              mingetty-service  ; deprecated
>              mingetty-service-type
> @@ -1242,20 +1247,33 @@ (define-record-type* <mingetty-configuration>
>    mingetty-configuration?
>    (mingetty         mingetty-configuration-mingetty ;file-like
>                      (default mingetty))
> -  (tty              mingetty-configuration-tty)     ;string
> -  (auto-login       mingetty-auto-login             ;string | #f
> +  (tty              mingetty-configuration-tty)       ;string
> +  (auto-login       mingetty-configuration-auto-login ;string | #f
>                      (default #f))
> -  (login-program    mingetty-login-program          ;gexp
> +  (login-program    mingetty-configuration-login-program ;gexp
>                      (default #f))
> -  (login-pause?     mingetty-login-pause?           ;Boolean
> +  (login-pause?     mingetty-configuration-login-pause? ;Boolean
>                      (default #f))
> -  (clear-on-logout? mingetty-clear-on-logout?       ;Boolean
> -                    (default #t)))
> +  (clear-on-logout? mingetty-configuration-clear-on-logout? ;Boolean
> +                    (default #t))
> +  (delay            mingetty-configuration-delay ;Integer | #f
> +                    (default #f))
> +  (print-issue      mingetty-configuration-print-issue ;Boolean | Symbol
> +                    (default #t))
> +  (print-hostname   mingetty-configuration-print-hostname ;Boolean | Symbol
> +                    (default #t))
> +  (nice             mingetty-configuration-nice ;Integer | #f
> +                    (default #f))
> +  (chdir            mingetty-configuration-chdir ;String | #f
> +                    (default #f))
> +  (chroot           mingetty-configuration-chroot ;String | #f
> +                    (default #f)))
>  
>  (define (mingetty-shepherd-service config)
>    (match-record config <mingetty-configuration>
> -    (mingetty tty auto-login login-program
> -              login-pause? clear-on-logout?)
> +                (mingetty tty auto-login login-program
> +                          login-pause? clear-on-logout? delay
> +                          print-issue print-hostname nice chdir chroot)
>      (list
>       (shepherd-service
>        (documentation "Run mingetty on an tty.")
> @@ -1286,6 +1304,32 @@ (define (mingetty-shepherd-service config)
>                                #~())
>                         #$@(if login-pause?
>                                #~("--loginpause")
> +                              #~())
> +                       #$@(if delay
> +                              #~("--delay" #$(number->string delay))
> +                              #~())
> +                       #$@(match print-issue
> +                            (#t
> +                             #~())
> +                            ('no-nl
> +                             #~("--nonewline"))
> +                            (#f
> +                             #~("--noissue")))
> +                       #$@(match print-hostname
> +                            (#t
> +                             #~())
> +                            ('long
> +                             #~("--long-hostname"))
> +                            (#f
> +                             #~("--nohostname")))
> +                       #$@(if nice
> +                              #~("--nice" #$(number->string nice))
> +                              #~())
> +                       #$@(if chdir
> +                              #~("--chdir" #$chdir)
> +                              #~())
> +                       #$@(if chroot
> +                              #~("--chroot" #$chroot)
>                                #~()))))
>        (stop   #~(make-kill-destructor))))))

Neatly crafted.  Could you please send a v2 with my small comments above
addressed, such as splitting in two commits, and not removing the
'mingetty-configuration-mingetty' accessor?  Other than that, LGTM.

-- 
Thanks,
Maxim