unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Brainstorming ideas for define-configuration
@ 2023-03-09  2:28 Bruno Victal
  2023-03-09  8:13 ` Attila Lendvai
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Bruno Victal @ 2023-03-09  2:28 UTC (permalink / raw)
  To: guix-devel
  Cc: Felix Lechner, mirai, Liliana Marie Prikler, Maxim Cournoyer,
	Ludovic Courtès

Co-authored-by: Felix Lechner


After spending some time with old and new Guix services, I'd like to
suggest some potential improvements to our define-configuration macro:


User-specified sanitizer support
===============================================================================

The sanitizers should be integrated with the type. Otherwise, they are
tedious to use and appear verbose when repeatedly applied to multiple fields.

--8<---------------cut here---------------start------------->8---
;; Suggestion #1
;; The procedure could return a sanitized value. Upon failure, there are
;; the following options:
;;    - The procedure returns only a special value (akin to %unset-value)
;;      and an error message, as a pair.
;;      Exception raising is done by define-sanitized macro behind the scenes
;;      which makes the procedure easier to write.
;;    - The procedure raises an exception. There would be no consistency
;;      on the message formats, however, except for any agreed convention.
;;      This would involve some code duplication.
;; Cons: too specific, not extensible.

(define-sanitized typename procedure
  (prefix ...))


;; Suggestion #2
;; A user-supplied procedure ('procname' below) would work just like the
;; procedure in option #1.
;; There is some similiarity to the Guix record-type*.
;; This could be extended more easily in the future should it be required.
(define-type typename        ; maybe call this 'define-configuration-type' ?
  (sanitizer procname)
  (maybe-type? #t)
  ;; The properties below are service specific.
  ;; If this is implemented with Guix record-type* then we could have a
  ;; module containing generic types and do something along the lines of:
  ;; (define-type foo-ip-address
  ;;    (inherit generic-ip-address)
  ;;    (serializer ...))
  (serializer procname)      ; define-type/no-serialization = sets this field to #f ?
  (prefix ...))
--8<---------------cut here---------------end--------------->8---

The original motivation for this proposal stems from the attempts to
resolve issue #61570. There, one potential fix was to handle the user and
group fields similarly to the way greetd service handles them.
There is some opportunity for generalization here, types that might be
useful elsewhere, such as a port number or a host name, could be placed in
a separate module.

This proposal should also make it easier to handle situations when fields
become obsolete.


Record Validator
===============================================================================

There is also a need to validate records. Matching fields alone do not actually
ensure that the configuration is coherent and usable. For example, some fields
may be mutually incompatible with others.

We could provide procedures that validate each record type within
define-configuration itself instead of validating the value at runtime (i.e.
within the body of the service-type).

--8<---------------cut here---------------start------------->8---
;; the common case
(define-configuration foo-configuration
  (name
   string
   "Lorem ipsum...")

  ;; ...

  (validator procname))

;; [bonus] Simpler configurations that only care for mutually-exclusive fields
(define-configuration foo-configuration
  (name
   string
   "Lorem ipsum...")

  (title
   string
   "Lorem ipsum..."
   (conflicts 'name)))
--8<---------------cut here---------------end--------------->8---


Comments regarding literal placement
-------------------------------------------------------------------------------

Does the placement order matter for the extra fields/literals for define-configuration?
Can they be placed arbitrarily or only at the bottom? (where custom-serializer is specified)

Another point, extra parameters given to define-configuration should never
clash with field names. For example, it should be possible to name a
field 'prefix', 'location' or similar without causing issues like #59423.


Coalesced documentation
===============================================================================

Currently, we manually edit the texinfo output from configuration->documentation
if we're unsatisfied with the generated result.
For instance, substituting @item with an @itemx marker for fields whose
documentation is similar.

Instead, we could embed hints in define-configuration that affect the texinfo
generation in smarter ways.

In the long term, guix.texi should source some portions of the documentation
directly from the code base. The current workflow involving copy and paste
the output from the evaluation of configuration->documentation carries the
unnecessary risk that future documentation patches are done against guix.texi
rather than the .scm file from where it was generated. (issue #60582)

Snippet based on mympd-ip-acl (gnu/services/audio.scm):
--8<---------------cut here---------------start------------->8---
(define-configuration/no-serialization mympd-ip-acl
  (allow
   (list-of-string '())
   "Allowed/Disallowed IP addresses.")

  (deny
   (list-of-string '())
   (documentation 'allow)))  ; group with field 'allow

;;; --- texi output below ---

@c %start of fragment
@deftp {Data Type} mympd-ip-acl
Available @code{mympd-ip-acl} fields are:

@table @asis
@item @code{allow}
@itemx @code{deny} (default: @code{()}) (type: list-of-string)
Allowed/Disallowed IP addresses.

@end table
@end deftp
@c %end of fragment
--8<---------------cut here---------------end--------------->8---


Serializer access to other fields
===============================================================================

Serialization procedures generally only have access to the values of its own field. That
may be insufficient in some cases as whether a field can be serialized
or how that is done, for example, can depend on the value of other fields.

mympd-service-type is one example where each serialized field depends on the value of
another field. Our standard serializer procedures were useless for that case.

As a side note, the 'this-record' macro does not work with
define-configuration which made it impossible to use
(custom-serializer) for the same effect.

Instead, serializer procedures could take additional keyword arguments:
--8<---------------cut here---------------start------------->8---
(define* (serialize-string field-name value (#:key config))
   (let ((baz-value (assoc-ref config 'baz)))
      (string-append baz-value "/" value)
      ...))
--8<---------------cut here---------------end--------------->8---


Inheritable record-type definition
===============================================================================

The openvpn-service pairs in (gnu services vpn) define a special
macro define-split-configuration with the purpose to avoid code duplication
since the service pairs have multiple fields in common.

Perhaps in a similar vein to SRFI-136, we could have:

--8<---------------cut here---------------start------------->8---
(define-configuration openvpn-common-configuration
  (openvpn
    (file-like openvpn)
    "The OpenVPN package.")
  ;; ...
)

(define-configuration openvpn-server-configuration openvpn-common-configuration
  (tls-auth
    (tls-auth-server #f)
    "Add an additional layer of HMAC authentication on top of the TLS control
channel to protect against DoS attacks.")
  ;; ...
)

;;; or through a literal/keyword approach

(define-configuration openvpn-server-configuration
  (tls-auth
    (tls-auth-server #f)
    "Add an additional layer of HMAC authentication on top of the TLS control
channel to protect against DoS attacks.")

  (parent openvpn-common-configuration))

--8<---------------cut here---------------end--------------->8---


Generic serialize-configuration
===============================================================================

The procedure serialize-configuration inherently assumes that the serialized
configuration must be a single string. This assumption needn't always hold, especially
if the service in question is not a shepherd service.

We could possibly make this procedure a bit more "generic", maybe picking some ideas
from SRFI-171 Transducers.

Another improvement to this procedure is to eliminate (or make optional) the second
parameter, the configuration fields. It's unclear what value it brings since one
can't use fields from another configuration type here.
An argument can be made for selectively serializing some of the fields but then
it would be more practical to make this an optional parameter.



PS.
===============================================================================

Kind of a late realization, but couldn't many of the items above be satisfied
by improvements to define-record-type* instead?
(define-record-type* paired with a documentation literal is nearly equivalent to define-configuration,
sans the serialization scaffolding)



Cheers,
Bruno


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Brainstorming ideas for define-configuration
  2023-03-09  2:28 Brainstorming ideas for define-configuration Bruno Victal
@ 2023-03-09  8:13 ` Attila Lendvai
  2023-03-09 21:05   ` Josselin Poiret
  2023-03-09 14:40 ` Joshua Branson
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Attila Lendvai @ 2023-03-09  8:13 UTC (permalink / raw)
  To: Bruno Victal
  Cc: guix-devel, Felix Lechner, Liliana Marie Prikler, Maxim Cournoyer,
	Ludovic Courtès

