GNU bug report logs - #61587
[PATCH 0/8] networking services refactoring

Previous Next

Package: guix-patches;

Reported by: Bruno Victal <mirai <at> makinata.eu>

Date: Fri, 17 Feb 2023 21:14:02 UTC

Severity: normal

Tags: patch

Done: Ludovic Courtès <ludo <at> gnu.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 61587 in the body.
You can then email your comments to 61587 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox

Report forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Fri, 17 Feb 2023 21:14:02 GMT) Full text and rfc822 format available.
Acknowledgement sent to Bruno Victal <mirai <at> makinata.eu>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Fri, 17 Feb 2023 21:14:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: guix-patches <at> gnu.org
Cc: Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH 0/8] networking services refactoring
Date: Fri, 17 Feb 2023 21:12:53 +0000

Changes of interest:

* Make wpa_supplicant (or iwd) optional.
* Fixes networking for NetworkManager starting too early (see [1] e [2])
* Introduces canonical and virtual names for connman and NetworkManager
services.


[1]: https://issues.guix.gnu.org/60300
[2]: https://issues.guix.gnu.org/47253

Bruno Victal (8):
  services: network-manager: Add 'shepherd-requirement' field.
  services: network-manager: Deprecate 'iwd?' field.
  services: connman: Use match-record and export accessors.
  services: connman: Add 'shepherd-requirement' field.
  services: connman: Deprecate 'iwd?' field.
  services: network-manager: Await for NetworkManager to finish starting
    up.
  services: network-manager: Set service canonical-name to
    NetworkManager.
  services: connman: Set service canonical-name to connman.

 doc/guix.texi               |  18 +++--
 gnu/services/networking.scm | 132 ++++++++++++++++++++++++------------
 2 files changed, 99 insertions(+), 51 deletions(-)


base-commit: 8d8e1438ae5a2e50005b500dacd0a26be540fe69
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Fri, 17 Feb 2023 21:16:01 GMT) Full text and rfc822 format available.

Message #8 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH 1/8] services: network-manager: Add 'shepherd-requirement'
 field.
Date: Fri, 17 Feb 2023 21:14:52 +0000

Note: This also makes wpa-supplicant an optional requirement.

* gnu/services/networking.scm (<network-manager-configuration>)
[shepherd-requirement]: New field.
(network-manager-shepherd-service): Honor it.
(network-manager-configuration-shepherd-requirement): Export accessor.
* doc/guix.texi (Networking Setup): Document it.
---
 doc/guix.texi               |  6 ++++++
 gnu/services/networking.scm | 10 +++++++---
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 44e2165a82..46e0f637d0 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19704,6 +19704,12 @@ Networking Setup
 @item @code{network-manager} (default: @code{network-manager})
 The NetworkManager package to use.
 
