Subject: bug#48934: closed (Re: bug#48934: [PATCH 0/2] Some improvements to (gnu services configuration))

From: [email-masked] (GNU bug Tracking System)
Date: Mon, 02 Aug 2021 18:22:02 +0000

Your bug report

#48934: [PATCH 0/2] Some improvements to (gnu services configuration)

which was filed against the guix-patches package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
48934: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=48934
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#48934: [PATCH 0/2] Some improvements to (gnu services configuration)
From: Maxim Cournoyer <[email-masked]>
To: Xinglu Chen <[email-masked]>
Cc: [email-masked]
Date: Mon, 02 Aug 2021 14:21:07 -0400
List-Id: <debbugs-submit.debbugs.gnu.org>

Hello again,

Xinglu Chen <[email-masked]> writes:

> * gnu/services/configuration.scm (generate-documentation): If the default
>   value of a field is a package, show the value of the ‘name’ field of the
>   package.  This might not be the correct name in some cases though.

Here also, I've edited the commit message like so:

  services: configuration: Derive the default value from the package variable.

  If the type of a configuration field is a package, show the name of its
  package *variable* as the default value.

  * gnu/services/configuration.scm (generate-documentation){show-default}
  {package->symbol}: New nested procedures.  Use them to format the field
  entries.

> ---
>  gnu/services/configuration.scm | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
> index abcbc70520..99687d065a 100644
> --- a/gnu/services/configuration.scm
> +++ b/gnu/services/configuration.scm
> @@ -252,12 +252,20 @@ does not have a default value" field kind)))
>                                          (configuration-field-default-value-thunk f)
>                                          (lambda _ '%invalid))))
>                           (define (show-default? val)
> -                           (or (string? val) (number? val) (boolean? val)
> +                           (or (string? val) (number? val) (boolean? val) (package? val)
>                                 (and (symbol? val) (not (eq? val '%invalid)))
>                                 (and (list? val) (and-map show-default? val))))
> +
> +                         (define (show-default val)
> +                           (cond
> +                            ((package? val)
> +                             ;; Maybe not always correct.
> +                             (package-name val))
> +                            (else (str val))))
> +
>                           `(entry (% (heading (code ,(str field-name))
>                                               ,@(if (show-default? default)
> -                                                   `(" (default: " (code ,(str default)) ")")
> +                                                   `(" (default: " (code ,(show-default default)) ")")
>                                                     '())
>                                               " (type: "
>                                               ,(str field-type)

I've found a (rather hacky?) way to derive a package's symbol name
instead of using its name, which eliminates the risk of a mismatch,
using such procedure:

--8<---------------cut here---------------start------------->8---
@@ -252,6 +254,21 @@ does not have a default value" field kind)))
 ;; A little helper to make it easier to document all those fields.
 (define (generate-documentation documentation documentation-name)
   (define (str x) (object->string x))
+
+  (define (package->symbol package)
+    "Return the first symbol name of a package that matches PACKAGE, else #f."
+    (let* ((module (file-name->module-name
+                    (location-file (package-location package))))
+           (symbols (filter-map
+                     identity
+                     (module-map (lambda (symbol var)
+                                   (and (equal? package (variable-ref var))
+                                        symbol))
+                                 (resolve-module module)))))
+      (if (null? symbols)
+          #f
+          (first symbols))))
+
   (define (generate configuration-name)
     (match (assq-ref documentation configuration-name)
       ((fields . sub-documentation)
@@ -270,14 +287,21 @@ does not have a default value" field kind)))
                                   (lambda _ '%invalid))))
                    (define (show-default? val)
                      (or (string? val) (number? val) (boolean? val)
+                         (package? val)
                          (and (symbol? val) (not (eq? val '%invalid)))
                          (and (list? val) (and-map show-default? val))))
 
+                   (define (show-default val)
+                     (cond
+                      ((package? val)
+                       (symbol->string (package->symbol val)))
+                      (else (str val))))
+
                    `(entry (% (heading
                                (code ,(str field-name))
                                ,@(if (show-default? default)
                                      `(" (default: "
--8<---------------cut here---------------end--------------->8---

Tested it to generate the new Jami service documentation, and pushed as
commit 8e1f94421873777c6bb0b83daa4f81cbacc8b3ff.

I think (guix services configuration) is starting to look good!  Thanks
to your efforts toward improving the module.

Closing.

Maxim

Subject: [PATCH 0/2] Some improvements to (gnu services configuration)
From: Xinglu Chen <[email-masked]>
To: [email-masked]
Cc: Maxim Cournoyer <[email-masked]>
Date: Wed, 09 Jun 2021 15:04:31 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

This series contains some improvements to the (gnu services
configuration) module.

The first patch changes the formatting of the generated documentation
for configuration records.  Previously, the generated documentation
looked a bit different from the ones that were to generated, compare the
docs for ‘getmail-configuration’ (generated) and ‘openssh-configuration’
(not generated).

--8<---------------cut here---------------start------------->8---
   Available ‘getmail-configuration’ fields are:

 -- ‘getmail-configuration’ parameter: symbol name
     A symbol to identify the getmail service.

     Defaults to ‘"unset"’.

 -- ‘getmail-configuration’ parameter: package package
     The getmail package to use.
--8<---------------cut here---------------end--------------->8---

--8<---------------cut here---------------start------------->8---
 -- Data Type: openssh-configuration
     This is the configuration record for OpenSSH’s ‘sshd’.

     ‘openssh’ (default OPENSSH)
          The Openssh package to use.

     ‘pid-file’ (default: ‘"/var/run/sshd.pid"’)
          Name of the file where ‘sshd’ writes its PID.

     ‘port-number’ (default: ‘22’)
          TCP port on which ‘sshd’ listens for incoming connections.
--8<---------------cut here---------------end--------------->8---

The first patch will make the generated documentation look at lot more
similiar to the hand-written ones.

--8<---------------cut here---------------start------------->8---
 -- Data Type: getmail-configuration
     Available ‘getmail-configuration’ fields are:

     ‘name’ (default: ‘"unset"’) (type: symbol)
          A symbol to identify the getmail service.

     ‘package’ (default: ‘getmail’) (type: package)
          The getmail package to use.
--8<---------------cut here---------------end--------------->8---

If you paid close attention you will also notice that the old generated
docs didn’t specify the default value of the ‘package’ field, whereas
the new docs do.  This brings us to the second patch, it looks the
package and shows the value of the ‘name’ field of the package.  This
will only show the correct package name if the ‘name’ field and the
Scheme variable corresponding to the package are the same, in most cases
it is, so I don’t think it would be a huge deal.

Xinglu Chen (2):
  services: configuration: Change formatting of generated documentation.
  services: configuration: Show default value when it is a package.

 gnu/services/configuration.scm | 62 ++++++++++++++++++++--------------
 1 file changed, 36 insertions(+), 26 deletions(-)

base-commit: 86bb77608d375043f837583332a7c852ea2080ec
-- 
2.32.0