bug#56799: (gnu services configuration) usage of *unspecified* is problematic

[Maxim Cournoyer <maxim.cournoyer@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3331 bytes --]

Hi,

Ludovic Courtès <ludo@gnu.org> writes:

> Howdy,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

[...]

>> I think mostly because few services make use of define-configuration.
>> While attempting to write a new VNC service, it quickly became a visible
>> annoyance:
>>
>> (define-configuration/no-serialization xvnc-configuration
>
> [...]
>
>>   (port
>>    maybe-port
>>    "The port on which to listen for connections from viewers.  When left
>> unspecified, it defaults to 5900 plus the display number.")
>
> [...]
>
>> (define (xvnc-shepherd-service config)
>>   "Return a <shepherd-service> for Xvnc with CONFIG."
>>   ;; XXX: Ensure all the *unspecified* values are handled outside of gexps, as
>>   ;; they are not valid gexp input (they are not self-quoting/serializable).
>>   ;; This would otherwise cause problem during 'guix deploy'.
>>   (let* ((display-number (xvnc-configuration-display-number config))
>>          (port (if (unspecified? (xvnc-configuration-port config))
>>                    #f
>>                    (xvnc-configuration-port config)))
>
> OK, I see.  I guess most of the time, we just call
> ‘serialize-xyz-configuration’, which automatically handles *unspecified*
> values.  In this case, ‘port’ is treated specially and instead passed as
> a command-line argument.

Yes, and that provides much welcome flexibility, as records are
serializable (as vectors) so long as the values they contain also are.

> Other ways to address that come to mind include: adding ‘port’ to the
> config file instead of on the command line (if possible), or doing:
>
>   (serialize-configuration config
>                            (find (lambda (field)
>                                    (eq? (configuration-field-name field)
>                                         'port))
>                                  xvnc-configuration-fields))
>
> That’s a mouthful but maybe it could be abstracted.  It does sound less
> convenient though.

Xvnc takes no config switch, and I agree the above is not
intuitive/great.  It also wouldn't fix the use case of places it is more
convenient to have the maybe value expanded into the gexp and dealt with
there, such as used in the jami-service-type and in my previous example
making use of a inetd style service, where shepherd-side only bindings
such as endpoint makes it convenient to place the ports logic inside the
service gexp.

> That said, whether it’s ‘unspecified?’ or something else, you have to
> have a check in place, right?  With the new interface it becomes:
>
>  (if (eq? port 'unset) #f port)
>
> Or you can provide an actual default value (an integer in this case),
> but that’s possible whether or not *unspecified* is the default value.

To me, the main use of maybe values is to allow for the daemon to use
its own default when no value is specified.  A maybe value with a
default value makes little sense to me, so we should move toward
eliminating that style and support for it from define-configuration, in
my opinion.  Things would be a bit easier to reason with then.

As a baby step toward making 'unset a properly hidden implementation
detail, I'd suggest the attached patch, which adds a means to check if a
value was set.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-services-configuration-Add-a-maybe-value-set-procedu.patch --]
[-- Type: text/x-patch, Size: 2435 bytes --]

From ac84f836a5a6c451465a879521a306b6fbcbed50 Mon Sep 17 00:00:00 2001
From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Date: Fri, 5 Aug 2022 15:03:55 -0400
Subject: [PATCH] services: configuration: Add a 'maybe-value-set?' procedure.

* gnu/services/configuration.scm (maybe-value-set?): New procedure.
* doc/guix.texi (Complex Configurations): Document it.  Remove comment showing
usage of 'maybe-string' with a default value, which doesn't make sense.
---
 doc/guix.texi                  | 7 ++++++-
 gnu/services/configuration.scm | 5 +++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index eb3a1a4eb5..ccdd01f321 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -38998,7 +38998,7 @@ to be a string, or left unspecified.
   (name
    ;; If set to a string, the `serialize-string' procedure will be used
    ;; to serialize the string.  Otherwise this field is not serialized.
-   maybe-string    ; equivalent to (maybe-string *unspecified*)
+   maybe-string
    "The name of this module."))
 @end lisp
 
@@ -39029,6 +39029,11 @@ whether its value is set or not.
 @end lisp
 @end deffn
 
+@deffn (Scheme Procedure) maybe-value-set? @var{value}
+Predicate to check whether a user explicitly specified the value of a
+maybe field.
+@end deffn
+
 @deffn {Scheme Procedure} serialize-configuration @var{configuration} @
 @var{fields}
 Return a G-expression that contains the values corresponding to the
diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index 3007e8de35..b41b4d2e62 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -57,6 +57,7 @@ (define-module (gnu services configuration)
             serialize-configuration
             define-maybe
             define-maybe/no-serialization
+            maybe-value-set?
             generate-documentation
             configuration->documentation
             empty-serializer
@@ -300,6 +301,10 @@ (define-configuration stem (field field-type+def
 (define (empty-serializer field-name val) "")
 (define serialize-package empty-serializer)
 
+(define (maybe-value-set? value)
+  "Predicate to check whether a 'maybe' value was explicitly provided."
+  (not (eq? 'unset value)))
+
 ;; A little helper to make it easier to document all those fields.
 (define (generate-documentation documentation documentation-name)
   (define (str x) (object->string x))
-- 
2.36.1


[-- Attachment #3: Type: text/plain, Size: 642 bytes --]


[...]

> With the xvnc example, I better understand the kind of situation where
> a field value might end up directly in a gexp, though I’m still unsure
> whether use of *unspecified* really makes things worse.  At least,
> because it lacks a read syntax, the problem is caught early on; whereas
> with 'unset, you might end up stuffing 'unset in your config file
> without noticing.

I've been experimenting a lot with define-configuration recently and it
gives you little chance to shoot yourself in the foot; it knows maybe
values are not to be serialized, and won't wrote such values to config
files.

Thanks,

Maxim