> Record Validator
> ===============================================================================
> 
> There is also a need to validate records. Matching fields alone do
> not actually ensure that the configuration is coherent and
> usable. For example, some fields may be mutually incompatible with
> others.
>
> We could provide procedures that validate each record type within
> define-configuration itself instead of validating the value at
> runtime (i.e.  within the body of the service-type).


in my service i have a non-trivial logic regarding defaults (e.g. there are cross-field dependencies while setting some defaults).

this means that all the entry points to my service code have a line like this at the very beginning:

(set! config (apply-config-defaults config))

maybe this validator logic could be implemented so that the validator may return a new config instance, and all entry points to the service's code would be called with that new instance?



> Coalesced documentation
> ================================================================



it's a tangential here, but i think demanding a documentation for fields is just wrong. all it does is annoy the programmer who puts an empty "", after wasting some time on a failed compilation. especially in a phase where the code is still in a prototype phase.


> Kind of a late realization, but couldn't many of the items above be satisfied
> by improvements to define-record-type* instead?
> (define-record-type* paired with a documentation literal is nearly equivalent to define-configuration,
> sans the serialization scaffolding)


even if so, i'd still maintain a thin layer of abstraction for communicating the intention, and also for possible future extensions.

-- 
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“He who flatters a man is his enemy. He who tells him of his faults is his maker.”
	— Confucius (551–479 BC)



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Brainstorming ideas for define-configuration
  2023-03-09  2:28 Brainstorming ideas for define-configuration Bruno Victal
  2023-03-09  8:13 ` Attila Lendvai
@ 2023-03-09 14:40 ` Joshua Branson
  2023-03-09 20:23 ` Liliana Marie Prikler
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Joshua Branson @ 2023-03-09 14:40 UTC (permalink / raw)
  To: Bruno Victal
  Cc: guix-devel, Felix Lechner, Liliana Marie Prikler, Maxim Cournoyer,
	Ludovic Courtès

Bruno Victal <mirai@makinata.eu> writes:

> Co-authored-by: Felix Lechner
>
>
> After spending some time with old and new Guix services, I'd like to
> suggest some potential improvements to our define-configuration macro:
>
>
> User-specified sanitizer support
> ===============================================================================
>

Yes please!  Thanks for working on these improvements!  It would make my
opensmtpd-service updates much easier (though I haven't really touched
the code in a while).

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

>
> The sanitizers should be integrated with the type. Otherwise, they are
> tedious to use and appear verbose when repeatedly applied to multiple fields.
>
> ;; Suggestion #1
> ;; The procedure could return a sanitized value. Upon failure, there are
> ;; the following options:
> ;;    - The procedure returns only a special value (akin to %unset-value)
> ;;      and an error message, as a pair.
> ;;      Exception raising is done by define-sanitized macro behind the scenes
> ;;      which makes the procedure easier to write.
> ;;    - The procedure raises an exception. There would be no consistency
> ;;      on the message formats, however, except for any agreed convention.
> ;;      This would involve some code duplication.
> ;; Cons: too specific, not extensible.
>
> (define-sanitized typename procedure
>   (prefix ...))
>
>
> ;; Suggestion #2
> ;; A user-supplied procedure ('procname' below) would work just like the
> ;; procedure in option #1.
> ;; There is some similiarity to the Guix record-type*.
> ;; This could be extended more easily in the future should it be required.
> (define-type typename        ; maybe call this 'define-configuration-type' ?
>   (sanitizer procname)
>   (maybe-type? #t)
>   ;; The properties below are service specific.
>   ;; If this is implemented with Guix record-type* then we could have a
>   ;; module containing generic types and do something along the lines of:
>   ;; (define-type foo-ip-address
>   ;;    (inherit generic-ip-address)
>   ;;    (serializer ...))
>   (serializer procname)      ; define-type/no-serialization = sets this field to #f ?
>   (prefix ...))
>
>
> Record Validator
> ===============================================================================
>
> There is also a need to validate records. Matching fields alone do not actually
> ensure that the configuration is coherent and usable. For example, some fields
> may be mutually incompatible with others.
>
> We could provide procedures that validate each record type within
> define-configuration itself instead of validating the value at runtime (i.e.
> within the body of the service-type).

This is also a great idea!

> Cheers,
> Bruno


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Brainstorming ideas for define-configuration
  2023-03-09  2:28 Brainstorming ideas for define-configuration Bruno Victal
  2023-03-09  8:13 ` Attila Lendvai
  2023-03-09 14:40 ` Joshua Branson
@ 2023-03-09 20:23 ` Liliana Marie Prikler
  2023-03-10 14:10 ` Maxim Cournoyer
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Liliana Marie Prikler @ 2023-03-09 20:23 UTC (permalink / raw)
  To: Bruno Victal, guix-devel
  Cc: Felix Lechner, Maxim Cournoyer, Ludovic Courtès

Hi,

Am Donnerstag, dem 09.03.2023 um 02:28 +0000 schrieb Bruno Victal:
> After spending some time with old and new Guix services, I'd like to
> suggest some potential improvements to our define-configuration
> macro:
> 
> 
> User-specified sanitizer support
> =====================================================================
> ==========
> 
> The sanitizers should be integrated with the type. Otherwise, they
> are tedious to use and appear verbose when repeatedly applied to
> multiple fields.
> 
> --8<---------------cut here---------------start------------->8---
> ;; Suggestion #1
> ;; The procedure could return a sanitized value. Upon failure, there
> are
> ;; the following options:
> ;;    - The procedure returns only a special value (akin to %unset-
> value)
> ;;      and an error message, as a pair.
> ;;      Exception raising is done by define-sanitized macro behind
> the scenes
> ;;      which makes the procedure easier to write.
> ;;    - The procedure raises an exception. There would be no
> consistency
> ;;      on the message formats, however, except for any agreed
> convention.
> ;;      This would involve some code duplication.
> ;; Cons: too specific, not extensible.
> 
> (define-sanitized typename procedure
>   (prefix ...))
> 
> 
> ;; Suggestion #2
> ;; A user-supplied procedure ('procname' below) would work just like
> the
> ;; procedure in option #1.
> ;; There is some similiarity to the Guix record-type*.
> ;; This could be extended more easily in the future should it be
> required.
> (define-type typename        ; maybe call this 'define-configuration-
> type' ?
>   (sanitizer procname)
>   (maybe-type? #t)
>   ;; The properties below are service specific.
>   ;; If this is implemented with Guix record-type* then we could have
> a
>   ;; module containing generic types and do something along the lines
> of:
>   ;; (define-type foo-ip-address
>   ;;    (inherit generic-ip-address)
>   ;;    (serializer ...))
>   (serializer procname)      ; define-type/no-serialization = sets
> this field to #f ?
>   (prefix ...))
> --8<---------------cut here---------------end--------------->8---
Rather than creating yet another syntax, I think we should instead
extend define-configuration to support this use-case in a backwards-
compatible manner.

As for error handling, there are basically two options:
1. Let the sanitizer raise any exception and do regular stack 
   unwinding.
   1b. Provide a source-location to the sanitizer so that it can use
       it in case of an error to provide better fix hints.
2. Let the sanitizer raise any exception, catch it and enrich it with
   the source-location.  Also possibly cross-check the resulting value
   so that e.g. #f is not used if it's not a maybe type or a boolean.

> The original motivation for this proposal stems from the attempts to
> resolve issue #61570. There, one potential fix was to handle the user
> and group fields similarly to the way greetd service handles them.
> There is some opportunity for generalization here, types that might
> be useful elsewhere, such as a port number or a host name, could be
> placed in a separate module.
I think you are blending several concerns together here, which is fine
insofar as sanitizers are powerful enough to mix those but perhaps not
the wisest idea from a software engineering perspective.  Coercing an
account name into a user account is a distinct operation from checking
whether an integer is indeed a port – the latter is a simple range
check that can already be performed with the existing framework.

> Record Validator
> =====================================================================
> ==========
> 
> There is also a need to validate records. Matching fields alone do
> not actually ensure that the configuration is coherent and usable.
> For example, some fields may be mutually incompatible with others.
I smell bad code ahead.

> We could provide procedures that validate each record type within
> define-configuration itself instead of validating the value at
> runtime (i.e. within the body of the service-type).
> 
> --8<---------------cut here---------------start------------->8---
> ;; the common case
> (define-configuration foo-configuration
>   (name
>    string
>    "Lorem ipsum...")
> 
>   ;; ...
> 
>   (validator procname))
> 
> ;; [bonus] Simpler configurations that only care for mutually-
> exclusive fields
> (define-configuration foo-configuration
>   (name
>    string
>    "Lorem ipsum...")
> 
>   (title
>    string
>    "Lorem ipsum..."
>    (conflicts 'name)))
> --8<---------------cut here---------------end--------------->8---
Instead of providing both a name field and a title field, you might
provide a field that can either be a name or a title or allow an even
more powerful value type as long as it makes sense.

> Comments regarding literal placement
> ---------------------------------------------------------------------
> ----------
> 
> Does the placement order matter for the extra fields/literals for
> define-configuration?  Can they be placed arbitrarily or only at the
> bottom? (where custom-serializer is specified)
To keep backwards compatibility, I'd use the place where custom-
serializer currently lies to add these new customizations, but allow
arbitrary order within them.

Using only sanitizer and serializer for the sake of simplicity, the
following forms would be allowed:

- (name type doc): Implicit serializer
- (name type doc serializer): Explicit serializer with deprecation
warning
- (name type doc (serialize serializer)): Explicit serializer, new form
- (name type doc (sanitize sanitizer)): Explicit sanitization, implicit
  serialization
- (name type doc (sanitize ...) (serialize ...)): Sanitization and 
  serialization explicit.

> Another point, extra parameters given to define-configuration should
> never clash with field names. For example, it should be possible to
> name a field 'prefix', 'location' or similar without causing issues
> like #59423.
> 
> 
> Coalesced documentation
> =====================================================================
> ==========
> 
> Currently, we manually edit the texinfo output from configuration-
> >documentation if we're unsatisfied with the generated result. For
> instance, substituting @item with an @itemx marker for fields whose
> documentation is similar.
> 
> Instead, we could embed hints in define-configuration that affect the
> texinfo generation in smarter ways.
> 
> In the long term, guix.texi should source some portions of the
> documentation directly from the code base. The current workflow
> involving copy and paste the output from the evaluation of
> configuration->documentation carries the unnecessary risk that future
> documentation patches are done against guix.texi rather than the .scm
> file from where it was generated. (issue #60582)
> 
> Snippet based on mympd-ip-acl (gnu/services/audio.scm):
> --8<---------------cut here---------------start------------->8---
> (define-configuration/no-serialization mympd-ip-acl
>   (allow
>    (list-of-string '())
>    "Allowed/Disallowed IP addresses.")
> 
>   (deny
>    (list-of-string '())
>    (documentation 'allow)))  ; group with field 'allow
> 
> ;;; --- texi output below ---
> 
> @c %start of fragment
> @deftp {Data Type} mympd-ip-acl
> Available @code{mympd-ip-acl} fields are:
> 
> @table @asis
> @item @code{allow}
> @itemx @code{deny} (default: @code{()}) (type: list-of-string)
> Allowed/Disallowed IP addresses.
> 
> @end table
> @end deftp
> @c %end of fragment
> --8<---------------cut here---------------end--------------->8---
Sounds good, but you'd have to take extra caution here to not end up in
weird situations.  Consider for example:

(define-configuration micromanaged-acl
  (allow
   (list-of-string '())
   "Allowed/Disallowed IP addresses.")

  (ask
   (list-of-string '())
   "IP addresses which are allowed only after manual confirmation.")

  (deny
   (list-of-string '())
   (documentation 'allow)))  ; group with field 'allow

> Serializer access to other fields
> =====================================================================
> ==========
> 
> Serialization procedures generally only have access to the values of
> its own field. That may be insufficient in some cases as whether a
> field can be serialized or how that is done, for example, can depend
> on the value of other fields.
That sounds like a very, very bad idea.  Instead of having values that
provide no complete information of their own, group them into records.

> mympd-service-type is one example where each serialized field depends
> on the value of another field. Our standard serializer procedures
> were useless for that case.
Well, I'd for one send a patch upstream to separate configuration from
whatever is going on in the "working directory" – that pattern's
hazardous as fuck already.  Other than that, it wouldn't even strictly
be necessary to depend on this or rather, you are actually abusing the
serialization feature to implement a service type that wasn't meant to
be implemented in this manner.

> As a side note, the 'this-record' macro does not work with
> define-configuration which made it impossible to use
> (custom-serializer) for the same effect.
> 
> Instead, serializer procedures could take additional keyword
> arguments:
> --8<---------------cut here---------------start------------->8---
> (define* (serialize-string field-name value (#:key config))
>    (let ((baz-value (assoc-ref config 'baz)))
>       (string-append baz-value "/" value)
>       ...))
> --8<---------------cut here---------------end--------------->8---
Note that for the mympd-case you'd have to repeat the same boilerplate
over and over, hardly making any improvement over how the service is
currently written.

> Inheritable record-type definition
> =====================================================================
> ==========
> 
> The openvpn-service pairs in (gnu services vpn) define a special
> macro define-split-configuration with the purpose to avoid code
> duplication since the service pairs have multiple fields in common.
> 
> Perhaps in a similar vein to SRFI-136, we could have:
> 
> --8<---------------cut here---------------start------------->8---
> (define-configuration openvpn-common-configuration
>   (openvpn
>     (file-like openvpn)
>     "The OpenVPN package.")
>   ;; ...
> )
> 
> (define-configuration openvpn-server-configuration openvpn-common-
> configuration
>   (tls-auth
>     (tls-auth-server #f)
>     "Add an additional layer of HMAC authentication on top of the TLS
> control
> channel to protect against DoS attacks.")
>   ;; ...
> )
> 
> ;;; or through a literal/keyword approach
> 
> (define-configuration openvpn-server-configuration
>   (tls-auth
>     (tls-auth-server #f)
>     "Add an additional layer of HMAC authentication on top of the TLS
> control
> channel to protect against DoS attacks.")
> 
>   (parent openvpn-common-configuration))
> 
> --8<---------------cut here---------------end--------------->8---
Note that with the keyword approach you can no longer name fields
"parent", which might not be what you intended.

> Generic serialize-configuration
> =====================================================================
> ==========
> 
> The procedure serialize-configuration inherently assumes that the
> serialized configuration must be a single string. This assumption
> needn't always hold, especially if the service in question is not a
> shepherd service.
> 
> We could possibly make this procedure a bit more "generic", maybe
> picking some ideas from SRFI-171 Transducers.
> 
> Another improvement to this procedure is to eliminate (or make
> optional) the second parameter, the configuration fields. It's
> unclear what value it brings since one can't use fields from another
> configuration type here.  An argument can be made for selectively
> serializing some of the fields but then it would be more practical to
> make this an optional parameter.
I think it's fine to assume the most common case and let the users
handle their own uncommon beasts.

> PS.
> =====================================================================
> ==========
> 
> Kind of a late realization, but couldn't many of the items above be
> satisfied by improvements to define-record-type* instead?
Isn't the point that started this the realization that define-record-
type* has a feature that's currently inaccessible through define-
configuration?  🙂️

Cheers



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Brainstorming ideas for define-configuration
  2023-03-09  8:13 ` Attila Lendvai
