[bug#77204,1/3] services: dnsmasq: Provide shepherd-provision and shepherd-requirement fields.

Message ID 242697cdc4ed0e257df4ba35500ae2a3b3a00a0b.1742725327.git.levenson@mmer.org
State New
Headers
Series dnsmasq service changes |

Commit Message

Alexey Abramov March 23, 2025, 10:27 a.m. UTC
  * doc/guix.texi: Document the change.
* gnu/services/dns.scm (<dnsmasq-configuration>)[provision]: Rename the field to
[shepherd-provision] for consistency with other services.
* gnu/services/dns.scm (<dnsmasq-configuration>)[shepherd-requirement]: New field.
* gnu/services/dns.scm (dnsmasq-shepherd-service): Use newly-created fields.
---
 doc/guix.texi        | 11 ++++++++---
 gnu/services/dns.scm | 12 ++++++++----
 2 files changed, 16 insertions(+), 7 deletions(-)
  

Comments

45mg March 23, 2025, 10:50 a.m. UTC | #1
Hi,

Alexey Abramov via Guix-patches via <guix-patches@gnu.org> writes:

> * gnu/services/dns.scm (<dnsmasq-configuration>)[provision]: Rename the field to
> [shepherd-provision] for consistency with other services.

If this is accepted, you'll probably need to add an entry to
etc/news.scm, since it's a breaking change.

You'll also need to update the Guix Cookbook. Specifically, the recent
commit da090138028894c6b00c21730aa3a02cda57fb24 uses the existing
'provision' field of `dnsmasq-service-type` in an example.

With that said, I think the rename is a sensible change. (Just in case
it matters, I was the one who introduced the 'provision' field, as well
as that Cookbook example).
  
Alexey Abramov March 23, 2025, 12:28 p.m. UTC | #2
Hi, 
45mg <45mg.writes@gmail.com> writes:

> Hi,
>
> Alexey Abramov via Guix-patches via <guix-patches@gnu.org> writes:
>
>> * gnu/services/dns.scm (<dnsmasq-configuration>)[provision]: Rename the field to
>> [shepherd-provision] for consistency with other services.
>
> If this is accepted, you'll probably need to add an entry to
> etc/news.scm, since it's a breaking change.
>
> You'll also need to update the Guix Cookbook. Specifically, the recent
> commit da090138028894c6b00c21730aa3a02cda57fb24 uses the existing
> 'provision' field of `dnsmasq-service-type` in an example.

Thanks for letting me know.

> With that said, I think the rename is a sensible change. (Just in case
> it matters, I was the one who introduced the 'provision' field, as well
> as that Cookbook example).

I see. I guess there are not that many users who overrides provision for
dnsmasq at the moment. AFAIK we do want shepherd-provision and
shepherd-requirement insead of just provision/requirement. I have this
script (see attachements)

and run it withing the repo as

./pre-inst-env guix repl guix-services-info.scm

--8<---------------cut here---------------start------------->8---
(<log-rotation-configuration> (provision requirement))
(<mympd-configuration> (shepherd-requirement))
(<mpd-configuration> (shepherd-requirement))
(<restic-backup-job> (requirement))
(<agetty-configuration> (shepherd-requirement))
(<mingetty-configuration> (shepherd-requirement))
(<static-networking> (provision requirement))
(<dnsmasq-configuration> (provision))
(<oci-container-configuration> (provision requirement))
(<live-service> (provision requirement))
(<opensmtpd-configuration> (shepherd-requirement))
(<connman-configuration> (shepherd-requirement))
(<network-manager-configuration> (shepherd-requirement))
(<wpa-supplicant-configuration> (requirement))
(<dhcp-client-configuration> (shepherd-requirement shepherd-provision))
(<system-log-configuration> (provision requirement))
(<shepherd-service> (provision requirement))
(<shepherd-configuration> (shepherd))
(<wireguard-configuration> (shepherd-requirement))
(<nginx-configuration> (shepherd-requirement))
(<slim-configuration> (shepherd))
--8<---------------cut here---------------end--------------->8---

I will add the change for the cookbook no problem. Regarding the news, I
am not sure if I can do this, because it requires 'commit'.
  
Maxim Cournoyer March 23, 2025, 1:04 p.m. UTC | #3
Hi,

Alexey Abramov <levenson@mmer.org> writes:

> Hi, 
> 45mg <45mg.writes@gmail.com> writes:
>
>> Hi,
>>
>> Alexey Abramov via Guix-patches via <guix-patches@gnu.org> writes:
>>
>>> * gnu/services/dns.scm (<dnsmasq-configuration>)[provision]: Rename the field to
>>> [shepherd-provision] for consistency with other services.
>>
>> If this is accepted, you'll probably need to add an entry to
>> etc/news.scm, since it's a breaking change.

Instead of a news item, I'd add a deprecated alias for the old field, so
that it remains usable but warns.  There should be various examples in
the code.
  

Patch

diff --git a/doc/guix.texi b/doc/guix.texi
index eecc0aec52c..fd6a0176348 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -35172,9 +35172,14 @@  DNS Services
 @item @code{package} (default: @var{dnsmasq})
 Package object of the dnsmasq server.
 
-@item @code{provision} (default: @code{'(dnsmasq)})
-A list of symbols for the Shepherd service corresponding to this dnsmasq
-configuration.
+@item @code{shepherd-provision} (default: @code{'(dnsmasq)})
+@itemx @code{shepherd-requirement} (default: @code{'(user-processes networking)})
+This option can be used to provide a list of Shepherd service names
+(symbols) provided by this service. You might want to change the default
+value if you intend to run several @command{dnsmasq} instances.
+
+Likewise, @code{shepherd-requirement} is a list of Shepherd service names
+(symbols) that this service will depend on.
 
 @item @code{no-hosts?} (default: @code{#f})
 When true, don't read the hostnames in /etc/hosts.
diff --git a/gnu/services/dns.scm b/gnu/services/dns.scm
index 05291eb65d9..9276504ffd0 100644
--- a/gnu/services/dns.scm
+++ b/gnu/services/dns.scm
@@ -742,8 +742,10 @@  (define-record-type* <dnsmasq-configuration>
   dnsmasq-configuration?
   (package          dnsmasq-configuration-package
                     (default dnsmasq))  ;file-like
-  (provision        dnsmasq-provision
-                    (default '(dnsmasq)))
+  (shepherd-provision           dnsmasq-configuration-shepherd-provision
+                                (default '(dnsmasq)))
+  (shepherd-requirement         dnsmasq-configuration-shepherd-requirement
+                                (default '(user-processes networking)))
   (no-hosts?        dnsmasq-configuration-no-hosts?
                     (default #f))       ;boolean
   (port             dnsmasq-configuration-port
@@ -802,6 +804,8 @@  (define-record-type* <dnsmasq-configuration>
 (define (dnsmasq-shepherd-service config)
   (match-record config <dnsmasq-configuration>
     (package
+     shepherd-provision
+     shepherd-requirement
      no-hosts?
      port local-service? listen-addresses
      resolv-file no-resolv?
@@ -815,8 +819,8 @@  (define (dnsmasq-shepherd-service config)
      tftp-lowercase? tftp-port-range
      tftp-root tftp-unique-root extra-options)
     (shepherd-service
-     (provision (dnsmasq-provision config))
-     (requirement '(user-processes networking))
+     (provision shepherd-provision)
+     (requirement shepherd-requirement)
      (documentation "Run the dnsmasq DNS server.")
      (start #~(make-forkexec-constructor
                (list