unofficial mirror of bug-guix@gnu.org 
 help / color / mirror / code / Atom feed
From: "Ludovic Courtès" <ludo@gnu.org>
To: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Cc: 56799@debbugs.gnu.org, attila@lendvai.name
Subject: bug#56799: (gnu services configuration) usage of *unspecified* is problematic
Date: Thu, 04 Aug 2022 14:19:59 +0200	[thread overview]
Message-ID: <877d3omc9c.fsf@gnu.org> (raw)
In-Reply-To: <87sfme1y8m.fsf@gmail.com> (Maxim Cournoyer's message of "Tue, 02 Aug 2022 11:06:17 -0400")

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




  reply	other threads:[~2022-08-04 12:59 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-27 16:23 bug#56799: (gnu services configuration) usage of *unspecified* is problematic Maxim Cournoyer
2022-07-27 16:43 ` Tobias Geerinckx-Rice via Bug reports for GNU Guix
2022-07-27 18:27   ` Attila Lendvai
2022-07-28 15:15     ` Maxim Cournoyer
2022-07-27 18:31   ` Maxim Cournoyer
2022-07-27 18:45     ` Tobias Geerinckx-Rice via Bug reports for GNU Guix
2022-07-27 19:09       ` Maxim Cournoyer
2022-07-27 19:45         ` bug#56799: [PATCH] services: configuration: Step back from *unspecified* Maxim Cournoyer
2022-07-27 19:46         ` bug#56799: (gnu services configuration) usage of *unspecified* is problematic Maxim Cournoyer
2022-07-27 20:20           ` bug#56799: [PATCH v2] gexp: Handle *unspecified* as a gexp input Maxim Cournoyer
2022-07-27 21:43             ` Maxime Devos
2022-07-28 14:58               ` Maxim Cournoyer
2022-07-28  4:41           ` bug#56799: [PATCH v3] " Maxim Cournoyer
2022-08-01  5:08             ` bug#56799: (gnu services configuration) usage of *unspecified* is problematic Maxim Cournoyer
2022-08-01 10:00               ` Maxime Devos
2022-08-01 12:46                 ` Maxim Cournoyer
2022-08-01 13:44             ` Ludovic Courtès
2022-08-01 16:55       ` Maxim Cournoyer
2022-07-28  4:55     ` bokr
2022-07-28 10:26       ` Maxime Devos
2022-07-28 15:09         ` Maxim Cournoyer
2022-08-01 13:49 ` Ludovic Courtès
2022-08-01 15:55   ` Maxim Cournoyer
2022-08-02  7:31     ` Ludovic Courtès
2022-08-02  8:45       ` bokr
2022-08-02 15:06       ` Maxim Cournoyer
2022-08-04 12:19         ` Ludovic Courtès [this message]
2022-08-07 22:44           ` Ludovic Courtès
2022-08-08 22:27           ` Attila Lendvai
2022-08-08 23:35             ` Attila Lendvai
2022-08-10  2:17               ` Maxim Cournoyer
2022-08-10  3:26             ` Maxim Cournoyer
2022-08-11 10:15               ` Attila Lendvai
2022-08-13  6:31                 ` Maxim Cournoyer
2022-08-13 16:47                   ` Attila Lendvai
2022-08-14  2:57                     ` Maxim Cournoyer
2022-08-16 14:00                       ` Attila Lendvai
2022-08-17 13:16                         ` Maxim Cournoyer
2022-08-17 16:00                           ` paren--- via Bug reports for GNU Guix
2022-08-10  0:43           ` Maxim Cournoyer
2022-08-24 12:40 ` bug#56799: [PATCH 1/5] services: configuration: Add a 'maybe-value-set?' procedure Attila Lendvai
2022-08-24 12:40   ` bug#56799: [PATCH 2/5] services: configuration: Add %unset-value exported variable Attila Lendvai
2022-08-24 12:40   ` bug#56799: [PATCH 3/5] services: configuration: Add maybe-value exported procedure Attila Lendvai
2022-08-24 12:40   ` bug#56799: [PATCH 4/5] services: Use the new maybe/unset API Attila Lendvai
2022-08-25  4:18     ` bug#56799: (gnu services configuration) usage of *unspecified* is problematic Maxim Cournoyer
2022-08-24 12:40   ` bug#56799: [PATCH 5/5] services: configuration: Change the value of the unset marker Attila Lendvai
2022-08-25  4:14     ` bug#56799: (gnu services configuration) usage of *unspecified* is problematic Maxim Cournoyer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=877d3omc9c.fsf@gnu.org \
    --to=ludo@gnu.org \
    --cc=56799@debbugs.gnu.org \
    --cc=attila@lendvai.name \
    --cc=maxim.cournoyer@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).