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

* gnu/services/dns.scm (<dnsmasq-configuration>)[provision]: Mark as
  deprecated with a warning. Set default to #f.
* gnu/services/dns.scm (<dnsmasq-configuration>)[shepherd-provision]:
  Add new field for consistency with other services.
* gnu/services/dns.scm (<dnsmasq-configuration>)[shepherd-requirement]:
  Add new field.
* gnu/services/dns.scm (dnsmasq-shepherd-service): Use the new fields.
* doc/guix.texi: Document these changes.
* doc/guix-cookbook.texi (Custom NAT-based network for libvirt):
  Update example to use shepherd-provision instead of provision.
---
 doc/guix-cookbook.texi |  4 ++--
 doc/guix.texi          | 11 ++++++++---
 gnu/services/dns.scm   | 24 ++++++++++++++++++++----
 3 files changed, 30 insertions(+), 9 deletions(-)
  

Hi Alexey,

Alexey Abramov <levenson@mmer.org> writes:

> * gnu/services/dns.scm (<dnsmasq-configuration>)[provision]: Mark as
>   deprecated with a warning. Set default to #f.
> * gnu/services/dns.scm (<dnsmasq-configuration>)[shepherd-provision]:
>   Add new field for consistency with other services.
> * gnu/services/dns.scm (<dnsmasq-configuration>)[shepherd-requirement]:
>   Add new field.
> * gnu/services/dns.scm (dnsmasq-shepherd-service): Use the new fields.
> * doc/guix.texi: Document these changes.
> * doc/guix-cookbook.texi (Custom NAT-based network for libvirt):
>   Update example to use shepherd-provision instead of provision.

Same file names in changelog should only appear once.  See (info
"(standards) Style of Change Logs") (that's a manual part of our
'gnu-standards' package) for more guidance, or 'git log' for examples.

> ---
>  doc/guix-cookbook.texi |  4 ++--
>  doc/guix.texi          | 11 ++++++++---
>  gnu/services/dns.scm   | 24 ++++++++++++++++++++----
>  3 files changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/doc/guix-cookbook.texi b/doc/guix-cookbook.texi
> index fe4cac79c3a..d4832b9bb40 100644
> --- a/doc/guix-cookbook.texi
> +++ b/doc/guix-cookbook.texi
> @@ -4031,8 +4031,8 @@ Custom NAT-based network for libvirt
>  (service dnsmasq-service-type
>           (dnsmasq-configuration
>            ;; You can have multiple instances of `dnsmasq-service-type` as long
> -          ;; as each one has a different provision.
> -          (provision '(dnsmasq-virbr0))
> +          ;; as each one has a different shepherd-provision.
> +          (shepherd-provision '(dnsmasq-virbr0))
>            (extra-options (list
>                            ;; Only bind to the virtual bridge. This
>                            ;; avoids conflicts with other running

Looks reasonable; it does seem like 'shepherd-provision' and
'shepherd-requirement' are the prevalent and preferable names to
distinguish them from other configuration options.

> diff --git a/doc/guix.texi b/doc/guix.texi
> index bcb1f9d9cf8..90fa6779657 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -35067,9 +35067,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
                                      ^

Use double space to separate sentences in the Guix sources (whether in
doc or comments, etc); see (info "(standards) Comments").
                                      
> +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..f617f26891d 100644
> --- a/gnu/services/dns.scm
> +++ b/gnu/services/dns.scm
> @@ -27,6 +27,7 @@ (define-module (gnu services dns)
>    #:use-module (gnu system shadow)
>    #:use-module (gnu packages admin)
>    #:use-module (gnu packages dns)
> +  #:use-module (guix deprecation)
>    #:use-module (guix packages)
>    #:use-module (guix records)
>    #:use-module (guix gexp)
> @@ -742,8 +743,13 @@ (define-record-type* <dnsmasq-configuration>
>    dnsmasq-configuration?
>    (package          dnsmasq-configuration-package
>                      (default dnsmasq))  ;file-like
> -  (provision        dnsmasq-provision
> -                    (default '(dnsmasq)))
> +  (provision                    dnsmasq-configuration-provision ; deprecated
> +                                (default #f)
> +                                (sanitize warn-deprecated-dnsmasq-configuration-provision))
> +  (shepherd-provision           dnsmasq-configuration-shepherd-provision
> +                                (default '(dnsmasq)))
> +  (shepherd-requirement         dnsmasq-configuration-shepherd-requirement
> +                                (default '(user-processes networking)))

Since we're busting our 80 columns max width coding style here, I'd
refrain from indenting the right hand side as a block.  You may need to
reformat lines (break them) so that it fits under 80 chars too.

>    (no-hosts?        dnsmasq-configuration-no-hosts?
>                      (default #f))       ;boolean
>    (port             dnsmasq-configuration-port
> @@ -799,9 +805,19 @@ (define-record-type* <dnsmasq-configuration>
>    (tftp-unique-root dnsmasq-tftp-unique-root
>                      (default #f)))      ;"" or "ip" or "mac"
>  
> +(define (warn-deprecated-dnsmasq-configuration-provision value)
> +  (when (pair? value)
> +    (warn-about-deprecation
> +     'provision #f
> +     #:replacement 'shepherd-provision))
> +  value)
> +

Yeah, I think that's the best we can do currently with deprecation for
guix record fields.  It'd be nice to add deprecation support builtin and
have source info, maybe.

>  (define (dnsmasq-shepherd-service config)
>    (match-record config <dnsmasq-configuration>
>      (package
> +     provision
> +     shepherd-provision
> +     shepherd-requirement
>       no-hosts?
>       port local-service? listen-addresses
>       resolv-file no-resolv?
> @@ -815,8 +831,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 (or provision shepherd-provision))
> +     (requirement shepherd-requirement)
>       (documentation "Run the dnsmasq DNS server.")
>       (start #~(make-forkexec-constructor
>                 (list

Other than these nitpicks, it LGTM.  I'm not done reviewing 2 and 3, but
after I do so, could you please send a v2 with the above adjusted?