diff mbox series

[bug#47979,v5,1/2] installer: Recommend 'ntp-service-type' for non-graphical systems.

Message ID f28531869bc7135b4710edfe57a3a88ed2777575.1640369080.git.leo@famulari.name
State Accepted
Headers show
Series [bug#47979,v5,1/2] installer: Recommend 'ntp-service-type' for non-graphical systems. | expand

Checks

Context Check Description
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Leo Famulari Dec. 24, 2021, 6:04 p.m. UTC
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(-)

Comments

Leo Famulari Dec. 24, 2021, 6:14 p.m. UTC | #1
On Fri, Dec 24, 2021 at 01:04:40PM -0500, Leo Famulari wrote:
> 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.
diff mbox series

Patch

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