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

  • Done
  • quality assurance status badge
Details
2 participants
  • Maxim Cournoyer
  • Xinglu Chen
Owner
unassigned
Submitted by
Xinglu Chen
Severity
normal
X
X
Xinglu Chen wrote on 9 Jun 2021 15:04
(address . guix-patches@gnu.org)(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
cover.1623243063.git.public@yoctocell.xyz
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).

Toggle snippet (11 lines)
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.

Toggle snippet (13 lines)
-- 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.

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

Toggle snippet (10 lines)
-- 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.

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
-----BEGIN PGP SIGNATURE-----

iQJJBAEBCAAzFiEEAVhh4yyK5+SEykIzrPUJmaL7XHkFAmDAvF8VHHB1YmxpY0B5
b2N0b2NlbGwueHl6AAoJEKz1CZmi+1x579cP/jGOaroYfsMlBf+SY0/QSHBFBzh8
+3DGfyA5/ltMzBLHAe7aFSWUItHJqrUfGxkix2preh8iVHCq9KoGMxMr0BdEP80L
t7nj+ciTE8ua10NcIW7RJTvaAeHkpb6pcEXh2mynS5sw2/z9GlcG9rX2TuyFE9mA
Ov7rEp64Czg+yfJEmRy+D9G6JUzhJzPVzk6Uu5j9lL/7oNUB/pdEOteY/kQo+LOD
aP0dM5UZeSzVxT0RtZcSR5pwZEtUiNht/5VesPJKtXk5lK2bgHYKfDaQjYt3kTcF
vdNbZvrQYsJGSW5VORP2kKFpE9+iNebi5rP40dPnpT45H9NqgI98WM5USIauTyLU
9cxG13R+BV6F50eHSX+hF4/nOh5PmUtR2gmdv9sFLUzsq682GzAU1WYDkwxgg8qZ
ciTYb0C0PiTdLmxKF6Tw3VibBcZWpE6SdugxqDfvaBzphzpkZ91oFPbExnQJQoy2
rIigctfHHegnIqdbkanuZkkgCQYL63W+qf7UtXf5Pw6NNG6M+Uh9BU3sQoMjGRLS
CJvdq8/WbLs8/wh9YV2QG8HaMlCOV2YiAQ6UJj6WZz12DNZ0G+5KRsAzwIdNPZ6X
6u0AzLH5Wk6aYxBQ3iQgLmQKRGk4SVFhNukjL5Bgj0fFC7ITTpWQZlEpw9LvouEk
n2YqoFUdNT22EWOI
=cAuz
-----END PGP SIGNATURE-----

X
X
Xinglu Chen wrote on 9 Jun 2021 15:06
[PATCH 1/2] services: configuration: Change formatting of generated
(address . 48934@debbugs.gnu.org)
07aed2e6ffe6c6fdab4a38207cafb7d3169c6e11.1623243063.git.public@yoctocell.xyz
* gnu/services/configuration (generate-documentation): Make the formatting of
the generated docs more consistent with the rest of the docs in the “Services”
section of the manual.
---
gnu/services/configuration.scm | 54 ++++++++++++++++++----------------
1 file changed, 28 insertions(+), 26 deletions(-)