@ 2023-03-09 21:05   ` Josselin Poiret
  0 siblings, 0 replies; 9+ messages in thread
From: Josselin Poiret @ 2023-03-09 21:05 UTC (permalink / raw)
  To: Attila Lendvai, Bruno Victal
  Cc: guix-devel, Felix Lechner, Liliana Marie Prikler, Maxim Cournoyer,
	Ludovic Courtès

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

Hi Attila,

Attila Lendvai <attila@lendvai.name> writes:


> (set! config (apply-config-defaults config))

You can simply shadow the config variable rather than setting its value,
using (let ((config (...))) ...) or even define if you're in the right
form.

Best,
-- 
Josselin Poiret

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Brainstorming ideas for define-configuration
  2023-03-09  2:28 Brainstorming ideas for define-configuration Bruno Victal
                   ` (2 preceding siblings ...)
  2023-03-09 20:23 ` Liliana Marie Prikler
@ 2023-03-10 14:10 ` Maxim Cournoyer
  2023-03-10 20:15 ` jbranso
  2023-03-15 16:27 ` Ludovic Courtès
  5 siblings, 0 replies; 9+ messages in thread
From: Maxim Cournoyer @ 2023-03-10 14:10 UTC (permalink / raw)
  To: Bruno Victal
  Cc: guix-devel, Felix Lechner, Liliana Marie Prikler,
	Ludovic Courtès