+@item @code{shepherd-requirement} (default: @code{()})
+This option can be used to provide a list of symbols naming Shepherd services
+that this service will depend on, such as @code{'wpa-supplicant} or
+@code{'iwd} if you require authenticated access for encrypted WiFi or Ethernet
+networks.
+
 @item @code{dns} (default: @code{"default"})
 Processing mode for DNS, which affects how NetworkManager uses the
 @code{resolv.conf} configuration file.
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index dacf64c2d1..5284855b83 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -143,6 +143,7 @@ (define-module (gnu services networking)
 
             network-manager-configuration
             network-manager-configuration?
+            network-manager-configuration-shepherd-requirement
             network-manager-configuration-dns
             network-manager-configuration-vpn-plugins
             network-manager-service-type
@@ -1140,6 +1141,8 @@ (define-record-type* <network-manager-configuration>
   network-manager-configuration?
   (network-manager network-manager-configuration-network-manager
                    (default network-manager))
+  (shepherd-requirement network-manager-configuration-shepherd-requirement
+                        (default '()))
   (dns network-manager-configuration-dns
        (default "default"))
   (vpn-plugins network-manager-configuration-vpn-plugins ;list of file-like
@@ -1200,7 +1203,7 @@ (define (network-manager-environment config)
 
 (define (network-manager-shepherd-service config)
   (match-record config <network-manager-configuration>
-    (network-manager dns vpn-plugins iwd?)
+    (network-manager shepherd-requirement dns vpn-plugins iwd?)
     (let ((conf (plain-file "NetworkManager.conf"
                             (string-append
                              "[main]\ndns=" dns "\n"
@@ -1209,8 +1212,9 @@ (define (network-manager-shepherd-service config)
       (list (shepherd-service
              (documentation "Run the NetworkManager.")
              (provision '(networking))
-             (requirement (append '(user-processes dbus-system loopback)
-                                  (if iwd? '(iwd) '(wpa-supplicant))))
+             (requirement `(user-processes dbus-system loopback
+                            ,@shepherd-requirement
+                            ,@(if iwd? '(iwd) '())))
              (start #~(make-forkexec-constructor
                        (list (string-append #$network-manager
                                             "/sbin/NetworkManager")
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Fri, 17 Feb 2023 21:16:02 GMT) Full text and rfc822 format available.

Message #11 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH 2/8] services: network-manager: Deprecate 'iwd?' field.
Date: Fri, 17 Feb 2023 21:14:53 +0000

* gnu/services/networking.scm (warn-iwd?-field-deprecation): New procedure,
helper for deprecated field.
(<network-manager-configuration>)[iwd?]: Use helper to warn deprecated field.
(network-manager-shepherd-service): Make iwd? a local variable independent
from the deprecated field.
* doc/guix.texi (Networking Setup): Remove mention of iwd? field.
---
 doc/guix.texi               |  4 ----
 gnu/services/networking.scm | 20 ++++++++++++++++++--
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 46e0f637d0..8ccd727e6e 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19758,10 +19758,6 @@ Networking Setup
 (VPNs).  An example of this is the @code{network-manager-openvpn}
 package, which allows NetworkManager to manage VPNs @i{via} OpenVPN.
 
-@item @code{iwd?} (default: @code{#f})
-NetworkManager will use iwd as a backend for wireless networking if this
-option is set to @code{#t}, otherwise it will use wpa-supplicant.
-
 @end table
 @end deftp
 
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 5284855b83..ddf2e20791 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -1136,6 +1136,15 @@ (define-record-type* <modem-manager-configuration>
 ;;; NetworkManager
 ;;;
 
+;; TODO: deprecated field, remove later.
+(define-with-syntax-properties (warn-iwd?-field-deprecation
+                                (value properties))
+  (when value
+    (warning (source-properties->location properties)
+             (G_ "the 'iwd?' field is deprecated, please use \
+'shepherd-requirement' field instead~%")))
+  value)
+
 (define-record-type* <network-manager-configuration>
   network-manager-configuration make-network-manager-configuration
   network-manager-configuration?
@@ -1147,7 +1156,9 @@ (define-record-type* <network-manager-configuration>
        (default "default"))
   (vpn-plugins network-manager-configuration-vpn-plugins ;list of file-like
                (default '()))
-  (iwd? network-manager-configuration-iwd? (default #f)))
+  (iwd? network-manager-configuration-iwd?  ; TODO: deprecated field, remove.
+        (default #f)
+        (sanitize warn-iwd?-field-deprecation)))
 
 (define (network-manager-activation config)
   ;; Activation gexp for NetworkManager
@@ -1204,7 +1215,10 @@ (define (network-manager-environment config)
 (define (network-manager-shepherd-service config)
   (match-record config <network-manager-configuration>
     (network-manager shepherd-requirement dns vpn-plugins iwd?)
-    (let ((conf (plain-file "NetworkManager.conf"
+    (let ((iwd? (or iwd?  ; TODO: deprecated field, remove later.
+                    (and shepherd-requirement
+                         (memq 'iwd shepherd-requirement))))
+          (conf (plain-file "NetworkManager.conf"
                             (string-append
                              "[main]\ndns=" dns "\n"
                              (if iwd? "[device]\nwifi.backend=iwd\n" ""))))
@@ -1214,6 +1228,8 @@ (define (network-manager-shepherd-service config)
              (provision '(networking))
              (requirement `(user-processes dbus-system loopback
                             ,@shepherd-requirement
+                            ;; TODO: iwd? is deprecated and should be passed
+                            ;; with shepherd-requirement, remove later.
                             ,@(if iwd? '(iwd) '())))
              (start #~(make-forkexec-constructor
                        (list (string-append #$network-manager
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Fri, 17 Feb 2023 21:16:02 GMT) Full text and rfc822 format available.

Message #14 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH 3/8] services: connman: Use match-record and export accessors.
Date: Fri, 17 Feb 2023 21:14:54 +0000

* gnu/services/networking.scm (connman-shepherd-service): Use match-record.
(connman-configuration-connman, connman-configuration-disable-vpn?)
(connman-configuration-iwd?): Export accessors.
---
 gnu/services/networking.scm | 52 ++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 27 deletions(-)

diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index ddf2e20791..5cba3a9a3f 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -150,6 +150,9 @@ (define-module (gnu services networking)
 
             connman-configuration
             connman-configuration?
+            connman-configuration-connman
+            connman-configuration-disable-vpn?
+            connman-configuration-iwd?
             connman-service-type
 
             modem-manager-configuration
@@ -1300,33 +1303,28 @@ (define (connman-activation config)
             (mkdir-p "/var/lib/connman-vpn/"))))))
 
 (define (connman-shepherd-service config)
-  "Return a shepherd service for Connman"
-  (and
-   (connman-configuration? config)
-   (let ((connman      (connman-configuration-connman config))
-         (disable-vpn? (connman-configuration-disable-vpn? config))
-         (iwd?         (connman-configuration-iwd? config)))
-     (list (shepherd-service
-            (documentation "Run Connman")
-            (provision '(networking))
-            (requirement
-             (append '(user-processes dbus-system loopback)
-                     (if iwd? '(iwd) '())))
-            (start #~(make-forkexec-constructor
-                      (list (string-append #$connman
-                                           "/sbin/connmand")
-                            "--nodaemon"
-                            "--nodnsproxy"
-                            #$@(if disable-vpn? '("--noplugin=vpn") '())
-                            #$@(if iwd? '("--wifi=iwd_agent") '()))
-
-                      ;; As connman(8) notes, when passing '-n', connman
-                      ;; "directs log output to the controlling terminal in
-                      ;; addition to syslog."  Redirect stdout and stderr
-                      ;; to avoid spamming the console (XXX: for some reason
-                      ;; redirecting to /dev/null doesn't work.)
-                      #:log-file "/var/log/connman.log"))
-            (stop #~(make-kill-destructor)))))))
+  (match-record config <connman-configuration> (connman disable-vpn? iwd?)
+    (list (shepherd-service
+           (documentation "Run Connman")
+           (provision '(networking))
+           (requirement
+            (append '(user-processes dbus-system loopback)
+                    (if iwd? '(iwd) '())))
+           (start #~(make-forkexec-constructor
+                     (list (string-append #$connman
+                                          "/sbin/connmand")
+                           "--nodaemon"
+                           "--nodnsproxy"
+                           #$@(if disable-vpn? '("--noplugin=vpn") '())
+                           #$@(if iwd? '("--wifi=iwd_agent") '()))
+
+                     ;; As connman(8) notes, when passing '-n', connman
+                     ;; "directs log output to the controlling terminal in
+                     ;; addition to syslog."  Redirect stdout and stderr
+                     ;; to avoid spamming the console (XXX: for some reason
+                     ;; redirecting to /dev/null doesn't work.)
+                     #:log-file "/var/log/connman.log"))
+           (stop #~(make-kill-destructor))))))
 
 (define %connman-log-rotation
   (list (log-rotation
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Fri, 17 Feb 2023 21:16:02 GMT) Full text and rfc822 format available.

Message #17 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH 4/8] services: connman: Add 'shepherd-requirement' field.
Date: Fri, 17 Feb 2023 21:14:55 +0000

* gnu/services/networking.scm (<connman-configuration>)
[shepherd-requirement]: New field.
(connman-shepherd-service): Honor it.
(connman-configuration-shepherd-requirement): Export accessor.
* doc/guix.texi (Networking Setup): Document it.
---
 doc/guix.texi               |  6 ++++++
 gnu/services/networking.scm | 12 ++++++++----
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 8ccd727e6e..0a0080b117 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19785,6 +19785,12 @@ Networking Setup
 @item @code{connman} (default: @var{connman})
 The connman package to use.
 
+@item @code{shepherd-requirement} (default: @code{()})
+This option can be used to provide a list of symbols naming Shepherd services
+that this service will depend on, such as @code{'wpa-supplicant} or
+@code{'iwd} if you require authenticated access for encrypted WiFi or Ethernet
+networks.
+
 @item @code{disable-vpn?} (default: @code{#f})
 When true, disable connman's vpn plugin.
 
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 5cba3a9a3f..30a4d6a6d8 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -151,6 +151,7 @@ (define-module (gnu services networking)
             connman-configuration
             connman-configuration?
             connman-configuration-connman
+            connman-configuration-shepherd-requirement
             connman-configuration-disable-vpn?
             connman-configuration-iwd?
             connman-service-type
@@ -1288,6 +1289,8 @@ (define-record-type* <connman-configuration>
   connman-configuration?
   (connman      connman-configuration-connman
                 (default connman))
+  (shepherd-requirement connman-configuration-shepherd-requirement
+                        (default '()))
   (disable-vpn? connman-configuration-disable-vpn?
                 (default #f))
   (iwd?         connman-configuration-iwd?
@@ -1303,13 +1306,14 @@ (define (connman-activation config)
             (mkdir-p "/var/lib/connman-vpn/"))))))
 
 (define (connman-shepherd-service config)
-  (match-record config <connman-configuration> (connman disable-vpn? iwd?)
+  (match-record config <connman-configuration> (connman shepherd-requirement
+                                                disable-vpn? iwd?)
     (list (shepherd-service
            (documentation "Run Connman")
            (provision '(networking))
-           (requirement
-            (append '(user-processes dbus-system loopback)
-                    (if iwd? '(iwd) '())))
+           (requirement `(user-processes dbus-system loopback
+                          ,@shepherd-requirement
+                          ,@(if iwd? '(iwd) '())))
            (start #~(make-forkexec-constructor
                      (list (string-append #$connman
                                           "/sbin/connmand")
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Fri, 17 Feb 2023 21:17:02 GMT) Full text and rfc822 format available.

Message #20 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH 6/8] services: network-manager: Await for NetworkManager to
 finish starting up.
Date: Fri, 17 Feb 2023 21:14:57 +0000

This is similar to its NetworkManager-wait-online.service systemd counterpart,
with the main difference being that we handle it all in 'networking symbol, rather than
introduce a new 'networking-online symbol. (see discussion #47253)

As a result of this change, with opensmtpd-service-type as an example,
manual 'herd restart smtpd' after system bootups are no longer required
when opensmtpd is configured with a smtpd.conf containing non-loopback interfaces.
(this issue is described in more detail at #60300)

Addresses #60300.
Supersedes #47253. (Note: Shepherd no longer blocks since shepherd 0.9.3)

* gnu/services/networking.scm (network-manager-shepherd-service): Await for
NetworkManager to finish starting up.
---
 gnu/services/networking.scm | 36 +++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 13816327b0..55bc2cf362 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -1235,17 +1235,31 @@ (define (network-manager-shepherd-service config)
                             ;; TODO: iwd? is deprecated and should be passed
                             ;; with shepherd-requirement, remove later.
                             ,@(if iwd? '(iwd) '())))
-             (start #~(make-forkexec-constructor
-                       (list (string-append #$network-manager
-                                            "/sbin/NetworkManager")
-                             (string-append "--config=" #$conf)
-                             "--no-daemon")
-                       #:environment-variables
-                       (list (string-append "NM_VPN_PLUGIN_DIR=" #$vpn
-                                            "/lib/NetworkManager/VPN")
-                             ;; Override non-existent default users
-                             "NM_OPENVPN_USER="
-                             "NM_OPENVPN_GROUP=")))
+             (start
+              #~(lambda args
+                  (let ((constructor
+                         (apply
+                          (make-forkexec-constructor
+                           (list #$(file-append network-manager
+                                                "/sbin/NetworkManager")
+                                 (string-append "--config=" #$conf)
+                                 "--no-daemon")
+                           #:environment-variables
+                           (list (string-append "NM_VPN_PLUGIN_DIR=" #$vpn
+                                                "/lib/NetworkManager/VPN")
+                                 ;; Override non-existent default users
+                                 "NM_OPENVPN_USER="
+                                 "NM_OPENVPN_GROUP=")) args)))
+                    ;; XXX: Despite the "online" name, this doesn't guarantee
+                    ;; WAN connectivity, it merely waits for NetworkManager
+                    ;; to finish starting-up. This is required otherwise
+                    ;; services will fail since the network interfaces be
+                    ;; absent until NetworkManager finishes setting them up.
+                    (system* #$(file-append network-manager "/bin/nm-online")
+                             "--wait-for-startup" "--quiet")
+                    ;; XXX: Finally, return the value from running
+                    ;; make-forkexec-constructor to shepherd.
+                    constructor)))
              (stop #~(make-kill-destructor)))))))
 
 (define network-manager-service-type
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Fri, 17 Feb 2023 21:17:02 GMT) Full text and rfc822 format available.

Message #23 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH 7/8] services: network-manager: Set service canonical-name to
 NetworkManager.
Date: Fri, 17 Feb 2023 21:14:58 +0000

According to the semantics in [1], 'networking should be a "virtual service"
and NetworkManager its canonical-name. This does not influence
existing services and they should continue to use the 'networking symbol.

One visible change is that 'herd status' doesn't show 'networking' anymore,
instead listing 'NetworkManager' in its place but both symbols are can be used
to start and stop the same service.

Note: Though the symbol NetworkManager doesn't really conform with the overall kebab-case
used throughout Guix, this is intentional as we really want to make it clear that
that the symbol NetworkManager really refers to the software called NetworkManager,
since it's a canonical name here. (rather than risk misleading the user to interpret
the symbol network-manager as a symbol for some unspecific network management software)


[1]: https://www.gnu.org/software/shepherd/manual/html_node/Jump-Start.html

* gnu/services/networking.scm (network-manager-shepherd-service): Make 'networking a
virtual service and set 'NetworkManager as its canonical name.
---
 gnu/services/networking.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 55bc2cf362..bcde0d0db8 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -1229,7 +1229,7 @@ (define (network-manager-shepherd-service config)
           (vpn  (vpn-plugin-directory vpn-plugins)))
       (list (shepherd-service
              (documentation "Run the NetworkManager.")
-             (provision '(networking))
+             (provision '(NetworkManager networking))
              (requirement `(user-processes dbus-system loopback
                             ,@shepherd-requirement
                             ;; TODO: iwd? is deprecated and should be passed
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Fri, 17 Feb 2023 21:17:03 GMT) Full text and rfc822 format available.

Message #26 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH 8/8] services: connman: Set service canonical-name to connman.
Date: Fri, 17 Feb 2023 21:14:59 +0000

* gnu/services/networking.scm (connman-shepherd-service): Make 'networking a
virtual service and set 'connman as its canonical name.
---
 gnu/services/networking.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index bcde0d0db8..7c35ff72eb 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -1328,7 +1328,7 @@ (define (connman-shepherd-service config)
                          (memq 'iwd shepherd-requirement)))))
       (list (shepherd-service
              (documentation "Run Connman")
-             (provision '(networking))
+             (provision '(connman networking))
              (requirement `(user-processes dbus-system loopback
                                            ,@shepherd-requirement
                                            ;; TODO: iwd? is deprecated and should be passed
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Fri, 17 Feb 2023 21:25:02 GMT) Full text and rfc822 format available.

Message #29 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH 5/8] services: connman: Deprecate 'iwd?' field.
Date: Fri, 17 Feb 2023 21:14:56 +0000

* gnu/services/networking.scm (<connman-configuration>)
[iwd?]: Use helper to warn deprecated field.
(connman-shepherd-service): Make iwd? a local variable independent from
the deprecated field.
* doc/guix.texi (Networking Setup): Remove mention of iwd? field.
---
 doc/guix.texi               |  2 --
 gnu/services/networking.scm | 50 +++++++++++++++++++++----------------
 2 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 0a0080b117..67c518ca7d 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19794,8 +19794,6 @@ Networking Setup
 @item @code{disable-vpn?} (default: @code{#f})
 When true, disable connman's vpn plugin.
 
-@item @code{iwd?} (default: @code{#f})
-When true, ConnMan uses iwd to connect to wireless networks.
 @end table
 @end deftp
 
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 30a4d6a6d8..13816327b0 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -1294,7 +1294,8 @@ (define-record-type* <connman-configuration>
   (disable-vpn? connman-configuration-disable-vpn?
                 (default #f))
   (iwd?         connman-configuration-iwd?
-                (default #f)))
+                (default #f)
+                (sanitize warn-iwd?-field-deprecation)))
 
 (define (connman-activation config)
   (let ((disable-vpn? (connman-configuration-disable-vpn? config)))
@@ -1308,27 +1309,32 @@ (define (connman-activation config)
 (define (connman-shepherd-service config)
   (match-record config <connman-configuration> (connman shepherd-requirement
                                                 disable-vpn? iwd?)
-    (list (shepherd-service
-           (documentation "Run Connman")
-           (provision '(networking))
-           (requirement `(user-processes dbus-system loopback
-                          ,@shepherd-requirement
-                          ,@(if iwd? '(iwd) '())))
-           (start #~(make-forkexec-constructor
-                     (list (string-append #$connman
-                                          "/sbin/connmand")
-                           "--nodaemon"
-                           "--nodnsproxy"
-                           #$@(if disable-vpn? '("--noplugin=vpn") '())
-                           #$@(if iwd? '("--wifi=iwd_agent") '()))
-
-                     ;; As connman(8) notes, when passing '-n', connman
-                     ;; "directs log output to the controlling terminal in
-                     ;; addition to syslog."  Redirect stdout and stderr
-                     ;; to avoid spamming the console (XXX: for some reason
-                     ;; redirecting to /dev/null doesn't work.)
-                     #:log-file "/var/log/connman.log"))
-           (stop #~(make-kill-destructor))))))
+    (let ((iwd? (or iwd?  ; TODO: deprecated field, remove later.
+                    (and shepherd-requirement
+                         (memq 'iwd shepherd-requirement)))))
+      (list (shepherd-service
+             (documentation "Run Connman")
+             (provision '(networking))
+             (requirement `(user-processes dbus-system loopback
+                                           ,@shepherd-requirement
+                                           ;; TODO: iwd? is deprecated and should be passed
+                                           ;; with shepherd-requirement, remove later.
+                                           ,@(if iwd? '(iwd) '())))
+             (start #~(make-forkexec-constructor
+                       (list (string-append #$connman
+                                            "/sbin/connmand")
+                             "--nodaemon"
+                             "--nodnsproxy"
+                             #$@(if disable-vpn? '("--noplugin=vpn") '())
+                             #$@(if iwd? '("--wifi=iwd_agent") '()))
+
+                       ;; As connman(8) notes, when passing '-n', connman
+                       ;; "directs log output to the controlling terminal in
+                       ;; addition to syslog."  Redirect stdout and stderr
+                       ;; to avoid spamming the console (XXX: for some reason
+                       ;; redirecting to /dev/null doesn't work.)
+                       #:log-file "/var/log/connman.log"))
+             (stop #~(make-kill-destructor)))))))
 
 (define %connman-log-rotation
   (list (log-rotation
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Fri, 03 Mar 2023 17:14:01 GMT) Full text and rfc822 format available.

Message #32 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Bruno Victal <mirai <at> makinata.eu>
Cc: 61587 <at> debbugs.gnu.org
Subject: Re: bug#61587: [PATCH 0/8] networking services refactoring
Date: Fri, 03 Mar 2023 18:13:41 +0100

Hi,

Bruno Victal <mirai <at> makinata.eu> skribis:

> This is similar to its NetworkManager-wait-online.service systemd counterpart,
> with the main difference being that we handle it all in 'networking symbol, rather than
> introduce a new 'networking-online symbol. (see discussion #47253)
>
> As a result of this change, with opensmtpd-service-type as an example,
> manual 'herd restart smtpd' after system bootups are no longer required
> when opensmtpd is configured with a smtpd.conf containing non-loopback interfaces.
> (this issue is described in more detail at #60300)
>
> Addresses #60300.

Please write: “Fixes <https://issues.guix.gnu.org/60300>.”
Possibly along with a “Reported by” line (see the Git log for inspiration).

> Supersedes #47253. (Note: Shepherd no longer blocks since shepherd 0.9.3)

What does that mean?

> * gnu/services/networking.scm (network-manager-shepherd-service): Await for
> NetworkManager to finish starting up.

[...]

> +             (start
> +              #~(lambda args
> +                  (let ((constructor
> +                         (apply
> +                          (make-forkexec-constructor
> +                           (list #$(file-append network-manager
> +                                                "/sbin/NetworkManager")
> +                                 (string-append "--config=" #$conf)
> +                                 "--no-daemon")

Rather:

  (let ((pid (fork+exec-command (list …))))
    …
    pid)

Ludo’.
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Fri, 03 Mar 2023 17:16:02 GMT) Full text and rfc822 format available.

Message #35 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Bruno Victal <mirai <at> makinata.eu>
Cc: 61587 <at> debbugs.gnu.org
Subject: Re: bug#61587: [PATCH 0/8] networking services refactoring
Date: Fri, 03 Mar 2023 18:15:33 +0100

Bruno Victal <mirai <at> makinata.eu> skribis:

>        (list (shepherd-service
>               (documentation "Run the NetworkManager.")
> -             (provision '(networking))
> +             (provision '(NetworkManager networking))

We could discuss this at length, but I’m in favor of either the status
quo, or:

  (provision '(networking network-manager))

WDYT?

Ludo’.
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Fri, 03 Mar 2023 17:27:02 GMT) Full text and rfc822 format available.

Message #38 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 61587 <at> debbugs.gnu.org
Subject: Re: bug#61587: [PATCH 0/8] networking services refactoring
Date: Fri, 3 Mar 2023 17:26:51 +0000

On 2023-03-03 17:13, Ludovic Courtès wrote:> Bruno Victal <mirai <at> makinata.eu> skribis:
> 
>> Supersedes #47253. (Note: Shepherd no longer blocks since shepherd 0.9.3)
> 
> What does that mean?

Oops, I didn't intend to include this in the commit message, it was for the mail body.
At the time I didn't notice that #47253 already had a patch to solve this issue which resulted
in independent rewrite of the same fix. I only noticed it while I was searching for open issues to
attach to the message as potential issues that would be closed by this.

The note refers to what made #47253 untenable back then.

>> * gnu/services/networking.scm (network-manager-shepherd-service): Await for
>> NetworkManager to finish starting up.
> 
> [...]
> 
>> +             (start
>> +              #~(lambda args
>> +                  (let ((constructor
>> +                         (apply
>> +                          (make-forkexec-constructor
>> +                           (list #$(file-append network-manager
>> +                                                "/sbin/NetworkManager")
>> +                                 (string-append "--config=" #$conf)
>> +                                 "--no-daemon")
> 
> Rather:
> 
>   (let ((pid (fork+exec-command (list …))))
>     …
>     pid)

I'll send a v2 with the required touch-ups.


Cheers,
Bruno
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Fri, 03 Mar 2023 17:30:02 GMT) Full text and rfc822 format available.

Message #41 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 61587 <at> debbugs.gnu.org
Subject: Re: bug#61587: [PATCH 0/8] networking services refactoring
Date: Fri, 3 Mar 2023 17:20:03 +0000

On 2023-03-03 17:15, Ludovic Courtès wrote:
> Bruno Victal <mirai <at> makinata.eu> skribis:
> 
>>        (list (shepherd-service
>>               (documentation "Run the NetworkManager.")
>> -             (provision '(networking))
>> +             (provision '(NetworkManager networking))
> 
> We could discuss this at length, but I’m in favor of either the status
> quo, or:
> 
>   (provision '(networking network-manager))
> 
> WDYT?

The ordering matters here, putting 'networking first means that 'network-manager is the virtual service and that something like
connman can provide 'network-manager which is absurd.


Cheers,
Bruno
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Fri, 03 Mar 2023 23:00:03 GMT) Full text and rfc822 format available.

Message #44 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH v2 1/8] services: network-manager: Add 'shepherd-requirement'
 field.
Date: Fri,  3 Mar 2023 22:59:06 +0000

Note: This also makes wpa-supplicant an optional requirement.

* gnu/services/networking.scm (<network-manager-configuration>)
[shepherd-requirement]: New field.
(network-manager-shepherd-service): Honor it.
(network-manager-configuration-shepherd-requirement): Export accessor.
* doc/guix.texi (Networking Setup): Document it.
---
 doc/guix.texi               |  6 ++++++
 gnu/services/networking.scm | 10 +++++++---
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 74658dbc86..4599c87a72 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19836,6 +19836,12 @@ Networking Setup
 @item @code{network-manager} (default: @code{network-manager})
 The NetworkManager package to use.
 
+@item @code{shepherd-requirement} (default: @code{()})
+This option can be used to provide a list of symbols naming Shepherd services
+that this service will depend on, such as @code{'wpa-supplicant} or
+@code{'iwd} if you require authenticated access for encrypted WiFi or Ethernet
+networks.
+
 @item @code{dns} (default: @code{"default"})
 Processing mode for DNS, which affects how NetworkManager uses the
 @code{resolv.conf} configuration file.
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index dacf64c2d1..5284855b83 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -143,6 +143,7 @@ (define-module (gnu services networking)
 
             network-manager-configuration
             network-manager-configuration?
+            network-manager-configuration-shepherd-requirement
             network-manager-configuration-dns
             network-manager-configuration-vpn-plugins
             network-manager-service-type
@@ -1140,6 +1141,8 @@ (define-record-type* <network-manager-configuration>
   network-manager-configuration?
   (network-manager network-manager-configuration-network-manager
                    (default network-manager))
+  (shepherd-requirement network-manager-configuration-shepherd-requirement
+                        (default '()))
   (dns network-manager-configuration-dns
        (default "default"))
   (vpn-plugins network-manager-configuration-vpn-plugins ;list of file-like
@@ -1200,7 +1203,7 @@ (define (network-manager-environment config)
 
 (define (network-manager-shepherd-service config)
   (match-record config <network-manager-configuration>
-    (network-manager dns vpn-plugins iwd?)
+    (network-manager shepherd-requirement dns vpn-plugins iwd?)
     (let ((conf (plain-file "NetworkManager.conf"
                             (string-append
                              "[main]\ndns=" dns "\n"
@@ -1209,8 +1212,9 @@ (define (network-manager-shepherd-service config)
       (list (shepherd-service
              (documentation "Run the NetworkManager.")
              (provision '(networking))
-             (requirement (append '(user-processes dbus-system loopback)
-                                  (if iwd? '(iwd) '(wpa-supplicant))))
+             (requirement `(user-processes dbus-system loopback
+                            ,@shepherd-requirement
+                            ,@(if iwd? '(iwd) '())))
              (start #~(make-forkexec-constructor
                        (list (string-append #$network-manager
                                             "/sbin/NetworkManager")

base-commit: 6a1464b0cc8c0b3e53d2580661a8c69d79f183ab
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Fri, 03 Mar 2023 23:00:03 GMT) Full text and rfc822 format available.

Message #47 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH v2 8/8] services: connman: Set service canonical-name to
 connman.
Date: Fri,  3 Mar 2023 22:59:13 +0000

* gnu/services/networking.scm (connman-shepherd-service): Make 'networking a
virtual service and set 'connman as its canonical name.
---
 gnu/services/networking.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 66fd9121e0..680d6f6b4f 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -1327,7 +1327,7 @@ (define (connman-shepherd-service config)
                          (memq 'iwd shepherd-requirement)))))
       (list (shepherd-service
              (documentation "Run Connman")
-             (provision '(networking))
+             (provision '(connman networking))
              (requirement `(user-processes dbus-system loopback
                                            ,@shepherd-requirement
                                            ;; TODO: iwd? is deprecated and should be passed
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Fri, 03 Mar 2023 23:00:04 GMT) Full text and rfc822 format available.

Message #50 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH v2 5/8] services: connman: Deprecate 'iwd?' field.
Date: Fri,  3 Mar 2023 22:59:10 +0000

* gnu/services/networking.scm (<connman-configuration>)
[iwd?]: Use helper to warn deprecated field.
(connman-shepherd-service): Make iwd? a local variable independent from
the deprecated field.
* doc/guix.texi (Networking Setup): Remove mention of iwd? field.
---
 doc/guix.texi               |  2 --
 gnu/services/networking.scm | 50 +++++++++++++++++++++----------------
 2 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index dceef5cbd5..fcaa5f69be 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19926,8 +19926,6 @@ Networking Setup
 @item @code{disable-vpn?} (default: @code{#f})
 When true, disable connman's vpn plugin.
 
-@item @code{iwd?} (default: @code{#f})
-When true, ConnMan uses iwd to connect to wireless networks.
 @end table
 @end deftp
 
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 30a4d6a6d8..13816327b0 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -1294,7 +1294,8 @@ (define-record-type* <connman-configuration>
   (disable-vpn? connman-configuration-disable-vpn?
                 (default #f))
   (iwd?         connman-configuration-iwd?
-                (default #f)))
+                (default #f)
+                (sanitize warn-iwd?-field-deprecation)))
 
 (define (connman-activation config)
   (let ((disable-vpn? (connman-configuration-disable-vpn? config)))
@@ -1308,27 +1309,32 @@ (define (connman-activation config)
 (define (connman-shepherd-service config)
   (match-record config <connman-configuration> (connman shepherd-requirement
                                                 disable-vpn? iwd?)
-    (list (shepherd-service
-           (documentation "Run Connman")
-           (provision '(networking))
-           (requirement `(user-processes dbus-system loopback
-                          ,@shepherd-requirement
-                          ,@(if iwd? '(iwd) '())))
-           (start #~(make-forkexec-constructor
-                     (list (string-append #$connman
-                                          "/sbin/connmand")
-                           "--nodaemon"
-                           "--nodnsproxy"
-                           #$@(if disable-vpn? '("--noplugin=vpn") '())
-                           #$@(if iwd? '("--wifi=iwd_agent") '()))
-
-                     ;; As connman(8) notes, when passing '-n', connman
-                     ;; "directs log output to the controlling terminal in
-                     ;; addition to syslog."  Redirect stdout and stderr
-                     ;; to avoid spamming the console (XXX: for some reason
-                     ;; redirecting to /dev/null doesn't work.)
-                     #:log-file "/var/log/connman.log"))
-           (stop #~(make-kill-destructor))))))
+    (let ((iwd? (or iwd?  ; TODO: deprecated field, remove later.
+                    (and shepherd-requirement
+                         (memq 'iwd shepherd-requirement)))))
+      (list (shepherd-service
+             (documentation "Run Connman")
+             (provision '(networking))
+             (requirement `(user-processes dbus-system loopback
+                                           ,@shepherd-requirement
+                                           ;; TODO: iwd? is deprecated and should be passed
+                                           ;; with shepherd-requirement, remove later.
+                                           ,@(if iwd? '(iwd) '())))
+             (start #~(make-forkexec-constructor
+                       (list (string-append #$connman
+                                            "/sbin/connmand")
+                             "--nodaemon"
+                             "--nodnsproxy"
+                             #$@(if disable-vpn? '("--noplugin=vpn") '())
+                             #$@(if iwd? '("--wifi=iwd_agent") '()))
+
+                       ;; As connman(8) notes, when passing '-n', connman
+                       ;; "directs log output to the controlling terminal in
+                       ;; addition to syslog."  Redirect stdout and stderr
+                       ;; to avoid spamming the console (XXX: for some reason
+                       ;; redirecting to /dev/null doesn't work.)
+                       #:log-file "/var/log/connman.log"))
+             (stop #~(make-kill-destructor)))))))
 
 (define %connman-log-rotation
   (list (log-rotation
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Fri, 03 Mar 2023 23:00:05 GMT) Full text and rfc822 format available.

Message #53 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH v2 4/8] services: connman: Add 'shepherd-requirement' field.
Date: Fri,  3 Mar 2023 22:59:09 +0000

* gnu/services/networking.scm (<connman-configuration>)
[shepherd-requirement]: New field.
(connman-shepherd-service): Honor it.
(connman-configuration-shepherd-requirement): Export accessor.
* doc/guix.texi (Networking Setup): Document it.
---
 doc/guix.texi               |  6 ++++++
 gnu/services/networking.scm | 12 ++++++++----
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 00313c54e6..dceef5cbd5 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19917,6 +19917,12 @@ Networking Setup
 @item @code{connman} (default: @var{connman})
 The connman package to use.
 
+@item @code{shepherd-requirement} (default: @code{()})
+This option can be used to provide a list of symbols naming Shepherd services
+that this service will depend on, such as @code{'wpa-supplicant} or
+@code{'iwd} if you require authenticated access for encrypted WiFi or Ethernet
+networks.
+
 @item @code{disable-vpn?} (default: @code{#f})
 When true, disable connman's vpn plugin.
 
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 5cba3a9a3f..30a4d6a6d8 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -151,6 +151,7 @@ (define-module (gnu services networking)
             connman-configuration
             connman-configuration?
             connman-configuration-connman
+            connman-configuration-shepherd-requirement
             connman-configuration-disable-vpn?
             connman-configuration-iwd?
             connman-service-type
@@ -1288,6 +1289,8 @@ (define-record-type* <connman-configuration>
   connman-configuration?
   (connman      connman-configuration-connman
                 (default connman))
+  (shepherd-requirement connman-configuration-shepherd-requirement
+                        (default '()))
   (disable-vpn? connman-configuration-disable-vpn?
                 (default #f))
   (iwd?         connman-configuration-iwd?
@@ -1303,13 +1306,14 @@ (define (connman-activation config)
             (mkdir-p "/var/lib/connman-vpn/"))))))
 
 (define (connman-shepherd-service config)
-  (match-record config <connman-configuration> (connman disable-vpn? iwd?)
+  (match-record config <connman-configuration> (connman shepherd-requirement
+                                                disable-vpn? iwd?)
     (list (shepherd-service
            (documentation "Run Connman")
            (provision '(networking))
-           (requirement
-            (append '(user-processes dbus-system loopback)
-                    (if iwd? '(iwd) '())))
+           (requirement `(user-processes dbus-system loopback
+                          ,@shepherd-requirement
+                          ,@(if iwd? '(iwd) '())))
            (start #~(make-forkexec-constructor
                      (list (string-append #$connman
                                           "/sbin/connmand")
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Fri, 03 Mar 2023 23:00:05 GMT) Full text and rfc822 format available.

Message #56 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH v2 6/8] services: network-manager: Await for NetworkManager to
 finish starting up.
Date: Fri,  3 Mar 2023 22:59:11 +0000

This is similar to its NetworkManager-wait-online.service systemd counterpart,
with the main difference being that we handle it all in 'networking symbol, rather than
introduce a new 'networking-online symbol. (see discussion #47253)

As a result of this change, with opensmtpd-service-type as an example,
manual 'herd restart smtpd' after system bootups are no longer required
when opensmtpd is configured with a smtpd.conf containing non-loopback interfaces.
(this issue is described in more detail at #60300)

Fixes <https://issues.guix.gnu.org/60300>.

* gnu/services/networking.scm (network-manager-shepherd-service): Await for
NetworkManager to finish starting up.
---
 gnu/services/networking.scm | 35 ++++++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 13816327b0..76674f346a 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -1235,17 +1235,30 @@ (define (network-manager-shepherd-service config)
                             ;; TODO: iwd? is deprecated and should be passed
                             ;; with shepherd-requirement, remove later.
                             ,@(if iwd? '(iwd) '())))
-             (start #~(make-forkexec-constructor
-                       (list (string-append #$network-manager
-                                            "/sbin/NetworkManager")
-                             (string-append "--config=" #$conf)
-                             "--no-daemon")
-                       #:environment-variables
-                       (list (string-append "NM_VPN_PLUGIN_DIR=" #$vpn
-                                            "/lib/NetworkManager/VPN")
-                             ;; Override non-existent default users
-                             "NM_OPENVPN_USER="
-                             "NM_OPENVPN_GROUP=")))
+             (start
+              #~(lambda _
+                  (let ((pid
+                         (fork+exec-command
+                          (list #$(file-append network-manager
+                                               "/sbin/NetworkManager")
+                                (string-append "--config=" #$conf)
+                                "--no-daemon")
+                          #:environment-variables
+                          (list (string-append "NM_VPN_PLUGIN_DIR=" #$vpn
+                                               "/lib/NetworkManager/VPN")
+                                ;; Override non-existent default users
+                                "NM_OPENVPN_USER="
+                                "NM_OPENVPN_GROUP="))))
+                    ;; XXX: Despite the "online" name, this doesn't guarantee
+                    ;; WAN connectivity, it merely waits for NetworkManager
+                    ;; to finish starting-up. This is required otherwise
+                    ;; services will fail since the network interfaces be
+                    ;; absent until NetworkManager finishes setting them up.
+                    (system* #$(file-append network-manager "/bin/nm-online")
+                             "--wait-for-startup" "--quiet")
+                    ;; XXX: Finally, return the pid from running
+                    ;; fork+exec-command to shepherd.
+                    pid)))
              (stop #~(make-kill-destructor)))))))
 
 (define network-manager-service-type
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Fri, 03 Mar 2023 23:00:06 GMT) Full text and rfc822 format available.

Message #59 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH v2 2/8] services: network-manager: Deprecate 'iwd?' field.
Date: Fri,  3 Mar 2023 22:59:07 +0000

* gnu/services/networking.scm (warn-iwd?-field-deprecation): New procedure,
helper for deprecated field.
(<network-manager-configuration>)[iwd?]: Use helper to warn deprecated field.
(network-manager-shepherd-service): Make iwd? a local variable independent
from the deprecated field.
* doc/guix.texi (Networking Setup): Remove mention of iwd? field.
---
 doc/guix.texi               |  4 ----
 gnu/services/networking.scm | 20 ++++++++++++++++++--
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 4599c87a72..00313c54e6 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19890,10 +19890,6 @@ Networking Setup
 (VPNs).  An example of this is the @code{network-manager-openvpn}
 package, which allows NetworkManager to manage VPNs @i{via} OpenVPN.
 
-@item @code{iwd?} (default: @code{#f})
-NetworkManager will use iwd as a backend for wireless networking if this
-option is set to @code{#t}, otherwise it will use wpa-supplicant.
-
 @end table
 @end deftp
 
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 5284855b83..ddf2e20791 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -1136,6 +1136,15 @@ (define-record-type* <modem-manager-configuration>
 ;;; NetworkManager
 ;;;
 
+;; TODO: deprecated field, remove later.
+(define-with-syntax-properties (warn-iwd?-field-deprecation
+                                (value properties))
+  (when value
+    (warning (source-properties->location properties)
+             (G_ "the 'iwd?' field is deprecated, please use \
+'shepherd-requirement' field instead~%")))
+  value)
+
 (define-record-type* <network-manager-configuration>
   network-manager-configuration make-network-manager-configuration
   network-manager-configuration?
@@ -1147,7 +1156,9 @@ (define-record-type* <network-manager-configuration>
        (default "default"))
   (vpn-plugins network-manager-configuration-vpn-plugins ;list of file-like
                (default '()))
-  (iwd? network-manager-configuration-iwd? (default #f)))
+  (iwd? network-manager-configuration-iwd?  ; TODO: deprecated field, remove.
+        (default #f)
+        (sanitize warn-iwd?-field-deprecation)))
 
 (define (network-manager-activation config)
   ;; Activation gexp for NetworkManager
@@ -1204,7 +1215,10 @@ (define (network-manager-environment config)
 (define (network-manager-shepherd-service config)
   (match-record config <network-manager-configuration>
     (network-manager shepherd-requirement dns vpn-plugins iwd?)
-    (let ((conf (plain-file "NetworkManager.conf"
+    (let ((iwd? (or iwd?  ; TODO: deprecated field, remove later.
+                    (and shepherd-requirement
+                         (memq 'iwd shepherd-requirement))))
+          (conf (plain-file "NetworkManager.conf"
                             (string-append
                              "[main]\ndns=" dns "\n"
                              (if iwd? "[device]\nwifi.backend=iwd\n" ""))))
@@ -1214,6 +1228,8 @@ (define (network-manager-shepherd-service config)
              (provision '(networking))
              (requirement `(user-processes dbus-system loopback
                             ,@shepherd-requirement
+                            ;; TODO: iwd? is deprecated and should be passed
+                            ;; with shepherd-requirement, remove later.
                             ,@(if iwd? '(iwd) '())))
              (start #~(make-forkexec-constructor
                        (list (string-append #$network-manager
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Fri, 03 Mar 2023 23:00:06 GMT) Full text and rfc822 format available.

Message #62 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH v2 7/8] services: network-manager: Set service canonical-name
 to NetworkManager.
Date: Fri,  3 Mar 2023 22:59:12 +0000

According to the semantics in [1], 'networking should be a "virtual service"
and NetworkManager its canonical-name. This does not influence
existing services and they should continue to use the 'networking symbol.

One visible change is that 'herd status' doesn't show 'networking' anymore,
instead listing 'NetworkManager' in its place but both symbols are can be used
to start and stop the same service.

Note: Though the symbol NetworkManager doesn't really conform with the overall kebab-case
used throughout Guix, this is intentional as we really want to make it clear that
that the symbol NetworkManager really refers to the software called NetworkManager,
since it's a canonical name here. (rather than risk misleading the user to interpret
the symbol network-manager as a symbol for some unspecific network management software)


[1]: https://www.gnu.org/software/shepherd/manual/html_node/Jump-Start.html

* gnu/services/networking.scm (network-manager-shepherd-service): Make 'networking a
virtual service and set 'NetworkManager as its canonical name.
---
 gnu/services/networking.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 76674f346a..66fd9121e0 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -1229,7 +1229,7 @@ (define (network-manager-shepherd-service config)
           (vpn  (vpn-plugin-directory vpn-plugins)))
       (list (shepherd-service
              (documentation "Run the NetworkManager.")
-             (provision '(networking))
+             (provision '(NetworkManager networking))
              (requirement `(user-processes dbus-system loopback
                             ,@shepherd-requirement
                             ;; TODO: iwd? is deprecated and should be passed
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Fri, 03 Mar 2023 23:00:06 GMT) Full text and rfc822 format available.

Message #65 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH v2 3/8] services: connman: Use match-record and export
 accessors.
Date: Fri,  3 Mar 2023 22:59:08 +0000

* gnu/services/networking.scm (connman-shepherd-service): Use match-record.
(connman-configuration-connman, connman-configuration-disable-vpn?)
(connman-configuration-iwd?): Export accessors.
---
 gnu/services/networking.scm | 52 ++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 27 deletions(-)

diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index ddf2e20791..5cba3a9a3f 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -150,6 +150,9 @@ (define-module (gnu services networking)
 
             connman-configuration
             connman-configuration?
+            connman-configuration-connman
+            connman-configuration-disable-vpn?
+            connman-configuration-iwd?
             connman-service-type
 
             modem-manager-configuration
@@ -1300,33 +1303,28 @@ (define (connman-activation config)
             (mkdir-p "/var/lib/connman-vpn/"))))))
 
 (define (connman-shepherd-service config)
-  "Return a shepherd service for Connman"
-  (and
-   (connman-configuration? config)
-   (let ((connman      (connman-configuration-connman config))
-         (disable-vpn? (connman-configuration-disable-vpn? config))
-         (iwd?         (connman-configuration-iwd? config)))
-     (list (shepherd-service
-            (documentation "Run Connman")
-            (provision '(networking))
-            (requirement
-             (append '(user-processes dbus-system loopback)
-                     (if iwd? '(iwd) '())))
-            (start #~(make-forkexec-constructor
-                      (list (string-append #$connman
-                                           "/sbin/connmand")
-                            "--nodaemon"
-                            "--nodnsproxy"
-                            #$@(if disable-vpn? '("--noplugin=vpn") '())
-                            #$@(if iwd? '("--wifi=iwd_agent") '()))
-
-                      ;; As connman(8) notes, when passing '-n', connman
-                      ;; "directs log output to the controlling terminal in
-                      ;; addition to syslog."  Redirect stdout and stderr
-                      ;; to avoid spamming the console (XXX: for some reason
-                      ;; redirecting to /dev/null doesn't work.)
-                      #:log-file "/var/log/connman.log"))
-            (stop #~(make-kill-destructor)))))))
+  (match-record config <connman-configuration> (connman disable-vpn? iwd?)
+    (list (shepherd-service
+           (documentation "Run Connman")
+           (provision '(networking))
+           (requirement
+            (append '(user-processes dbus-system loopback)
+                    (if iwd? '(iwd) '())))
+           (start #~(make-forkexec-constructor
+                     (list (string-append #$connman
+                                          "/sbin/connmand")
+                           "--nodaemon"
+                           "--nodnsproxy"
+                           #$@(if disable-vpn? '("--noplugin=vpn") '())
+                           #$@(if iwd? '("--wifi=iwd_agent") '()))
+
+                     ;; As connman(8) notes, when passing '-n', connman
+                     ;; "directs log output to the controlling terminal in
+                     ;; addition to syslog."  Redirect stdout and stderr
+                     ;; to avoid spamming the console (XXX: for some reason
+                     ;; redirecting to /dev/null doesn't work.)
+                     #:log-file "/var/log/connman.log"))
+           (stop #~(make-kill-destructor))))))
 
 (define %connman-log-rotation
   (list (log-rotation
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Mon, 06 Mar 2023 15:21:01 GMT) Full text and rfc822 format available.

Message #68 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Bruno Victal <mirai <at> makinata.eu>
Cc: 61587 <at> debbugs.gnu.org
Subject: Re: bug#61587: [PATCH 0/8] networking services refactoring
Date: Mon, 06 Mar 2023 16:20:19 +0100

Bruno Victal <mirai <at> makinata.eu> skribis:

> On 2023-03-03 17:15, Ludovic Courtès wrote:
>> Bruno Victal <mirai <at> makinata.eu> skribis:
>> 
>>>        (list (shepherd-service
>>>               (documentation "Run the NetworkManager.")
>>> -             (provision '(networking))
>>> +             (provision '(NetworkManager networking))
>> 
>> We could discuss this at length, but I’m in favor of either the status
>> quo, or:
>> 
>>   (provision '(networking network-manager))
>> 
>> WDYT?
>
> The ordering matters here, putting 'networking first means that 'network-manager is the virtual service and that something like
> connman can provide 'network-manager which is absurd.

Hmm OK, I don’t mind the ordering.

In the Shepherd there’s really the “canonical name” and “other names”;
that’s all ordering changes.

Ludo’.
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Tue, 07 Mar 2023 10:02:02 GMT) Full text and rfc822 format available.

Message #71 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Bruno Victal <mirai <at> makinata.eu>
Cc: 61587 <at> debbugs.gnu.org
Subject: Re: bug#61587: [PATCH 0/8] networking services refactoring
Date: Tue, 07 Mar 2023 11:01:30 +0100

Hi,

Bruno Victal <mirai <at> makinata.eu> skribis:

> Note: This also makes wpa-supplicant an optional requirement.
>
> * gnu/services/networking.scm (<network-manager-configuration>)
> [shepherd-requirement]: New field.
> (network-manager-shepherd-service): Honor it.
> (network-manager-configuration-shepherd-requirement): Export accessor.
> * doc/guix.texi (Networking Setup): Document it.

[...]

> +@item @code{shepherd-requirement} (default: @code{()})
> +This option can be used to provide a list of symbols naming Shepherd services
> +that this service will depend on, such as @code{'wpa-supplicant} or
> +@code{'iwd} if you require authenticated access for encrypted WiFi or Ethernet
> +networks.

For clarity, I’d write the list: @code{'(wpa-supplicant)}.

> +  (shepherd-requirement network-manager-configuration-shepherd-requirement
> +                        (default '()))

[...]

> -             (requirement (append '(user-processes dbus-system loopback)
> -                                  (if iwd? '(iwd) '(wpa-supplicant))))
> +             (requirement `(user-processes dbus-system loopback
> +                            ,@shepherd-requirement
> +                            ,@(if iwd? '(iwd) '())))

To preserve backward compatibility and to provide a reasonable default
(with working WiFi), I think the default for ‘shepherd-requirement’
should be '(wpa-supplicant) rather than the empty list.

(BTW, it seems that wpa-supplicant can be DBus-activated, maybe that’s
what we should do instead?  But let’s forget about it for this patch
series.)

Thanks,
Ludo’.
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Tue, 07 Mar 2023 10:05:02 GMT) Full text and rfc822 format available.

Message #74 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Bruno Victal <mirai <at> makinata.eu>
Cc: 61587 <at> debbugs.gnu.org
Subject: Re: bug#61587: [PATCH 0/8] networking services refactoring
Date: Tue, 07 Mar 2023 11:04:32 +0100

Hi,

Ludovic Courtès <ludo <at> gnu.org> skribis:

> Bruno Victal <mirai <at> makinata.eu> skribis:
>
>> Note: This also makes wpa-supplicant an optional requirement.
>>
>> * gnu/services/networking.scm (<network-manager-configuration>)
>> [shepherd-requirement]: New field.
>> (network-manager-shepherd-service): Honor it.
>> (network-manager-configuration-shepherd-requirement): Export accessor.
>> * doc/guix.texi (Networking Setup): Document it.
>
> [...]
>
>> +@item @code{shepherd-requirement} (default: @code{()})
>> +This option can be used to provide a list of symbols naming Shepherd services
>> +that this service will depend on, such as @code{'wpa-supplicant} or
>> +@code{'iwd} if you require authenticated access for encrypted WiFi or Ethernet
>> +networks.
>
> For clarity, I’d write the list: @code{'(wpa-supplicant)}.
>
>> +  (shepherd-requirement network-manager-configuration-shepherd-requirement
>> +                        (default '()))
>
> [...]
>
>> -             (requirement (append '(user-processes dbus-system loopback)
>> -                                  (if iwd? '(iwd) '(wpa-supplicant))))
>> +             (requirement `(user-processes dbus-system loopback
>> +                            ,@shepherd-requirement
>> +                            ,@(if iwd? '(iwd) '())))
>
> To preserve backward compatibility and to provide a reasonable default
> (with working WiFi), I think the default for ‘shepherd-requirement’
> should be '(wpa-supplicant) rather than the empty list.

The rest of the patch series (v2) LGTM.

You can send an updated version of the patch above or I can fix it up on
your behalf.

Thanks!

Ludo’.
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Tue, 07 Mar 2023 12:45:02 GMT) Full text and rfc822 format available.

Message #77 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org, Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH v3 1/8] services: network-manager: Add 'shepherd-requirement'
 field.
Date: Tue,  7 Mar 2023 12:43:58 +0000

Note: This also makes wpa-supplicant an optional requirement.

* gnu/services/networking.scm (<network-manager-configuration>)
[shepherd-requirement]: New field.
(network-manager-shepherd-service): Honor it.
(network-manager-configuration-shepherd-requirement): Export accessor.
* doc/guix.texi (Networking Setup): Document it.
---
 doc/guix.texi               |  6 ++++++
 gnu/services/networking.scm | 10 +++++++---
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 6671ba9305..ed518cbcb8 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19836,6 +19836,12 @@ Networking Setup
 @item @code{network-manager} (default: @code{network-manager})
 The NetworkManager package to use.
 
+@item @code{shepherd-requirement} (default: @code{'(wpa-supplicant)})
+This option can be used to provide a list of symbols naming Shepherd services
+that this service will depend on, such as @code{'wpa-supplicant} or
+@code{'iwd} if you require authenticated access for encrypted WiFi or Ethernet
+networks.
+
 @item @code{dns} (default: @code{"default"})
 Processing mode for DNS, which affects how NetworkManager uses the
 @code{resolv.conf} configuration file.
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index dacf64c2d1..4a3d5b887f 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -143,6 +143,7 @@ (define-module (gnu services networking)
 
             network-manager-configuration
             network-manager-configuration?
+            network-manager-configuration-shepherd-requirement
             network-manager-configuration-dns
             network-manager-configuration-vpn-plugins
             network-manager-service-type
@@ -1140,6 +1141,8 @@ (define-record-type* <network-manager-configuration>
   network-manager-configuration?
   (network-manager network-manager-configuration-network-manager
                    (default network-manager))
+  (shepherd-requirement network-manager-configuration-shepherd-requirement
+                        (default '(wpa-supplicant)))
   (dns network-manager-configuration-dns
        (default "default"))
   (vpn-plugins network-manager-configuration-vpn-plugins ;list of file-like
@@ -1200,7 +1203,7 @@ (define (network-manager-environment config)
 
 (define (network-manager-shepherd-service config)
   (match-record config <network-manager-configuration>
-    (network-manager dns vpn-plugins iwd?)
+    (network-manager shepherd-requirement dns vpn-plugins iwd?)
     (let ((conf (plain-file "NetworkManager.conf"
                             (string-append
                              "[main]\ndns=" dns "\n"
@@ -1209,8 +1212,9 @@ (define (network-manager-shepherd-service config)
       (list (shepherd-service
              (documentation "Run the NetworkManager.")
              (provision '(networking))
-             (requirement (append '(user-processes dbus-system loopback)
-                                  (if iwd? '(iwd) '(wpa-supplicant))))
+             (requirement `(user-processes dbus-system loopback
+                            ,@shepherd-requirement
+                            ,@(if iwd? '(iwd) '())))
              (start #~(make-forkexec-constructor
                        (list (string-append #$network-manager
                                             "/sbin/NetworkManager")
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Tue, 07 Mar 2023 12:45:02 GMT) Full text and rfc822 format available.

Message #80 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org, Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH v3 2/8] services: network-manager: Deprecate 'iwd?' field.
Date: Tue,  7 Mar 2023 12:43:59 +0000

* gnu/services/networking.scm (warn-iwd?-field-deprecation): New procedure,
helper for deprecated field.
(<network-manager-configuration>)[iwd?]: Use helper to warn deprecated field.
(network-manager-shepherd-service): Make iwd? a local variable independent
from the deprecated field.
* doc/guix.texi (Networking Setup): Remove mention of iwd? field.
---
 doc/guix.texi               |  4 ----
 gnu/services/networking.scm | 20 ++++++++++++++++++--
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index ed518cbcb8..ccaca0a71b 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19890,10 +19890,6 @@ Networking Setup
 (VPNs).  An example of this is the @code{network-manager-openvpn}
 package, which allows NetworkManager to manage VPNs @i{via} OpenVPN.
 
-@item @code{iwd?} (default: @code{#f})
-NetworkManager will use iwd as a backend for wireless networking if this
-option is set to @code{#t}, otherwise it will use wpa-supplicant.
-
 @end table
 @end deftp
 
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 4a3d5b887f..f572de1279 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -1136,6 +1136,15 @@ (define-record-type* <modem-manager-configuration>
 ;;; NetworkManager
 ;;;
 
+;; TODO: deprecated field, remove later.
+(define-with-syntax-properties (warn-iwd?-field-deprecation
+                                (value properties))
+  (when value
+    (warning (source-properties->location properties)
+             (G_ "the 'iwd?' field is deprecated, please use \
+'shepherd-requirement' field instead~%")))
+  value)
+
 (define-record-type* <network-manager-configuration>
   network-manager-configuration make-network-manager-configuration
   network-manager-configuration?
@@ -1147,7 +1156,9 @@ (define-record-type* <network-manager-configuration>
        (default "default"))
   (vpn-plugins network-manager-configuration-vpn-plugins ;list of file-like
                (default '()))
-  (iwd? network-manager-configuration-iwd? (default #f)))
+  (iwd? network-manager-configuration-iwd?  ; TODO: deprecated field, remove.
+        (default #f)
+        (sanitize warn-iwd?-field-deprecation)))
 
 (define (network-manager-activation config)
   ;; Activation gexp for NetworkManager
@@ -1204,7 +1215,10 @@ (define (network-manager-environment config)
 (define (network-manager-shepherd-service config)
   (match-record config <network-manager-configuration>
     (network-manager shepherd-requirement dns vpn-plugins iwd?)
-    (let ((conf (plain-file "NetworkManager.conf"
+    (let ((iwd? (or iwd?  ; TODO: deprecated field, remove later.
+                    (and shepherd-requirement
+                         (memq 'iwd shepherd-requirement))))
+          (conf (plain-file "NetworkManager.conf"
                             (string-append
                              "[main]\ndns=" dns "\n"
                              (if iwd? "[device]\nwifi.backend=iwd\n" ""))))
@@ -1214,6 +1228,8 @@ (define (network-manager-shepherd-service config)
              (provision '(networking))
              (requirement `(user-processes dbus-system loopback
                             ,@shepherd-requirement
+                            ;; TODO: iwd? is deprecated and should be passed
+                            ;; with shepherd-requirement, remove later.
                             ,@(if iwd? '(iwd) '())))
              (start #~(make-forkexec-constructor
                        (list (string-append #$network-manager
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Tue, 07 Mar 2023 12:45:03 GMT) Full text and rfc822 format available.

Message #83 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org, Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH v3 3/8] services: connman: Use match-record and export
 accessors.
Date: Tue,  7 Mar 2023 12:44:00 +0000

* gnu/services/networking.scm (connman-shepherd-service): Use match-record.
(connman-configuration-connman, connman-configuration-disable-vpn?)
(connman-configuration-iwd?): Export accessors.
---
 gnu/services/networking.scm | 52 ++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 27 deletions(-)

diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index f572de1279..abfaba8004 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -150,6 +150,9 @@ (define-module (gnu services networking)
 
             connman-configuration
             connman-configuration?
+            connman-configuration-connman
+            connman-configuration-disable-vpn?
+            connman-configuration-iwd?
             connman-service-type
 
             modem-manager-configuration
@@ -1300,33 +1303,28 @@ (define (connman-activation config)
             (mkdir-p "/var/lib/connman-vpn/"))))))
 
 (define (connman-shepherd-service config)
-  "Return a shepherd service for Connman"
-  (and
-   (connman-configuration? config)
-   (let ((connman      (connman-configuration-connman config))
-         (disable-vpn? (connman-configuration-disable-vpn? config))
-         (iwd?         (connman-configuration-iwd? config)))
-     (list (shepherd-service
-            (documentation "Run Connman")
-            (provision '(networking))
-            (requirement
-             (append '(user-processes dbus-system loopback)
-                     (if iwd? '(iwd) '())))
-            (start #~(make-forkexec-constructor
-                      (list (string-append #$connman
-                                           "/sbin/connmand")
-                            "--nodaemon"
-                            "--nodnsproxy"
-                            #$@(if disable-vpn? '("--noplugin=vpn") '())
-                            #$@(if iwd? '("--wifi=iwd_agent") '()))
-
-                      ;; As connman(8) notes, when passing '-n', connman
-                      ;; "directs log output to the controlling terminal in
-                      ;; addition to syslog."  Redirect stdout and stderr
-                      ;; to avoid spamming the console (XXX: for some reason
-                      ;; redirecting to /dev/null doesn't work.)
-                      #:log-file "/var/log/connman.log"))
-            (stop #~(make-kill-destructor)))))))
+  (match-record config <connman-configuration> (connman disable-vpn? iwd?)
+    (list (shepherd-service
+           (documentation "Run Connman")
+           (provision '(networking))
+           (requirement
+            (append '(user-processes dbus-system loopback)
+                    (if iwd? '(iwd) '())))
+           (start #~(make-forkexec-constructor
+                     (list (string-append #$connman
+                                          "/sbin/connmand")
+                           "--nodaemon"
+                           "--nodnsproxy"
+                           #$@(if disable-vpn? '("--noplugin=vpn") '())
+                           #$@(if iwd? '("--wifi=iwd_agent") '()))
+
+                     ;; As connman(8) notes, when passing '-n', connman
+                     ;; "directs log output to the controlling terminal in
+                     ;; addition to syslog."  Redirect stdout and stderr
+                     ;; to avoid spamming the console (XXX: for some reason
+                     ;; redirecting to /dev/null doesn't work.)
+                     #:log-file "/var/log/connman.log"))
+           (stop #~(make-kill-destructor))))))
 
 (define %connman-log-rotation
   (list (log-rotation
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Tue, 07 Mar 2023 12:45:03 GMT) Full text and rfc822 format available.

Message #86 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org, Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH v3 4/8] services: connman: Add 'shepherd-requirement' field.
Date: Tue,  7 Mar 2023 12:44:01 +0000

* gnu/services/networking.scm (<connman-configuration>)
[shepherd-requirement]: New field.
(connman-shepherd-service): Honor it.
(connman-configuration-shepherd-requirement): Export accessor.
* doc/guix.texi (Networking Setup): Document it.
---
 doc/guix.texi               |  6 ++++++
 gnu/services/networking.scm | 12 ++++++++----
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index ccaca0a71b..ece0c0354d 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19917,6 +19917,12 @@ Networking Setup
 @item @code{connman} (default: @var{connman})
 The connman package to use.
 
+@item @code{shepherd-requirement} (default: @code{()})
+This option can be used to provide a list of symbols naming Shepherd services
+that this service will depend on, such as @code{'wpa-supplicant} or
+@code{'iwd} if you require authenticated access for encrypted WiFi or Ethernet
+networks.
+
 @item @code{disable-vpn?} (default: @code{#f})
 When true, disable connman's vpn plugin.
 
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index abfaba8004..6a09f6e728 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -151,6 +151,7 @@ (define-module (gnu services networking)
             connman-configuration
             connman-configuration?
             connman-configuration-connman
+            connman-configuration-shepherd-requirement
             connman-configuration-disable-vpn?
             connman-configuration-iwd?
             connman-service-type
@@ -1288,6 +1289,8 @@ (define-record-type* <connman-configuration>
   connman-configuration?
   (connman      connman-configuration-connman
                 (default connman))
+  (shepherd-requirement connman-configuration-shepherd-requirement
+                        (default '()))
   (disable-vpn? connman-configuration-disable-vpn?
                 (default #f))
   (iwd?         connman-configuration-iwd?
@@ -1303,13 +1306,14 @@ (define (connman-activation config)
             (mkdir-p "/var/lib/connman-vpn/"))))))
 
 (define (connman-shepherd-service config)
-  (match-record config <connman-configuration> (connman disable-vpn? iwd?)
+  (match-record config <connman-configuration> (connman shepherd-requirement
+                                                disable-vpn? iwd?)
     (list (shepherd-service
            (documentation "Run Connman")
            (provision '(networking))
-           (requirement
-            (append '(user-processes dbus-system loopback)
-                    (if iwd? '(iwd) '())))
+           (requirement `(user-processes dbus-system loopback
+                          ,@shepherd-requirement
+                          ,@(if iwd? '(iwd) '())))
            (start #~(make-forkexec-constructor
                      (list (string-append #$connman
                                           "/sbin/connmand")
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Tue, 07 Mar 2023 12:45:04 GMT) Full text and rfc822 format available.

Message #89 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org, Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH v3 7/8] services: network-manager: Set service canonical-name
 to NetworkManager.
Date: Tue,  7 Mar 2023 12:44:04 +0000

According to the semantics in [1], 'networking should be a "virtual service"
and NetworkManager its canonical-name. This does not influence
existing services and they should continue to use the 'networking symbol.

One visible change is that 'herd status' doesn't show 'networking' anymore,
instead listing 'NetworkManager' in its place but both symbols are can be used
to start and stop the same service.

Note: Though the symbol NetworkManager doesn't really conform with the overall kebab-case
used throughout Guix, this is intentional as we really want to make it clear that
that the symbol NetworkManager really refers to the software called NetworkManager,
since it's a canonical name here. (rather than risk misleading the user to interpret
the symbol network-manager as a symbol for some unspecific network management software)


[1]: https://www.gnu.org/software/shepherd/manual/html_node/Jump-Start.html

* gnu/services/networking.scm (network-manager-shepherd-service): Make 'networking a
virtual service and set 'NetworkManager as its canonical name.
---
 gnu/services/networking.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 7cf92dc8c4..5899e0977b 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -1229,7 +1229,7 @@ (define (network-manager-shepherd-service config)
           (vpn  (vpn-plugin-directory vpn-plugins)))
       (list (shepherd-service
              (documentation "Run the NetworkManager.")
-             (provision '(networking))
+             (provision '(NetworkManager networking))
              (requirement `(user-processes dbus-system loopback
                             ,@shepherd-requirement
                             ;; TODO: iwd? is deprecated and should be passed
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Tue, 07 Mar 2023 12:45:04 GMT) Full text and rfc822 format available.

Message #92 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org, Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH v3 6/8] services: network-manager: Await for NetworkManager to
 finish starting up.
Date: Tue,  7 Mar 2023 12:44:03 +0000

This is similar to its NetworkManager-wait-online.service systemd counterpart,
with the main difference being that we handle it all in 'networking symbol, rather than
introduce a new 'networking-online symbol. (see discussion #47253)

As a result of this change, with opensmtpd-service-type as an example,
manual 'herd restart smtpd' after system bootups are no longer required
when opensmtpd is configured with a smtpd.conf containing non-loopback interfaces.
(this issue is described in more detail at #60300)

Fixes <https://issues.guix.gnu.org/60300>.

* gnu/services/networking.scm (network-manager-shepherd-service): Await for
NetworkManager to finish starting up.
---
 gnu/services/networking.scm | 35 ++++++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 0ed467f9d8..7cf92dc8c4 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -1235,17 +1235,30 @@ (define (network-manager-shepherd-service config)
                             ;; TODO: iwd? is deprecated and should be passed
                             ;; with shepherd-requirement, remove later.
                             ,@(if iwd? '(iwd) '())))
-             (start #~(make-forkexec-constructor
-                       (list (string-append #$network-manager
-                                            "/sbin/NetworkManager")
-                             (string-append "--config=" #$conf)
-                             "--no-daemon")
-                       #:environment-variables
-                       (list (string-append "NM_VPN_PLUGIN_DIR=" #$vpn
-                                            "/lib/NetworkManager/VPN")
-                             ;; Override non-existent default users
-                             "NM_OPENVPN_USER="
-                             "NM_OPENVPN_GROUP=")))
+             (start
+              #~(lambda _
+                  (let ((pid
+                         (fork+exec-command
+                          (list #$(file-append network-manager
+                                               "/sbin/NetworkManager")
+                                (string-append "--config=" #$conf)
+                                "--no-daemon")
+                          #:environment-variables
+                          (list (string-append "NM_VPN_PLUGIN_DIR=" #$vpn
+                                               "/lib/NetworkManager/VPN")
+                                ;; Override non-existent default users
+                                "NM_OPENVPN_USER="
+                                "NM_OPENVPN_GROUP="))))
+                    ;; XXX: Despite the "online" name, this doesn't guarantee
+                    ;; WAN connectivity, it merely waits for NetworkManager
+                    ;; to finish starting-up. This is required otherwise
+                    ;; services will fail since the network interfaces be
+                    ;; absent until NetworkManager finishes setting them up.
+                    (system* #$(file-append network-manager "/bin/nm-online")
+                             "--wait-for-startup" "--quiet")
+                    ;; XXX: Finally, return the pid from running
+                    ;; fork+exec-command to shepherd.
+                    pid)))
              (stop #~(make-kill-destructor)))))))
 
 (define network-manager-service-type
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Tue, 07 Mar 2023 12:50:02 GMT) Full text and rfc822 format available.

Message #95 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org, Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH v3 5/8] services: connman: Deprecate 'iwd?' field.
Date: Tue,  7 Mar 2023 12:44:02 +0000

* gnu/services/networking.scm (<connman-configuration>)
[iwd?]: Use helper to warn deprecated field.
(connman-shepherd-service): Make iwd? a local variable independent from
the deprecated field.
* doc/guix.texi (Networking Setup): Remove mention of iwd? field.
---
 doc/guix.texi               |  2 --
 gnu/services/networking.scm | 50 +++++++++++++++++++++----------------
 2 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index ece0c0354d..b098e45a50 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19926,8 +19926,6 @@ Networking Setup
 @item @code{disable-vpn?} (default: @code{#f})
 When true, disable connman's vpn plugin.
 
-@item @code{iwd?} (default: @code{#f})
-When true, ConnMan uses iwd to connect to wireless networks.
 @end table
 @end deftp
 
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 6a09f6e728..0ed467f9d8 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -1294,7 +1294,8 @@ (define-record-type* <connman-configuration>
   (disable-vpn? connman-configuration-disable-vpn?
                 (default #f))
   (iwd?         connman-configuration-iwd?
-                (default #f)))
+                (default #f)
+                (sanitize warn-iwd?-field-deprecation)))
 
 (define (connman-activation config)
   (let ((disable-vpn? (connman-configuration-disable-vpn? config)))
@@ -1308,27 +1309,32 @@ (define (connman-activation config)
 (define (connman-shepherd-service config)
   (match-record config <connman-configuration> (connman shepherd-requirement
                                                 disable-vpn? iwd?)
-    (list (shepherd-service
-           (documentation "Run Connman")
-           (provision '(networking))
-           (requirement `(user-processes dbus-system loopback
-                          ,@shepherd-requirement
-                          ,@(if iwd? '(iwd) '())))
-           (start #~(make-forkexec-constructor
-                     (list (string-append #$connman
-                                          "/sbin/connmand")
-                           "--nodaemon"
-                           "--nodnsproxy"
-                           #$@(if disable-vpn? '("--noplugin=vpn") '())
-                           #$@(if iwd? '("--wifi=iwd_agent") '()))
-
-                     ;; As connman(8) notes, when passing '-n', connman
-                     ;; "directs log output to the controlling terminal in
-                     ;; addition to syslog."  Redirect stdout and stderr
-                     ;; to avoid spamming the console (XXX: for some reason
-                     ;; redirecting to /dev/null doesn't work.)
-                     #:log-file "/var/log/connman.log"))
-           (stop #~(make-kill-destructor))))))
+    (let ((iwd? (or iwd?  ; TODO: deprecated field, remove later.
+                    (and shepherd-requirement
+                         (memq 'iwd shepherd-requirement)))))
+      (list (shepherd-service
+             (documentation "Run Connman")
+             (provision '(networking))
+             (requirement `(user-processes dbus-system loopback
+                                           ,@shepherd-requirement
+                                           ;; TODO: iwd? is deprecated and should be passed
+                                           ;; with shepherd-requirement, remove later.
+                                           ,@(if iwd? '(iwd) '())))
+             (start #~(make-forkexec-constructor
+                       (list (string-append #$connman
+                                            "/sbin/connmand")
+                             "--nodaemon"
+                             "--nodnsproxy"
+                             #$@(if disable-vpn? '("--noplugin=vpn") '())
+                             #$@(if iwd? '("--wifi=iwd_agent") '()))
+
+                       ;; As connman(8) notes, when passing '-n', connman
+                       ;; "directs log output to the controlling terminal in
+                       ;; addition to syslog."  Redirect stdout and stderr
+                       ;; to avoid spamming the console (XXX: for some reason
+                       ;; redirecting to /dev/null doesn't work.)
+                       #:log-file "/var/log/connman.log"))
+             (stop #~(make-kill-destructor)))))))
 
 (define %connman-log-rotation
   (list (log-rotation
-- 
2.39.1
Information forwarded to guix-patches <at> gnu.org:
bug#61587; Package guix-patches. (Tue, 07 Mar 2023 13:00:02 GMT) Full text and rfc822 format available.

Message #98 received at 61587 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 61587 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org, Bruno Victal <mirai <at> makinata.eu>
Subject: [PATCH v3 8/8] services: connman: Set service canonical-name to
 connman.
Date: Tue,  7 Mar 2023 12:44:05 +0000

* gnu/services/networking.scm (connman-shepherd-service): Make 'networking a
virtual service and set 'connman as its canonical name.
---
 gnu/services/networking.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 5899e0977b..5c6ad4ce3b 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -1327,7 +1327,7 @@ (define (connman-shepherd-service config)
                          (memq 'iwd shepherd-requirement)))))
       (list (shepherd-service
              (documentation "Run Connman")
-             (provision '(networking))
+             (provision '(connman networking))
              (requirement `(user-processes dbus-system loopback
                                            ,@shepherd-requirement
                                            ;; TODO: iwd? is deprecated and should be passed
-- 
2.39.1
Reply sent to Ludovic Courtès <ludo <at> gnu.org>:
You have taken responsibility. (Fri, 10 Mar 2023 13:22:01 GMT) Full text and rfc822 format available.
Notification sent to Bruno Victal <mirai <at> makinata.eu>:
bug acknowledged by developer. (Fri, 10 Mar 2023 13:22:02 GMT) Full text and rfc822 format available.

Message #103 received at 61587-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Bruno Victal <mirai <at> makinata.eu>
Cc: 61587-done <at> debbugs.gnu.org
Subject: Re: [PATCH v3 1/8] services: network-manager: Add
 'shepherd-requirement' field.
Date: Fri, 10 Mar 2023 14:21:37 +0100

Hi Bruno,

Applied v3, thank you!

Ludo’.
bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sat, 08 Apr 2023 11:24:10 GMT) Full text and rfc822 format available.
This bug report was last modified 1 year and 12 days ago.

Previous Next

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