Message ID | 20221029041649.12144-1-maxim.cournoyer@gmail.com |
---|---|
State | New |
Headers | show |
Series | [bug#58855,1/5] services: configuration: Re-order generated record fields. | expand |
Context | Check | Description |
---|---|---|
cbaines/comparison | success | View comparision |
cbaines/git-branch | success | View Git branch |
cbaines/applying patch | success | |
cbaines/issue | success | View issue |
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’.
diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm index 636c49ccba..dacfc52ba9 100644 --- a/gnu/services/configuration.scm +++ 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))) (define #,(id #'stem #'stem #'-fields) (list (configuration-field diff --git a/gnu/services/monitoring.scm b/gnu/services/monitoring.scm index 9c8704092c..b19c6c9f18 100644 --- a/gnu/services/monitoring.scm +++ b/gnu/services/monitoring.scm @@ -622,8 +622,8 @@ (define-configuration zabbix-front-end-configuration (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) (mixed-text-file "zabbix.conf.php" "\ <?php