[PATCH] installer: Recommend 'ntp-service-type' for non-graphical systems.

Done

Details

3 participants

Leo Famulari
Ludovic Courtès
Mathieu Othacehe

Owner: unassigned

Submitted by: Leo Famulari

Severity: important

Leo Famulari wrote on 23 Apr 2021 20:53

Recipients:(address . guix-patches@gnu.org)

Message-ID:b0a1ebc01ab84f43d5bddff5f54cea44dc04e513.1619203994.git.leo@famulari.name

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/services.scm | 6 ++++++

1 file changed, 6 insertions(+)

Toggle diff (26 lines)diff --git a/gnu/installer/services.scm b/gnu/installer/services.scm
index ec5ea30594..440b8c67db 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.
 ;;;
@@ -103,6 +104,11 @@
       (type 'networking)
       (packages '((specification->package "nss-certs")))
       (recommended? #t))
+     (system-service
+       (name (G_ "Network time service, to set the clock automatically"))
+       (type 'network-management)
+       (snippet '((service ntp-service-type)))
+       (recommended? #t))
 
      ;; Network connectivity management.
      (system-service
-- 
2.31.1

Leo Famulari wrote on 23 Apr 2021 20:56

Recipients:(address . 47979@debbugs.gnu.org)

Message-ID:YIMYaG4+hTJj5/s9@jasmine.lan

Hey everyone, please disregard this patch.

It doesn't do what I meant. Sorry for being hasty and sending the wrong

thing.

Leo Famulari wrote on 24 Apr 2021 00:45

Recipients:(address . 47979@debbugs.gnu.org)

Message-ID:YINOJ3HMdKFFKASf@jasmine.lan

Okay, here is a patch that does what I want.

That is, when the user does not select any desktop environments, it
prompts them about enabling NTP, with the checkbox already checked.

However, with this patch, the installer crashes after partitioning, like
this:

------
In ./gnu/installer/services.scm:
     38:0 19 (%system-service-snippet-procedure (#<<system-service> name: "Network time service, to set the clock automatically" type: administration recommended?: #t snippet: ((service ntp-servi…>))
In ice-9/boot-9.scm:
  1669:16 18 (raise-exception _ #:continuable? _)
  1667:16 17 (raise-exception _ #:continuable? _)
  1667:16 16 (raise-exception _ #:continuable? _)
  1667:16 15 (raise-exception _ #:continuable? _)
  1667:16 14 (raise-exception _ #:continuable? _)
  1667:16 13 (raise-exception _ #:continuable? _)
  1667:16 12 (raise-exception _ #:continuable? _)
  1667:16 11 (raise-exception _ #:continuable? _)
  1667:16 10 (raise-exception _ #:continuable? _)
  1667:16  9 (raise-exception _ #:continuable? _)
  1667:16  8 (raise-exception _ #:continuable? _)
  1667:16  7 (raise-exception _ #:continuable? _)
  1764:13  6 (_ #<&compound-exception components: (#<&assertion-failure> #<&origin origin: "struct-vtable"> #<&message message: "Wrong type argument in position 1 (expecting struct): ~S"> #<&irrit…>)
In ice-9/eval.scm:
    619:8  5 (_ #(#(#<directory (guile-user) 7f8be12f4c80> #<<installer> name: newt init: #<procedure init ()> exit: #<procedure exit ()> exit-error: #<procedure exit-error (file key args)> f…>) …))
    619:8  4 (_ #(#(#(#<directory (guile-user) 7f8be12f4c80> #<<installer> name: newt init: #<procedure init ()> exit: #<procedure exit ()> exit-error: #<procedure exit-error (file key arg…>) …) #))
In ice-9/ports.scm:
   463:17  3 (call-with-output-file _ _ #:binary _ #:encoding _)
In ice-9/eval.scm:
    619:8  2 (_ #(#(#<directory (guile-user) 7f8be12f4c80> wrong-type-arg ("struct-vtable" "Wrong type argument in position 1 (expecting struct): ~S" ((#<<system-service> name: "Network …>)) …)) #))
    159:9  1 (_ #(#(#<directory (guile-user) 7f8be12f4c80> wrong-type-arg ("struct-vtable" "Wrong type argument in position 1 (expecting struct): ~S" ((#<<system-service> name: "Network …>)) …)) #))
In unknown file:
           0 (make-stack #t)
ice-9/eval.scm:159:9: In procedure struct-vtable: Wrong type argument in position 1 (expecting struct): (#<<system-service> name: "Network time service, to set the clock automatically" type: administration recommended?: #t snippet: ((service ntp-service-type)) packages: ()>)
------

So, I've done something wrong. But I don't know what :)

I'm going to keep trying but hopefully somebody can help me.

From c70ed1b6ed621a56ab258cf5cfce45c50cb1831f Mon Sep 17 00:00:00 2001
From: Leo Famulari <leo@famulari.name>
Date: Fri, 23 Apr 2021 14:50:15 -0400
Subject: [PATCH] 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.
---
 gnu/installer/newt/services.scm | 23 ++++++++++++++++++++++-
 gnu/installer/services.scm      |  8 ++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

Toggle diff (74 lines)diff --git a/gnu/installer/newt/services.scm b/gnu/installer/newt/services.scm
index 74f28e41ba..60c62a52b1 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 adminstrative 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 system 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))
+                (list (run-network-management-page)
+                      (run-system-administration-cbt-page))
                 '()))))
diff --git a/gnu/installer/services.scm b/gnu/installer/services.scm
index ec5ea30594..17ff31d2ec 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))
 
+     ;; System administration
+     (system-service
+       (name (G_ "Network time service, to set the clock automatically"))
+       (type 'administration)
+       (snippet '((service ntp-service-type)))
+       (recommended? #t))
+
      ;; Network connectivity management.
      (system-service
       (name (G_ "NetworkManager network connection manager"))
-- 
2.31.1

Leo Famulari wrote on 24 Apr 2021 01:20

Recipients:(address . 47979@debbugs.gnu.org)

Message-ID:YINWQz00YwrQP2l1@jasmine.lan

On Fri, Apr 23, 2021 at 06:45:59PM -0400, Leo Famulari wrote:

Toggle quote (3 lines)

> However, with this patch, the installer crashes after partitioning, like

> this:

By the way, I'm testing this by building the installer in the same way

as our Makefile:

$ guix environment guix -- ./pre-inst-env guix system image -t iso9660 --label="GUIX_x86_64-linux-leo" --system=x86_64-linux gnu/system/install.scm

I create a QEMU disk:

$ qemu-img create -f qcow2 guix-system.img 50G

And then I use the ISO I built, after copying it out of the store:

$ qemu-system-x86_64 -m 1024 -smp 1 -enable-kvm -nic user,model=virtio-net-pci -boot menu=on,order=d -drive file=guix-system.img -drive media=cdrom,file=image.iso

Leo Famulari wrote on 24 Apr 2021 02:56

Re: [PATCH v3] installer: Recommend 'ntp-service-type' for non-graphical systems.

Recipients:(address . 47979@debbugs.gnu.org)

Message-ID:YINsy8LNhQd4NYmq@jasmine.lan

On Fri, Apr 23, 2021 at 06:45:59PM -0400, Leo Famulari wrote:

Toggle quote (2 lines)

> ice-9/eval.scm:159:9: In procedure struct-vtable: Wrong type argument in position 1 (expecting struct): (#<<system-service> name: "Network time service, to set the clock automatically" type: administration recommended?: #t snippet: ((service ntp-service-type)) packages: ()>)

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.
---
 gnu/installer/newt/services.scm | 23 ++++++++++++++++++++++-
 gnu/installer/services.scm      |  8 ++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

Toggle diff (74 lines)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"))
-- 
2.31.1

From a00274e70d700b9325ad9ab7cd1804d444a5dc37 Mon Sep 17 00:00:00 2001
From: Leo Famulari <leo@famulari.name>
Date: Fri, 23 Apr 2021 20:42:28 -0400
Subject: [PATCH 2/2] installer: Offer 'gpm-service-type' for non-graphical
 systems.

* gnu/installer/services.scm (%system-services): Add the gpm-service-type.
---
 gnu/installer/services.scm | 4 ++++
 1 file changed, 4 insertions(+)

Toggle diff (17 lines)diff --git a/gnu/installer/services.scm b/gnu/installer/services.scm
index b50bd3e412..27beb5da66 100644
--- a/gnu/installer/services.scm
+++ b/gnu/installer/services.scm
@@ -111,6 +111,10 @@
        (type 'administration)
        (recommended? #t)
        (snippet '((service ntp-service-type))))
+     (system-service
+       (name (G_ "GPM mouse daemon, to use the mouse in the console"))
+       (type 'administration)
+       (snippet '((service gpm-service-type))))
 
      ;; Network connectivity management.
      (system-service
-- 
2.31.1

Leo Famulari wrote on 2 May 2021 06:36

Recipients:(address . 47979@debbugs.gnu.org)

Message-ID:YI4sPu8mD8UKwG9d@jasmine.lan

On Fri, Apr 23, 2021 at 08:56:43PM -0400, Leo Famulari wrote:

Toggle quote (23 lines)> 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.

Mathieu Othacehe wrote on 2 May 2021 11:25

Re: [bug#47979] [PATCH v3] installer: Recommend 'ntp-service-type' for non-graphical systems.

Recipients:(name . Leo Famulari)(address . leo@famulari.name)(address . 47979@debbugs.gnu.org)

Message-ID:87sg351o9a.fsf@gnu.org

Hey Leo,

Toggle quote (2 lines)

> 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

Leo Famulari wrote on 2 May 2021 17:22

Recipients:(name . Mathieu Othacehe)(address . othacehe@gnu.org)(address . 47979@debbugs.gnu.org)

Message-ID:YI7Do9YQ7+gAsuh2@jasmine.lan

On Sun, May 02, 2021 at 11:25:05AM +0200, Mathieu Othacehe wrote:

Toggle quote (11 lines)> 
> 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.

Mathieu Othacehe wrote on 21 May 2021 14:58

Re: bug#47979: [PATCH] installer: Recommend 'ntp-service-type' for non-graphical systems.

Recipients:(name . Leo Famulari)(address . leo@famulari.name)(address . 47979@debbugs.gnu.org)

Message-ID:87lf88qm47.fsf_-_@gnu.org

Hey Leo,

Toggle quote (19 lines)> +(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,

Toggle quote (8 lines)

> + ;; 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,

Toggle quote (5 lines)

> + (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:

Toggle snippet (5 lines)

((checkbox-list (title "Miscellaneous services") (text _)

(items ,services))

(filter choose-misc-service? services))

in the (gnu installer tests) module, to fix this error:

Toggle snippet (13 lines)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?>))

Otherwise, it looks fine :)

Thanks,

Mathieu

Leo Famulari wrote on 21 May 2021 15:37

Recipients:(name . Mathieu Othacehe)(address . othacehe@gnu.org)(address . 47979@debbugs.gnu.org)

Message-ID:YKe3hmapmNSvhemn@jasmine.lan

Thanks for the review.

On Fri, May 21, 2021 at 02:58:48PM +0200, Mathieu Othacehe wrote:

Toggle quote (38 lines)> > +(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.

Toggle quote (11 lines)> 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.

Leo Famulari wrote on 21 May 2021 16:21

Recipients:(name . Mathieu Othacehe)(address . othacehe@gnu.org)(address . 47979@debbugs.gnu.org)

Message-ID:YKfB/hBQUwLPrWci@jasmine.lan

On Fri, May 21, 2021 at 02:58:48PM +0200, Mathieu Othacehe wrote:

Toggle quote (11 lines)> 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`...

Leo Famulari wrote on 21 May 2021 16:26

Recipients:(name . Mathieu Othacehe)(address . othacehe@gnu.org)(address . 47979@debbugs.gnu.org)

Message-ID:YKfC+mRQMycoW/4d@jasmine.lan

On Fri, May 21, 2021 at 10:21:50AM -0400, Leo Famulari wrote:

Toggle quote (15 lines)> 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`.

Mathieu Othacehe wrote on 21 May 2021 16:39

Recipients:(name . Leo Famulari)(address . leo@famulari.name)(address . 47979@debbugs.gnu.org)

Message-ID:875yzcqhg4.fsf@gnu.org

Toggle quote (3 lines)

> 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:

Toggle snippet (4 lines)

make check-system TESTS="gui-installed-os gui-uefi-installed-os

gui-installed-os-encrypted gui-installed-desktop-os-encrypted"

Fixing the "gui-installed-os" test should be enough to fix all of them

though.

Thanks,

Mathieu

Leo Famulari wrote on 21 May 2021 23:01

Recipients:(name . Mathieu Othacehe)(address . othacehe@gnu.org)(address . 47979@debbugs.gnu.org)

Message-ID:YKgfsIih02iFbR6o@jasmine.lan

Attachment: file

Leo Famulari wrote on 21 May 2021 23:44

Recipients:(name . Mathieu Othacehe)(address . othacehe@gnu.org)(address . 47979@debbugs.gnu.org)

Message-ID:YKgpxUpoZcU1IvPs@jasmine.lan

On Fri, May 21, 2021 at 05:01:36PM -0400, Leo Famulari wrote:

Toggle quote (3 lines)

> I tried copying existing code, e.g. the choose-network-service?

> part of the tests.

The tests succeed when I amend the v4 patch to do "(choose-misc-service?

(const #f))" in (gnu installer tests).

I'm not really sure what it means :/

Mathieu Othacehe wrote on 1 Jun 2021 11:49

Recipients:(name . Leo Famulari)(address . leo@famulari.name)(address . 47979@debbugs.gnu.org)

Message-ID:87tumh6hj0.fsf@gnu.org

Hey Leo,

Toggle quote (2 lines)

> cannot build derivation `/gnu/store/20flyxbjjl8as5aca4gzf3b54as1gr9z-ntp-4.2.8p15.drv': 1 dependencies couldn't be built

The installation tests are run in a network isolated environment. This

means that we need to make sure that every single derivation that will

be needed during the installation is made available.

It is for sure a tricky business. If you have a look to the error line I

pasted above, you will see that the installer tries to build the ntp

package because the ntp-service-type is now included.

The following patch fixes the "gui-installed-os" test by making sure

that the ntp package is available in the installation environment. I did

not check for the other installation tests.

Toggle snippet (18 lines)diff --git a/gnu/tests/install.scm b/gnu/tests/install.scm
index b5263f5f0d..e9ebc63b64 100644
--- a/gnu/tests/install.scm
+++ b/gnu/tests/install.scm
@@ -1608,8 +1608,9 @@ build (current-guix) and then store a couple of full system images.")
     (swap-devices (if encrypted?
                       '()
                       (list (uuid "11111111-2222-3333-4444-123456789abc"))))
-    (services (cons (service dhcp-client-service-type)
-                    (operating-system-user-services %minimal-os-on-vda)))))
+    (services (cons* (service dhcp-client-service-type)
+                     (service ntp-service-type)
+                     (operating-system-user-services %minimal-os-on-vda)))))
 
 (define* (installation-target-desktop-os-for-gui-tests
           #:key (encrypted? #f))

Thanks,

Mathieu

Ludovic Courtès wrote on 10 Jun 2021 10:29

control message for bug #47979

Recipients:(address . control@debbugs.gnu.org)

Message-ID:8735tqazrf.fsf@gnu.org

severity 47979 important

quit

Leo Famulari wrote on 24 Dec 2021 19:04

[PATCH v5] gnu: binutils: Fix CVE-2021-45078.

Recipients:(address . 47979@debbugs.gnu.org)

Message-ID:802baa40ed72d1020c15226ec295187bbb1565be.1640369055.git.leo@famulari.name

* gnu/packages/patches/binutils-CVE-2021-45078.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/base.scm (binutils)[source]: Use it.
---
 gnu/local.mk                                  |   1 +
 gnu/packages/base.scm                         |   3 +-
 .../patches/binutils-CVE-2021-45078.patch     | 257 ++++++++++++++++++
 3 files changed, 260 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/binutils-CVE-2021-45078.patch

Toggle diff (291 lines)diff --git a/gnu/local.mk b/gnu/local.mk
index aa0117a3f5..546b940eb6 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -906,6 +906,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/binutils-loongson-workaround.patch	\
   %D%/packages/patches/binutils-mingw-w64-timestamp.patch	\
   %D%/packages/patches/binutils-mingw-w64-deterministic.patch	\
+  %D%/packages/patches/binutils-CVE-2021-45078.patch		\
   %D%/packages/patches/blender-2.79-gcc8.patch			\
   %D%/packages/patches/blender-2.79-gcc9.patch			\
   %D%/packages/patches/blender-2.79-newer-ffmpeg.patch		\
diff --git a/gnu/packages/base.scm b/gnu/packages/base.scm
index 12e4de52d4..be6fbf608d 100644
--- a/gnu/packages/base.scm
+++ b/gnu/packages/base.scm
@@ -521,7 +521,8 @@ (define-public binutils
       (sha256
        (base32 "1m3b2rdfv1dmdpd0bzg1hy7i8a2qng53szc6livyi3nh6101mz37"))
       (patches (search-patches "binutils-loongson-workaround.patch"
-                               "binutils-2.37-file-descriptor-leak.patch"))))
+                               "binutils-2.37-file-descriptor-leak.patch"
+                               "binutils-CVE-2021-45078.patch"))))
    (build-system gnu-build-system)
    (arguments
     `(#:configure-flags '(;; Add `-static-libgcc' to not retain a dependency
diff --git a/gnu/packages/patches/binutils-CVE-2021-45078.patch b/gnu/packages/patches/binutils-CVE-2021-45078.patch
new file mode 100644
index 0000000000..fca692bdb5
--- /dev/null
+++ b/gnu/packages/patches/binutils-CVE-2021-45078.patch
@@ -0,0 +1,257 @@
+Fix CVE-2021-45078 (incomplete fix for CVE-2018-12699):
+
+https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-45078
+https://sourceware.org/bugzilla/show_bug.cgi?id=28694
+
+Patch copied from upstream source repository:
+
+https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=161e87d12167b1e36193385485c1f6ce92f74f02
+
+From 161e87d12167b1e36193385485c1f6ce92f74f02 Mon Sep 17 00:00:00 2001
+From: Alan Modra <amodra@gmail.com>
+Date: Wed, 15 Dec 2021 11:48:42 +1030
+Subject: [PATCH] PR28694, Out-of-bounds write in stab_xcoff_builtin_type
+
+	PR 28694
+	* stabs.c (stab_xcoff_builtin_type): Make typenum unsigned.
+	Negate typenum earlier, simplifying bounds checking.  Correct
+	off-by-one indexing.  Adjust switch cases.
+---
+ binutils/stabs.c | 87 ++++++++++++++++++++++++------------------------
+ 1 file changed, 43 insertions(+), 44 deletions(-)
+
+diff --git a/binutils/stabs.c b/binutils/stabs.c
+index 274bfb0e7fa..83ee3ea5fa4 100644
+--- a/binutils/stabs.c
++++ b/binutils/stabs.c
+@@ -202,7 +202,7 @@ static debug_type stab_find_type (void *, struct stab_handle *, const int *);
+ static bool stab_record_type
+   (void *, struct stab_handle *, const int *, debug_type);
+ static debug_type stab_xcoff_builtin_type
+-  (void *, struct stab_handle *, int);
++  (void *, struct stab_handle *, unsigned int);
+ static debug_type stab_find_tagged_type
+   (void *, struct stab_handle *, const char *, int, enum debug_type_kind);
+ static debug_type *stab_demangle_argtypes
+@@ -3496,166 +3496,167 @@ stab_record_type (void *dhandle ATTRIBUTE_UNUSED, struct stab_handle *info,
+ 
+ static debug_type
+ stab_xcoff_builtin_type (void *dhandle, struct stab_handle *info,
+-			 int typenum)
++			 unsigned int typenum)
+ {
+   debug_type rettype;
+   const char *name;
+ 
+-  if (typenum >= 0 || typenum < -XCOFF_TYPE_COUNT)
++  typenum = -typenum - 1;
++  if (typenum >= XCOFF_TYPE_COUNT)
+     {
+-      fprintf (stderr, _("Unrecognized XCOFF type %d\n"), typenum);
++      fprintf (stderr, _("Unrecognized XCOFF type %d\n"), -typenum - 1);
+       return DEBUG_TYPE_NULL;
+     }
+-  if (info->xcoff_types[-typenum] != NULL)
+-    return info->xcoff_types[-typenum];
++  if (info->xcoff_types[typenum] != NULL)
++    return info->xcoff_types[typenum];
+ 
+-  switch (-typenum)
++  switch (typenum)
+     {
+-    case 1:
++    case 0:
+       /* The size of this and all the other types are fixed, defined
+ 	 by the debugging format.  */
+       name = "int";
+       rettype = debug_make_int_type (dhandle, 4, false);
+       break;
+-    case 2:
++    case 1:
+       name = "char";
+       rettype = debug_make_int_type (dhandle, 1, false);
+       break;
+-    case 3:
++    case 2:
+       name = "short";
+       rettype = debug_make_int_type (dhandle, 2, false);
+       break;
+-    case 4:
++    case 3:
+       name = "long";
+       rettype = debug_make_int_type (dhandle, 4, false);
+       break;
+-    case 5:
++    case 4:
+       name = "unsigned char";
+       rettype = debug_make_int_type (dhandle, 1, true);
+       break;
+-    case 6:
++    case 5:
+       name = "signed char";
+       rettype = debug_make_int_type (dhandle, 1, false);
+       break;
+-    case 7:
++    case 6:
+       name = "unsigned short";
+       rettype = debug_make_int_type (dhandle, 2, true);
+       break;
+-    case 8:
++    case 7:
+       name = "unsigned int";
+       rettype = debug_make_int_type (dhandle, 4, true);
+       break;
+-    case 9:
++    case 8:
+       name = "unsigned";
+       rettype = debug_make_int_type (dhandle, 4, true);
+       break;
+-    case 10:
++    case 9:
+       name = "unsigned long";
+       rettype = debug_make_int_type (dhandle, 4, true);
+       break;
+-    case 11:
++    case 10:
+       name = "void";
+       rettype = debug_make_void_type (dhandle);
+       break;
+-    case 12:
++    case 11:
+       /* IEEE single precision (32 bit).  */
+       name = "float";
+       rettype = debug_make_float_type (dhandle, 4);
+       break;
+-    case 13:
++    case 12:
+       /* IEEE double precision (64 bit).  */
+       name = "double";
+       rettype = debug_make_float_type (dhandle, 8);
+       break;
+-    case 14:
++    case 13:
+       /* This is an IEEE double on the RS/6000, and different machines
+ 	 with different sizes for "long double" should use different
+ 	 negative type numbers.  See stabs.texinfo.  */
+       name = "long double";
+       rettype = debug_make_float_type (dhandle, 8);
+       break;
+-    case 15:
++    case 14:
+       name = "integer";
+       rettype = debug_make_int_type (dhandle, 4, false);
+       break;
+-    case 16:
++    case 15:
+       name = "boolean";
+       rettype = debug_make_bool_type (dhandle, 4);
+       break;
+-    case 17:
++    case 16:
+       name = "short real";
+       rettype = debug_make_float_type (dhandle, 4);
+       break;
+-    case 18:
++    case 17:
+       name = "real";
+       rettype = debug_make_float_type (dhandle, 8);
+       break;
+-    case 19:
++    case 18:
+       /* FIXME */
+       name = "stringptr";
+       rettype = NULL;
+       break;
+-    case 20:
++    case 19:
+       /* FIXME */
+       name = "character";
+       rettype = debug_make_int_type (dhandle, 1, true);
+       break;
+-    case 21:
++    case 20:
+       name = "logical*1";
+       rettype = debug_make_bool_type (dhandle, 1);
+       break;
+-    case 22:
++    case 21:
+       name = "logical*2";
+       rettype = debug_make_bool_type (dhandle, 2);
+       break;
+-    case 23:
++    case 22:
+       name = "logical*4";
+       rettype = debug_make_bool_type (dhandle, 4);
+       break;
+-    case 24:
++    case 23:
+       name = "logical";
+       rettype = debug_make_bool_type (dhandle, 4);
+       break;
+-    case 25:
++    case 24:
+       /* Complex type consisting of two IEEE single precision values.  */
+       name = "complex";
+       rettype = debug_make_complex_type (dhandle, 8);
+       break;
+-    case 26:
++    case 25:
+       /* Complex type consisting of two IEEE double precision values.  */
+       name = "double complex";
+       rettype = debug_make_complex_type (dhandle, 16);
+       break;
+-    case 27:
++    case 26:
+       name = "integer*1";
+       rettype = debug_make_int_type (dhandle, 1, false);
+       break;
+-    case 28:
++    case 27:
+       name = "integer*2";
+       rettype = debug_make_int_type (dhandle, 2, false);
+       break;
+-    case 29:
++    case 28:
+       name = "integer*4";
+       rettype = debug_make_int_type (dhandle, 4, false);
+       break;
+-    case 30:
++    case 29:
+       /* FIXME */
+       name = "wchar";
+       rettype = debug_make_int_type (dhandle, 2, false);
+       break;
+-    case 31:
++    case 30:
+       name = "long long";
+       rettype = debug_make_int_type (dhandle, 8, false);
+       break;
+-    case 32:
++    case 31:
+       name = "unsigned long long";
+       rettype = debug_make_int_type (dhandle, 8, true);
+       break;
+-    case 33:
++    case 32:
+       name = "logical*8";
+       rettype = debug_make_bool_type (dhandle, 8);
+       break;
+-    case 34:
++    case 33:
+       name = "integer*8";
+       rettype = debug_make_int_type (dhandle, 8, false);
+       break;
+@@ -3664,9 +3665,7 @@ stab_xcoff_builtin_type (void *dhandle, struct stab_handle *info,
+     }
+ 
+   rettype = debug_name_type (dhandle, name, rettype);
+-
+-  info->xcoff_types[-typenum] = rettype;
+-
++  info->xcoff_types[typenum] = rettype;
+   return rettype;
+ }
+ 
+-- 
+2.27.0
+
-- 
2.34.0

Leo Famulari wrote on 24 Dec 2021 19:04

[PATCH v5 1/2] installer: Recommend 'ntp-service-type' for non-graphical systems.

Recipients:(address . 47979@debbugs.gnu.org)

Message-ID:f28531869bc7135b4710edfe57a3a88ed2777575.1640369080.git.leo@famulari.name

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.
* gnu/installer/tests.scm (choose-services): Add and use a
choose-misc-service?  procedure.
* gnu/tests/install.scm (installation-target-os-for-gui-tests)<services>: Add
ntp-service-type.
---
 gnu/installer/newt/services.scm | 23 ++++++++++++++++++++++-
 gnu/installer/services.scm      |  8 ++++++++
 gnu/installer/tests.scm         | 10 +++++++++-
 gnu/tests/install.scm           |  5 +++--
 4 files changed, 42 insertions(+), 4 deletions(-)

Toggle diff (119 lines)diff --git a/gnu/installer/newt/services.scm b/gnu/installer/newt/services.scm
index 1af4e7df2d..56cb3f6787 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.
 ;;;
@@ -90,6 +91,25 @@ (define (run-other-services-cbt-page)
         (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")))
@@ -121,6 +141,7 @@ (define (run-services-page)
     (append desktop
             (run-networking-cbt-page)
             (if (null? desktop)
-                (list (run-network-management-page))
+                (cons (run-network-management-page)
+                      (run-system-administration-cbt-page))
                 '())
             (run-other-services-cbt-page))))
diff --git a/gnu/installer/services.scm b/gnu/installer/services.scm
index 341d8b69c8..94c49307f0 100644
--- a/gnu/installer/services.scm
+++ b/gnu/installer/services.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2019 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.
 ;;;
@@ -104,6 +105,13 @@ (define %system-services
       (packages '((specification->package "nss-certs")))
       (recommended? #t))
 
+     ;; Miscellaneous system administration services.
+     (system-service
+       (name (G_ "Network time service (NTP), 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"))
diff --git a/gnu/installer/tests.scm b/gnu/installer/tests.scm
index 12d1d91608..dc2371ce97 100644
--- a/gnu/installer/tests.scm
+++ b/gnu/installer/tests.scm
@@ -221,7 +221,11 @@ (define* (choose-services port
                           (choose-network-management-tool?
                            (lambda (service)
                              (string-contains service "DHCP")))
-                          (choose-other-service? (const #f)))
+                          (choose-other-service? (const #f))
+                          (choose-misc-service?
+                           (lambda (service)
+                             (string-contains service "NTP"))))
+
   "Converse over PORT to choose services."
   (define desktop-environments '())
 
@@ -243,6 +247,10 @@ (define desktop-environments '())
      (null? desktop-environments)
      (find choose-network-management-tool? services))
 
+    ((checkbox-list (title "Miscellaneous services") (text _)
+                     (items ,services))
+     (filter choose-misc-service? services))
+
     ((checkbox-list (title "Other services") (text _)
                     (items ,services))
      (filter choose-other-service? services))))
diff --git a/gnu/tests/install.scm b/gnu/tests/install.scm
index 154f98b2e1..ae8c6051f1 100644
--- a/gnu/tests/install.scm
+++ b/gnu/tests/install.scm
@@ -1685,8 +1685,9 @@ (define* (installation-target-os-for-gui-tests
          (list
           (swap-space
            (target (uuid "11111111-2222-3333-4444-123456789abc"))))))
-    (services (cons (service dhcp-client-service-type)
-                    (operating-system-user-services %minimal-os-on-vda)))))
+    (services (cons* (service dhcp-client-service-type)
+                     (service ntp-service-type)
+                     (operating-system-user-services %minimal-os-on-vda)))))
 
 (define* (installation-target-desktop-os-for-gui-tests
           #:key (encrypted? #f))
-- 
2.34.0

Leo Famulari wrote on 24 Dec 2021 19:04

[PATCH v5 2/2] installer: Offer 'gpm-service-type' for non-graphical systems.

Recipients:(address . 47979@debbugs.gnu.org)

Message-ID:3f4e7ef8e9e4088cd567d6c0b6d5effa7daf59c9.1640369080.git.leo@famulari.name

* gnu/installer/services.scm (%system-services): Add the gpm-service-type.

---

gnu/installer/services.scm | 4 ++++

1 file changed, 4 insertions(+)

Toggle diff (17 lines)diff --git a/gnu/installer/services.scm b/gnu/installer/services.scm
index 94c49307f0..be6e75cbc4 100644
--- a/gnu/installer/services.scm
+++ b/gnu/installer/services.scm
@@ -111,6 +111,10 @@ (define %system-services
        (type 'administration)
        (recommended? #t)
        (snippet '((service ntp-service-type))))
+     (system-service
+       (name (G_ "GPM mouse daemon, to use the mouse in the console"))
+       (type 'administration)
+       (snippet '((service gpm-service-type))))
 
      ;; Network connectivity management.
      (system-service
-- 
2.34.0

Leo Famulari wrote on 24 Dec 2021 19:14

Re: [PATCH v5 1/2] installer: Recommend 'ntp-service-type' for non-graphical systems.

Recipients:(address . 47979@debbugs.gnu.org)

Message-ID:YcYN9fZvhxYIQDVd@jasmine.lan

On Fri, Dec 24, 2021 at 01:04:40PM -0500, Leo Famulari wrote:

Toggle quote (14 lines)> 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.
> * gnu/installer/tests.scm (choose-services): Add and use a
> choose-misc-service?  procedure.
> * gnu/tests/install.scm (installation-target-os-for-gui-tests)<services>: Add
> ntp-service-type.

This v5 patch series is rebased on current master.

It passes `make check-system TESTS="gui-installed-os"`.

When a desktop environment is not selected, it adds a page to the

installer suggesting NTP, and offering GPM (console mouse).

I would have said that it's ready to go, but commit 6f13881f1e92

"installer: Offer the CUPS printing service." also added a page to the

installer offering NTP, unconditionally. That is because the

run-other-services page that it adds offers all recommended services.

https://git.savannah.gnu.org/cgit/guix.git/commit/?id=6f13881f1e92832023caadb3cb51ce393e685e58

However, that breaks the installation, because NTP is provided by

%desktop-services, and services may not be duplicated in config.scm.

So, we need to adjust the functionality added by 6f13881f1e92. I think

we should only offer NTP when the user does not choose any desktops.

If you'd like to test these patches, you can follow the steps described

previously:

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=47979#14

I still think we need to ensure that NTP is either enabled or suggested

for all installations. I remember the wave of bug reports due to "clock

is wrong" after 1.3.0.

Leo Famulari wrote on 26 Dec 2021 21:31

[PATCH v6 1/3] installer: Offer the CUPS printing service on a dedicated page.

Recipients:(address . 47979@debbugs.gnu.org)

Message-ID:18b693c017c779578de0a6a367ebf0eab95c7054.1640550687.git.leo@famulari.name

Currently, this page of the installer offers to the user all installer services

that are not of the types 'desktop', 'network-management', or 'networking'.

Concretely, this means that it offers the CUPS printing service, because that

is the only service of a different type defined in the installer.

In later commits, we will add some services of a new type, and we only want them

to be offered when the user is installing a non-graphical system.

At least one of these new services (NTP) is part of %desktop-services. If it

were offered on run-other-services-cbt-page, and the user had configured a

system using %desktop-services, the user could accidentally add NTP to their

services twice, which is an error and would break installation.

So, this commit makes the run-other-services-cbt-page be more specific about

what services to offer. This allows us to later create a new checkbox page that

only runs when %desktop-services is not selected.

* gnu/installer/newt/services.scm (run-other-services-cbt-page): Rename to ...

(run-printing-services-cbt-page): ... new variable, and select only 'document' services.

(run-services-page): Adjust accordingly.

* gnu/installer/tests.scm (choose-services): Adjust accordingly.

---

gnu/installer/newt/services.scm | 16 +++++++---------

gnu/installer/tests.scm | 2 +-

2 files changed, 8 insertions(+), 10 deletions(-)

Toggle diff (50 lines)diff --git a/gnu/installer/newt/services.scm b/gnu/installer/newt/services.scm
index 1af4e7df2d..b4b5219b20 100644
--- a/gnu/installer/newt/services.scm
+++ b/gnu/installer/newt/services.scm
@@ -68,18 +68,16 @@ (define (run-networking-cbt-page)
         (condition
          (&installer-step-abort)))))))
 
-(define (run-other-services-cbt-page)
-  "Run a page allowing the user to select other services."
+(define (run-printing-services-cbt-page)
+  "Run a page allowing the user to select document services such as CUPS."
   (let ((items (filter (lambda (service)
-                         (not (member (system-service-type service)
-                                      '(desktop
-                                        network-management
-                                        networking))))
+                         (eq? 'document
+                              (system-service-type service)))
                        %system-services)))
     (run-checkbox-tree-page
-     #:info-text (G_ "You can now select other services to run on your \
+     #:info-text (G_ "You can now select the CUPS printing service to run on your \
 system.")
-     #:title (G_ "Other services")
+     #:title (G_ "Printing and document services")
      #:items items
      #:selection (map system-service-recommended? items)
      #:item->text (compose G_ system-service-name)
@@ -123,4 +121,4 @@ (define (run-services-page)
             (if (null? desktop)
                 (list (run-network-management-page))
                 '())
-            (run-other-services-cbt-page))))
+            (run-printing-services-cbt-page))))
diff --git a/gnu/installer/tests.scm b/gnu/installer/tests.scm
index 12d1d91608..5d931149b8 100644
--- a/gnu/installer/tests.scm
+++ b/gnu/installer/tests.scm
@@ -243,7 +243,7 @@ (define desktop-environments '())
      (null? desktop-environments)
      (find choose-network-management-tool? services))
 
-    ((checkbox-list (title "Other services") (text _)
+    ((checkbox-list (title "Printing and document services") (text _)
                     (items ,services))
      (filter choose-other-service? services))))
 
-- 
2.34.0

Leo Famulari wrote on 26 Dec 2021 21:31

[PATCH v6 2/3] installer: Recommend 'ntp-service-type' for non-graphical systems.

Recipients:(address . 47979@debbugs.gnu.org)

Message-ID:817dd7dd12e866f87428dcac82fff96e42c4ba77.1640550687.git.leo@famulari.name

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.
* gnu/installer/tests.scm (choose-services): Add and use a
choose-misc-service?  procedure.
* gnu/tests/install.scm (installation-target-os-for-gui-tests)<services>: Add
ntp-service-type.
---
 gnu/installer/newt/services.scm | 25 ++++++++++++++++++++++++-
 gnu/installer/services.scm      |  8 ++++++++
 gnu/installer/tests.scm         | 10 +++++++++-
 gnu/tests/install.scm           |  5 +++--
 4 files changed, 44 insertions(+), 4 deletions(-)

Toggle diff (121 lines)diff --git a/gnu/installer/newt/services.scm b/gnu/installer/newt/services.scm
index b4b5219b20..c218825813 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.
 ;;;
@@ -88,6 +89,27 @@ (define (run-printing-services-cbt-page)
         (condition
          (&installer-step-abort)))))))
 
+(define (run-console-services-cbt-page)
+  "Run a page to select various system adminstration services for non-graphical
+systems."
+  (let ((items (filter (lambda (service)
+                         (eq? 'administration
+                              (system-service-type service)))
+                       %system-services)))
+    (run-checkbox-tree-page
+      #:title (G_ "Console services")
+      #:info-text (G_ "Select miscellaneous services to run on your \
+non-graphical 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")))
@@ -119,6 +141,7 @@ (define (run-services-page)
     (append desktop
             (run-networking-cbt-page)
             (if (null? desktop)
-                (list (run-network-management-page))
+                (cons (run-network-management-page)
+                      (run-console-services-cbt-page))
                 '())
             (run-printing-services-cbt-page))))
diff --git a/gnu/installer/services.scm b/gnu/installer/services.scm
index 341d8b69c8..94c49307f0 100644
--- a/gnu/installer/services.scm
+++ b/gnu/installer/services.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2019 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.
 ;;;
@@ -104,6 +105,13 @@ (define %system-services
       (packages '((specification->package "nss-certs")))
       (recommended? #t))
 
+     ;; Miscellaneous system administration services.
+     (system-service
+       (name (G_ "Network time service (NTP), 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"))
diff --git a/gnu/installer/tests.scm b/gnu/installer/tests.scm
index 5d931149b8..39be59f689 100644
--- a/gnu/installer/tests.scm
+++ b/gnu/installer/tests.scm
@@ -221,7 +221,11 @@ (define* (choose-services port
                           (choose-network-management-tool?
                            (lambda (service)
                              (string-contains service "DHCP")))
-                          (choose-other-service? (const #f)))
+                          (choose-other-service? (const #f))
+                          (choose-misc-service?
+                           (lambda (service)
+                             (string-contains service "NTP"))))
+
   "Converse over PORT to choose services."
   (define desktop-environments '())
 
@@ -243,6 +247,10 @@ (define desktop-environments '())
      (null? desktop-environments)
      (find choose-network-management-tool? services))
 
+    ((checkbox-list (title "Console services") (text _)
+                     (items ,services))
+     (filter choose-misc-service? services))
+
     ((checkbox-list (title "Printing and document services") (text _)
                     (items ,services))
      (filter choose-other-service? services))))
diff --git a/gnu/tests/install.scm b/gnu/tests/install.scm
index 154f98b2e1..ae8c6051f1 100644
--- a/gnu/tests/install.scm
+++ b/gnu/tests/install.scm
@@ -1685,8 +1685,9 @@ (define* (installation-target-os-for-gui-tests
          (list
           (swap-space
            (target (uuid "11111111-2222-3333-4444-123456789abc"))))))
-    (services (cons (service dhcp-client-service-type)
-                    (operating-system-user-services %minimal-os-on-vda)))))
+    (services (cons* (service dhcp-client-service-type)
+                     (service ntp-service-type)
+                     (operating-system-user-services %minimal-os-on-vda)))))
 
 (define* (installation-target-desktop-os-for-gui-tests
           #:key (encrypted? #f))
-- 
2.34.0

Leo Famulari wrote on 26 Dec 2021 21:31

[PATCH v6 3/3] installer: Offer 'gpm-service-type' for non-graphical systems.

Recipients:(address . 47979@debbugs.gnu.org)

Message-ID:1a104bb17072170af44b1d2886e98f2377c28f74.1640550687.git.leo@famulari.name

* gnu/installer/services.scm (%system-services): Add the gpm-service-type.

---

gnu/installer/services.scm | 4 ++++

1 file changed, 4 insertions(+)

Toggle diff (17 lines)diff --git a/gnu/installer/services.scm b/gnu/installer/services.scm
index 94c49307f0..be6e75cbc4 100644
--- a/gnu/installer/services.scm
+++ b/gnu/installer/services.scm
@@ -111,6 +111,10 @@ (define %system-services
        (type 'administration)
        (recommended? #t)
        (snippet '((service ntp-service-type))))
+     (system-service
+       (name (G_ "GPM mouse daemon, to use the mouse in the console"))
+       (type 'administration)
+       (snippet '((service gpm-service-type))))
 
      ;; Network connectivity management.
      (system-service
-- 
2.34.0

Leo Famulari wrote on 26 Dec 2021 21:33

Re: [PATCH v6 2/3] installer: Recommend 'ntp-service-type' for non-graphical systems.

Recipients:(address . 47979@debbugs.gnu.org)

Message-ID:YcjRpAVOIMwnax3F@jasmine.lan

On Sun, Dec 26, 2021 at 03:31:30PM -0500, Leo Famulari wrote:

Toggle quote (14 lines)> 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.
> * gnu/installer/tests.scm (choose-services): Add and use a
> choose-misc-service?  procedure.
> * gnu/tests/install.scm (installation-target-os-for-gui-tests)<services>: Add
> ntp-service-type.

Here's v6 of this patch series.

With it, the installer successfully installs systems with and without

%desktop-services, and correctly chooses whether or not to offer NTP and

GPM.

It also passes `make check-system TESTS="gui-installed-os"`.

I think it's ready to go.

Mathieu Othacehe wrote on 26 Dec 2021 22:06

Re: [bug#47979] [PATCH v6 2/3] installer: Recommend 'ntp-service-type' for non-graphical systems.

Recipients:(name . Leo Famulari)(address . leo@famulari.name)(address . 47979@debbugs.gnu.org)

Message-ID:87a6gnnkji.fsf@gnu.org

Hello Leo,

Toggle quote (2 lines)

> It also passes `make check-system TESTS="gui-installed-os"`.

Nice! You might also want to check that the

gui-installed-desktop-os-encrypted test is passing as it covers desktop

installation. Otherwise, it LGTM.

Thanks,

Mathieu

Leo Famulari wrote on 27 Dec 2021 01:57

Recipients:(name . Mathieu Othacehe)(address . othacehe@gnu.org)(address . 47979@debbugs.gnu.org)

Message-ID:YckPjP6k7ivQtcqq@jasmine.lan

On Sun, Dec 26, 2021 at 10:06:09PM +0100, Mathieu Othacehe wrote:

Toggle quote (4 lines)

> Nice! You might also want to check that the

> gui-installed-desktop-os-encrypted test is passing as it covers desktop

> installation. Otherwise, it LGTM.

Good idea, these changes did break that test. I'm tweaking the patches

now and re-testing.

Leo Famulari wrote on 27 Dec 2021 23:24

[PATCH v7 1/3] installer: Offer the CUPS printing service on a dedicated page.

Recipients:(address . 47979@debbugs.gnu.org)

Message-ID:422b32c84a0ae293398777f5233565b9c282d5da.1640643713.git.leo@famulari.name