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

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

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