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

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

[Maxim Cournoyer]

Hello Guix,

Since commit 8cb1a49a3998c39f315a4199b7d4a121a6d66449, the
define-configuration machinery in (gnu services configuration) uses
*unspecified* instead of 'disabled for an unspecified field value.

While this is indeed an improvement in readability, it introduces an
extra complication: because this new value is not self-quoting, it
cannot be used as is in G-Exps, and values using it must be carefully
expanded outside the gexp context, which is error prone.

This broke the jami-service-type, when partially specifying a
jami-account like so:

--8<---------------cut here---------------start------------->8---
(service jami-service-type
               (jami-configuration
                (accounts
                 (list (jami-account
                        (archive "/etc/jami/some-jami-account.gz"))))))
--8<---------------cut here---------------end--------------->8---

When building the operating system containing the above fragment, the
following error is throw:

--8<---------------cut here---------------start------------->8---
guix system: error: #<unspecified>: invalid G-expression input
--8<---------------cut here---------------end--------------->8---

The following change to the jami-provisioning test can also reproduce
the problem:

--8<---------------cut here---------------start------------->8---
modified   gnu/tests/telephony.scm
@@ -60,7 +60,7 @@ (define %moderators '("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
 (define %dummy-jami-account (jami-account
                              (archive %dummy-jami-account-archive)
                              (allowed-contacts %allowed-contacts)
-                             (moderators %moderators)
+;                             (moderators %moderators)
                              (rendezvous-point? #t)
                              (peer-discovery? #f)
                              (bootstrap-hostnames '("bootstrap.me"
--8<---------------cut here---------------end--------------->8---

--8<---------------cut here---------------start------------->8---
$ make check-system TESTS=jami-provisioning
Selected 1 system tests...
guix build: error: #<unspecified>: invalid G-expression input
make: *** [Makefile:6734: check-system] Error 1
--8<---------------cut here---------------end--------------->8---

I'd suggest we revisit 8cb1a49a3998c39f315a4199b7d4a121a6d66449 to use
'unspecified (the symbol) instead of *unspecified*, which *can* be
serialized without any fuss in gexps.

Thoughts?

Thanks,

Maxim

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

[Tobias Geerinckx-Rice via Bug reports for GNU Guix]

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

Hi Maxim,

Maxim Cournoyer 写道：
> I'd suggest we revisit 8cb1a49a3998c39f315a4199b7d4a121a6d66449 
> to use
> 'unspecified (the symbol) instead of *unspecified*, which *can* 
> be
> serialized without any fuss in gexps.

Bah.  Could we provide our own reader?

I'd much rather this be addressed in Guile (or failing that, 
transparently by Guix) than have to deal with some magical symbol. 
IIRC that was the argument for using *unspecified* in the first 
place, and I think it makes sense.

This looks more like an unexplored oversight than a well-reasoned 
restriction to me.

Kind regards,

T G-R

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]

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

[Attila Lendvai]

hi,

sorry for the headaches!

the original discussion is here (well, i think. site is down right now):

https://issues.guix.gnu.org/54674

'UNSPECIFIED would satisfy SYMBOL?, i.e. a source of headaches/confusion. it used to be 'DISABLED, which was even worse as it can be confused/conflated with a user specified value.

i suggested the use of srfi-189, but it was rejected as unwelcome complexity.

https://srfi.schemers.org/srfi-189/srfi-189.html

i think it makes sense to change Guile to make *unspecified* self-evaluating, but looking back, maybe the use of srfi-189 would have been better.

i need to run now, and i'll be offline for a week or two. i can't look the example in depth now, but my gut instinct says that it's a bug if *unspecified* reaches any GExp machinery.

more later,

--
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“If you are neutral in situations of injustice, you have chosen the side of the oppressor.”
	— Desmond Tutu (1931–)

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

[Maxim Cournoyer]

Hi Attila,

[...]

> i need to run now, and i'll be offline for a week or two. i can't look
> the example in depth now, but my gut instinct says that it's a bug if
> *unspecified* reaches any GExp machinery.

I don't think it's reasonable to burden users with normalizing their
G-exp input data, where *unspecified* may appear in nested data
structures (such as used by the jami-service-type: jami-accounts has
maybe fields end is used as a nested data type to jami-configuration).

I think v3 of this patch enables us to continue with our current ways
and is a non-invasive change, so I'll merge it soon if there are no
objections.

Thanks,

Maxim

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

[Maxim Cournoyer]

Hi,

Tobias Geerinckx-Rice <me@tobias.gr> writes:

> Hi Maxim,
>
> Maxim Cournoyer 写道：
>> I'd suggest we revisit 8cb1a49a3998c39f315a4199b7d4a121a6d66449 to
>> use
>> 'unspecified (the symbol) instead of *unspecified*, which *can* be
>> serialized without any fuss in gexps.
>
> Bah.  Could we provide our own reader?
>
> I'd much rather this be addressed in Guile (or failing that,
> transparently by Guix) than have to deal with some magical
> symbol. IIRC that was the argument for using *unspecified* in the
> first place, and I think it makes sense.
>
> This looks more like an unexplored oversight than a well-reasoned
> restriction to me.

This was my original impression, but thinking more about it, it became
apparent that *unspecified* is well, unspecified and shouldn't be relied
on by people to be something well defined.  For some background reading,
see [0].  So it seems wrong in Scheme to actively set things to
*unspecified*, and give a specific meaning to that.

I think the semantic of the language is that it is to be used as the
lack of a return value from a procedure or syntax, e.g.:

(unspecified? (if #f 'one-arm-if)) -> #t

Having 'unspecified?' even defined in Guile seems to go against that
idea; perhaps because Wingo themselves seems to disagree in [0].

I'm also thinking 'unspecified being too close to *unspecified* is
probably going to cause confusion down the line.  Reverting to the
originally used 'disabled may be the lesser evil.

Other thoughts?

Thanks,

Maxim

[0]  https://scheme-reports.scheme-reports.narkive.com/QSQtJSAh/unspecified-values

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

[Tobias Geerinckx-Rice via Bug reports for GNU Guix]

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

Hi Maxim,

Maxim Cournoyer 写道：
> For some background reading, see [0].

Thanks for the well-thought-out reply, and sharing this 
interesting link!

Now, it's just the musings of one person, but now I think I do 
agree with (what I think is) the underlying vision: to hush up 
*unspecified* and sneakily replace it with true nothingness.  OK, 
I can live with that.  :-)

> I think the semantic of the language is that it is to be used as 
> the
> lack of a return value from a procedure or syntax, e.g.:
>
> (unspecified? (if #f 'one-arm-if)) -> #t

Well… in the above context I'd hesitate to even imply ‘semantics’. 
It's like undefined behaviour in C.  Ascribe it meaning at your 
peril.  Otherwise, point taken.

> Having 'unspecified?' even defined in Guile seems to go against 
> that
> idea; perhaps because Wingo themselves seems to disagree in [0].

Agreed.  *This* was one of my reasons for supporting (field 
*unspecified*), so it's nice to have it validated, even if it is 
rejected.

> I'm also thinking 'unspecified being too close to *unspecified* 
> is
> probably going to cause confusion down the line.  Reverting to 
> the
> originally used 'disabled may be the lesser evil.

Ah, here I can concentrate all my previous disagreement: hell no 
:-)

It is the worstest evil; literally anything is better than 
(enable-foo? 'disabled) defaulting to #t.

Bikeshed this all y'all want, but 'default or 'unset or 'whatever 
are miles better.

Kind regards,

T G-R

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]

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

[Maxim Cournoyer]

Hi,

Tobias Geerinckx-Rice <me@tobias.gr> writes:

> Hi Maxim,
>
> Maxim Cournoyer 写道：
>> For some background reading, see [0].
>
> Thanks for the well-thought-out reply, and sharing this interesting
> link!
>
> Now, it's just the musings of one person, but now I think I do agree
> with (what I think is) the underlying vision: to hush up *unspecified*
> and sneakily replace it with true nothingness.  OK, I can live with
> that.  :-)
>
>> I think the semantic of the language is that it is to be used as the
>> lack of a return value from a procedure or syntax, e.g.:
>>
>> (unspecified? (if #f 'one-arm-if)) -> #t
>
> Well… in the above context I'd hesitate to even imply
> ‘semantics’. It's like undefined behaviour in C.  Ascribe it meaning
> at your peril.  Otherwise, point taken.
>
>> Having 'unspecified?' even defined in Guile seems to go against that
>> idea; perhaps because Wingo themselves seems to disagree in [0].
>
> Agreed.  *This* was one of my reasons for supporting (field
> *unspecified*), so it's nice to have it validated, even if it is
> rejected.

Good to know I wasn't the only one nudged into thinking the
'unspecified?' procedure somehow justified using *unspecified* directly.

>> I'm also thinking 'unspecified being too close to *unspecified* is
>> probably going to cause confusion down the line.  Reverting to the
>> originally used 'disabled may be the lesser evil.

> Ah, here I can concentrate all my previous disagreement: hell no :-)
>
> It is the worstest evil; literally anything is better than
> (enable-foo? 'disabled) defaulting to #t.
>
> Bikeshed this all y'all want, but 'default or 'unset or 'whatever are
> miles better.

Thanks for sharing your idea!  The neat thing here, is that even if we
disagree about 'unspecified in the patch I'm about to send, we can
discuss it to length then fix the end result in a second using sed ;-).

Thanks for your input!

Maxim

bug#56799: [PATCH] services: configuration: Step back from *unspecified*.

[Maxim Cournoyer]

Fixes <https://issues.guix.gnu.org/56799>.

This partially reverts 8cb1a49a3998c39f315a4199b7d4a121a6d66449.

Rationale: *unspecified* cannot be serialized thus used as a G-Expression
input, which is problematic/inconvenient when using deeply nested records.  As
an example, jami-service-type was broken when using partially defined
<jami-account> records.

* gnu/services/configuration.scm (define-maybe-helper): Check against the
'unspecified symbol.
(normalize-field-type+def): Adjust value to 'unspecified.
(define-configuration-helper): Use 'unspecified as the default value thunk.
* gnu/services/file-sharing.scm (serialize-maybe-string): Check against the
'unspecified symbol.
(serialize-maybe-file-object): Likewise.
* gnu/services/messaging.scm (define-all-configurations): Use 'unspecified as
value.
(raw-content?): Check against 'unspecified symbol.
(prosody-configuration)[http-max-content-size]: Default to 'unspecified.
[http-external-url]: Likewise.
[mod-muc]: Likewise.
[raw-content]: Likewise.
* gnu/services/networking.scm (opendht-configuration): Adjust documentation.
* gnu/services/telephony.scm (jami-shepherd-services): Replace *undefined*
with the 'unspecified symbol.
* tests/services/configuration.scm ("maybe type, no default"): Check against
the 'unspecified symbol.
* doc/guix.texi: Regenerate the opendht-configuration,
openvpn-client-configuration and openvpn-server-configuration documentation.
---
 doc/guix.texi                    | 367 +++++++------------------------
 gnu/services/configuration.scm   |  11 +-
 gnu/services/file-sharing.scm    |   4 +-
 gnu/services/messaging.scm       |  12 +-
 gnu/services/networking.scm      |   6 +-
 gnu/services/telephony.scm       |   6 +-
 tests/services/configuration.scm |   6 +-
 7 files changed, 102 insertions(+), 310 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 12ecc1b952..a2ccf913da 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19767,75 +19767,46 @@ The value of this service is a @code{opendht-configuration} object, as
 described below.
 @end defvr
 
-@deftp {Data Type} opendht-configuration
-This is the data type for the OpenDHT service configuration.
-
 @c The fields documentation has been auto-generated using the
 @c configuration->documentation procedure from
 @c (gnu services configuration).
+@deftp {Data Type} opendht-configuration
 Available @code{opendht-configuration} fields are:
 
-@deftypevr {@code{opendht-configuration} parameter} package opendht
+@table @asis
+@item @code{opendht} (default: @code{opendht}) (type: file-like)
 The @code{opendht} package to use.
 
-@end deftypevr
-
-@deftypevr {@code{opendht-configuration} parameter} boolean peer-discovery?
+@item @code{peer-discovery?} (default: @code{#f}) (type: boolean)
 Whether to enable the multicast local peer discovery mechanism.
 
-Defaults to @samp{#f}.
-
-@end deftypevr
-
-@deftypevr {@code{opendht-configuration} parameter} boolean enable-logging?
+@item @code{enable-logging?} (default: @code{#f}) (type: boolean)
 Whether to enable logging messages to syslog.  It is disabled by default
 as it is rather verbose.
 
-Defaults to @samp{#f}.
-
-@end deftypevr
-
-@deftypevr {@code{opendht-configuration} parameter} boolean debug?
+@item @code{debug?} (default: @code{#f}) (type: boolean)
 Whether to enable debug-level logging messages.  This has no effect if
 logging is disabled.
 
-Defaults to @samp{#f}.
-
-@end deftypevr
-
-@deftypevr {@code{opendht-configuration} parameter} maybe-string bootstrap-host
+@item @code{bootstrap-host} (default: @code{"bootstrap.jami.net:4222"}) (type: maybe-string)
 The node host name that is used to make the first connection to the
 network.  A specific port value can be provided by appending the
 @code{:PORT} suffix.  By default, it uses the Jami bootstrap nodes, but
 any host can be specified here.  It's also possible to disable
-bootsrapping by explicitly setting this to the @code{*unspecified*}
-value.
+bootstrapping by explicitly setting this field to the
+@code{'unspecified} value.
 
-Defaults to @samp{"bootstrap.jami.net:4222"}.
-
-@end deftypevr
-
-@deftypevr {@code{opendht-configuration} parameter} maybe-number port
-The UDP port to bind to.  When explicitly set to @code{*unspecified*},
-an available port is automatically selected.
-
-Defaults to @samp{4222}.
-
-@end deftypevr
+@item @code{port} (default: @code{4222}) (type: maybe-number)
+The UDP port to bind to.  When left unspecified, an available port is
+automatically selected.
 
-@deftypevr {@code{opendht-configuration} parameter} maybe-number proxy-server-port
+@item @code{proxy-server-port} (type: maybe-number)
 Spawn a proxy server listening on the specified port.
 
-Defaults to @samp{disabled}.
-
-@end deftypevr
-
-@deftypevr {@code{opendht-configuration} parameter} maybe-number proxy-server-port-tls
+@item @code{proxy-server-port-tls} (type: maybe-number)
 Spawn a proxy server listening to TLS connections on the specified port.
 
-Defaults to @samp{disabled}.
-
-@end deftypevr
+@end table
 @end deftp
 
 @cindex Tor
@@ -30525,362 +30496,184 @@ Both can be run simultaneously.
 
 @c %automatically generated documentation
 
+@deftp {Data Type} openvpn-client-configuration
 Available @code{openvpn-client-configuration} fields are:
 
-@deftypevr {@code{openvpn-client-configuration} parameter} package openvpn
+@table @asis
+@item @code{openvpn} (default: @code{openvpn}) (type: file-like)
 The OpenVPN package.
 
-@end deftypevr
-
-@deftypevr {@code{openvpn-client-configuration} parameter} string pid-file
+@item @code{pid-file} (default: @code{"/var/run/openvpn/openvpn.pid"}) (type: string)
 The OpenVPN pid file.
 
-Defaults to @samp{"/var/run/openvpn/openvpn.pid"}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-client-configuration} parameter} proto proto
+@item @code{proto} (default: @code{udp}) (type: proto)
 The protocol (UDP or TCP) used to open a channel between clients and
 servers.
 
-Defaults to @samp{udp}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-client-configuration} parameter} dev dev
+@item @code{dev} (default: @code{tun}) (type: dev)
 The device type used to represent the VPN connection.
 
-Defaults to @samp{tun}.
-
-@end deftypevr
-
-If you do not have some of these files (eg.@: you use a username and
-password), you can disable any of the following three fields by setting
-it to @code{*unspecified*}.
-
-@deftypevr {@code{openvpn-client-configuration} parameter} maybe-string ca
+@item @code{ca} (default: @code{"/etc/openvpn/ca.crt"}) (type: maybe-string)
 The certificate authority to check connections against.
 
-Defaults to @samp{"/etc/openvpn/ca.crt"}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-client-configuration} parameter} maybe-string cert
+@item @code{cert} (default: @code{"/etc/openvpn/client.crt"}) (type: maybe-string)
 The certificate of the machine the daemon is running on.  It should be
 signed by the authority given in @code{ca}.
 
-Defaults to @samp{"/etc/openvpn/client.crt"}.
+@item @code{key} (default: @code{"/etc/openvpn/client.key"}) (type: maybe-string)
+The key of the machine the daemon is running on.  It must be the key
+whose certificate is @code{cert}.
 
-@end deftypevr
-
-@deftypevr {@code{openvpn-client-configuration} parameter} maybe-string key
-The key of the machine the daemon is running on.  It must be the key whose
-certificate is @code{cert}.
-
-Defaults to @samp{"/etc/openvpn/client.key"}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-client-configuration} parameter} boolean comp-lzo?
+@item @code{comp-lzo?} (default: @code{#t}) (type: boolean)
 Whether to use the lzo compression algorithm.
 
-Defaults to @samp{#t}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-client-configuration} parameter} boolean persist-key?
+@item @code{persist-key?} (default: @code{#t}) (type: boolean)
 Don't re-read key files across SIGUSR1 or --ping-restart.
 
-Defaults to @samp{#t}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-client-configuration} parameter} boolean persist-tun?
+@item @code{persist-tun?} (default: @code{#t}) (type: boolean)
 Don't close and reopen TUN/TAP device or run up/down scripts across
 SIGUSR1 or --ping-restart restarts.
 
-Defaults to @samp{#t}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-client-configuration} parameter} boolean fast-io?
+@item @code{fast-io?} (default: @code{#f}) (type: boolean)
 (Experimental) Optimize TUN/TAP/UDP I/O writes by avoiding a call to
 poll/epoll/select prior to the write operation.
 
-Defaults to @samp{#f}.
-@end deftypevr
-
-@deftypevr {@code{openvpn-client-configuration} parameter} number verbosity
+@item @code{verbosity} (default: @code{3}) (type: number)
 Verbosity level.
 
-Defaults to @samp{3}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-client-configuration} parameter} tls-auth-client tls-auth
+@item @code{tls-auth} (default: @code{#f}) (type: tls-auth-client)
 Add an additional layer of HMAC authentication on top of the TLS control
 channel to protect against DoS attacks.
 
-Defaults to @samp{#f}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-client-configuration} parameter} maybe-string auth-user-pass
+@item @code{auth-user-pass} (type: maybe-string)
 Authenticate with server using username/password.  The option is a file
-containing username/password on 2 lines.  Do not use a file-like object as it
-would be added to the store and readable by any user.
+containing username/password on 2 lines.  Do not use a file-like object
+as it would be added to the store and readable by any user.
 
-@end deftypevr
-
-@deftypevr {@code{openvpn-client-configuration} parameter} key-usage verify-key-usage?
+@item @code{verify-key-usage?} (default: @code{#t}) (type: key-usage)
 Whether to check the server certificate has server usage extension.
 
-Defaults to @samp{#t}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-client-configuration} parameter} bind bind?
+@item @code{bind?} (default: @code{#f}) (type: bind)
 Bind to a specific local port number.
 
-Defaults to @samp{#f}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-client-configuration} parameter} resolv-retry resolv-retry?
+@item @code{resolv-retry?} (default: @code{#t}) (type: resolv-retry)
 Retry resolving server address.
 
-Defaults to @samp{#t}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-client-configuration} parameter} openvpn-remote-list remote
+@item @code{remote} (default: @code{()}) (type: openvpn-remote-list)
 A list of remote servers to connect to.
 
-Defaults to @samp{()}.
-
+@deftp {Data Type} openvpn-remote-configuration
 Available @code{openvpn-remote-configuration} fields are:
 
-@deftypevr {@code{openvpn-remote-configuration} parameter} string name
+@table @asis
+@item @code{name} (default: @code{"my-server"}) (type: string)
 Server name.
 
-Defaults to @samp{"my-server"}.
+@item @code{port} (default: @code{1194}) (type: number)
+Port number the server listens to.
 
-@end deftypevr
+@end table
 
-@deftypevr {@code{openvpn-remote-configuration} parameter} number port
-Port number the server listens to.
+@end deftp
 
-Defaults to @samp{1194}.
+@end table
 
-@end deftypevr
+@end deftp
 
-@end deftypevr
 @c %end of automatic openvpn-client documentation
 
 @c %automatically generated documentation
 
+@deftp {Data Type} openvpn-server-configuration
 Available @code{openvpn-server-configuration} fields are:
 
-@deftypevr {@code{openvpn-server-configuration} parameter} package openvpn
+@table @asis
+@item @code{openvpn} (default: @code{openvpn}) (type: file-like)
 The OpenVPN package.
 
-@end deftypevr
-
-@deftypevr {@code{openvpn-server-configuration} parameter} string pid-file
+@item @code{pid-file} (default: @code{"/var/run/openvpn/openvpn.pid"}) (type: string)
 The OpenVPN pid file.
 
-Defaults to @samp{"/var/run/openvpn/openvpn.pid"}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-server-configuration} parameter} proto proto
+@item @code{proto} (default: @code{udp}) (type: proto)
 The protocol (UDP or TCP) used to open a channel between clients and
 servers.
 
-Defaults to @samp{udp}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-server-configuration} parameter} dev dev
+@item @code{dev} (default: @code{tun}) (type: dev)
 The device type used to represent the VPN connection.
 
-Defaults to @samp{tun}.
-
-@end deftypevr
-
-If you do not have some of these files (eg.@: you use a username and
-password), you can disable any of the following three fields by setting
-it to @code{*unspecified*}.
-
-@deftypevr {@code{openvpn-server-configuration} parameter} maybe-string ca
+@item @code{ca} (default: @code{"/etc/openvpn/ca.crt"}) (type: maybe-string)
 The certificate authority to check connections against.
 
-Defaults to @samp{"/etc/openvpn/ca.crt"}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-server-configuration} parameter} maybe-string cert
+@item @code{cert} (default: @code{"/etc/openvpn/client.crt"}) (type: maybe-string)
 The certificate of the machine the daemon is running on.  It should be
 signed by the authority given in @code{ca}.
 
-Defaults to @samp{"/etc/openvpn/client.crt"}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-server-configuration} parameter} maybe-string key
-The key of the machine the daemon is running on.  It must be the key whose
-certificate is @code{cert}.
-
-Defaults to @samp{"/etc/openvpn/client.key"}.
-
-@end deftypevr
+@item @code{key} (default: @code{"/etc/openvpn/client.key"}) (type: maybe-string)
+The key of the machine the daemon is running on.  It must be the key
+whose certificate is @code{cert}.
 
-@deftypevr {@code{openvpn-server-configuration} parameter} boolean comp-lzo?
+@item @code{comp-lzo?} (default: @code{#t}) (type: boolean)
 Whether to use the lzo compression algorithm.
 
-Defaults to @samp{#t}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-server-configuration} parameter} boolean persist-key?
+@item @code{persist-key?} (default: @code{#t}) (type: boolean)
 Don't re-read key files across SIGUSR1 or --ping-restart.
 
-Defaults to @samp{#t}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-server-configuration} parameter} boolean persist-tun?
+@item @code{persist-tun?} (default: @code{#t}) (type: boolean)
 Don't close and reopen TUN/TAP device or run up/down scripts across
 SIGUSR1 or --ping-restart restarts.
 
-Defaults to @samp{#t}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-server-configuration} parameter} boolean fast-io?
+@item @code{fast-io?} (default: @code{#f}) (type: boolean)
 (Experimental) Optimize TUN/TAP/UDP I/O writes by avoiding a call to
 poll/epoll/select prior to the write operation.
 
-Defaults to @samp{#f}.
-@end deftypevr
-
-@deftypevr {@code{openvpn-server-configuration} parameter} number verbosity
+@item @code{verbosity} (default: @code{3}) (type: number)
 Verbosity level.
 
-Defaults to @samp{3}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-server-configuration} parameter} tls-auth-server tls-auth
+@item @code{tls-auth} (default: @code{#f}) (type: tls-auth-server)
 Add an additional layer of HMAC authentication on top of the TLS control
 channel to protect against DoS attacks.
 
-Defaults to @samp{#f}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-server-configuration} parameter} number port
+@item @code{port} (default: @code{1194}) (type: number)
 Specifies the port number on which the server listens.
 
-Defaults to @samp{1194}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-server-configuration} parameter} ip-mask server
+@item @code{server} (default: @code{"10.8.0.0 255.255.255.0"}) (type: ip-mask)
 An ip and mask specifying the subnet inside the virtual network.
 
-Defaults to @samp{"10.8.0.0 255.255.255.0"}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-server-configuration} parameter} cidr6 server-ipv6
+@item @code{server-ipv6} (default: @code{#f}) (type: cidr6)
 A CIDR notation specifying the IPv6 subnet inside the virtual network.
 
-Defaults to @samp{#f}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-server-configuration} parameter} string dh
+@item @code{dh} (default: @code{"/etc/openvpn/dh2048.pem"}) (type: string)
 The Diffie-Hellman parameters file.
 
-Defaults to @samp{"/etc/openvpn/dh2048.pem"}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-server-configuration} parameter} string ifconfig-pool-persist
+@item @code{ifconfig-pool-persist} (default: @code{"/etc/openvpn/ipp.txt"}) (type: string)
 The file that records client IPs.
 
-Defaults to @samp{"/etc/openvpn/ipp.txt"}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-server-configuration} parameter} gateway redirect-gateway?
+@item @code{redirect-gateway?} (default: @code{#f}) (type: gateway)
 When true, the server will act as a gateway for its clients.
 
-Defaults to @samp{#f}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-server-configuration} parameter} boolean client-to-client?
+@item @code{client-to-client?} (default: @code{#f}) (type: boolean)
 When true, clients are allowed to talk to each other inside the VPN.
 
-Defaults to @samp{#f}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-server-configuration} parameter} keepalive keepalive
+@item @code{keepalive} (default: @code{(10 120)}) (type: keepalive)
 Causes ping-like messages to be sent back and forth over the link so
 that each side knows when the other side has gone down.  @code{keepalive}
 requires a pair.  The first element is the period of the ping sending,
 and the second element is the timeout before considering the other side
 down.
 
-@end deftypevr
-
-@deftypevr {@code{openvpn-server-configuration} parameter} number max-clients
+@item @code{max-clients} (default: @code{100}) (type: number)
 The maximum number of clients.
 
-Defaults to @samp{100}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-server-configuration} parameter} string status
+@item @code{status} (default: @code{"/var/run/openvpn/status"}) (type: string)
 The status file.  This file shows a small report on current connection.
 It is truncated and rewritten every minute.
 
-Defaults to @samp{"/var/run/openvpn/status"}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-server-configuration} parameter} openvpn-ccd-list client-config-dir
+@item @code{client-config-dir} (default: @code{()}) (type: openvpn-ccd-list)
 The list of configuration for some clients.
 
-Defaults to @samp{()}.
-
-Available @code{openvpn-ccd-configuration} fields are:
-
-@deftypevr {@code{openvpn-ccd-configuration} parameter} string name
-Client name.
-
-Defaults to @samp{"client"}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-ccd-configuration} parameter} ip-mask iroute
-Client own network
-
-Defaults to @samp{#f}.
-
-@end deftypevr
-
-@deftypevr {@code{openvpn-ccd-configuration} parameter} ip-mask ifconfig-push
-Client VPN IP.
-
-Defaults to @samp{#f}.
-
-@end deftypevr
+@end table
 
-@end deftypevr
+@end deftp
 
 @c %end of automatic openvpn-server documentation
 
@@ -31512,7 +31305,7 @@ Each parameter definition is preceded by its type; for example,
 @samp{boolean foo} indicates that the @code{foo} parameter should be
 specified as a boolean.  Types starting with @code{maybe-} denote
 parameters that won't show up in TLP config file when their value is
-left unset, or is explicitly set to the @code{*unspecified*} value.
+left unset, or is explicitly set to the @code{'unspecified} value.
 
 @c The following documentation was initially generated by
 @c (generate-tlp-documentation) in (gnu services pm).  Manually maintained
@@ -39129,7 +38922,7 @@ macro which is a shorthand of this.
 Sometimes a field should not be serialized if the user doesn’t specify a
 value.  To achieve this, you can use the @code{define-maybe} macro to
 define a ``maybe type''; if the value of a maybe type is left unset, or
-is set to the @code{*unspecified*} value, then it will not be
+is set to the @code{'unspecified} value, then it will not be
 serialized.
 
 When defining a ``maybe type'', the corresponding serializer for the
diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index e3c101d042..3758b4e09a 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -3,7 +3,7 @@
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
 ;;; Copyright © 2017, 2018 Clément Lassieur <clement@lassieur.org>
 ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
-;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
+;;; Copyright © 2021, 2022 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;; Copyright © 2021 Andrew Tropin <andrew@trop.in>
 ;;; Copyright © 2022 Maxime Devos <maximedevos@telenet.be>
 ;;;
@@ -142,8 +142,7 @@ (define (define-maybe-helper serialize? prefix syn)
                                     (id #'stem #'serialize-maybe- #'stem))))
        #`(begin
            (define (maybe-stem? val)
-             (or (unspecified? val)
-                 (stem? val)))
+             (or (eq? val 'unspecified) (stem? val)))
            #,@(if serialize?
                   (list #'(define (serialize-maybe-stem field-name val)
                             (if (stem? val)
@@ -171,10 +170,10 @@ (define (normalize-field-type+def s)
      (values #'(field-type def)))
     ((field-type)
      (identifier? #'field-type)
-     (values #'(field-type *unspecified*)))
+     (values #'(field-type 'unspecified)))
     (field-type
      (identifier? #'field-type)
-     (values #'(field-type *unspecified*)))))
+     (values #'(field-type 'unspecified)))))
 
 (define (define-configuration-helper serialize? serializer-prefix syn)
   (syntax-case syn ()
@@ -262,7 +261,7 @@ (define #,(id #'stem #'stem #'-fields)
                         (lambda ()
                           (display '#,(id #'stem #'% #'stem))
                           (if (eq? (syntax->datum field-default)
-                                   '*unspecified*)
+                                   'unspecified)
                               (configuration-missing-default-value
                                '#,(id #'stem #'% #'stem) 'field)
                               field-default)))
diff --git a/gnu/services/file-sharing.scm b/gnu/services/file-sharing.scm
index e32d1f145d..8110bb0cce 100644
--- a/gnu/services/file-sharing.scm
+++ b/gnu/services/file-sharing.scm
@@ -115,7 +115,7 @@ (define-maybe string)
 (set! serialize-maybe-string
   (lambda (field-name val)
     (serialize-string field-name
-                      (if (unspecified? val)
+                      (if (eq? val 'unspecified)
                           ""
                           val))))
 
@@ -180,7 +180,7 @@ (define (serialize-file-object field-name val)
 (define-maybe file-object)
 (set! serialize-maybe-file-object
   (lambda (field-name val)
-    (if (unspecified? val)
+    (if (eq? val 'unspecified)
         (serialize-string field-name "")
         (serialize-file-object field-name val))))
 
diff --git a/gnu/services/messaging.scm b/gnu/services/messaging.scm
index 651f90adb2..abe814e7f5 100644
--- a/gnu/services/messaging.scm
+++ b/gnu/services/messaging.scm
@@ -90,7 +90,7 @@ (define (make-pred arg)
                      ((new-def ...)
                       (map (lambda (def target)
                              (if (eq? 'common (syntax->datum target))
-                                 #'*unspecified* def))
+                                 #''unspecified def))
                            #'(def ...) #'(target ...)))
                      ((new-doc ...)
                       (map (lambda (doc target)
@@ -200,7 +200,7 @@ (define (serialize-file-object-list field-name val)
 (define-maybe file-object-list)
 
 (define (raw-content? val)
-  (not (unspecified? val)))
+  (not (eq? val 'unspecified)))
 (define (serialize-raw-content field-name val)
   val)
 (define-maybe raw-content)
@@ -474,12 +474,12 @@ (define-all-configurations prosody-configuration
      global)
 
     (http-max-content-size
-     (maybe-non-negative-integer *unspecified*)
+     (maybe-non-negative-integer 'unspecified)
      "Maximum allowed size of the HTTP body (in bytes)."
      common)
 
     (http-external-url
-     (maybe-string *unspecified*)
+     (maybe-string 'unspecified)
      "Some modules expose their own URL in various ways.  This URL is built
 from the protocol, host and port used.  If Prosody sits behind a proxy, the
 public URL will be @code{http-external-url} instead.  See
@@ -556,7 +556,7 @@ (define-all-configurations prosody-configuration
      int-component)
 
     (mod-muc
-     (maybe-mod-muc-configuration *unspecified*)
+     (maybe-mod-muc-configuration 'unspecified)
      "Multi-user chat (MUC) is Prosody's module for allowing you to create
 hosted chatrooms/conferences for XMPP users.
 
@@ -573,7 +573,7 @@ (define-all-configurations prosody-configuration
      ext-component)
 
     (raw-content
-     (maybe-raw-content *unspecified*)
+     (maybe-raw-content 'unspecified)
      "Raw content that will be added to the configuration file."
      common)))
 
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index b555c46040..a5f0924984 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -772,11 +772,11 @@ (define-configuration/no-serialization opendht-configuration
 network.  A specific port value can be provided by appending the @code{:PORT}
 suffix.  By default, it uses the Jami bootstrap nodes, but any host can be
 specified here.  It's also possible to disable bootstrapping by explicitly
-setting this field to the @code{*unspecified*} value.")
+setting this field to the @code{'unspecified} value.")
   (port
    (maybe-number 4222)
-   "The UDP port to bind to.  When set to @code{*unspecified*}, an available
-port is automatically selected.")
+   "The UDP port to bind to.  When left unspecified, an available port is
+automatically selected.")
   (proxy-server-port
    maybe-number
    "Spawn a proxy server listening on the specified port.")
diff --git a/gnu/services/telephony.scm b/gnu/services/telephony.scm
index e8bfbc88c5..f099b60a0e 100644
--- a/gnu/services/telephony.scm
+++ b/gnu/services/telephony.scm
@@ -307,7 +307,7 @@ (define (jami-shepherd-services config)
          (dbus (jami-configuration-dbus config))
          (dbus-daemon (file-append dbus "/bin/dbus-daemon"))
          (accounts (jami-configuration-accounts config))
-         (declarative-mode? (not (unspecified? accounts))))
+         (declarative-mode? (not (eq? 'unspecified accounts))))
 
     (with-extensions (list guile-packrat ;used by guile-ac-d-bus
                            guile-ac-d-bus
@@ -649,7 +649,7 @@ (define (archive-name->username archive)
                                           account-details)
                            (let ((username (archive-name->username
                                             archive)))
-                             (when (not (unspecified? allowed-contacts))
+                             (when (not (eq? 'unspecified allowed-contacts))
                                ;; Reject calls from unknown contacts.
                                (set-account-details
                                 '(("DHT.PublicInCalls" . "false")) username)
@@ -659,7 +659,7 @@ (define (archive-name->username archive)
                                ;; Add allowed ones.
                                (for-each (cut add-contact <> username)
                                          allowed-contacts))
-                             (when (not (unspecified? moderators))
+                             (when (not (eq? 'unspecified moderators))
                                ;; Disable the 'AllModerators' property.
                                (set-all-moderators #f username)
                                ;; Remove all moderators.
diff --git a/tests/services/configuration.scm b/tests/services/configuration.scm
index 6268525317..9fea65ba58 100644
--- a/tests/services/configuration.scm
+++ b/tests/services/configuration.scm
@@ -151,9 +151,9 @@ (define-configuration config-with-maybe-string/no-serialization
   (not (defined? 'serialize-maybe-string)))
 
 (test-assert "maybe type, no default"
-  (unspecified?
-   (config-with-maybe-string/no-serialization-name
-    (config-with-maybe-string/no-serialization))))
+  (eq? 'unspecified
+       (config-with-maybe-string/no-serialization-name
+        (config-with-maybe-string/no-serialization))))
 
 (test-assert "maybe type, with default"
   (equal?
-- 
2.36.1

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

[Maxim Cournoyer]

Hi again,

I just sent a first patch.

Another idea would be to add a gexp compiler that would simply turn
*unimplemented* into '*unimplemented* (i.e., quote it).  I've never
added a gexp compiler so I don't know how easy/difficult that is.

Thanks,

Maxim

bug#56799: [PATCH v2] gexp: Handle *unspecified* as a gexp input.

[Maxim Cournoyer]

Fixes <https://issues.guix.gnu.org/56799>.

* guix/gexp.scm (gexp->sexp)[*unspecified*]: Quote value when encountering it.
---
 guix/gexp.scm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/guix/gexp.scm b/guix/gexp.scm
index ef92223048..e05aed6f32 100644
--- a/guix/gexp.scm
+++ b/guix/gexp.scm
@@ -1380,6 +1380,8 @@ (define* (reference->sexp ref #:optional native?)
                                 #:output output)))
         (($ <gexp-input> (? self-quoting? x))
          (return x))
+        (($ <gexp-input> (? unspecified? x))
+         (return (quote x)))
         (($ <gexp-input> x)
          (raise (condition (&gexp-input-error (input x)))))
         (x
-- 
2.36.1

bug#56799: [PATCH v2] gexp: Handle *unspecified* as a gexp input.

[Maxime Devos]

[-- Attachment #1.1.1: Type: text/plain, Size: 660 bytes --]

On 27-07-2022 22:20, Maxim Cournoyer wrote:

> Fixes <https://issues.guix.gnu.org/56799>.
>
> * guix/gexp.scm (gexp->sexp)[*unspecified*]: Quote value when encountering it.

I recommend writing a test case -- this patch does not actually do that.

Instead, it returns the symbol x.

In a ./pre-inst-env guix repl:

> scheme@(guix-user)> ,use (guix)
> scheme@(guix-user)> ,m (guix gexp)
> scheme@(guix gexp)> ,enter-store-monad
> store-monad@(guix gexp) (gexp->sexp #~(list #$*unspecified*) 
> "x86_64-linux" "aarch64-linux-gnu")
> $1 = (list x)
I don't know what exact semantics you had in mind but probably not this.

Greetings,
Maxime.

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

bug#56799: [PATCH v2] gexp: Handle *unspecified* as a gexp input.

[Maxim Cournoyer]

Hi,

Maxime Devos <maximedevos@telenet.be> writes:

> On 27-07-2022 22:20, Maxim Cournoyer wrote:
>
>> Fixes <https://issues.guix.gnu.org/56799>.
>>
>> * guix/gexp.scm (gexp->sexp)[*unspecified*]: Quote value when encountering it.
>
> I recommend writing a test case -- this patch does not actually do that.
>
> Instead, it returns the symbol x.
>
> In a ./pre-inst-env guix repl:
>
>> scheme@(guix-user)> ,use (guix)
>> scheme@(guix-user)> ,m (guix gexp)
>> scheme@(guix gexp)> ,enter-store-monad
>> store-monad@(guix gexp) (gexp->sexp #~(list #$*unspecified*)
>> "x86_64-linux" "aarch64-linux-gnu")
>> $1 = (list x)
> I don't know what exact semantics you had in mind but probably not this.

Indeed.  Fixed in v3 (which includes a test).

Thanks!

Maxim

bug#56799: [PATCH v3] gexp: Handle *unspecified* as a gexp input.

[Maxim Cournoyer]

Fixes <https://issues.guix.gnu.org/56799>.

* guix/gexp.scm (gexp->sexp)[*unspecified*]: Quote value when encountering it.
---
 guix/gexp.scm  | 2 ++
 tests/gexp.scm | 6 ++++++
 2 files changed, 8 insertions(+)

diff --git a/guix/gexp.scm b/guix/gexp.scm
index ef92223048..fe47a116f6 100644
--- a/guix/gexp.scm
+++ b/guix/gexp.scm
@@ -1380,6 +1380,8 @@ (define* (reference->sexp ref #:optional native?)
                                 #:output output)))
         (($ <gexp-input> (? self-quoting? x))
          (return x))
+        (($ <gexp-input> (? unspecified?))
+         (return '*unspecified*))
         (($ <gexp-input> x)
          (raise (condition (&gexp-input-error (input x)))))
         (x
diff --git a/tests/gexp.scm b/tests/gexp.scm
index 07e940ffdc..cad139fde7 100644
--- a/tests/gexp.scm
+++ b/tests/gexp.scm
@@ -1128,6 +1128,12 @@ (define (matching-input drv output)
     (run-with-store %store
       (lower-gexp #~(foo #$+)))))
 
+(test-equal "lower-gexp, *unspecified* input"
+  '(*unspecified*)
+  (lowered-gexp-sexp
+   (run-with-store %store
+     (lower-gexp #~(#$*unspecified*)))))
+
 (test-equal "lower-gexp, character literal"
   '(#\+)
   (lowered-gexp-sexp
-- 
2.36.1

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

[Maxim Cournoyer]

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

Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Fixes <https://issues.guix.gnu.org/56799>.
>
> * guix/gexp.scm (gexp->sexp)[*unspecified*]: Quote value when encountering it.

Sadly, this doesn't fix the jami service when using a partially defined
jami-account as part of the jami-configuration; the error is:

--8<---------------cut here---------------start------------->8---
In procedure for-each: Wrong type argument: *unspecified*
--8<---------------cut here---------------end--------------->8---

Here's what the generated shepherd-jami.scm file look like:

--8<---------------cut here---------------start------------->8---
#:start
(lambda args
  (define
    (delete-file-recursively/safe file)
    (let
        ((parent-directory
          (dirname file)))
      (if
       (eq?
        (quote symlink)
        (stat:type
         (stat parent-directory)))
       (error "abnormality detected; unexpected symlink found at" parent-directory)
       (delete-file-recursively file))))
  (when #t
    (catch #t
      (lambda
          ()
        (for-each
         (cut delete-file-recursively/safe <>)
         (quote
          ("/var/lib/jami/.cache/jami" "/var/lib/jami/.config/jami" "/var/lib/jami/.local/share/jami" "/var/lib/jami/accounts"))))
      (lambda args #t))
    (let*
        ((accounts-dir "/var/lib/jami/accounts/")
         (pwd
          (getpwnam "jami"))
         (user
          (passwd:uid pwd))
         (group
          (passwd:gid pwd)))
      (mkdir-p accounts-dir)
      (chown accounts-dir user group)
      (for-each
       (lambda
           (f)
         (let
             ((dest
               (string-append accounts-dir
                              (basename f))))
           (copy-file f dest)
           (chown dest user group)))
       (quote
        ("/gnu/store/14flr53fr0hs7mzfwn93kmyzrnb3fhjz-dummy-jami-account.gz")))))
  (define daemon-pid
    ((make-forkexec-constructor/container
      (quote
       ("/gnu/store/z7qlqkb0qwnpcs5kbbf2z2js0k1xgkbv-libjami-20220726.1515.da8d1da/libexec/jamid" "--persistent" "--debug"))
      #:mappings
      (list
       (file-system-mapping
        (source "/dev/log")
        (target source))
       (file-system-mapping
        (source "/var/lib/jami")
        (target source)
        (writable? #t))
       (file-system-mapping
        (source "/var/run/jami")
        (target source)
        (writable? #t))
       (file-system-mapping
        (source "/gnu/store/mjmpb4k2g21p7hyx9zq57p9xymbl16ac-nss-certs-3.71/etc/ssl/certs")
        (target "/etc/ssl/certs")))
      #:user "jami" #:group "jami" #:environment-variables
      (list
       (string-append "DBUS_SESSION_BUS_ADDRESS=" "unix:path=/var/run/jami/bus")
       "SSL_CERT_DIR=/etc/ssl/certs"))))
  (setenv "DBUS_SESSION_BUS_ADDRESS" "unix:path=/var/run/jami/bus")
  (with-retries 20 1
    (jami-service-available?))
  (when #t
    (let*
        ((jami-account-archives
          (map
           (cut string-append "/var/lib/jami/accounts/" <>)
           (scandir "/var/lib/jami/accounts/"
                    (lambda
                        (f)
                      (not
                       (member f
                               (quote
                                ("." ".."))))))))
         (usernames
          (map-in-order
           (cut add-account <>)
           jami-account-archives)))
      (define
        (archive-name->username archive)
        (list-ref usernames
                  (list-index
                   (lambda
                       (f)
                     (string-suffix?
                      (basename archive)
                      f))
                   jami-account-archives)))
      (for-each
       (lambda
           (archive allowed-contacts moderators account-details)
         (let
             ((username
               (archive-name->username archive)))
           (when
               (not
                (unspecified? allowed-contacts))
             (set-account-details
              (quote
               (("DHT.PublicInCalls" . "false")))
              username)
             (for-each
              (cut remove-contact <> username)
              (username->contacts username))
             (for-each
              (cut add-contact <> username)
              allowed-contacts))
           (when
               (not
                (unspecified? moderators))
             (set-all-moderators #f username)
             (for-each
              (cut set-moderator <> #f username)
              (username->moderators username))
             (for-each
              (cut set-moderator <> #t username)
              moderators))
           (set-account-details account-details username)))
       (quote
        ("/gnu/store/14flr53fr0hs7mzfwn93kmyzrnb3fhjz-dummy-jami-account.gz"))
       (quote
        (*unspecified*))
       (quote
        (*unspecified*))
       (quote
        ((("Account.rendezVous" . "true")
          ("Account.peerDiscovery" . "false")
          ("Account.hostname" . "bootstrap.me;fallback.another.host")
          ("RingNS.uri" . "https://my.name.server")))))))
  daemon-pid)
--8<---------------cut here---------------end--------------->8---

Can you spot where the problem is?

Attached is the extended test I've used to test.  You generate a VM
with:

--8<---------------cut here---------------start------------->8---
./pre-inst-env guix system vm --no-graphics \
  -e '(@@ (gnu tests telephony) %jami-os-provisioning-partial)
--8<---------------cut here---------------end--------------->8---

To test with a more hands-on approach.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-telephony-Add-a-Jami-test-for-a-partially-define.patch --]
[-- Type: text/x-patch, Size: 5267 bytes --]

From 4ccaa9109c67174d428512b16a5c1ee77e66f491 Mon Sep 17 00:00:00 2001
From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Date: Mon, 1 Aug 2022 00:49:07 -0400
Subject: [PATCH] gnu: telephony: Add a Jami test for a partially defined
 jami-account.

* gnu/tests/telephony.scm (%dummy-jami-account-partial): New variable.
(make-jami-os): Add a PARTIAL? argument and use it to select the jami-account
variant to use.
(%jami-os-provisioning-partial): New variable.
(run-jami-test): Add a PARTIAL? argument, and use it to select operating
system variant.  Skip allowed-contacts and moderators test when PARTIAL? is
true.
(%test-jami-provisioning-partial): New test.
---
 gnu/tests/telephony.scm | 49 +++++++++++++++++++++++++++++++++--------
 1 file changed, 40 insertions(+), 9 deletions(-)

diff --git a/gnu/tests/telephony.scm b/gnu/tests/telephony.scm
index 16ee313f69..b505f26e8d 100644
--- a/gnu/tests/telephony.scm
+++ b/gnu/tests/telephony.scm
@@ -31,7 +31,8 @@ (define-module (gnu tests telephony)
   #:use-module (guix gexp)
   #:use-module (guix modules)
   #:export (%test-jami
-            %test-jami-provisioning))
+            %test-jami-provisioning
+            %test-jami-provisioning-partial))
 
 ;;;
 ;;; Jami daemon.
@@ -67,7 +68,18 @@ (define %dummy-jami-account (jami-account
                                                     "fallback.another.host"))
                              (name-server-uri "https://my.name.server")))
 
-(define* (make-jami-os #:key provisioning?)
+;;; Like %dummy-jami-account, but with allowed-contacts and moderators left
+;;; unset (thus taking the value *unspecified*).
+(define %dummy-jami-account-partial
+  (jami-account
+   (archive %dummy-jami-account-archive)
+   (rendezvous-point? #t)
+   (peer-discovery? #f)
+   (bootstrap-hostnames '("bootstrap.me"
+                          "fallback.another.host"))
+   (name-server-uri "https://my.name.server")))
+
+(define* (make-jami-os #:key provisioning? partial?)
   (operating-system
     (host-name "jami")
     (timezone "America/Montreal")
@@ -87,7 +99,10 @@ (define* (make-jami-os #:key provisioning?)
                               (if provisioning?
                                   (jami-configuration
                                    (debug? #t)
-                                   (accounts (list %dummy-jami-account)))
+                                   (accounts
+                                    (list (if partial?
+                                              %dummy-jami-account-partial
+                                              %dummy-jami-account))))
                                   (jami-configuration
                                    (debug? #t))))
                      (service dbus-root-service-type)
@@ -109,12 +124,18 @@ (define %jami-os
 (define %jami-os-provisioning
   (make-jami-os #:provisioning? #t))
 
-(define* (run-jami-test #:key provisioning?)
-  "Run tests in %JAMI-OS.  When PROVISIONING? is true, test the
-accounts provisioning feature of the service."
+(define %jami-os-provisioning-partial
+  (make-jami-os #:provisioning? #t #:partial? #t))
+
+(define* (run-jami-test #:key provisioning? partial?)
+  "Run tests in %JAMI-OS.  When PROVISIONING? is true, test the accounts
+provisioning feature of the service.  When PARTIAL? is #t, some fields of the
+jami account used as part of the jami configuration are left *unspecified*."
   (define os (marionette-operating-system
               (if provisioning?
-                  %jami-os-provisioning
+                  (if partial?
+                      %jami-os-provisioning-partial
+                      %jami-os-provisioning)
                   %jami-os)
               #:imported-modules '((gnu services herd)
                                    (guix combinators))))
@@ -202,7 +223,7 @@ (define marionette
                                                      "Account.username")))))))
                marionette))
 
-            (unless #$provisioning? (test-skip 1))
+            (unless #$(and provisioning? (not partial?)) (test-skip 1))
             (test-assert "jami accounts provisioning, allowed-contacts"
               (marionette-eval
                '(begin
@@ -224,7 +245,7 @@ (define marionette
                       (assert (lset= string-ci=? contacts '#$%allowed-contacts)))))
                marionette))
 
-            (unless #$provisioning? (test-skip 1))
+            (unless #$(and provisioning? (not partial?)) (test-skip 1))
             (test-assert "jami accounts provisioning, moderators"
               (marionette-eval
                '(begin
@@ -341,3 +362,13 @@ (define %test-jami-provisioning
    (name "jami-provisioning")
    (description "Provisioning test for the jami service.")
    (value (run-jami-test #:provisioning? #t))))
+
+;;; Thi test verifies that <jami-account> values can be left unspecified
+;;; without causing any issue (see: https://issues.guix.gnu.org/56799).
+(define %test-jami-provisioning-partial
+  (system-test
+   (name "jami-provisioning-partial")
+   (description "Provisioning test for the jami service, when some of the
+'maybe' fields aren't provided (such that their value end up being
+*unspecified*.")
+   (value (run-jami-test #:provisioning? #t #:partial? #t))))
-- 
2.36.1


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


Thanks,

Maxim

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

[Maxime Devos]

[-- Attachment #1.1.1: Type: text/plain, Size: 563 bytes --]


On 01-08-2022 07:08, Maxim Cournoyer wrote:
>         (quote
>          ("/gnu/store/14flr53fr0hs7mzfwn93kmyzrnb3fhjz-dummy-jami-account.gz"))
>         (quote
>          (*unspecified*))
>         (quote
>          (*unspecified*))

These lines look suspicious to me -- should they have done (list 
*unspecified*) instead of '(*unspecified*)?

The former uses the actual unspecified object, the latter uses the 
symbol '*unspecified*' that merely happens to be the name of a variable 
the unspecified object is bound to.

Greetings,
Maxime.


[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

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

[Maxim Cournoyer]

Hi,

Maxime Devos <maximedevos@telenet.be> writes:

> On 01-08-2022 07:08, Maxim Cournoyer wrote:
>>         (quote
>>          ("/gnu/store/14flr53fr0hs7mzfwn93kmyzrnb3fhjz-dummy-jami-account.gz"))
>>         (quote
>>          (*unspecified*))
>>         (quote
>>          (*unspecified*))
>
> These lines look suspicious to me -- should they have done (list
> *unspecified*) instead of '(*unspecified*)?
>
> The former uses the actual unspecified object, the latter uses the
> symbol '*unspecified*' that merely happens to be the name of a
> variable the unspecified object is bound to.

Indeed; '*unspecified ain't the same as *unspecified, and it isn't
magically evaluated after being lowered in a G-exp:

--8<---------------cut here---------------start------------->8---
(map unspecified?
     (quote (*unspecified* *unspecified* *unspecified*
             *unspecified* *unspecified*)))
$1 = (#f #f #f #f #f)             
--8<---------------cut here---------------end--------------->8---

So in essence the idea to simply quote *unspecified* from patch v3 is
flawed.

v1 works though (using a symbol), so unless someone has a better idea
right now I'm thinking that using 'unset instead of *unspecified* may be
the simplest working solution.

Thanks,

Maxim

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

[Ludovic Courtès]

Hi!

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

> Fixes <https://issues.guix.gnu.org/56799>.
>
> * guix/gexp.scm (gexp->sexp)[*unspecified*]: Quote value when encountering it.

I think we need to take a step back.  Overall, I’m reluctant to
modifying a core primitive like ‘gexp->sexp’ “just” to address this
higher-level problem that we have.

> +        (($ <gexp-input> (? unspecified?))
> +         (return '*unspecified*))

Incidentally, this is “unhygienic”, meaning that it relies on
‘*unspecified*’ being bound to what we want.  For example, if I do this:

  #~(let ((*unspecified* 'hi!))
      #$*unspecified*)

… I won’t get the desired output.

Ludo’, who now goes back to the beginning of the thread.  :-)

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

[Maxim Cournoyer]

Hi,

Tobias Geerinckx-Rice <me@tobias.gr> writes:

> Hi Maxim,
>
> Maxim Cournoyer 写道：
>> For some background reading, see [0].
>
> Thanks for the well-thought-out reply, and sharing this interesting
> link!
>
> Now, it's just the musings of one person, but now I think I do agree
> with (what I think is) the underlying vision: to hush up *unspecified*
> and sneakily replace it with true nothingness.  OK, I can live with
> that.  :-)
>
>> I think the semantic of the language is that it is to be used as the
>> lack of a return value from a procedure or syntax, e.g.:
>>
>> (unspecified? (if #f 'one-arm-if)) -> #t
>
> Well… in the above context I'd hesitate to even imply
> ‘semantics’. It's like undefined behaviour in C.  Ascribe it meaning
> at your peril.  Otherwise, point taken.
>
>> Having 'unspecified?' even defined in Guile seems to go against that
>> idea; perhaps because Wingo themselves seems to disagree in [0].
>
> Agreed.  *This* was one of my reasons for supporting (field
> *unspecified*), so it's nice to have it validated, even if it is
> rejected.
>
>> I'm also thinking 'unspecified being too close to *unspecified* is
>> probably going to cause confusion down the line.  Reverting to the
>> originally used 'disabled may be the lesser evil.
>
> Ah, here I can concentrate all my previous disagreement: hell no :-)
>
> It is the worstest evil; literally anything is better than
> (enable-foo? 'disabled) defaulting to #t.
>
> Bikeshed this all y'all want, but 'default or 'unset or 'whatever are
> miles better.

OK.  The v2 and v3 idea ended up not working, among lesser issues :-).

So I went with v1, renaming the default value to 'unset; see commit
a2b89a3319dc1d621c546855f578acae5baaf6da.  Thanks for the naming
suggestions.

I also added a 'jami-provisioning-partial' system test to ensure it
doesn't regress again if we decide to revisit this.

Thanks,

Closing.

Maxim

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

[bokr]

Hi,

On +2022-07-27 14:31:32 -0400, Maxim Cournoyer wrote:
> Hi,
> 
> Tobias Geerinckx-Rice <me@tobias.gr> writes:
> 
> > Hi Maxim,
> >
> > Maxim Cournoyer 写道：
> >> I'd suggest we revisit 8cb1a49a3998c39f315a4199b7d4a121a6d66449 to
> >> use
> >> 'unspecified (the symbol) instead of *unspecified*, which *can* be
> >> serialized without any fuss in gexps.
> >
> > Bah.  Could we provide our own reader?
> >
> > I'd much rather this be addressed in Guile (or failing that,
> > transparently by Guix) than have to deal with some magical
> > symbol. IIRC that was the argument for using *unspecified* in the
> > first place, and I think it makes sense.
> >
> > This looks more like an unexplored oversight than a well-reasoned
> > restriction to me.
> 
> This was my original impression, but thinking more about it, it became
> apparent that *unspecified* is well, unspecified and shouldn't be relied
> on by people to be something well defined.  For some background reading,
> see [0].  So it seems wrong in Scheme to actively set things to
> *unspecified*, and give a specific meaning to that.
>
> I think the semantic of the language is that it is to be used as the
> lack of a return value from a procedure or syntax, e.g.:
> 
> (unspecified? (if #f 'one-arm-if)) -> #t
> 
> Having 'unspecified?' even defined in Guile seems to go against that
> idea; perhaps because Wingo themselves seems to disagree in [0].
> 
> I'm also thinking 'unspecified being too close to *unspecified* is
> probably going to cause confusion down the line.  Reverting to the
> originally used 'disabled may be the lesser evil.
> 
> Other thoughts?
> 
> Thanks,
> 
> Maxim
> 
> [0]  https://scheme-reports.scheme-reports.narkive.com/QSQtJSAh/unspecified-values
> 
> 
>

Lots of systems are dealing with this issue, it seems, judging from
[1] https://en.wikipedia.org/wiki/Bottom_type

I think the problem is you really need a tuple to return both data and metadata
unambiguously from anything that produces a result (or not, which is a result).
Something like read-delimited with the 'split option, or using catch.

Personally, if I were designing a language :), my goal would be to have
nothing unspecified, and no undefined behaviour outside of physical failures ;-)

*unspecified* seems me like an ok word for the unasserted/high-impedance
state of tri-state memory address bus electronic logic,
but IMO the example above
--8<---------------cut here---------------start------------->8---
> (unspecified? (if #f 'one-arm-if)) -> #t
--8<---------------cut here---------------end--------------->8---

is not nice. (A nit, but For one thing "specified?" ought to be the question IMO,
if you are ging to have that concept, not "unspecified?" :)

What about using characters from some private upper unicode section
to represent various kinds of unspecified things? E.g.,
as guile named chars,

#\unspecified_function_retval
#\unspecified_function_error
#\unspecified_macro_err
#\unspecified_exception

#\nil or #\not_an_object -- or #\nao -- can't use #\n :)
#\paradox  -- e.g., (eval-nl-string "this sentence is lying")
#\nonsense -- e.g. when a question is based on false premises
             (eval-nl-string "Bob is bareheaded: Bob, is your hat too tight?")   

             Hm, one could argue that (+ "ab" "cd") could be based on the false
             premise that + was overloaded like
--8<---------------cut here---------------start------------->8---
scheme@(guile-user) [1]> (let*( (+ string-append) (sum (+ "ab" "cd"))) sum)
$8 = "abcd"
--8<---------------cut here---------------end--------------->8---
             and if it wasn't, should return #\nonsense :)
             (though maybe as part of the exception, which is practical for debugging etc :)
#\
#\guix_bottom -- a private unicode rather than U+22A5 which could be
                 returned as a valid character value by some functon.
--8<---------------cut here---------------start------------->8---
$ echo -en "\u22a5"|unicode-info
"⊥":
    glyph  codepoint .....int  name...
    _⊥_     +U0022a5     8869  UP TACK  
--8<---------------cut here---------------end--------------->8---

Well, hope you can extract something useful from the above :)

BTW, I didn't get far via the link [0] :(
--8<---------------cut here---------------start------------->8---
🤖 Hungry for data? 🤖

   As you guessed, this page is to confirm your affiliation to the human race.

   about - legalese

   Loading...
--8<---------------cut here---------------end--------------->8---
Ok machine, you identified me as human, and kept me out. Happy?
No, I know, machines can only fake that.
--
Regards,
Bengt Richter

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

[Maxime Devos]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 1080 bytes --]


On 28-07-2022 06:55, bokr@bokr.com wrote:
> Lots of systems are dealing with this issue, it seems, judging from
> [1]https://en.wikipedia.org/wiki/Bottom_type

I don't think it's a bottom type -- *unspecified* _is_ a value, so if we 
assign a type to it, it is inhabited, and hence not a bottom type. I

> What about using characters from some private upper unicode section
> to represent various kinds of unspecified things? E.g.,
> as guile named chars,
>
> #\unspecified_function_retval
> #\unspecified_function_error
> #\unspecified_macro_err
> #\unspecified_exception
  suppose you could subdivide the *unspecified* value, but why characters?

I think it would be better to:

  * gradually move away from *unspecified* to (values)
  * and this way, gradually change the meaning of *unspecified* from "an
    unspecified value" to 'an atom you can do with as you want"
  * after this, unspecified? and making #<unspecified> readable by the
    reader aren't weird anymore

though more something for guile-devel@ I suppose.

Greetings,
Maxime


[-- Attachment #1.1.1.2: Type: text/html, Size: 1849 bytes --]

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

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

[Maxim Cournoyer]

Hi Maxime,

Maxime Devos <maximedevos@telenet.be> writes:

[...]

> I think it would be better to:
>
>  * gradually move away from *unspecified* to (values)
>  * and this way, gradually change the meaning of *unspecified* from "an
>    unspecified value" to 'an atom you can do with as you want"
>  * after this, unspecified? and making #<unspecified> readable by the
>    reader aren't weird anymore

Or perhaps simply introduce a #none or other special syntax similar to
None in Python, that would serve a different purpose that *unspecified*,
and be orthogonal to it.  It seems that'd be less confusing and simpler.

Thanks,

Maxim

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

[Ludovic Courtès]

Hi!

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

> Since commit 8cb1a49a3998c39f315a4199b7d4a121a6d66449, the
> define-configuration machinery in (gnu services configuration) uses
> *unspecified* instead of 'disabled for an unspecified field value.

As Attila wrote, the rationale as discussed in
<https://issues.guix.gnu.org/54674> was to specifically use a “special”
value without a read syntax in lieu of a symbol like 'disabled.

> While this is indeed an improvement in readability, it introduces an
> extra complication: because this new value is not self-quoting, it
> cannot be used as is in G-Exps, and values using it must be carefully
> expanded outside the gexp context, which is error prone.

Could you give a simple example of how this can happen?

In my experience, one would use ‘define-maybe’ and appropriate field
serializers such that *unspecified* never goes through.  Previously
you’d check for (eq? x 'disabled) and now you just check for
(unspecified? x).

Thanks,
Ludo’.

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

[Maxim Cournoyer]

Hi Ludo,

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

> Hi!
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> Since commit 8cb1a49a3998c39f315a4199b7d4a121a6d66449, the
>> define-configuration machinery in (gnu services configuration) uses
>> *unspecified* instead of 'disabled for an unspecified field value.
>
> As Attila wrote, the rationale as discussed in
> <https://issues.guix.gnu.org/54674> was to specifically use a “special”
> value without a read syntax in lieu of a symbol like 'disabled.
>
>> While this is indeed an improvement in readability, it introduces an
>> extra complication: because this new value is not self-quoting, it
>> cannot be used as is in G-Exps, and values using it must be carefully
>> expanded outside the gexp context, which is error prone.
>
> Could you give a simple example of how this can happen?
>
> In my experience, one would use ‘define-maybe’ and appropriate field
> serializers such that *unspecified* never goes through.  Previously
> you’d check for (eq? x 'disabled) and now you just check for
> (unspecified? x).

Yes, I understand that.  What changed is that previously you could have
the configuration serialized and used on the service side, which is what
using *unspecified* made impossible.

Granted, few services outside of Jami probably made use of this, but it
was nevertheless a useful property.

Thanks,

Maxim

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

[Ludovic Courtès]

Hi,

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

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Hi!
>>
>> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>>
>>> Since commit 8cb1a49a3998c39f315a4199b7d4a121a6d66449, the
>>> define-configuration machinery in (gnu services configuration) uses
>>> *unspecified* instead of 'disabled for an unspecified field value.
>>
>> As Attila wrote, the rationale as discussed in
>> <https://issues.guix.gnu.org/54674> was to specifically use a “special”
>> value without a read syntax in lieu of a symbol like 'disabled.
>>
>>> While this is indeed an improvement in readability, it introduces an
>>> extra complication: because this new value is not self-quoting, it
>>> cannot be used as is in G-Exps, and values using it must be carefully
>>> expanded outside the gexp context, which is error prone.
>>
>> Could you give a simple example of how this can happen?
>>
>> In my experience, one would use ‘define-maybe’ and appropriate field
>> serializers such that *unspecified* never goes through.  Previously
>> you’d check for (eq? x 'disabled) and now you just check for
>> (unspecified? x).
>
> Yes, I understand that.  What changed is that previously you could have
> the configuration serialized and used on the service side, which is what
> using *unspecified* made impossible.

Do you have an example?  Even on the service side, I imagine one could
check for ‘unspecified?’ just like one would check for 'disabled, no?

> Granted, few services outside of Jami probably made use of this, but it
> was nevertheless a useful property.

I don’t know of any.

Having spent time reviewing the original change that Attila proposed and
then chiming in on this issue, I would have hoped for a longer
discussion before enacting the change in
a2b89a3319dc1d621c546855f578acae5baaf6da.

In addition to these issues around the process, I think we should strive
for more stability.  One of the reasons it took time to review
<https://issues.guix.gnu.org/54674> is that interface changes are a
commitment.  Now commit a2b89a3319dc1d621c546855f578acae5baaf6da
introduces a second interface change for reasons that are unclear to me
(if the conclusion had been to revert, I’d have favored an actual revert
rather than introducing 'unset).

How should we move forward?

Thanks,
Ludo’.

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

[bokr]

Hi,

On +2022-08-02 09:31:14 +0200, Ludovic Courtès wrote:
> Hi,
> 
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
> 
> > Ludovic Courtès <ludo@gnu.org> writes:
> >
> >> Hi!
> >>
> >> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
> >>
> >>> Since commit 8cb1a49a3998c39f315a4199b7d4a121a6d66449, the
> >>> define-configuration machinery in (gnu services configuration) uses
> >>> *unspecified* instead of 'disabled for an unspecified field value.
> >>
> >> As Attila wrote, the rationale as discussed in
> >> <https://issues.guix.gnu.org/54674> was to specifically use a “special”
> >> value without a read syntax in lieu of a symbol like 'disabled.
> >>
> >>> While this is indeed an improvement in readability, it introduces an
> >>> extra complication: because this new value is not self-quoting, it
> >>> cannot be used as is in G-Exps, and values using it must be carefully
> >>> expanded outside the gexp context, which is error prone.
> >>
> >> Could you give a simple example of how this can happen?
> >>
> >> In my experience, one would use ‘define-maybe’ and appropriate field
> >> serializers such that *unspecified* never goes through.  Previously
> >> you’d check for (eq? x 'disabled) and now you just check for
> >> (unspecified? x).
> >
> > Yes, I understand that.  What changed is that previously you could have
> > the configuration serialized and used on the service side, which is what
> > using *unspecified* made impossible.
> 
> Do you have an example?  Even on the service side, I imagine one could
> check for ‘unspecified?’ just like one would check for 'disabled, no?
> 
> > Granted, few services outside of Jami probably made use of this, but it
> > was nevertheless a useful property.
> 
> I don’t know of any.
> 
> Having spent time reviewing the original change that Attila proposed and
> then chiming in on this issue, I would have hoped for a longer
> discussion before enacting the change in
> a2b89a3319dc1d621c546855f578acae5baaf6da.
> 
> In addition to these issues around the process, I think we should strive
> for more stability.  One of the reasons it took time to review
> <https://issues.guix.gnu.org/54674> is that interface changes are a
> commitment.  Now commit a2b89a3319dc1d621c546855f578acae5baaf6da
> introduces a second interface change for reasons that are unclear to me
> (if the conclusion had been to revert, I’d have favored an actual revert
> rather than introducing 'unset).
> 
> How should we move forward?
>

My 2¢ :

Maybe separate commmit churn more formally into a
release candidate series like Linus does for linux kernel,
and have a long term stable guix only get what is agreed
with solid consensus?

AND, importantly: where issues involve subtleties
of abstract entities vs their concrete representations,
make sure this is clearly documented in the commit rationale,
e.g., maybe using denottional semantics[1] like r5rs ?

[1]:  <https://en.wikipedia.org/wiki/Denotational_semantics>

:)

> Thanks,
> Ludo’.
>
--
Regards,
Bengt Richter
OT PS: Has Boot-to-guile been updated by anyone?
Kudos for the original! :) A RISCV qemu image? :)

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

[Maxim Cournoyer]

Hi Ludovic,

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

[...]

>> Granted, few services outside of Jami probably made use of this, but it
>> was nevertheless a useful property.
>
> I don’t know of any.

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:

--8<---------------cut here---------------start------------->8---
(define-configuration/no-serialization xvnc-configuration
  (xvnc
   (file-like tigervnc-server)
   "The package that provides the Xvnc binary.")
  (display-number
   (number 0)
   "The display number used by Xvnc.  You should set this to a number not
already used a Xorg server.")
  (geometry
   (string "1024x768")
   "The size of the desktop to be created.")
  (depth
   (color-depth 24)
   "The pixel depth in bits of the desktop to be created.  Accepted values are
16, 24 or 32.")
  (port
   maybe-port
   "The port on which to listen for connections from viewers.  When left
unspecified, it defaults to 5900 plus the display number.")
  (ipv4?
   (boolean #t)
   "Use IPv4 for incoming and outgoing connections.")
  (ipv6?
   (boolean #t)
   "Use IPv6 for incoming and outgoing connections.")
  (password-file
   maybe-string
   "The password file to use, if any.  Refer to vncpasswd(1) to learn how to
generate such a file.")
  (frame-rate
   (number 60)
   "The maximum number of updates per second sent to each client.")
  (security-types
   (security-types (list "None"))
   (format #f "The allowed security schemes to use for incoming connections.
The default is \"None\", which is safe given that Xvnc is configured to
authenticate the user via the display manager, and only for local connections.
Accepted values are any of the following: ~s" %security-types))
  (localhost?
   (boolean #t)
   "Only allow connections from the same machine.  It is set to #true by
default for security, which means SSH or another secure means should be used
to expose the remote port.")
  (log-level
   (log-level 30)
   "The log level, a number between 0 and 100, 100 meaning most verbose
output.  The log messages are output to syslog.")
  (extra-options
   (strings '())
   "This can be used to provide extra Xvnc options not exposed via this
<xvnc-configuration> record."))

[...]

(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)))
         (port* (or port (+ 5900 display-number)))
         (inaddr (if (xvnc-configuration-localhost? config)
                     INADDR_LOOPBACK
                     INADDR_ANY))
         (in6addr (if (xvnc-configuration-localhost? config)
                      IN6ADDR_LOOPBACK
                      IN6ADDR_ANY))
         (ipv4-socket (and (xvnc-configuration-ipv4? config)
                           (make-socket-address AF_INET inaddr
                                                port*)))
         (ipv6-socket (and (xvnc-configuration-ipv6? config)
                           (make-socket-address AF_INET6 in6addr
                                                port*))))
    (shepherd-service
     (provision '(xvnc vncserver))
     (documentation "Run the Xvnc server.")
     (requirement '(networking syslogd))
     (start #~(make-inetd-constructor
               #$(xvnc-configuration->command-line-arguments config)
               `(,@(if #$ipv4-socket
                       (list (endpoint #$ipv4-socket))
                       '())
                 ,@(if #$ipv6-socket
                       (list (endpoint #$ipv6-socket))
                       '()))))
     (stop #~(make-inetd-destructor)))))
--8<---------------cut here---------------end--------------->8---

As you can see, the *unspecified* values need to be carefully massaged
out before starting to assemble the G-exp, as they aren't valid G-Exp
inputs.

> Having spent time reviewing the original change that Attila proposed and
> then chiming in on this issue, I would have hoped for a longer
> discussion before enacting the change in
> a2b89a3319dc1d621c546855f578acae5baaf6da.

OK.

> In addition to these issues around the process, I think we should strive
> for more stability.  One of the reasons it took time to review
> <https://issues.guix.gnu.org/54674> is that interface changes are a
> commitment.  Now commit a2b89a3319dc1d621c546855f578acae5baaf6da
> introduces a second interface change for reasons that are unclear to me
> (if the conclusion had been to revert, I’d have favored an actual revert
> rather than introducing 'unset).

I like to think of *unspecified* or 'unset as an uninteresting
implementation detail that shouldn't be part of the public API.  It's an
implementation detail of the 'maybe' types/predicates generator of the
(gnu services configuration) machinery.  Perhaps we could introduce a
'maybe-set?' predicate to check them, to avoid leaking the
implementation detail (the 'unset symbol).  Also, after reading more on
the topic, it became clear to me that *unspecified* is not meant as a
value to be actively used by programmers in Scheme; it seems it's rather
a value meant to be returned when the behavior of a procedure is
unspecified [0].  Why 'unspecified?' even exist in Guile I don't know, I
suppose because of some disagreement on the matter, as hinted in the
previous link.

The reason I did not simply revert was because the change also
introduced something useful in the same code change, which is to lift
the requirement to specify a default value for maybe-* fields.

> How should we move forward?

Perhaps we can discuss any issues I may have missed that arise from the
this change, or possible future directions that would improve things.

Thanks,

Maxim

[0]  https://scheme-reports.scheme-reports.narkive.com/QSQtJSAh/unspecified-values

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

[Ludovic Courtès]

Howdy,

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

>>> Granted, few services outside of Jami probably made use of this, but it
>>> was nevertheless a useful property.
>>
>> I don’t know of any.
>
> 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.

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.

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.

WDYT?

>> In addition to these issues around the process, I think we should strive
>> for more stability.  One of the reasons it took time to review
>> <https://issues.guix.gnu.org/54674> is that interface changes are a
>> commitment.  Now commit a2b89a3319dc1d621c546855f578acae5baaf6da
>> introduces a second interface change for reasons that are unclear to me
>> (if the conclusion had been to revert, I’d have favored an actual revert
>> rather than introducing 'unset).
>
> I like to think of *unspecified* or 'unset as an uninteresting
> implementation detail that shouldn't be part of the public API.

It is part of the API for people who write services (but it’s definitely
an implementation detail for someone who just uses services).

> It's an implementation detail of the 'maybe' types/predicates
> generator of the (gnu services configuration) machinery.  Perhaps we
> could introduce a 'maybe-set?' predicate to check them, to avoid
> leaking the implementation detail (the 'unset symbol).  Also, after
> reading more on the topic, it became clear to me that *unspecified* is
> not meant as a value to be actively used by programmers in Scheme; it
> seems it's rather a value meant to be returned when the behavior of a
> procedure is unspecified [0].  Why 'unspecified?' even exist in Guile
> I don't know, I suppose because of some disagreement on the matter, as
> hinted in the previous link.

Right, even cleaner would be to have a specific value for this, like:

  (define &default-value (list 'default)) ;or something w/o read syntax
  (define (default? x) (eq? x &default-value))

But IMO it’s OK.

> The reason I did not simply revert was because the change also
> introduced something useful in the same code change, which is to lift
> the requirement to specify a default value for maybe-* fields.

Right, that makes sense to me.

>> How should we move forward?
>
> Perhaps we can discuss any issues I may have missed that arise from the
> this change, or possible future directions that would improve things.

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.

Thanks for taking the time to discuss!

Ludo’.

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

[Ludovic Courtès]

Hello,

I just hit this while running ‘guix home reconfigure’ from
f0ae9da3210cc6d87ca519545203daf9751f3465:

  home-config.scm:24:16: error: invalid value unset for field 'address-family'

It’s an ‘openssh-host’ record:

  (openssh-host
    (name "xyz")
    (host-name "xyz.example.org")
    (user "ludo"))

⇒

  #<<openssh-host> %location: #f name: "xyz" host-name: "xyz.example.org" address-family: #<unspecified> identity-file: #<unspecified> port: #<unspecified> user: "ludo" forward-x11?: #f forward-x11-trusted?: #f forward-agent?: #f compression?: #f proxy-command: #<unspecified> host-key-algorithms: #<unspecified> accepted-key-types: #<unspecified> extra-content: "">

We have a new bug to address.

Ludo’.

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

[Attila Lendvai]

i got back online...


> 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.


i agree.


> 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 agree with this, too.

also, seems like it didn't register in this discussion, so i press it once again: if the default/unspecified value is a symbol (any symbol), then those configuration fields, whose type is set to be of symbol, will not error when they are left unspecified. (see the FIELD-SANITIZER: it simply does a (IF (PRED VALUE) ...) check, which doesn't fail because 'UNSET satisfies SYMBOL?). i should have added a unit test for this...

i think moving back from *UNSPECIFIED* to using 'UNSET only has drawbacks. and if it works (read: it doesn't error out while guix is deploying the config), then it probably produces illegal config files by serializing the unspecified value into an "unset" string in the config files.

i'm obviously not aware of the entire complexity here, othrwise there wouldn't have remained a bug... but regardless of the actual API/value used, i don't see how any of this could work without the service code explicitly checking for the unspecified value for fields that have a maybe type (i.e. whose type allows the value to be unspecified). i think using a symbol instead of *unspecified* only pushes the appearance of the symptoms farther away both in time and space (otherwise there should have been a trivial fix to this without changing *unspecified* back to 'unset).

either way, if there was a failing test case that i could run locally, then i would have been more than happy to fix it... now i'm only dismayed that all my service code, and my entire channel got broken. seems like i went offline in the worst two weeks.

sidenote: what srfi-189 does is basically introduce such a special value that we are trying to hone in on here (i.e. Nothing, which is implemented as a record instance) in a standardized way, and also an API to deal with this special value in various different contexts (i.e. it exports stuff like MAYBE-IF, MAYBE-FOLD, MAYBE-AND, etc).

https://srfi.schemers.org/srfi-189/srfi-189.html

--
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“Although teachers do care and do work very, very hard, the institution is psychopathic-it has no conscience. It rings a bell and the young man in the middle of writing a poem must close his notebook and move to a different cell where he must memorize that humans and monkeys derive from a common ancestor.”
	— John Taylor Gatto (1935–), 'Dumbing Us Down: The Hidden Curriculum of Compulsory Education' (1992)

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

[Attila Lendvai]

> i'm obviously not aware of the entire complexity here, othrwise
> there wouldn't have remained a bug... but regardless of the actual
> API/value used, i don't see how any of this could work without the
> service code explicitly checking for the unspecified value for
> fields that have a maybe type (i.e. whose type allows the value to
> be unspecified). i think using a symbol instead of unspecified only
> pushes the appearance of the symptoms farther away both in time and
> space (otherwise there should have been a trivial fix to this
> without changing unspecified back to 'unset).


sorry, i was wrong/slow here^.

i think i finally understand what the original issue was that triggered the rollback:

the *UNSPECIFIED* value cannot get through the GExp serialize/deserialize operation between the host/builder (or how do we call it?) and Shepherd. the checks in the service code that handle the unspecified field values only happen when Shepeherd is executing the deserialized GExp's.

the fix i would propose is to smarten up GExp serialization to handle whichever value we use as the marker, be it 1) *UNSPECIFIED*, or 2) Nothing from srfi-189, or 3) a record instance that we define/instantiate ourselves.

i don't recommend 3). we should rather use srfi-189 then, because it sandardizes widely known concepts.

so, would you accept a patch that implements 1) or 2) ?

2) has non-trivial uncertainties/complexities in making srfi-189 available in all the required contexts, and not introducing any bootstrap related issues in the process. because of that i would recommend getting to 2) by first implementing 1) and then working towards 2) -- if we want to use srfi-189 at all, that is.

--
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“What's done to children, they will do to society.”
	— Karl A. Menninger (http://psychohistory.com/)

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

[Maxim Cournoyer]

Hi Attila,

Attila Lendvai <attila@lendvai.name> writes:

>> i'm obviously not aware of the entire complexity here, othrwise
>> there wouldn't have remained a bug... but regardless of the actual
>> API/value used, i don't see how any of this could work without the
>> service code explicitly checking for the unspecified value for
>> fields that have a maybe type (i.e. whose type allows the value to
>> be unspecified). i think using a symbol instead of unspecified only
>> pushes the appearance of the symptoms farther away both in time and
>> space (otherwise there should have been a trivial fix to this
>> without changing unspecified back to 'unset).
>
>
> sorry, i was wrong/slow here^.
>
> i think i finally understand what the original issue was that triggered the rollback:
>
> the *UNSPECIFIED* value cannot get through the GExp
> serialize/deserialize operation between the host/builder (or how do we
> call it?) and Shepherd. the checks in the service code that handle the
> unspecified field values only happen when Shepeherd is executing the
> deserialized GExp's.

That's the issue, yes!  the *unspecified* value cannot cross the
host/build border, which forces someone to pre-compute all things before
serialization happens, which is inconvenient at best in some scenarios I
hinted at (a recent one is defining inetd-style services, which
endpoints need to be computed on the target (inside the service gexp)).

> the fix i would propose is to smarten up GExp serialization to handle
> whichever value we use as the marker, be it 1) *UNSPECIFIED*, or 2)
> Nothing from srfi-189, or 3) a record instance that we
> define/instantiate ourselves.

That's what I first attempted, without success.  That *unspecified*
expands to some random object on every usage seems meant to discourage
its usage as a specific value (its name, too!)

> i don't recommend 3). we should rather use srfi-189 then, because it
> sandardizes widely known concepts

srfi-189 sounds fancy, but how would it be better than the 'unset
symbol?  Could you please restate the problems faced when using a
symbol?  Would it be lowerable into Gexp, unlike *unspecified*?  To me,
in Guile this means having a readily implemented reader syntax.

> so, would you accept a patch that implements 1) or 2) ?

As I mentioned earlier, the specific value used for a maybe value is an
implementation detail, or should be made one, in my opinion.  So it
doesn't matter to me, as long as it makes writing services convenient
and passes all the tests (I recently augmented the jami-service-type
test which such a case to avoid any regressions).

Thanks,

Maxim

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

[Maxim Cournoyer]

Hi Attila,

Attila Lendvai <attila@lendvai.name> writes:

[...]

> also, seems like it didn't register in this discussion, so i press it
> once again: if the default/unspecified value is a symbol (any symbol),
> then those configuration fields, whose type is set to be of symbol,
> will not error when they are left unspecified. (see the
> FIELD-SANITIZER: it simply does a (IF (PRED VALUE) ...) check, which
> doesn't fail because 'UNSET satisfies SYMBOL?). i should have added a
> unit test for this...

OK, I've reread this, and it is indeed a risk, that 'unset could leak in
the case of a serializable configuration making use of a maybe-value
field of type maybe-symbol.  I've added the unit test suggested as
97cb43e732a38758c95b7caf3963507188d011cf (currently marked as 'expected
to fail').  Luckily no current service uses that.

I think the tension between serializable vs non-serializable unspecified
values comes from the two kind of usages of define-configuration: one is
to generate config files, in which unspecified means "nothing", and
shouldn't touch disk, while in the case it's used as a more general data
type (define-configuration/no-serialization), there is value in being
able to lower that data type on the build/target so that things can be
lazily computed where needed, without loss of information.

We should add one more unit test to exercise this last usage in action
(currently only jami-service-type makes use of that, but I have an
upcoming Xvnc service that benefits from that as well).

Thanks,

Maxim

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

[Attila Lendvai]

> OK, I've reread this, and it is indeed a risk, that 'unset could leak in
> the case of a serializable configuration making use of a maybe-value
> field of type maybe-symbol. I've added the unit test suggested as
> 97cb43e732a38758c95b7caf3963507188d011cf (currently marked as 'expected
> to fail'). Luckily no current service uses that.

thank you for that Maxim!

and sorry for my initial, somewhat reactive, and emotionally driven response earlier! maintaining a channel with complex services, and finally getting the changes i needed merged into Guix proper was a source of frustration for me.

i've looked at the current state of the code, and it looks good to me. the only issues i have left are the following:

1) the (eq 'unset ...) scattered around the code; it should be hidden behind an explicit abstraction, but you yourself mentioned this already in an earlier mail. i'd call it CONFIGURATION-FIELD-SET? (instead of MAYBE-SET?). it's longer, but we have completion in emacs, and it won't be used a gazillion times all around the code either.

2) the lack of an abstraction for the unset/unspecified value. whatever we use as the marker should be hidden behind either an exported global variable, or a function called UNSET-CONFIGURATION-FIELD! (or something alike). i should have introduced these myself, and then your fix would have been as simple as replacing *UNSPECIFIED* with 'UNSET in the abstraction.

3) the SYMBOL? corner case that your test captures, but it's not a burning issue for me (it doesn't affect the user facing API, once the above leakages are fixed).

do you agree? if yes, will you implement it, or shall i prepare a patch?

one more note: sometimes it's useful to have a field with a maybe type that also has a default, together with the ability to explicitly unset this field.

an example would be a port specification for a torrent client: it has some default port, but it's possible to explicitly unset the port value to request the allocation of a random port at startup.

to better accommodate for this use case, 2) should probably be implemented not as an UNSET-FOO! function, but as a global variable holding the unset value marker. or maybe both?

--
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“There is only one thing more harmful to society than an elected official forgetting the promises he made in order to get elected; that's when he doesn't forget them.”
	— John McCarthy (1927–2011), father of Lisp

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

[Maxim Cournoyer]

Hi Attila!

Attila Lendvai <attila@lendvai.name> writes:

>> OK, I've reread this, and it is indeed a risk, that 'unset could leak in
>> the case of a serializable configuration making use of a maybe-value
>> field of type maybe-symbol. I've added the unit test suggested as
>> 97cb43e732a38758c95b7caf3963507188d011cf (currently marked as 'expected
>> to fail'). Luckily no current service uses that.
>
> thank you for that Maxim!

>
> and sorry for my initial, somewhat reactive, and emotionally driven
> response earlier! maintaining a channel with complex services, and
> finally getting the changes i needed merged into Guix proper was a
> source of frustration for me.

No worries.  We all get caught in emotions at times.  It's not
necessarily a bad thing, it's a sign we are invested and care.

> i've looked at the current state of the code, and it looks good to me. the only issues i have left are the following:
>
> 1) the (eq 'unset ...) scattered around the code; it should be hidden
> behind an explicit abstraction, but you yourself mentioned this
> already in an earlier mail. i'd call it CONFIGURATION-FIELD-SET?
> (instead of MAYBE-SET?). it's longer, but we have completion in emacs,
> and it won't be used a gazillion times all around the code either.

I had used maybe-value-set? because the maybe values are define via the
'define-maybe' syntax; they are not really part of
'define-configuration' and are sometimes used outside of it, such as in
(guix home ssh).

> 2) the lack of an abstraction for the unset/unspecified
> value. whatever we use as the marker should be hidden behind either an
> exported global variable, or a function called
> UNSET-CONFIGURATION-FIELD! (or something alike). i should have
> introduced these myself, and then your fix would have been as simple
> as replacing *UNSPECIFIED* with 'UNSET in the abstraction.

An exported variable seems simplest and perhaps less awkward to use,
e.g. %unset or similar, although it's a bit ugly that we need to reify
an unspecified value :-).

> 3) the SYMBOL? corner case that your test captures, but it's not a burning issue for me (it doesn't affect the user facing API, once the above leakages are fixed).
>
> do you agree? if yes, will you implement it, or shall i prepare a patch?

I sent a patch somewhere for the maybe-value-set?, see message-id
<87bkstnd2d.fsf@gmail.com> up this thread.  I'd be happy if you could
prepare a patch for the other things mentionned here (an exported
symbol).

> one more note: sometimes it's useful to have a field with a maybe type
> that also has a default, together with the ability to explicitly unset
> this field.

True.  I'd prefer if this never was true for simplicity (a maybe field
would not need to take a default value), but the reality is that we may
want to set a sane default value while allowing the user to clear the
field to have the software use its default behavior.

>
> an example would be a port specification for a torrent client: it has some default port, but it's possible to explicitly unset the port value to request the allocation of a random port at startup.
>
> to better accommodate for this use case, 2) should probably be
> implemented not as an UNSET-FOO! function, but as a global variable
> holding the unset value marker. or maybe both?

I'd keep things simple with the exported unspecified value rather than
both.  A single, obvious way to do things is simpler.

Thanks,

Maxim

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

[Attila Lendvai]

> I had used maybe-value-set? because the maybe values are define via the
> 'define-maybe' syntax; they are not really part of
> 'define-configuration' and are sometimes used outside of it, such as in
> (guix home ssh).


oh! i wasn't aware of that.


> An exported variable seems simplest and perhaps less awkward to use,
> e.g. %unset or similar, although it's a bit ugly that we need to reify
> an unspecified value :-).


yeah, i started work on implementing an UNSET! function, and its API
would be unnecessarily complex with a (type, field-name, instance)
signature, and going through generic slot access in its
implementation.


> prepare a patch for the other things mentionned here (an exported
> symbol).


i started implementing your suggestions, including the replacement of
the scattered usage of (eq? 'unset ...) pattern. what i found is that
the code is not very readable using MAYBE-VALUE-SET?, or at least not
for me.

first, it negates the boolean logic everywhere in the current code
(i.e. larger diff, and/or the use of (if (not ...) a b)).

and an example wrt readability:

(if (maybe-value-set? field-default)
    field-default
    (configuration-missing-default-value ...)

a value is never set, only places can be set to some value.

would you be fine if we renamed MAYBE-VALUE-SET? to UNSET-VALUE?

then the boolean logic would remain the same, and the above example
would look like:

(if (unset-value? field-default)
    (configuration-missing-default-value ...)
    field-default)

short of a response i'll continue working towards this in the
following days and send a proposal eventually, but if you're very much
unhappy about it, then let me know!

--
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
That, which is not falsifiable, can never be true.

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

[Maxim Cournoyer]

Hi Attila,

Attila Lendvai <attila@lendvai.name> writes:

[...]

>> prepare a patch for the other things mentionned here (an exported
>> symbol).

Thanks!

> i started implementing your suggestions, including the replacement of
> the scattered usage of (eq? 'unset ...) pattern. what i found is that
> the code is not very readable using MAYBE-VALUE-SET?, or at least not
> for me.
>
> first, it negates the boolean logic everywhere in the current code
> (i.e. larger diff, and/or the use of (if (not ...) a b)).
>
> and an example wrt readability:
>
> (if (maybe-value-set? field-default)
>     field-default
>     (configuration-missing-default-value ...)
>
> a value is never set, only places can be set to some value.

It's not clear to me why you think the above is less readable; in the
code I had to touch, the maybe-value-set? was more natural, as the cases
I dealt with often tested for (not (eq? 'unset ...)), so reversing the
logic allowed getting rid of the negation.  See
https://issues.guix.gnu.org/57168#13 for example.

>
> would you be fine if we renamed MAYBE-VALUE-SET? to UNSET-VALUE?

unset-value? sounds like an action; so I'd name it 'maybe-value-unset?';
but as I wrote above I don't really see the benefit/like the idea.

Thanks for working on it!

Maxim

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

[Attila Lendvai]

> > would you be fine if we renamed MAYBE-VALUE-SET? to UNSET-VALUE?
>
>
> unset-value? sounds like an action; so I'd name it 'maybe-value-unset?';
> but as I wrote above I don't really see the benefit/like the idea.


it's always funny when two non-native speakers (?) argue about english... :) maybe we should invite one into the conversation?

with that in mind: UNSET-VALUE? certainly doesn't look like an action to me due to the question mark at the end. if we want to clearly disambiguate this, then we should insert 'is' somewhere, e.g. IS-UNSET-VALUE? which is a short version of IS-IT-THE-UNSET-VALUE?, i.e. it is not a shorthand for IS-THIS-VALUE-UNSET?, because only places can be in the state of not set.

does this clarify my perspective?

it's not crucial, though. i can work my way through the changes without renaming this, but it'll be public API, so it's better to have something intuitive and non-misleading published at the earliest possible iteration.

--
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“I sincerely believe that banking establishments are more dangerous than standing armies, and that the principle of spending money to be paid by posterity, under the name of funding, is but swindling futurity on a large scale.”
	— Thomas Jefferson (1743–1826)

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

[Maxim Cournoyer]

Hi Attila,

Attila Lendvai <attila@lendvai.name> writes:

>> > would you be fine if we renamed MAYBE-VALUE-SET? to UNSET-VALUE?
>>
>>
>> unset-value? sounds like an action; so I'd name it 'maybe-value-unset?';
>> but as I wrote above I don't really see the benefit/like the idea.
>
>
> it's always funny when two non-native speakers (?) argue about
> english... :) maybe we should invite one into the conversation?

Eh.  I'm not a native English speaker either (although living in North
America you can't really escape being exposed to it), but the naming
issue here seems logical rather than subjective to me: unset can be both
a verb or an adjective; by moving it after the noun, it communicates
better that it acts as an adjective rather than as a verb.

So instead of 'unset-value?', I'd use 'value-unset?', but since in this
case we're dealing with a 'maybe' type defined with the 'define-maybe'
macro, I'd keep 'maybe-value-unset?'.

I hope this makes sense.

Native English speakers are welcome to tip in, of course :-).

Thanks,

Maxim

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

[paren--- via Bug reports for GNU Guix]

Hello Maxim,

On Wed Aug 17, 2022 at 2:16 PM BST, Maxim Cournoyer wrote:
> So instead of 'unset-value?', I'd use 'value-unset?', but since in this
> case we're dealing with a 'maybe' type defined with the 'define-maybe'
> macro, I'd keep 'maybe-value-unset?'.

How about `unset?` or `maybe-unset?`? (I am a native English speaker, but
to be honest I don't think that really matters much to this dicussion...)

    -- (

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

[Maxim Cournoyer]

[-- 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

bug#56799: [PATCH 1/5] services: configuration: Add a 'maybe-value-set?' procedure.

[Attila Lendvai]

From: Maxim Cournoyer <maxim.cournoyer@gmail.com>

* 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.

Co-authored-by: Attila Lendvai <attila@lendvai.name>
---
 doc/guix.texi                  |  7 ++++++-
 gnu/services/configuration.scm | 15 ++++++++++-----
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 86cfe7d49c..039df29ebc 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -38999,7 +38999,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
 
@@ -39030,6 +39030,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..e2c4fe9998 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
@@ -142,7 +143,8 @@ (define (define-maybe-helper serialize? prefix syn)
                                     (id #'stem #'serialize-maybe- #'stem))))
        #`(begin
            (define (maybe-stem? val)
-             (or (eq? val 'unset) (stem? val)))
+             (or (not (maybe-value-set? val))
+                 (stem? val)))
            #,@(if serialize?
                   (list #'(define (serialize-maybe-stem field-name val)
                             (if (stem? val)
@@ -260,11 +262,10 @@ (define #,(id #'stem #'stem #'-fields)
                       (default-value-thunk
                         (lambda ()
                           (display '#,(id #'stem #'% #'stem))
-                          (if (eq? (syntax->datum field-default)
-                                   'unset)
+                          (if (maybe-value-set? (syntax->datum field-default))
+                              field-default
                               (configuration-missing-default-value
-                               '#,(id #'stem #'% #'stem) 'field)
-                              field-default)))
+                               '#,(id #'stem #'% #'stem) 'field))))
                       (documentation doc))
                      ...))))))))
 
@@ -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.35.1

bug#56799: [PATCH 2/5] services: configuration: Add %unset-value exported variable.

[Attila Lendvai]

* gnu/services/configuration.scm (%unset-value): New variable.
(normalize-field-type+def): Use it.
(maybe-value-unset?): Use it.
---
 gnu/services/configuration.scm | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index e2c4fe9998..a9426066b9 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
+            %unset-value
             maybe-value-set?
             generate-documentation
             configuration->documentation
@@ -172,10 +173,10 @@ (define (normalize-field-type+def s)
      (values #'(field-type def)))
     ((field-type)
      (identifier? #'field-type)
-     (values #'(field-type 'unset)))
+     (values #'(field-type %unset-value)))
     (field-type
      (identifier? #'field-type)
-     (values #'(field-type 'unset)))))
+     (values #'(field-type %unset-value)))))
 
 (define (define-configuration-helper serialize? serializer-prefix syn)
   (syntax-case syn ()
@@ -301,9 +302,18 @@ (define-configuration stem (field field-type+def
 (define (empty-serializer field-name val) "")
 (define serialize-package empty-serializer)
 
+;; Ideally this should be an implementation detail, but we export it
+;; to provide a simpler API that enables unsetting a configuration
+;; field that has a maybe type, but also a default value.
+;;
+;; An example use-case would be something like a network application
+;; that uses a default port, but the field can explicitly be unset to
+;; request a random port at startup.
+(define %unset-value 'unset)
+
 (define (maybe-value-set? value)
   "Predicate to check whether a 'maybe' value was explicitly provided."
-  (not (eq? 'unset value)))
+  (not (eq? %unset-value value)))
 
 ;; A little helper to make it easier to document all those fields.
 (define (generate-documentation documentation documentation-name)
-- 
2.35.1

bug#56799: [PATCH 3/5] services: configuration: Add maybe-value exported procedure.

[Attila Lendvai]

* gnu/services/configuration.scm (maybe-value): New procedure.
---
 gnu/services/configuration.scm | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index a9426066b9..60965486a7 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -58,6 +58,7 @@ (define-module (gnu services configuration)
             define-maybe
             define-maybe/no-serialization
             %unset-value
+            maybe-value
             maybe-value-set?
             generate-documentation
             configuration->documentation
@@ -315,6 +316,15 @@ (define (maybe-value-set? value)
   "Predicate to check whether a 'maybe' value was explicitly provided."
   (not (eq? %unset-value value)))
 
+;; Ideally there should be a compiler macro for this predicate, that expands
+;; to a conditional that only instantiates the default value when needed.
+(define* (maybe-value value #:optional (default #f))
+  "Returns VALUE, unless it is the unset value, in which case it returns
+DEFAULT."
+  (if (maybe-value-set? value)
+      value
+      default))
+
 ;; 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.35.1

bug#56799: [PATCH 4/5] services: Use the new maybe/unset API.

[Attila Lendvai]

* gnu/home/services/ssh.scm (serialize-address-family): Use the public API of
the maybe infrastructure.
* gnu/services/file-sharing.scm (serialize-maybe-string): Use maybe-value.
(serialize-maybe-file-object): Use maybe-value-set?.
* gnu/services/getmail.scm (getmail-retriever-configuration): Don't use
internals in unset field declarations.
(getmail-destination-configuration): Ditto.
* gnu/services/messaging.scm (raw-content?): Use maybe-value-set?.
(prosody-configuration): Use %unset-value.
* gnu/services/telephony.scm (jami-shepherd-services): Use maybe-value-set?.
(archive-name->username): Use maybe-value-set?.
* tests/services/configuration.scm ("maybe type, no default"): Use
%unset-value.
---
 gnu/home/services/ssh.scm        | 12 +++++++-----
 gnu/services/file-sharing.scm    | 11 ++++-------
 gnu/services/getmail.scm         |  6 +++---
 gnu/services/kerberos.scm        |  1 +
 gnu/services/messaging.scm       | 15 ++++++++++-----
 gnu/services/networking.scm      |  2 +-
 gnu/services/telephony.scm       |  6 +++---
 tests/services/configuration.scm |  2 +-
 8 files changed, 30 insertions(+), 25 deletions(-)

diff --git a/gnu/home/services/ssh.scm b/gnu/home/services/ssh.scm
index f0987aee23..d15f5ee912 100644
--- a/gnu/home/services/ssh.scm
+++ b/gnu/home/services/ssh.scm
@@ -69,17 +69,19 @@ (define (serialize-string field value)
                  " " value "\n"))
 
 (define (address-family? obj)
-  (memv obj (list 'unset AF_INET AF_INET6)))
+  (memv obj (list AF_INET AF_INET6)))
+
+(define-maybe address-family)
 
 (define (serialize-address-family field family)
-  (if (eq? 'unset family)
-      ""
+  (if (maybe-value-set? family)
       (string-append "  " (serialize-field-name field) " "
                      (cond ((= family AF_INET) "inet")
                            ((= family AF_INET6) "inet6")
                            ;; The 'else' branch is unreachable.
                            (else (raise (condition (&error)))))
-                     "\n")))
+                     "\n")
+      ""))
 
 (define (natural-number? obj)
   (and (integer? obj) (exact? obj) (> obj 0)))
@@ -115,7 +117,7 @@ (define-configuration openssh-host
    maybe-string
    "Host name---e.g., @code{\"foo.example.org\"} or @code{\"192.168.1.2\"}.")
   (address-family
-   address-family
+   maybe-address-family
    "Address family to use when connecting to this host: one of
 @code{AF_INET} (for IPv4 only), @code{AF_INET6} (for IPv6 only).
 Additionally, the field can be left unset to allow any address family.")
diff --git a/gnu/services/file-sharing.scm b/gnu/services/file-sharing.scm
index 5df8b0d597..75e99f20b7 100644
--- a/gnu/services/file-sharing.scm
+++ b/gnu/services/file-sharing.scm
@@ -114,10 +114,7 @@ (define-maybe string)
 ;; name-value pair for the JSON builder.
 (set! serialize-maybe-string
   (lambda (field-name val)
-    (serialize-string field-name
-                      (if (eq? val 'unset)
-                          ""
-                          val))))
+    (serialize-string field-name (maybe-value val ""))))
 
 (define (string-list? val)
   (and (list? val)
@@ -180,9 +177,9 @@ (define (serialize-file-object field-name val)
 (define-maybe file-object)
 (set! serialize-maybe-file-object
   (lambda (field-name val)
-    (if (eq? val 'unset)
-        (serialize-string field-name "")
-        (serialize-file-object field-name val))))
+    (if (maybe-value-set? val)
+        (serialize-file-object field-name val)
+        (serialize-string field-name ""))))
 
 (define (file-object-list? val)
   (and (list? val)
diff --git a/gnu/services/getmail.scm b/gnu/services/getmail.scm
index ce124f6b11..0a1c34cfd3 100644
--- a/gnu/services/getmail.scm
+++ b/gnu/services/getmail.scm
@@ -111,10 +111,10 @@ (define-configuration getmail-retriever-configuration
    "The type of mail retriever to use.  Valid values include
 @samp{passwd} and @samp{static}.")
   (server
-   (string 'unset)
+   string
    "Name or IP address of the server to retrieve mail from.")
   (username
-   (string 'unset)
+   string
    "Username to login to the mail server with.")
   (port
    (non-negative-integer #f)
@@ -143,7 +143,7 @@ (define (serialize-getmail-destination-configuration field-name val)
 
 (define-configuration getmail-destination-configuration
   (type
-   (string 'unset)
+   string
    "The type of mail destination.  Valid values include @samp{Maildir},
 @samp{Mboxrd} and @samp{MDA_external}.")
   (path
diff --git a/gnu/services/kerberos.scm b/gnu/services/kerberos.scm
index f845c1bd89..c3c7872734 100644
--- a/gnu/services/kerberos.scm
+++ b/gnu/services/kerberos.scm
@@ -39,6 +39,7 @@ (define-module (gnu services kerberos)
 
 \f
 
+;; TODO Use %unset-value and the define-maybe infrastructure.
 (define unset-field (list 'unset-field))
 
 (define (predicate/unset pred)
diff --git a/gnu/services/messaging.scm b/gnu/services/messaging.scm
index 00a1c80a14..59cb486778 100644
--- a/gnu/services/messaging.scm
+++ b/gnu/services/messaging.scm
@@ -90,6 +90,11 @@ (define (make-pred arg)
                      ((new-def ...)
                       (map (lambda (def target)
                              (if (eq? 'common (syntax->datum target))
+                                 ;; TODO Use the %unset-value variable, or
+                                 ;; even better just simplify this so that it
+                                 ;; doesn't interfere with
+                                 ;; define-configuration and define-maybe
+                                 ;; internals.
                                  #''unset def))
                            #'(def ...) #'(target ...)))
                      ((new-doc ...)
@@ -200,7 +205,7 @@ (define (serialize-file-object-list field-name val)
 (define-maybe file-object-list)
 
 (define (raw-content? val)
-  (not (eq? val 'unset)))
+  (maybe-value-set? val))
 (define (serialize-raw-content field-name val)
   val)
 (define-maybe raw-content)
@@ -474,12 +479,12 @@ (define-all-configurations prosody-configuration
      global)
 
     (http-max-content-size
-     (maybe-non-negative-integer 'unset)
+     (maybe-non-negative-integer %unset-value)
      "Maximum allowed size of the HTTP body (in bytes)."
      common)
 
     (http-external-url
-     (maybe-string 'unset)
+     (maybe-string %unset-value)
      "Some modules expose their own URL in various ways.  This URL is built
 from the protocol, host and port used.  If Prosody sits behind a proxy, the
 public URL will be @code{http-external-url} instead.  See
@@ -556,7 +561,7 @@ (define-all-configurations prosody-configuration
      int-component)
 
     (mod-muc
-     (maybe-mod-muc-configuration 'unset)
+     (maybe-mod-muc-configuration %unset-value)
      "Multi-user chat (MUC) is Prosody's module for allowing you to create
 hosted chatrooms/conferences for XMPP users.
 
@@ -573,7 +578,7 @@ (define-all-configurations prosody-configuration
      ext-component)
 
     (raw-content
-     (maybe-raw-content 'unset)
+     (maybe-raw-content %unset-value)
      "Raw content that will be added to the configuration file."
      common)))
 
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 3c6395b6ca..9d85728371 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -772,7 +772,7 @@ (define-configuration/no-serialization opendht-configuration
 network.  A specific port value can be provided by appending the @code{:PORT}
 suffix.  By default, it uses the Jami bootstrap nodes, but any host can be
 specified here.  It's also possible to disable bootstrapping by explicitly
-setting this field to the @code{'unset} value.")
+setting this field to @code{%unset-value}.")
   (port
    (maybe-number 4222)
    "The UDP port to bind to.  When left unspecified, an available port is
diff --git a/gnu/services/telephony.scm b/gnu/services/telephony.scm
index 7152f3b38d..624b30116d 100644
--- a/gnu/services/telephony.scm
+++ b/gnu/services/telephony.scm
@@ -307,7 +307,7 @@ (define (jami-shepherd-services config)
          (dbus (jami-configuration-dbus config))
          (dbus-daemon (file-append dbus "/bin/dbus-daemon"))
          (accounts (jami-configuration-accounts config))
-         (declarative-mode? (not (eq? 'unset accounts))))
+         (declarative-mode? (maybe-value-set? accounts)))
 
     (with-extensions (list guile-packrat ;used by guile-ac-d-bus
                            guile-ac-d-bus
@@ -649,7 +649,7 @@ (define (archive-name->username archive)
                                           account-details)
                            (let ((username (archive-name->username
                                             archive)))
-                             (when (not (eq? 'unset allowed-contacts))
+                             (when (maybe-value-set? allowed-contacts)
                                ;; Reject calls from unknown contacts.
                                (set-account-details
                                 '(("DHT.PublicInCalls" . "false")) username)
@@ -659,7 +659,7 @@ (define (archive-name->username archive)
                                ;; Add allowed ones.
                                (for-each (cut add-contact <> username)
                                          allowed-contacts))
-                             (when (not (eq? 'unset moderators))
+                             (when (maybe-value-set? moderators)
                                ;; Disable the 'AllModerators' property.
                                (set-all-moderators #f username)
                                ;; Remove all moderators.
diff --git a/tests/services/configuration.scm b/tests/services/configuration.scm
index 649dad26e8..8ed4ed4b66 100644
--- a/tests/services/configuration.scm
+++ b/tests/services/configuration.scm
@@ -169,7 +169,7 @@ (define-configuration config-with-maybe-string/no-serialization
   (not (defined? 'serialize-maybe-string)))
 
 (test-assert "maybe type, no default"
-  (eq? 'unset
+  (eq? %unset-value
        (config-with-maybe-string/no-serialization-name
         (config-with-maybe-string/no-serialization))))
 
-- 
2.35.1

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

[Maxim Cournoyer]

Hi,

Attila Lendvai <attila@lendvai.name> writes:

> * gnu/home/services/ssh.scm (serialize-address-family): Use the public API of
> the maybe infrastructure.
> * gnu/services/file-sharing.scm (serialize-maybe-string): Use maybe-value.
> (serialize-maybe-file-object): Use maybe-value-set?.
> * gnu/services/getmail.scm (getmail-retriever-configuration): Don't use
> internals in unset field declarations.
> (getmail-destination-configuration): Ditto.
> * gnu/services/messaging.scm (raw-content?): Use maybe-value-set?.
> (prosody-configuration): Use %unset-value.
> * gnu/services/telephony.scm (jami-shepherd-services): Use maybe-value-set?.
> (archive-name->username): Use maybe-value-set?.
> * tests/services/configuration.scm ("maybe type, no default"): Use
> %unset-value.

[...]

> --- a/gnu/services/telephony.scm
> +++ b/gnu/services/telephony.scm
> @@ -307,7 +307,7 @@ (define (jami-shepherd-services config)
>           (dbus (jami-configuration-dbus config))
>           (dbus-daemon (file-append dbus "/bin/dbus-daemon"))
>           (accounts (jami-configuration-accounts config))
> -         (declarative-mode? (not (eq? 'unset accounts))))
> +         (declarative-mode? (maybe-value-set? accounts)))
>  
>      (with-extensions (list guile-packrat ;used by guile-ac-d-bus
>                             guile-ac-d-bus
> @@ -649,7 +649,7 @@ (define (archive-name->username archive)
>                                            account-details)
>                             (let ((username (archive-name->username
>                                              archive)))
> -                             (when (not (eq? 'unset allowed-contacts))
> +                             (when (maybe-value-set? allowed-contacts)
>                                 ;; Reject calls from unknown contacts.
>                                 (set-account-details
>                                  '(("DHT.PublicInCalls" . "false")) username)
> @@ -659,7 +659,7 @@ (define (archive-name->username archive)
>                                 ;; Add allowed ones.
>                                 (for-each (cut add-contact <> username)
>                                           allowed-contacts))
> -                             (when (not (eq? 'unset moderators))
> +                             (when (maybe-value-set? moderators)
>                                 ;; Disable the 'AllModerators' property.
>                                 (set-all-moderators #f username)
>                                 ;; Remove all moderators.

maybe-value-set? cannot be used there, as the code runs on the target,
not on the host, where (gnu services configuration) is not (and cannot)
be in scope.

I've made the simple change below and pushed:

--8<---------------cut here---------------start------------->8---
modified   gnu/services/telephony.scm
@@ -649,7 +649,7 @@ (define (archive-name->username archive)
                                           account-details)
                            (let ((username (archive-name->username
                                             archive)))
-                             (when (maybe-value-set? allowed-contacts)
+                             (when (not (eq? '#$%unset-value allowed-contacts))
                                ;; Reject calls from unknown contacts.
                                (set-account-details
                                 '(("DHT.PublicInCalls" . "false")) username)
@@ -659,7 +659,7 @@ (define (archive-name->username archive)
                                ;; Add allowed ones.
                                (for-each (cut add-contact <> username)
                                          allowed-contacts))
-                             (when (maybe-value-set? moderators)
+                             (when (not (eq? '#$%unset-value moderators))
                                ;; Disable the 'AllModerators' property.
                                (set-all-moderators #f username)
                                ;; Remove all moderators.
--8<---------------cut here---------------end--------------->8---

I've also adjusted a few 'unset mentions in the doc:

--8<---------------cut here---------------start------------->8---
modified   doc/guix.texi
@@ -19846,7 +19846,7 @@ network.  A specific port value can be provided by appending the
 @code{:PORT} suffix.  By default, it uses the Jami bootstrap nodes, but
 any host can be specified here.  It's also possible to disable
 bootstrapping by explicitly setting this field to the
-@code{'unset} value.
+@code{%unset-value} value.
 
 @item @code{port} (default: @code{4222}) (type: maybe-number)
 The UDP port to bind to.  When left unspecified, an available port is
@@ -31362,7 +31362,7 @@ Each parameter definition is preceded by its type; for example,
 @samp{boolean foo} indicates that the @code{foo} parameter should be
 specified as a boolean.  Types starting with @code{maybe-} denote
 parameters that won't show up in TLP config file when their value is
-left unset, or is explicitly set to the @code{'unset} value.
+left unset, or is explicitly set to the @code{%unset-value} value.
 
 @c The following documentation was initially generated by
 @c (generate-tlp-documentation) in (gnu services pm).  Manually maintained
@@ -38983,8 +38983,7 @@ macro which is a shorthand of this.
 Sometimes a field should not be serialized if the user doesn’t specify a
 value.  To achieve this, you can use the @code{define-maybe} macro to
 define a ``maybe type''; if the value of a maybe type is left unset, or
-is set to the @code{'unset} value, then it will not be
-serialized.
+is set to the @code{%unset-value} value, then it will not be serialized.
 
 When defining a ``maybe type'', the corresponding serializer for the
 regular type will be used by default.  For example, a field of type
--8<---------------cut here---------------end--------------->8---

I've made these changes and pushed, as mentioned in a previous email.

Thank you!

Maxim

bug#56799: [PATCH 5/5] services: configuration: Change the value of the unset marker.

[Attila Lendvai]

The new value of %unset-value sticks out more when something goes wrong, and
is also more unique; i.e. easier to search for.
---
 gnu/services/configuration.scm   | 5 +++--
 gnu/services/messaging.scm       | 2 +-
 tests/services/configuration.scm | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index 60965486a7..83da63c1b3 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -305,12 +305,13 @@ (define serialize-package empty-serializer)
 
 ;; Ideally this should be an implementation detail, but we export it
 ;; to provide a simpler API that enables unsetting a configuration
-;; field that has a maybe type, but also a default value.
+;; field that has a maybe type, but also a default value.  We give it
+;; a value that sticks out to the reader when something goes wrong.
 ;;
 ;; An example use-case would be something like a network application
 ;; that uses a default port, but the field can explicitly be unset to
 ;; request a random port at startup.
-(define %unset-value 'unset)
+(define %unset-value '%unset-marker%)
 
 (define (maybe-value-set? value)
   "Predicate to check whether a 'maybe' value was explicitly provided."
diff --git a/gnu/services/messaging.scm b/gnu/services/messaging.scm
index 59cb486778..02712ede7c 100644
--- a/gnu/services/messaging.scm
+++ b/gnu/services/messaging.scm
@@ -95,7 +95,7 @@ (define (make-pred arg)
                                  ;; doesn't interfere with
                                  ;; define-configuration and define-maybe
                                  ;; internals.
-                                 #''unset def))
+                                 #''%unset-marker% def))
                            #'(def ...) #'(target ...)))
                      ((new-doc ...)
                       (map (lambda (doc target)
diff --git a/tests/services/configuration.scm b/tests/services/configuration.scm
index 8ed4ed4b66..4f8a74dc8a 100644
--- a/tests/services/configuration.scm
+++ b/tests/services/configuration.scm
@@ -150,7 +150,7 @@ (define-configuration config-with-maybe-symbol
   (protocol maybe-symbol ""))
 
 ;;; Maybe symbol values are currently seen as serializable, because the
-;;; unspecified value is 'unset, which is a symbol itself.
+;;; unspecified value is '%unset-marker%, which is a symbol itself.
 ;;; TODO: Remove expected fail marker after resolution.
 (test-expect-fail 1)
 (test-equal "symbol maybe value serialization, unspecified"
-- 
2.35.1

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

[Maxim Cournoyer]

Hi Attila,

Attila Lendvai <attila@lendvai.name> writes:

> The new value of %unset-value sticks out more when something goes wrong, and
> is also more unique; i.e. easier to search for.
> ---
>  gnu/services/configuration.scm   | 5 +++--
>  gnu/services/messaging.scm       | 2 +-
>  tests/services/configuration.scm | 2 +-
>  3 files changed, 5 insertions(+), 4 deletions(-)

Thanks for the series!  I've now pushed all of the 5 commits.  Yay!

Closing!

Maxim