GNU bug report logs - #30355
[PATCH] services: agetty: Make tty optional and add agetty instance to base services.

Previous Next

Full log

View this message in rfc822 format

From: ludo <at> gnu.org (Ludovic Courtès)
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: 30355 <at> debbugs.gnu.org
Subject: [bug#30355] [PATCH] services: agetty: Make tty optional and add agetty instance to base services.
Date: Fri, 09 Feb 2018 14:51:12 +0100

Hi,

Danny Milosavljevic <dannym <at> scratchpost.org> skribis:

> * gnu/system/install.scm (agetty-default-service): Delete variable.
> (beaglebone-black-installation-os): Do not specify tty.
> (a20-olinuxino-lime-installation-os): Do not specify tty.
> (a20-olinuxino-lime2-emmc-installation-os): Do not specify tty.
> (a20-olinuxino-micro-installation-os): Do not specify tty.
> (banana-pi-m2-ultra-installation-os): Do not specify tty.
> (nintendo-nes-classic-edition-installation-os): Do not specify tty.
> (embedded-installation-os): Move agetty-service instantiation to...
> * gnu/services/base.scm (%base-services): ...here.
> (default-serial-port): New variable.
> (agetty-shepherd-service): Make tty optional, default to the above.
> * doc/guix.texi (agetty-configuration): Update "tty" documentation.

I would prefer the install.scm part to be a separate patch, if that’s
not too much of a burden for you.

I have a few minor comments:

>  @item @code{tty}
>  The name of the console this agetty runs on, as a string---e.g.,
> -@code{"ttyS0"}. This argument is mandatory.
> +@code{"ttyS0"}. This argument is optional, it will default to
> +a reasonable default serial port used by Linux.

s/Linux/the kernel Linux/ (to avoid ambiguities…)

> +For this, if there is a value for an option "agetty.tty" in the Linux
> +command line, agetty will extract the device name of the serial port
> +from it and use that.
> +
> +If not and if there is a value for an option "console" with a tty in
> +the Linux command line, agetty will extract the device name of the
> +serial port from it and use that.

@code{agetty.tty}, @code{console}, etc.

Write “kernel command line” instead of “Linux command line”.

> +In both cases, agetty will leave the other serial device settings
> +(baud rate etc) alone - in the hope that Linux pinned them to the
> +correct values.

s/etc/etc./
s/alone - in the hope/alone---in the hope/

(The three hyphens map to an em dash in the output.)

> +(define (default-serial-port)
> +  "Return a gexp that determines a reasonable default serial port
> +to use as the tty.  This is primarily useful for headless systems."
> +  #~(begin
> +      ;; console=device,options
> +      ;; device: can be tty0, ttyS0, lp0, ttyUSB0 (serial).
> +      ;; options: BBBBPNF. P n|o|e, N number of bits,
> +      ;; F flow control (r RTS)
> +      (use-modules (gnu build linux-boot))
> +      (let* ((not-comma (char-set-complement (char-set #\,)))
> +             (command (linux-command-line))
> +             (agetty-specs (find-long-options "agetty.tty" command))
> +             (console-specs (filter (lambda (spec)
> +                                     (not (or
> +                                            (string-prefix? "tty0" spec)
> +                                            (string-prefix? "tty1" spec)
> +                                            (string-prefix? "tty2" spec)
> +                                            (string-prefix? "tty3" spec)
> +                                            (string-prefix? "tty4" spec)
> +                                            (string-prefix? "tty5" spec)
> +                                            (string-prefix? "tty6" spec)
> +                                            (string-prefix? "tty7" spec)
> +                                            (string-prefix? "tty8" spec)
> +                                            (string-prefix? "tty9" spec))))
> +                                   (filter
> +                                     (lambda (spec)
> +                                       (string-prefix? "tty" spec))
> +                                     (find-long-options "console" command))))

Please make that a single ‘filter’ or ‘remove’ call, for clarity.

> +             (specs (append agetty-specs console-specs))
> +             (spec (if (null? specs)
> +                      #f
> +                      (car specs))))
> +        (and spec
> +             ;; Extract device name.
> +             (car (string-tokenize spec not-comma))))))

Rather:

  (match (append agetty-specs console-specs)
    (() #f)
    ((spec _ ...)
     (string-tokenize spec not-comma)))

The rest LGTM.  OK to push with these changes!

Thanks,
Ludo’.

Previous Next

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