Layout of ‘define-configuration’ records

["Ludovic Courtès" <ludo@gnu.org>]

Hi,

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

> This is so that the first field of the generated record matches the first one
> declared, which makes 'define-configuration' record API compatible with
> define-record-type* ones.
>
> * gnu/services/configuration.scm (define-configuration-helper): Move the
> %location field below the ones declared by the user.
> * gnu/services/monitoring.scm (zabbix-front-end-config): Adjust match pattern
> accordingly.

[...]

> +++ b/gnu/services/configuration.scm
> @@ -242,17 +242,17 @@ (define-record-type* #,(id #'stem #'< #'stem #'>)
>                 stem
>                 #,(id #'stem #'make- #'stem)
>                 #,(id #'stem #'stem #'?)
> -               (%location #,(id #'stem #'stem #'-location)
> -                          (default (and=> (current-source-location)
> -                                          source-properties->location))
> -                          (innate))
>                 #,@(map (lambda (name getter def)
>                           #`(#,name #,getter (default #,def)
>                                     (sanitize
>                                      #,(id #'stem #'validate- #'stem #'- name))))
>                         #'(field ...)
>                         #'(field-getter ...)
> -                       #'(field-default ...)))
> +                       #'(field-default ...))
> +               (%location #,(id #'stem #'stem #'-location)
> +                          (default (and=> (current-source-location)
> +                                          source-properties->location))
> +                          (innate)))

Moving the field last is problematic as we’ve seen for any user that
uses ‘match’ on records—something that’s not recommended but still used
a lot.

>  (define (zabbix-front-end-config config)
>    (match-record config <zabbix-front-end-configuration>
> -    (%location db-host db-port db-name db-user db-password db-secret-file
> -               zabbix-host zabbix-port)
> +    (db-host db-port db-name db-user db-password db-secret-file
> +             zabbix-host zabbix-port %location)

This change has no effect because ‘match-record’ matches fields by name,
precisely to avoid problems when changing the layout of record types.

I’m not sure what was meant by “compatible” in the commit log; how
fields are laid out is something user code should not be aware of.

The only thing to keep in mind is that records must not be matched with
‘match’ because then the code silently breaks when the record type
layout is changed.  This is why ‘match-record’ was introduced:

  https://issues.guix.gnu.org/28960#4

One last thing: placing ‘%location’ first can let us implement:

  (define (configuration-location config)
    (struct-ref config 0))

As if there were type inheritance.  I didn’t see any indication that
this is actually done anywhere, but it could have been the case (it’s
not an unusual pattern) and it’s still something we might want to do.

Does that make sense?

Thanks,
Ludo’.