Hi Bruno,

Bruno Victal <mirai@makinata.eu> writes:

> Co-authored-by: Felix Lechner
>
>
> After spending some time with old and new Guix services, I'd like to
> suggest some potential improvements to our define-configuration macro:

There seems to be some good suggestions in there, but I'm a bit
struggling to see the big picture.  Perhaps you could make this easier
by breaking down the suggested changes into smaller, easily digested
chunks that would be easier for us to evaluate (as Liliana suggested
too).

The define-configuration stuff is hard enough to stomach already, let's
try to make suggested changes to it as focused as possible :-).

Thanks for working on it!

-- 
Thanks,
Maxim


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Brainstorming ideas for define-configuration
  2023-03-09  2:28 Brainstorming ideas for define-configuration Bruno Victal
                   ` (3 preceding siblings ...)
  2023-03-10 14:10 ` Maxim Cournoyer
@ 2023-03-10 20:15 ` jbranso
  2023-03-10 22:37   ` Liliana Marie Prikler
  2023-03-15 16:27 ` Ludovic Courtès
  5 siblings, 1 reply; 9+ messages in thread
From: jbranso @ 2023-03-10 20:15 UTC (permalink / raw)
  To: Liliana Marie Prikler, Bruno Victal, guix-devel
  Cc: Felix Lechner, Maxim Cournoyer, Ludovic Courtès

March 9, 2023 3:25 PM, "Liliana Marie Prikler" <liliana.prikler@gmail.com> wrote:

> Hi,
> 
> Am Donnerstag, dem 09.03.2023 um 02:28 +0000 schrieb Bruno Victal:
> 
> I smell bad code ahead.
> 
>> We could provide procedures that validate each record type within
>> define-configuration itself instead of validating the value at
>> runtime (i.e. within the body of the service-type).
>> 
>> --8<---------------cut here---------------start------------->8---
>> ;; the common case
>> (define-configuration foo-configuration
>> (name
>> string
>> "Lorem ipsum...")
>> 
>> ;; ...
>> 
>> (validator procname))
>> 
>> ;; [bonus] Simpler configurations that only care for mutually-
>> exclusive fields
>> (define-configuration foo-configuration
>> (name
>> string
>> "Lorem ipsum...")
>> 
>> (title
>> string
>> "Lorem ipsum..."
>> (conflicts 'name)))
>> --8<---------------cut here---------------end--------------->8---
> 
> Instead of providing both a name field and a title field, you might
> provide a field that can either be a name or a title or allow an even
> more powerful value type as long as it makes sense.

While I would agree that a guix service writer should avoid mutually
exclusive fieldnames and instead prefer mutually exclusive records
(and 95% of that time that will work), but may we examine it from a
user's perspective? How does the service writer differentiate from
a string title or string name?

Suppose that you want to respond to a king's rudeness. You can
secretly insult him or obviously insult him:

===Mutually exclusive records===, which are better from a maintainer's
perspective, but perhaps cause the user to write more scheme:

"..your traitor brother. Maybe I’ll feed him to wolves after I’ve
caught him. Did I tell you, I intend to challenge him to single
combat?"

(insult-configuration
  (response
    (secret-insult-configuration
      (secret-insult “I should like to see that, Your Grace.”))))

OR

"You can't insult me."

(insult-configuration
  (response
    (obvious-insult-configuration
      (obvious-insult "We've had vicious kings and we've had idiot kings,
but I don't know if we've ever been cursed with a vicious idiot for
a king!"))))

===Mutually exclusive fieldnames===

"I am the KING!"

(insult-configuration
  (secret-insult "Any man who must say, 'I am the king' is no
true king. I'll show you that after I've won your war.")))

OR

"You are Kingsguard!"

(insult-configuration
  (obvious-insult "...F*ck the King."))))

These examples are pretty wonky I will admit, but I really like
an option of having mutually exclusive fieldnames.  Having said all of this,
I will agree that that mutually exclusive fieldnames are a bit like "goto"
in C.  You really should never use them, unless you absolutely have to.

Thanks,

Joshua

P.S.  I thought about not sending this email, then realized that someone
might find it funny.  Sorry if it wastes your time.  :(


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Brainstorming ideas for define-configuration
  2023-03-10 20:15 ` jbranso
