| Message ID | c53676fec26ad69d8d57641b2d00c9521c0a8323.1677884352.git.mirai@makinata.eu |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#61587,v2,1/8] services: network-manager: Add 'shepherd-requirement' field. | expand |
Hi, Bruno Victal <mirai@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’.
Hi, Ludovic Courtès <ludo@gnu.org> skribis: > Bruno Victal <mirai@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’.
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")