Message ID | YINsy8LNhQd4NYmq@jasmine.lan |
---|---|
State | Accepted |
Headers | show |
Series | [bug#47979,v3] installer: Recommend 'ntp-service-type' for non-graphical systems. | expand |
Context | Check | Description |
---|---|---|
cbaines/submitting builds | success | |
cbaines/comparison | success | View comparision |
cbaines/git branch | success | View Git branch |
cbaines/applying patch | fail | View Laminar job |
cbaines/issue | success | View issue |
On Fri, Apr 23, 2021 at 08:56:43PM -0400, Leo Famulari wrote: > With Leo Prikler's help on #guix [0], I got it to work :) > > I've attached the working patch, as well as a patch to offer the GPM > "console mouse" service on non-graphical systems. > > [0] http://logs.guix.gnu.org/guix/2021-04-24.log#012921 > From 6f2f131551247aa20794007c43ae61b82f6e34d6 Mon Sep 17 00:00:00 2001 > From: Leo Famulari <leo@famulari.name> > Date: Fri, 23 Apr 2021 14:50:15 -0400 > Subject: [PATCH 1/2] installer: Recommend 'ntp-service-type' for non-graphical > systems. > > We had several bug reports with a root cause of "the clock was > incorrect" from users who used the installer to install a non-graphical > Guix System. > > * gnu/installer/services.scm (%system-services): Add the ntp-service-type. > * gnu/installer/newt/services.scm (run-system-administration-cbt-page): New > variable. > (run-services-page): Use run-system-administration-cbt-page when not > installing a desktop. Ping! I suppose it's too late for 1.3.0, due to the string freeze. But I am asking for review anyways.
Hey Leo,
> I suppose it's too late for 1.3.0, due to the string freeze.
It looks rather nice, but I'll test it more thoroughly after the
release. Did you make sure that the installer tests are passing with:
make check-system TESTS="gui-installed-os gui-uefi-installed-os
gui-installed-os-encrypted gui-installed-desktop-os-encrypted"
Thanks,
Mathieu
On Sun, May 02, 2021 at 11:25:05AM +0200, Mathieu Othacehe wrote: > > Hey Leo, > > > I suppose it's too late for 1.3.0, due to the string freeze. > > It looks rather nice, but I'll test it more thoroughly after the > release. Did you make sure that the installer tests are passing with: > > make check-system TESTS="gui-installed-os gui-uefi-installed-os > gui-installed-os-encrypted gui-installed-desktop-os-encrypted" I tested it on "bare metal", but I will run the systems tests, too.
Hey Leo, > +(define (run-system-administration-cbt-page) > + "Run a page to select various system adminstration services." > + (let ((items (filter (lambda (service) > + (eq? 'administration > + (system-service-type service))) > + %system-services))) > + (run-checkbox-tree-page > + #:title (G_ "Miscellaneous services") > + #:info-text (G_ "Select miscellaneous services to run on your system.") > + #:items items > + #:selection (map system-service-recommended? items) > + #:item->text (compose G_ system-service-name) > + #:checkbox-tree-height 5 > + #:exit-button-callback-procedure > + (lambda () > + (raise > + (condition > + (&installer-step-abort))))))) Indentation is off here, > + ;; Miscellaneous system administration services. > + (system-service > + (name (G_ "Network time service, to set the clock automatically")) > + (type 'administration) > + (recommended? #t) > + (snippet '((service ntp-service-type)))) > + here, > + (system-service > + (name (G_ "GPM mouse daemon, to use the mouse in the console")) > + (type 'administration) > + (snippet '((service gpm-service-type)))) and here. Those patches are breaking the installer tests because they introduce a new dialog. You need to add something like: --8<---------------cut here---------------start------------->8--- ((checkbox-list (title "Miscellaneous services") (text _) (items ,services)) (filter choose-misc-service? services)) --8<---------------cut here---------------end--------------->8--- in the (gnu installer tests) module, to fix this error: --8<---------------cut here---------------start------------->8--- conversation expecting pattern ((quote list-selection) ((quote title) "Partitioning method") ((quote multiple-choices?) #f) ((quote items) (not-encrypted encrypted _ ...))) /gnu/store/h38i2hvaqy9bd0sbn9isdfzl7m56mngr-shepherd-marionette.scm:1:1718: ERROR: 1. &pattern-not-matched: pattern: ((quote list-selection) ((quote title) "Partitioning method") ((quote multiple-choices?) #f) ((quote items) (not-encrypted encrypted _ ...))) sexp: (checkbox-list (title "Miscellaneous services") (text "Select miscellaneous services to run on your system.") (items ("Network time service, to set the clock automatically" "GPM mouse daemon, to use the mouse in the console"))) May 21 14:53:11 localhost instaBacktrace: ller[180]: running form #<newt-form 79dc20> ("Miscellaneous services") with 1 clients 2 (primitive-load "/gnu/store/9vzfy688gawwn2p06nn75kiqqz3?") In ice-9/eval.scm: 191:35 1 (_ #f) 619:8 0 (_ #(#<directory (guile-user) 7ffff3bb3f00> #<variabl?>)) --8<---------------cut here---------------end--------------->8--- Otherwise, it looks fine :) Thanks, Mathieu
Thanks for the review. On Fri, May 21, 2021 at 02:58:48PM +0200, Mathieu Othacehe wrote: > > +(define (run-system-administration-cbt-page) > > + "Run a page to select various system adminstration services." > > + (let ((items (filter (lambda (service) > > + (eq? 'administration > > + (system-service-type service))) > > + %system-services))) > > + (run-checkbox-tree-page > > + #:title (G_ "Miscellaneous services") > > + #:info-text (G_ "Select miscellaneous services to run on your system.") > > + #:items items > > + #:selection (map system-service-recommended? items) > > + #:item->text (compose G_ system-service-name) > > + #:checkbox-tree-height 5 > > + #:exit-button-callback-procedure > > + (lambda () > > + (raise > > + (condition > > + (&installer-step-abort))))))) > > Indentation is off here, > > > + ;; Miscellaneous system administration services. > > + (system-service > > + (name (G_ "Network time service, to set the clock automatically")) > > + (type 'administration) > > + (recommended? #t) > > + (snippet '((service ntp-service-type)))) > > + > > here, > > > + (system-service > > + (name (G_ "GPM mouse daemon, to use the mouse in the console")) > > + (type 'administration) > > + (snippet '((service gpm-service-type)))) > > and here. I don't see the problems. I copied from existing pieces of similar code and I don't really have a sense of what's right and wrong for indenting this kind of thing. > Those patches are breaking the installer tests because they > introduce a new dialog. You need to add something like: > > --8<---------------cut here---------------start------------->8--- > ((checkbox-list (title "Miscellaneous services") (text _) > (items ,services)) > (filter choose-misc-service? services)) > --8<---------------cut here---------------end--------------->8--- > > in the (gnu installer tests) module, to fix this error: Okay, I'll work on that.
On Fri, May 21, 2021 at 02:58:48PM +0200, Mathieu Othacehe wrote: > Those patches are breaking the installer tests because they > introduce a new dialog. You need to add something like: > > --8<---------------cut here---------------start------------->8--- > ((checkbox-list (title "Miscellaneous services") (text _) > (items ,services)) > (filter choose-misc-service? services)) > --8<---------------cut here---------------end--------------->8--- > > in the (gnu installer tests) module, to fix this error: How do I run this test? I can never figure out the names of the tests used by `make check-system`...
On Fri, May 21, 2021 at 10:21:50AM -0400, Leo Famulari wrote: > On Fri, May 21, 2021 at 02:58:48PM +0200, Mathieu Othacehe wrote: > > Those patches are breaking the installer tests because they > > introduce a new dialog. You need to add something like: > > > > --8<---------------cut here---------------start------------->8--- > > ((checkbox-list (title "Miscellaneous services") (text _) > > (items ,services)) > > (filter choose-misc-service? services)) > > --8<---------------cut here---------------end--------------->8--- > > > > in the (gnu installer tests) module, to fix this error: > > How do I run this test? I can never figure out the names of the tests > used by `make check-system`... With informed use of grep I found it: `make check-system TESTS=iso-image-installer`.
> How do I run this test? I can never figure out the names of the tests > used by `make check-system`... The installer tests are defined in the (gnu tests install) module. You can then search for "%test-gui" for all the graphical installation tests. You can run all of them by typing: --8<---------------cut here---------------start------------->8--- make check-system TESTS="gui-installed-os gui-uefi-installed-os gui-installed-os-encrypted gui-installed-desktop-os-encrypted" --8<---------------cut here---------------end--------------->8--- Fixing the "gui-installed-os" test should be enough to fix all of them though. Thanks, Mathieu
diff --git a/gnu/installer/newt/services.scm b/gnu/installer/newt/services.scm index 74f28e41ba..ee003b24b1 100644 --- a/gnu/installer/newt/services.scm +++ b/gnu/installer/newt/services.scm @@ -3,6 +3,7 @@ ;;; Copyright © 2019, 2020 Ludovic Courtès <ludo@gnu.org> ;;; Copyright © 2020 Jan (janneke) Nieuwenhuizen <janneke@gnu.org> ;;; Copyright © 2021 Tobias Geerinckx-Rice <me@tobias.gr> +;;; Copyright © 2021 Leo Famulari <leo@famulari.name> ;;; ;;; This file is part of GNU Guix. ;;; @@ -68,6 +69,25 @@ system.") (condition (&installer-step-abort))))))) +(define (run-system-administration-cbt-page) + "Run a page to select various system adminstration services." + (let ((items (filter (lambda (service) + (eq? 'administration + (system-service-type service))) + %system-services))) + (run-checkbox-tree-page + #:title (G_ "Miscellaneous services") + #:info-text (G_ "Select miscellaneous services to run on your system.") + #:items items + #:selection (map system-service-recommended? items) + #:item->text (compose G_ system-service-name) + #:checkbox-tree-height 5 + #:exit-button-callback-procedure + (lambda () + (raise + (condition + (&installer-step-abort))))))) + (define (run-network-management-page) "Run a page to select among several network management methods." (let ((title (G_ "Network management"))) @@ -99,5 +119,6 @@ client may be enough for a server.") (append desktop (run-networking-cbt-page) (if (null? desktop) - (list (run-network-management-page)) + (cons (run-network-management-page) + (run-system-administration-cbt-page)) '())))) diff --git a/gnu/installer/services.scm b/gnu/installer/services.scm index ec5ea30594..b50bd3e412 100644 --- a/gnu/installer/services.scm +++ b/gnu/installer/services.scm @@ -2,6 +2,7 @@ ;;; Copyright © 2018 Mathieu Othacehe <m.othacehe@gmail.com> ;;; Copyright © 2019 Ludovic Courtès <ludo@gnu.org> ;;; Copyright © 2020 Jan (janneke) Nieuwenhuizen <janneke@gnu.org> +;;; Copyright © 2021 Leo Famulari <leo@famulari.name> ;;; ;;; This file is part of GNU Guix. ;;; @@ -104,6 +105,13 @@ (packages '((specification->package "nss-certs"))) (recommended? #t)) + ;; Miscellaneous system administration services. + (system-service + (name (G_ "Network time service, to set the clock automatically")) + (type 'administration) + (recommended? #t) + (snippet '((service ntp-service-type)))) + ;; Network connectivity management. (system-service (name (G_ "NetworkManager network connection manager"))