@ 2023-03-10 22:37   ` Liliana Marie Prikler
  0 siblings, 0 replies; 9+ messages in thread
From: Liliana Marie Prikler @ 2023-03-10 22:37 UTC (permalink / raw)
  To: jbranso, Bruno Victal, guix-devel
  Cc: Felix Lechner, Maxim Cournoyer, Ludovic Courtès

Am Freitag, dem 10.03.2023 um 20:15 +0000 schrieb jbranso@dismail.de:
> While I would agree that a guix service writer should avoid mutually
> exclusive fieldnames and instead prefer mutually exclusive records
> (and 95% of that time that will work), but may we examine it from a
> user's perspective? How does the service writer differentiate from
> a string title or string name?
> 
> Suppose that you want to respond to a king's rudeness. You can
> secretly insult him or obviously insult him:
> 
> ===Mutually exclusive records===, which are better from a
> maintainer's perspective, but perhaps cause the user to write more
> scheme:
> 
> "..your traitor brother. Maybe I’ll feed him to wolves after I’ve
> caught him. Did I tell you, I intend to challenge him to single
> combat?"
> 
> (insult-configuration
>   (response
>     (secret-insult-configuration
>       (secret-insult “I should like to see that, Your Grace.”))))
> 
> OR
> 
> "You can't insult me."
> 
> (insult-configuration
>   (response
>     (obvious-insult-configuration
>       (obvious-insult "We've had vicious kings and we've had idiot
> kings, but I don't know if we've ever been cursed with a vicious
> idiot for a king!"))))
> 
> ===Mutually exclusive fieldnames===
> 
> "I am the KING!"
> 
> (insult-configuration
>   (secret-insult "Any man who must say, 'I am the king' is no
> true king. I'll show you that after I've won your war.")))
> 
> OR
> 
> "You are Kingsguard!"
> 
> (insult-configuration
>   (obvious-insult "...F*ck the King."))))
> 
> These examples are pretty wonky I will admit, but I really like
> an option of having mutually exclusive fieldnames.  Having said all
> of this, I will agree that that mutually exclusive fieldnames are a
> bit like "goto" in C.  You really should never use them, unless you
> absolutely have to.
These insult configurations are a little crafted in that the insult-
configuration does not have enough fields to make the necessity of a
nested field obvious.  That is, you could just as easily write
  (insult-configuration
    (tone 'obvious)
    (insult "F*ck the King."))
and look at this record as a "single" value.

Note that neither serialization depends on the other.  For instance,
you can serialize this to XML as 
  <insult tone="obvious">F*ck the King.</insult>
where both fields just need to have their special characters escaped
and the serializer for insult-configuration stitch them together via
format strings.  Obviously, going through SXML would be better, but
it's possible.

Now, if you wanted to add more metadata, like the location at which
you've insulted the king and the time of day, none of which are of
interest for the content of your file, but perhaps of interest the
medium by which you choose to insult him, it would be better to keep
those concerns separate rather than merge them into a single record.

Cheers


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Brainstorming ideas for define-configuration
  2023-03-09  2:28 Brainstorming ideas for define-configuration Bruno Victal
                   ` (4 preceding siblings ...)
  2023-03-10 20:15 ` jbranso
@ 2023-03-15 16:27 ` Ludovic Courtès
  5 siblings, 0 replies; 9+ messages in thread