Toggle diff (67 lines)
diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index f23840ee6d..abcbc70520 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -239,32 +239,34 @@ does not have a default value" field kind)))
(define (generate configuration-name)
(match (assq-ref documentation configuration-name)
((fields . sub-documentation)
- `((para "Available " (code ,(str configuration-name)) " fields are:")
- ,@(map
- (lambda (f)
- (let ((field-name (configuration-field-name f))
- (field-type (configuration-field-type f))
- (field-docs (cdr (texi-fragment->stexi
- (configuration-field-documentation f))))
- (default (catch #t
- (configuration-field-default-value-thunk f)
- (lambda _ '%invalid))))
- (define (show-default? val)
- (or (string? val) (number? val) (boolean? val)
- (and (symbol? val) (not (eq? val '%invalid)))
- (and (list? val) (and-map show-default? val))))
- `(deftypevr (% (category
- (code ,(str configuration-name)) " parameter")
- (data-type ,(str field-type))
- (name ,(str field-name)))
- ,@field-docs
- ,@(if (show-default? default)
- `((para "Defaults to " (samp ,(str default)) "."))
- '())
- ,@(append-map
- generate
- (or (assq-ref sub-documentation field-name) '())))))
- fields)))))
+ `((deftp (% (category "Data Type") (name ,(str configuration-name)))
+ (para "Available " (code ,(str configuration-name)) " fields are:")
+ (table (% (formatter (asis)))
+ ,@(map
+ (lambda (f)
+ (let ((field-name (configuration-field-name f))
+ (field-type (configuration-field-type f))
+ (field-docs (cdr (texi-fragment->stexi
+ (configuration-field-documentation f))))
+ (default (catch #t
+ (configuration-field-default-value-thunk f)
+ (lambda _ '%invalid))))
+ (define (show-default? val)
+ (or (string? val) (number? val) (boolean? val)
+ (and (symbol? val) (not (eq? val '%invalid)))
+ (and (list? val) (and-map show-default? val))))
+ `(entry (% (heading (code ,(str field-name))
+ ,@(if (show-default? default)
+ `(" (default: " (code ,(str default)) ")")
+ '())
+ " (type: "
+ ,(str field-type)
+ ")"))
+ (para ,@field-docs)
+ ,@(append-map
+ generate
+ (or (assq-ref sub-documentation field-name) '())))))
+ fields)))))))
(stexi->texi `(*fragment* . ,(generate documentation-name))))
(define (configuration->documentation configuration-symbol)
--
2.32.0
X
X
Xinglu Chen wrote on 9 Jun 2021 15:06
[PATCH 2/2] services: configuration: Show default value when it is a
(address . 48934@debbugs.gnu.org)
9de0174c0818edd7c3f1f58a264a6ea3c5c3be50.1623243063.git.public@yoctocell.xyz
* 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.
---
gnu/services/configuration.scm | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

Toggle diff (29 lines)
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)
--
2.32.0
M
M
Maxim Cournoyer wrote on 2 Aug 2021 20:10
Re: bug#48934: [PATCH 0/2] Some improvements to (gnu services configuration)
(name . Xinglu Chen)(address . public@yoctocell.xyz)(address . 48934@debbugs.gnu.org)
87im0n90qu.fsf_-_@gmail.com
Hello Xinglu!

Xinglu Chen <public@yoctocell.xyz> writes:

Toggle quote (4 lines)
> * gnu/services/configuration (generate-documentation): Make the formatting of
> the generated docs more consistent with the rest of the docs in the “Services”
> section of the manual.

I've modified the commit message to be more in line with the GNU change
log style (see 'info (standards) Style of Change Logs'), like so:

services: configuration: Uniformize the generated documentation.

Make the formatting of the generated docs more consistent with the rest of the
docs in the “Services” section of the manual.

* gnu/services/configuration (generate-documentation): Represent the data type
documentation of a field using a DEFTP table rather than DEFTYPEVR elements.

Toggle quote (70 lines)
> ---
> gnu/services/configuration.scm | 54 ++++++++++++++++++----------------
> 1 file changed, 28 insertions(+), 26 deletions(-)
>
> diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
> index f23840ee6d..abcbc70520 100644
> --- a/gnu/services/configuration.scm
> +++ b/gnu/services/configuration.scm
> @@ -239,32 +239,34 @@ does not have a default value" field kind)))
> (define (generate configuration-name)
> (match (assq-ref documentation configuration-name)
> ((fields . sub-documentation)
> - `((para "Available " (code ,(str configuration-name)) " fields are:")
> - ,@(map
> - (lambda (f)
> - (let ((field-name (configuration-field-name f))
> - (field-type (configuration-field-type f))
> - (field-docs (cdr (texi-fragment->stexi
> - (configuration-field-documentation f))))
> - (default (catch #t
> - (configuration-field-default-value-thunk f)
> - (lambda _ '%invalid))))
> - (define (show-default? val)
> - (or (string? val) (number? val) (boolean? val)
> - (and (symbol? val) (not (eq? val '%invalid)))
> - (and (list? val) (and-map show-default? val))))
> - `(deftypevr (% (category
> - (code ,(str configuration-name)) " parameter")
> - (data-type ,(str field-type))
> - (name ,(str field-name)))
> - ,@field-docs
> - ,@(if (show-default? default)
> - `((para "Defaults to " (samp ,(str default)) "."))
> - '())
> - ,@(append-map
> - generate
> - (or (assq-ref sub-documentation field-name) '())))))
> - fields)))))
> + `((deftp (% (category "Data Type") (name ,(str configuration-name)))
> + (para "Available " (code ,(str configuration-name)) " fields are:")
> + (table (% (formatter (asis)))
> + ,@(map
> + (lambda (f)
> + (let ((field-name (configuration-field-name f))
> + (field-type (configuration-field-type f))
> + (field-docs (cdr (texi-fragment->stexi
> + (configuration-field-documentation f))))
> + (default (catch #t
> + (configuration-field-default-value-thunk f)
> + (lambda _ '%invalid))))
> + (define (show-default? val)
> + (or (string? val) (number? val) (boolean? val)
> + (and (symbol? val) (not (eq? val '%invalid)))
> + (and (list? val) (and-map show-default? val))))
> + `(entry (% (heading (code ,(str field-name))
> + ,@(if (show-default? default)
> + `(" (default: " (code ,(str default)) ")")
> + '())
> + " (type: "
> + ,(str field-type)
> + ")"))
> + (para ,@field-docs)
> + ,@(append-map
> + generate
> + (or (assq-ref sub-documentation field-name) '())))))
> + fields)))))))
> (stexi->texi `(*fragment* . ,(generate documentation-name))))
>
> (define (configuration->documentation configuration-symbol)

I've used this opportunity to re-indent the code a bit, so that it'd fit
under 80 characters column width.

Thanks for this neat improvement!

Maxim
M
M
Maxim Cournoyer wrote on 2 Aug 2021 20:21
(name . Xinglu Chen)(address . public@yoctocell.xyz)(address . 48934-done@debbugs.gnu.org)
87eebb908c.fsf_-_@gmail.com
Hello again,

Xinglu Chen <public@yoctocell.xyz> writes:

Toggle quote (4 lines)
> * 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.

Toggle quote (32 lines)
> ---
> 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:

Toggle snippet (42 lines)
@@ -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: "

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
Closed
X
X
Xinglu Chen wrote on 3 Aug 2021 09:24
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)(address . 48934-done@debbugs.gnu.org)
87fsvruh1d.fsf@yoctocell.xyz
On Mon, Aug 02 2021, Maxim Cournoyer wrote:

Toggle quote (19 lines)
> Hello again,
>
> Xinglu Chen <public@yoctocell.xyz> 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.

Thanks, I should probably re-read the manual. :)

Toggle quote (79 lines)
>> ---
>> 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---

Cool, that looks like a better way than just getting the package name.

Toggle quote (6 lines)
> 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.

You are welcome, happy to help out. The next step would probably be to
automatically generate the docs when invoking ‘make docs’.
-----BEGIN PGP SIGNATURE-----

iQJJBAEBCAAzFiEEAVhh4yyK5+SEykIzrPUJmaL7XHkFAmEI7z4VHHB1YmxpY0B5
b2N0b2NlbGwueHl6AAoJEKz1CZmi+1x5Vl0QAIf62F0s4sL92RGe1ffakRjO7PLd
dOKiCgSh2gMeyIm3rYBsHLZ5s2KKyuyB9kCavXSUoCjvdGCdZolD+dHfJeE85Pzi
srL8HmtZ7Fd5sE0YQcQFF1hcV7y4tvRmAqIHc30wn8QcU51haVgdie0E2suKECZp
/vz5GIyMQFWdRTA4TMfYZ9qOWAfLMpsJfWDLXhQtO7ZBgxwxRFx1vm6TeOH5YYYj
Vx1leFp+WeO4JGUVI0soTa4VXB/GC+QLSXHmmFihPvaU8VBvWENW+A9lVrc+eeUb
9TfWE6NHUyDc0uwB5zajZPreLhEZy7jaFRjuq0N9V0O03HuFlbiZCF0L5IelUG6/
26COz08wUBCihO/8rWawWXl1aPo6BFd8jmuMhLRimGMxzH0/xCDNtcJPSxjGuUW+
LKSp9y3cDbeESeMocW3vwnP3KOuRjx04mXcA+kwHB/FvifIMJAQNRMRa1lq0gjov
iweKsSD9xzCObKJ2wAMbBsORZ+TyumHmDHdhTTWQCaqpTkqIiGYpfKDvvrJKrwlG
oTVM0KnHZ8fGgBakTfeAUF+3F19s3KGQCCSGaV5x9W+wArxrei6HxHMCOjINmn/R
C/yTXdEpMlwC8Np+oRwW6Yb//XQdYgltSKkYEytaj1yhfXy4eHOWjO1jvUM8f10n
bkvcReGLpHH5aSGN
=iO7/
-----END PGP SIGNATURE-----

Closed
M
M
Maxim Cournoyer wrote on 3 Aug 2021 16:38
(name . Xinglu Chen)(address . public@yoctocell.xyz)(address . 48934-done@debbugs.gnu.org)
87a6ly7fv9.fsf@gmail.com
Hi,

Xinglu Chen <public@yoctocell.xyz> writes:

[...]

Toggle quote (6 lines)
>> I think (guix services configuration) is starting to look good! Thanks
>> to your efforts toward improving the module.
>
> You are welcome, happy to help out. The next step would probably be to
> automatically generate the docs when invoking ‘make docs’.

Agreed! I've left a TODO in the recent Jami service addition to
guix.texi that suggests to do this. It'd be useful!

Maxim
Closed
?