From: Ludovic Courtès @ 2023-03-15 16:27 UTC (permalink / raw)
  To: Bruno Victal
  Cc: guix-devel, Felix Lechner, Liliana Marie Prikler, Maxim Cournoyer

Hi,

Bruno Victal <mirai@makinata.eu> skribis:

> User-specified sanitizer support

Yay!

> ;; Suggestion #2
> ;; A user-supplied procedure ('procname' below) would work just like the
> ;; procedure in option #1.
> ;; There is some similiarity to the Guix record-type*.
> ;; This could be extended more easily in the future should it be required.
> (define-type typename        ; maybe call this 'define-configuration-type' ?
>   (sanitizer procname)
>   (maybe-type? #t)
>   ;; The properties below are service specific.
>   ;; If this is implemented with Guix record-type* then we could have a
>   ;; module containing generic types and do something along the lines of:
>   ;; (define-type foo-ip-address
>   ;;    (inherit generic-ip-address)
>   ;;    (serializer ...))
>   (serializer procname)      ; define-type/no-serialization = sets this field to #f ?
>   (prefix ...))

I think we should implement contracts at this point, and have per-field
contracts.  We need to look at what Racket is doing.

> Record Validator
> ===============================================================================
>
> There is also a need to validate records. Matching fields alone do not actually
> ensure that the configuration is coherent and usable. For example, some fields
> may be mutually incompatible with others.

Does that require special support though?  Currently that can be done at
serialization time, for example.

> Coalesced documentation
> ===============================================================================
>
> Currently, we manually edit the texinfo output from configuration->documentation
> if we're unsatisfied with the generated result.
> For instance, substituting @item with an @itemx marker for fields whose
> documentation is similar.

Good idea.

> Serializer access to other fields
> ===============================================================================
>
> Serialization procedures generally only have access to the values of its own field. That
> may be insufficient in some cases as whether a field can be serialized
> or how that is done, for example, can depend on the value of other fields.

Overall, I find it nice that serializers have access to nothing but
their own field value; that makes it easier to reason about the whole
process.

> mympd-service-type is one example where each serialized field depends on the value of
> another field. Our standard serializer procedures were useless for that case.

It’d be interesting to look more closely at this example and see if this
can be solved in some other way or if we really need
‘define-configuration’ support.  Would be nice to see if similar
situations arise with other records.

> Inheritable record-type definition

I’d like to support type inheritance in ‘define-record-type*’, and
‘define-configuration’ could build on top of that.

> Generic serialize-configuration
> ===============================================================================
>
> The procedure serialize-configuration inherently assumes that the serialized
> configuration must be a single string. This assumption needn't always hold, especially
> if the service in question is not a shepherd service.

Hmm, no opinion on that one.

Thanks for the grocery list!  :-)

Ludo’.


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-03-15 16:27 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-09  2:28 Brainstorming ideas for define-configuration Bruno Victal
2023-03-09  8:13 ` Attila Lendvai
2023-03-09 21:05   ` Josselin Poiret
2023-03-09 14:40 ` Joshua Branson
2023-03-09 20:23 ` Liliana Marie Prikler
2023-03-10 14:10 ` Maxim Cournoyer
2023-03-10 20:15 ` jbranso
2023-03-10 22:37   ` Liliana Marie Prikler
2023-03-15 16:27 ` Ludovic Courtès

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