Invalid 'location' field generated in dovecot configuration

Done

Details

5 participants

Ludovic Courtès
Maxim Cournoyer
mirai
Pierre Langlois
Fredrik Salomonsson

Owner: unassigned

Submitted by: Pierre Langlois

Severity: important

Pierre Langlois wrote on 20 Nov 2022 22:53

Recipients:(address . bug-guix@gnu.org)

Message-ID:87y1s5wa4p.fsf@gmx.com

Hi Guix!

After updating the system, the dovecot service got confused and started

moving around all mailboxes. I looked up the configuration and noticed

strange invalid syntax for the location field:

Toggle snippet (3 lines)

location=#<<location> file: "path/to/config.scm" line: 297 column: 20>

Because the # character is interpreted as a comment, dovecot doesn't

crash and instead moves mailboxes around in weird ways I don't quite

understand :-/.

This can actually be reproduced locally with the dovecot system test if

one dumps the following expression to check the configuration:

Toggle snippet (5 lines)

(format #t "~a\n" (wait-for-file "/etc/dovecot/dovecot.conf"

marionette

#:read 'get-string-all))

Giving us the snippets like this in the config:

Toggle snippet (39 lines)$ make check-system TESTS="dovecot"  VERBOSE=1
...
namespace inbox {
type=private
separator=
prefix=
location=#<<location> file: "gnu/tests/mail.scm" line: 297 column: 20>
inbox=yes
hidden=no
list=yes
subscriptions=yes
mailbox "Drafts" {
auto=no
special_use=\Drafts
}
mailbox "Junk" {
auto=no
special_use=\Junk
}
mailbox "Trash" {
auto=no
special_use=\Trash
}
mailbox "Sent" {
auto=no
special_use=\Sent
}
mailbox "Sent Messages" {
auto=no
special_use=\Sent
}
mailbox "Drafts" {
auto=no
special_use=\Drafts
}
}
...

I did a `git bisect` with `guix time-machine` (this tool is invaluable)

and found the issue started with this commit:

Toggle snippet (15 lines)commit 543d971ed2a1d9eb934af1f51930741d7cc4e7ef
Author: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Date:   Fri Oct 28 17:06:16 2022 -0400
    services: configuration: Re-order generated record fields.
    
    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.

Sooo, I'm guessing this is something to do with the configuration field
being named "location", and /maybe/ we're patching it with the origin
location of the configuration, something like that? I don't understand
how this works well enough to be able to thing of any fixes.

Thanks,
Pierre

Ludovic Courtès wrote on 22 Nov 2022 09:10

Recipients:(name . Pierre Langlois)(address . pierre.langlois@gmx.com)

Message-ID:87ilj7if3o.fsf@gnu.org

Hi,

Pierre Langlois <pierre.langlois@gmx.com> skribis:

Toggle quote (11 lines)> After updating the system, the dovecot service got confused and started
> moving around all mailboxes.  I looked up the configuration and noticed
> strange invalid syntax for the location field:
>
> location=#<<location> file: "path/to/config.scm" line: 297 column: 20>
>
>
> Because the # character is interpreted as a comment, dovecot doesn't
> crash and instead moves mailboxes around in weird ways I don't quite
> understand :-/.

Ouch, sorry about that.

Toggle quote (8 lines)

> I did a `git bisect` with `guix time-machine` (this tool is invaluable)

> and found the issue started with this commit:

> commit 543d971ed2a1d9eb934af1f51930741d7cc4e7ef

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

> Date: Fri Oct 28 17:06:16 2022 -0400

> services: configuration: Re-order generated record fields.

I believe this is now fixed.

Maxim, can you confirm?

Thanks,

Ludo’.

Maxim Cournoyer wrote on 25 Nov 2022 16:36

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)

Message-ID:87mt8fjbbo.fsf@gmail.com

Hi,

Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (17 lines)> Hi,
>
> Pierre Langlois <pierre.langlois@gmx.com> skribis:
>
>> After updating the system, the dovecot service got confused and started
>> moving around all mailboxes.  I looked up the configuration and noticed
>> strange invalid syntax for the location field:
>>
>> location=#<<location> file: "path/to/config.scm" line: 297 column: 20>
>>
>>
>> Because the # character is interpreted as a comment, dovecot doesn't
>> crash and instead moves mailboxes around in weird ways I don't quite
>> understand :-/.
>
> Ouch, sorry about that.

That's a bad situation indeed, apologies for the breakage!

Toggle quote (12 lines)>> I did a `git bisect` with `guix time-machine` (this tool is invaluable)
>> and found the issue started with this commit:
>>
>> commit 543d971ed2a1d9eb934af1f51930741d7cc4e7ef
>> Author: Maxim Cournoyer <maxim.cournoyer@gmail.com>
>> Date:   Fri Oct 28 17:06:16 2022 -0400
>>     services: configuration: Re-order generated record fields.
>
> I believe this is now fixed.
>
> Maxim, can you confirm?

Pierre, has it resolved on your side? I don't use dovecot myself, and

since it doesn't crash, I don't think the dovecot will be an indicator

of resolution.

At least, the %location field value look normal when excercised at the

REPL (#f).

Thanks,

Maxim

mirai wrote on 25 Nov 2022 20:17

Invalid 'location' field generated in dovecot configuration

Recipients:(address . 59423@debbugs.gnu.org)(address . maxim.cournoyer@gmail.com)

Message-ID:0d61b5ad-1523-3dc3-2014-76f16c7011f9@makinata.eu

I'm also experiencing the same issue (guix describe: 7e0ad0dd0f2829d6f3776648ba7c88acf9888d7a).

My guess is that 44554e7133aa60e1b453436be1e80394189cabd9 (which supersedes 543d971ed2a1d9eb934af1f51930741d7cc4e7ef)
introduces a '%location' field which conflicts with 'dovecot-configuration' itself also having a field called 'location'.

In fact, interesting things happen if you define a configuration with a 'location' field.
With 'guix repl':
```
$ guix repl
GNU Guile 3.0.8
Copyright (C) 1995-2021 Free Software Foundation, Inc.

Guile comes with ABSOLUTELY NO WARRANTY; for details type `,show w'.
This program is free software, and you are welcome to redistribute it
under certain conditions; type `,show c' for details.

Enter `,help' for help.
scheme@(guix-user)> (use-modules (gnu services configuration))
(define-configuration FOO-configuration
  (name
   (string "aaa")
   "")
  (location
   (string "bbb")
   ""))
;;; <stdin>:2:0: warning: shadows previous definition of `%FOO-configuration-location-procedure' at <stdin>:2:0
;;; <unknown-location>: warning: shadows previous definition of `FOO-configuration-location' at <unknown-location>
;;; <stdin>:2:0: warning: possibly unbound variable `serialize-string'
ice-9/boot-9.scm:1685:16: In procedure raise-exception:
error: serialize-string: unbound variable

Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
scheme@(guix-user) [1]>
```

Code snippet for convenience:
```
(use-modules (gnu services configuration))
(define-configuration FOO-configuration
  (name
   (string "aaa")
   "")
  (location
   (string "bbb")
   ""))
```

Maxim Cournoyer wrote on 25 Nov 2022 21:06

Recipients:(name . Pierre Langlois)(address . pierre.langlois@gmx.com)(address . 59423@debbugs.gnu.org)

Message-ID:87v8n2iytp.fsf@gmail.com

Hi Pierre,

Pierre Langlois <pierre.langlois@gmx.com> writes:

Toggle quote (60 lines)> Hi Guix!
>
> After updating the system, the dovecot service got confused and started
> moving around all mailboxes.  I looked up the configuration and noticed
> strange invalid syntax for the location field:
>
> location=#<<location> file: "path/to/config.scm" line: 297 column: 20>
>
>
> Because the # character is interpreted as a comment, dovecot doesn't
> crash and instead moves mailboxes around in weird ways I don't quite
> understand :-/.
>
> This can actually be reproduced locally with the dovecot system test if
> one dumps the following expression to check the configuration:
>
> (format #t "~a\n" (wait-for-file "/etc/dovecot/dovecot.conf"
>                                  marionette
>                                  #:read 'get-string-all))
>
>
> Giving us the snippets like this in the config:
>
> $ make check-system TESTS="dovecot"  VERBOSE=1
> ...
> namespace inbox {
> type=private
> separator=
> prefix=
> location=#<<location> file: "gnu/tests/mail.scm" line: 297 column: 20>
> inbox=yes
> hidden=no
> list=yes
> subscriptions=yes
> mailbox "Drafts" {
> auto=no
> special_use=\Drafts
> }
> mailbox "Junk" {
> auto=no
> special_use=\Junk
> }
> mailbox "Trash" {
> auto=no
> special_use=\Trash
> }
> mailbox "Sent" {
> auto=no
> special_use=\Sent
> }
> mailbox "Sent Messages" {
> auto=no
> special_use=\Sent
> }
> mailbox "Drafts" {
> auto=no
> special_use=\Drafts
> }
> }

I did:

$ ./pre-inst-env guix system build -e '(@@ (gnu tests mail) %dovecot-os)'

/gnu/store/gpl6g2ia84kc41zma7ik9y4p3kik5aiy-system

Then:

$ guix gc -R /gnu/store/gpl6g2ia84kc41zma7ik9y4p3kik5aiy-system | grep dovecot.conf

/gnu/store/1ijjsm3sj8v0qj88fhlwqxgdszd6q6h7-dovecot.conf

And what I see in this file is now:

Toggle snippet (36 lines)namespace inbox {
type=private
separator=
prefix=
location=
inbox=yes
hidden=no
list=yes
subscriptions=yes
mailbox "Drafts" {
auto=no
special_use=\Drafts
}
mailbox "Junk" {
auto=no
special_use=\Junk
}
mailbox "Trash" {
auto=no
special_use=\Trash
}
mailbox "Sent" {
auto=no
special_use=\Sent
}
mailbox "Sent Messages" {
auto=no
special_use=\Sent
}
mailbox "Drafts" {
auto=no
special_use=\Drafts
}
}

Notice that location is empty. So that's at least different to your

findings, on latest commit. Can you still reproduce?

Thanks,

Maxim

Pierre Langlois wrote on 25 Nov 2022 21:19

Recipients:(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)

Message-ID:87leny93yp.fsf@gmx.com

Hi,

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

Toggle quote (23 lines)> Hi,
>
> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Hi,
>>
>> Pierre Langlois <pierre.langlois@gmx.com> skribis:
>>
>>> After updating the system, the dovecot service got confused and started
>>> moving around all mailboxes.  I looked up the configuration and noticed
>>> strange invalid syntax for the location field:
>>>
>>> location=#<<location> file: "path/to/config.scm" line: 297 column: 20>
>>>
>>>
>>> Because the # character is interpreted as a comment, dovecot doesn't
>>> crash and instead moves mailboxes around in weird ways I don't quite
>>> understand :-/.
>>
>> Ouch, sorry about that.
>
> That's a bad situation indeed, apologies for the breakage!

No worries! Bugs happen :-). It was confusing but I didn't lose any

data, dovecot moved folders to archives so all I had to do was rolling

guix back and move folders again.

Toggle quote (16 lines)>>> I did a `git bisect` with `guix time-machine` (this tool is invaluable)
>>> and found the issue started with this commit:
>>>
>>> commit 543d971ed2a1d9eb934af1f51930741d7cc4e7ef
>>> Author: Maxim Cournoyer <maxim.cournoyer@gmail.com>
>>> Date:   Fri Oct 28 17:06:16 2022 -0400
>>>     services: configuration: Re-order generated record fields.
>>
>> I believe this is now fixed.
>>
>> Maxim, can you confirm?
>
> Pierre, has it resolved on your side?  I don't use dovecot myself, and
> since it doesn't crash, I don't think the dovecot will be an indicator
> of resolution.

I'm afraid I'm still experiencing the issue, I'll follow-up on the other

thread.

Thanks,

Pierre

Pierre Langlois wrote on 25 Nov 2022 21:25

Recipients:(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)

Message-ID:875yf293ph.fsf@gmx.com

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

Toggle quote (74 lines)> Hi Pierre,
>
> Pierre Langlois <pierre.langlois@gmx.com> writes:
>
>> Hi Guix!
>>
>> After updating the system, the dovecot service got confused and started
>> moving around all mailboxes.  I looked up the configuration and noticed
>> strange invalid syntax for the location field:
>>
>> location=#<<location> file: "path/to/config.scm" line: 297 column: 20>
>>
>>
>> Because the # character is interpreted as a comment, dovecot doesn't
>> crash and instead moves mailboxes around in weird ways I don't quite
>> understand :-/.
>>
>> This can actually be reproduced locally with the dovecot system test if
>> one dumps the following expression to check the configuration:
>>
>> (format #t "~a\n" (wait-for-file "/etc/dovecot/dovecot.conf"
>>                                  marionette
>>                                  #:read 'get-string-all))
>>
>>
>> Giving us the snippets like this in the config:
>>
>> $ make check-system TESTS="dovecot"  VERBOSE=1
>> ...
>> namespace inbox {
>> type=private
>> separator=
>> prefix=
>> location=#<<location> file: "gnu/tests/mail.scm" line: 297 column: 20>
>> inbox=yes
>> hidden=no
>> list=yes
>> subscriptions=yes
>> mailbox "Drafts" {
>> auto=no
>> special_use=\Drafts
>> }
>> mailbox "Junk" {
>> auto=no
>> special_use=\Junk
>> }
>> mailbox "Trash" {
>> auto=no
>> special_use=\Trash
>> }
>> mailbox "Sent" {
>> auto=no
>> special_use=\Sent
>> }
>> mailbox "Sent Messages" {
>> auto=no
>> special_use=\Sent
>> }
>> mailbox "Drafts" {
>> auto=no
>> special_use=\Drafts
>> }
>> }
>
> I did:
>
> $ ./pre-inst-env guix system build -e '(@@ (gnu tests mail) %dovecot-os)'
> /gnu/store/gpl6g2ia84kc41zma7ik9y4p3kik5aiy-system
>
> Then:
>
> $ guix gc -R /gnu/store/gpl6g2ia84kc41zma7ik9y4p3kik5aiy-system | grep dovecot.conf
> /gnu/store/1ijjsm3sj8v0qj88fhlwqxgdszd6q6h7-dovecot.conf

Oh that's a nice way of doing this, better than my hack to print the

config, I'll have to remember the `guix gc -R' flag.

Toggle quote (41 lines)>
> And what I see in this file is now:
>
> namespace inbox {
> type=private
> separator=
> prefix=
> location=
> inbox=yes
> hidden=no
> list=yes
> subscriptions=yes
> mailbox "Drafts" {
> auto=no
> special_use=\Drafts
> }
> mailbox "Junk" {
> auto=no
> special_use=\Junk
> }
> mailbox "Trash" {
> auto=no
> special_use=\Trash
> }
> mailbox "Sent" {
> auto=no
> special_use=\Sent
> }
> mailbox "Sent Messages" {
> auto=no
> special_use=\Sent
> }
> mailbox "Drafts" {
> auto=no
> special_use=\Drafts
> }
> }
>
> Notice that location is empty.  So that's at least different to your
> findings, on latest commit.  Can you still reproduce?

Yeah I'm afraid I still see the same issue after a `git pull` just now:

Toggle snippet (6 lines)

~/code/guix [env]$ ./pre-inst-env guix system build -e '(@@ (gnu tests mail) %dovecot-os)'

/gnu/store/ayfvf5s561q955kv8wrkklrvq3ga3qpy-system

~/code/guix [env]$ guix gc -R /gnu/store/ayfvf5s561q955kv8wrkklrvq3ga3qpy-system | grep dovecot\.conf | xargs grep "^location"

location=#<<location> file: "gnu/tests/mail.scm" line: 297 column: 20>

Have you tried to rebuild from scratch, after a `make clean-go'? When

first bisecting this, I was working from the git repo and couldn't

reproduce the bug. Then it worked by using `guix time-machine' to bisect

rather than work from git.

So I'm guessing the change being in a macro, there could be residue .go

files that need recompiling?

Thanks,

Pierre

Pierre Langlois wrote on 25 Nov 2022 21:50

Recipients:(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)

Message-ID:87y1ry7o69.fsf@gmx.com

Pierre Langlois <pierre.langlois@gmx.com> writes:

Toggle quote (139 lines)> [[PGP Signed Part:Undecided]]
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>
>> Hi Pierre,
>>
>> Pierre Langlois <pierre.langlois@gmx.com> writes:
>>
>>> Hi Guix!
>>>
>>> After updating the system, the dovecot service got confused and started
>>> moving around all mailboxes.  I looked up the configuration and noticed
>>> strange invalid syntax for the location field:
>>>
>>> location=#<<location> file: "path/to/config.scm" line: 297 column: 20>
>>>
>>>
>>> Because the # character is interpreted as a comment, dovecot doesn't
>>> crash and instead moves mailboxes around in weird ways I don't quite
>>> understand :-/.
>>>
>>> This can actually be reproduced locally with the dovecot system test if
>>> one dumps the following expression to check the configuration:
>>>
>>> (format #t "~a\n" (wait-for-file "/etc/dovecot/dovecot.conf"
>>>                                  marionette
>>>                                  #:read 'get-string-all))
>>>
>>>
>>> Giving us the snippets like this in the config:
>>>
>>> $ make check-system TESTS="dovecot"  VERBOSE=1
>>> ...
>>> namespace inbox {
>>> type=private
>>> separator=
>>> prefix=
>>> location=#<<location> file: "gnu/tests/mail.scm" line: 297 column: 20>
>>> inbox=yes
>>> hidden=no
>>> list=yes
>>> subscriptions=yes
>>> mailbox "Drafts" {
>>> auto=no
>>> special_use=\Drafts
>>> }
>>> mailbox "Junk" {
>>> auto=no
>>> special_use=\Junk
>>> }
>>> mailbox "Trash" {
>>> auto=no
>>> special_use=\Trash
>>> }
>>> mailbox "Sent" {
>>> auto=no
>>> special_use=\Sent
>>> }
>>> mailbox "Sent Messages" {
>>> auto=no
>>> special_use=\Sent
>>> }
>>> mailbox "Drafts" {
>>> auto=no
>>> special_use=\Drafts
>>> }
>>> }
>>
>> I did:
>>
>> $ ./pre-inst-env guix system build -e '(@@ (gnu tests mail) %dovecot-os)'
>> /gnu/store/gpl6g2ia84kc41zma7ik9y4p3kik5aiy-system
>>
>> Then:
>>
>> $ guix gc -R /gnu/store/gpl6g2ia84kc41zma7ik9y4p3kik5aiy-system | grep dovecot.conf
>> /gnu/store/1ijjsm3sj8v0qj88fhlwqxgdszd6q6h7-dovecot.conf
>
> <sidetrack/>
>
> Oh that's a nice way of doing this, better than my hack to print the
> config, I'll have to remember the `guix gc -R' flag.
>
>>
>> And what I see in this file is now:
>>
>> namespace inbox {
>> type=private
>> separator=
>> prefix=
>> location=
>> inbox=yes
>> hidden=no
>> list=yes
>> subscriptions=yes
>> mailbox "Drafts" {
>> auto=no
>> special_use=\Drafts
>> }
>> mailbox "Junk" {
>> auto=no
>> special_use=\Junk
>> }
>> mailbox "Trash" {
>> auto=no
>> special_use=\Trash
>> }
>> mailbox "Sent" {
>> auto=no
>> special_use=\Sent
>> }
>> mailbox "Sent Messages" {
>> auto=no
>> special_use=\Sent
>> }
>> mailbox "Drafts" {
>> auto=no
>> special_use=\Drafts
>> }
>> }
>>
>> Notice that location is empty.  So that's at least different to your
>> findings, on latest commit.  Can you still reproduce?
>
> Yeah I'm afraid I still see the same issue after a `git pull` just now:
>
> ~/code/guix [env]$ ./pre-inst-env guix system build -e '(@@ (gnu tests mail) %dovecot-os)'
> /gnu/store/ayfvf5s561q955kv8wrkklrvq3ga3qpy-system
> ~/code/guix [env]$ guix gc -R /gnu/store/ayfvf5s561q955kv8wrkklrvq3ga3qpy-system | grep dovecot\.conf | xargs grep "^location"
> location=#<<location> file: "gnu/tests/mail.scm" line: 297 column: 20>
>
> Have you tried to rebuild from scratch, after a `make clean-go'? When
> first bisecting this, I was working from the git repo and couldn't
> reproduce the bug. Then it worked by using `guix time-machine' to bisect
> rather than work from git.
>
> So I'm guessing the change being in a macro, there could be residue .go
> files that need recompiling?

Oh, I just realized the change was reverted with
44554e7133aa60e1b453436be1e80394189cabd9, then I'm probably the one who
needs to do a `make clean-go' :-).

Pierre Langlois wrote on 25 Nov 2022 22:09

Recipients:(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)

Message-ID:87cz9a7n4y.fsf@gmx.com

Pierre Langlois <pierre.langlois@gmx.com> writes:

Toggle quote (147 lines)> [[PGP Signed Part:Undecided]]
>
> Pierre Langlois <pierre.langlois@gmx.com> writes:
>
>> [[PGP Signed Part:Undecided]]
>>
>> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>>
>>> Hi Pierre,
>>>
>>> Pierre Langlois <pierre.langlois@gmx.com> writes:
>>>
>>>> Hi Guix!
>>>>
>>>> After updating the system, the dovecot service got confused and started
>>>> moving around all mailboxes.  I looked up the configuration and noticed
>>>> strange invalid syntax for the location field:
>>>>
>>>> location=#<<location> file: "path/to/config.scm" line: 297 column: 20>
>>>>
>>>>
>>>> Because the # character is interpreted as a comment, dovecot doesn't
>>>> crash and instead moves mailboxes around in weird ways I don't quite
>>>> understand :-/.
>>>>
>>>> This can actually be reproduced locally with the dovecot system test if
>>>> one dumps the following expression to check the configuration:
>>>>
>>>> (format #t "~a\n" (wait-for-file "/etc/dovecot/dovecot.conf"
>>>>                                  marionette
>>>>                                  #:read 'get-string-all))
>>>>
>>>>
>>>> Giving us the snippets like this in the config:
>>>>
>>>> $ make check-system TESTS="dovecot"  VERBOSE=1
>>>> ...
>>>> namespace inbox {
>>>> type=private
>>>> separator=
>>>> prefix=
>>>> location=#<<location> file: "gnu/tests/mail.scm" line: 297 column: 20>
>>>> inbox=yes
>>>> hidden=no
>>>> list=yes
>>>> subscriptions=yes
>>>> mailbox "Drafts" {
>>>> auto=no
>>>> special_use=\Drafts
>>>> }
>>>> mailbox "Junk" {
>>>> auto=no
>>>> special_use=\Junk
>>>> }
>>>> mailbox "Trash" {
>>>> auto=no
>>>> special_use=\Trash
>>>> }
>>>> mailbox "Sent" {
>>>> auto=no
>>>> special_use=\Sent
>>>> }
>>>> mailbox "Sent Messages" {
>>>> auto=no
>>>> special_use=\Sent
>>>> }
>>>> mailbox "Drafts" {
>>>> auto=no
>>>> special_use=\Drafts
>>>> }
>>>> }
>>>
>>> I did:
>>>
>>> $ ./pre-inst-env guix system build -e '(@@ (gnu tests mail) %dovecot-os)'
>>> /gnu/store/gpl6g2ia84kc41zma7ik9y4p3kik5aiy-system
>>>
>>> Then:
>>>
>>> $ guix gc -R /gnu/store/gpl6g2ia84kc41zma7ik9y4p3kik5aiy-system | grep dovecot.conf
>>> /gnu/store/1ijjsm3sj8v0qj88fhlwqxgdszd6q6h7-dovecot.conf
>>
>> <sidetrack/>
>>
>> Oh that's a nice way of doing this, better than my hack to print the
>> config, I'll have to remember the `guix gc -R' flag.
>>
>>>
>>> And what I see in this file is now:
>>>
>>> namespace inbox {
>>> type=private
>>> separator=
>>> prefix=
>>> location=
>>> inbox=yes
>>> hidden=no
>>> list=yes
>>> subscriptions=yes
>>> mailbox "Drafts" {
>>> auto=no
>>> special_use=\Drafts
>>> }
>>> mailbox "Junk" {
>>> auto=no
>>> special_use=\Junk
>>> }
>>> mailbox "Trash" {
>>> auto=no
>>> special_use=\Trash
>>> }
>>> mailbox "Sent" {
>>> auto=no
>>> special_use=\Sent
>>> }
>>> mailbox "Sent Messages" {
>>> auto=no
>>> special_use=\Sent
>>> }
>>> mailbox "Drafts" {
>>> auto=no
>>> special_use=\Drafts
>>> }
>>> }
>>>
>>> Notice that location is empty.  So that's at least different to your
>>> findings, on latest commit.  Can you still reproduce?
>>
>> Yeah I'm afraid I still see the same issue after a `git pull` just now:
>>
>> ~/code/guix [env]$ ./pre-inst-env guix system build -e '(@@ (gnu tests mail) %dovecot-os)'
>> /gnu/store/ayfvf5s561q955kv8wrkklrvq3ga3qpy-system
>> ~/code/guix [env]$ guix gc -R /gnu/store/ayfvf5s561q955kv8wrkklrvq3ga3qpy-system | grep dovecot\.conf | xargs grep "^location"
>> location=#<<location> file: "gnu/tests/mail.scm" line: 297 column: 20>
>>
>> Have you tried to rebuild from scratch, after a `make clean-go'? When
>> first bisecting this, I was working from the git repo and couldn't
>> reproduce the bug. Then it worked by using `guix time-machine' to bisect
>> rather than work from git.
>>
>> So I'm guessing the change being in a macro, there could be residue .go
>> files that need recompiling?
>
> Oh, I just realized the change was reverted with
> 44554e7133aa60e1b453436be1e80394189cabd9, then I'm probably the one who
> needs to do a `make clean-go' :-).

I'm afraid I can still reproduce the issue after a fresh rebuild, I can

also reproduce it outside of the git checkout:

Toggle snippet (30 lines)$ guix describe
Generation 1026 Nov 25 2022 20:11:23    (current)
  guix 7e0ad0d
    repository URL: https://git.savannah.gnu.org/git/guix.git
    branch: master
    commit: 7e0ad0dd0f2829d6f3776648ba7c88acf9888d7a
(... snip ...)
$ guix gc -R `guix system build -e '(@@ (gnu tests mail) %dovecot-os)'` | grep dovecot\.conf | xargs head -n 20
listen=*,::
dict {
}
passdb {
driver=pam
}
userdb {
driver=passwd
override_fields=
}
plugin {
}
namespace inbox {
type=private
separator=
prefix=
location=#<<location> file: "gnu/tests/mail.scm" line: 297 column: 20>
inbox=yes
hidden=no
list=yes

Now I'm confused :-/.

Maxim Cournoyer wrote on 26 Nov 2022 03:54

Recipients:(name . Pierre Langlois)(address . pierre.langlois@gmx.com)(address . 59423@debbugs.gnu.org)

Message-ID:87fse6fmrq.fsf@gmail.com

Hi Pierre,

Pierre Langlois <pierre.langlois@gmx.com> writes:

[...]

Toggle quote (10 lines)>>>> I did:
>>>>
>>>> $ ./pre-inst-env guix system build -e '(@@ (gnu tests mail) %dovecot-os)'
>>>> /gnu/store/gpl6g2ia84kc41zma7ik9y4p3kik5aiy-system
>>>>
>>>> Then:
>>>>
>>>> $ guix gc -R /gnu/store/gpl6g2ia84kc41zma7ik9y4p3kik5aiy-system | grep dovecot.conf
>>>> /gnu/store/1ijjsm3sj8v0qj88fhlwqxgdszd6q6h7-dovecot.conf

OK, after 'make clean-go' and rebuilding everything, I now see the issue

with the above:

Toggle snippet (3 lines)

location=#<<location> file: "gnu/tests/mail.scm" line: 297 column: 20>

So, that's that.

Toggle quote (21 lines)>>> Yeah I'm afraid I still see the same issue after a `git pull` just now:
>>>
>>> ~/code/guix [env]$ ./pre-inst-env guix system build -e '(@@ (gnu tests mail) %dovecot-os)'
>>> /gnu/store/ayfvf5s561q955kv8wrkklrvq3ga3qpy-system
>>> ~/code/guix [env]$ guix gc -R
>>> /gnu/store/ayfvf5s561q955kv8wrkklrvq3ga3qpy-system | grep
>>> dovecot\.conf | xargs grep "^location"
>>> location=#<<location> file: "gnu/tests/mail.scm" line: 297 column: 20>
>>>
>>> Have you tried to rebuild from scratch, after a `make clean-go'? When
>>> first bisecting this, I was working from the git repo and couldn't
>>> reproduce the bug. Then it worked by using `guix time-machine' to bisect
>>> rather than work from git.
>>>
>>> So I'm guessing the change being in a macro, there could be residue .go
>>> files that need recompiling?
>>
>> Oh, I just realized the change was reverted with
>> 44554e7133aa60e1b453436be1e80394189cabd9, then I'm probably the one who
>> needs to do a `make clean-go' :-).

The change was reinstated as part of the mcron update, in

44554e7133aa60e1b453436be1e80394189cabd9. The bit that seems to cause

the issue here (still not clearly understood) is probably this one:

Toggle snippet (29 lines)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

Reverting it would likely fix the issue (haven't tried), but it'd be

nice to have a clear understanding of what's going on. It may have

unmasked a bug waiting to bite.

The issue seems to be with the serialization of the

<namespace-configuration> object nested in the <dovecot-configuration>

record. I tried this at the REPL:

Toggle snippet (16 lines)scheme@(guile-user)> ,m (gnu services mail)
scheme@(gnu services mail)> (namespace-configuration (name "inbox"))
$8 = #<<namespace-configuration> name: "inbox" type: "private" separator: "" prefix: "" location: "" inbox?: #f hidden?: #f list?: #t subscriptions?: #t mailboxes: () %location: #f>
scheme@(gnu services mail)> (serialize-configuration $8 namespace-configuration-fields)
name=inbox
type=private
separator=
prefix=
location=#f
inbox=no
hidden=no
list=yes
subscriptions=yes
$9 = #<gexp  gnu/services/configuration.scm:123:2 7f78f494fde0>

But as you can see, it doesn't reproduce in this environment. I'll keep experimenting.

Thanks,

Maxim

Pierre Langlois wrote on 26 Nov 2022 20:32

Recipients:(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)

Message-ID:87cz995wwu.fsf@gmx.com

Hi Maxim,

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

[...]

Toggle quote (72 lines)>>>> Yeah I'm afraid I still see the same issue after a `git pull` just now:
>>>>
>>>> ~/code/guix [env]$ ./pre-inst-env guix system build -e '(@@ (gnu tests mail) %dovecot-os)'
>>>> /gnu/store/ayfvf5s561q955kv8wrkklrvq3ga3qpy-system
>>>> ~/code/guix [env]$ guix gc -R
>>>> /gnu/store/ayfvf5s561q955kv8wrkklrvq3ga3qpy-system | grep
>>>> dovecot\.conf | xargs grep "^location"
>>>> location=#<<location> file: "gnu/tests/mail.scm" line: 297 column: 20>
>>>>
>>>> Have you tried to rebuild from scratch, after a `make clean-go'? When
>>>> first bisecting this, I was working from the git repo and couldn't
>>>> reproduce the bug. Then it worked by using `guix time-machine' to bisect
>>>> rather than work from git.
>>>>
>>>> So I'm guessing the change being in a macro, there could be residue .go
>>>> files that need recompiling?
>>>
>>> Oh, I just realized the change was reverted with
>>> 44554e7133aa60e1b453436be1e80394189cabd9, then I'm probably the one who
>>> needs to do a `make clean-go' :-).
>
> The change was reinstated as part of the mcron update, in
> 44554e7133aa60e1b453436be1e80394189cabd9.  The bit that seems to cause
> the issue here (still not clearly understood) is probably this one:
>
> 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
>
>
> Reverting it would likely fix the issue (haven't tried), but it'd be
> nice to have a clear understanding of what's going on.  It may have
> unmasked a bug waiting to bite.
>
> The issue seems to be with the serialization of the
> <namespace-configuration> object nested in the <dovecot-configuration>
> record.  I tried this at the REPL:
>
> scheme@(guile-user)> ,m (gnu services mail)
> scheme@(gnu services mail)> (namespace-configuration (name "inbox"))
> $8 = #<<namespace-configuration> name: "inbox" type: "private" separator: "" prefix: "" location: "" inbox?: #f hidden?: #f list?: #t subscriptions?: #t mailboxes: () %location: #f>
> scheme@(gnu services mail)> (serialize-configuration $8 namespace-configuration-fields)
> name=inbox
> type=private
> separator=
> prefix=
> location=#f

The location here should probably be empty rather than `#f' no? It looks

as though the value is coming from the internal %location, rather than

the user-provided location. If the two fields can shadow each other,

then indeed, that looks like an existing bug that was exposed by the

reordering, rather than a bug with the reorder itself.

I'll if I can find anything the macro, it looks quite complex to me :-).

Toggle quote (8 lines)

> inbox=no

> hidden=no

> list=yes

> subscriptions=yes

> $9 = #<gexp gnu/services/configuration.scm:123:2 7f78f494fde0>

> But as you can see, it doesn't reproduce in this environment. I'll keep experimenting.

Thanks for looking into this!

Fredrik Salomonsson wrote on 27 Nov 2022 00:17

Invalid 'location' field generated in dovecot configuration

Recipients:(address . 59423@debbugs.gnu.org)

Message-ID:878rjxxq4o.fsf@posteo.net

Hello,

I think I encountered the same bug when I updated my home rofi service

to use a configuration. It also has a `location` entry but it takes an

integer.

Here is a repro to reproduce it using `guix repl`:

scheme@(guix-user)> (use-modules (gnu services configuration))

scheme@(guix-user)> (define (serialize-integer f v) (number->string v))

scheme@(guix-user)> (define-configuration repro-configuration (location (integer 2) "set location"))

;;; <stdin>:3:0: warning: shadows previous definition of `%repro-configuration-location-procedure' at <stdin>:3:0

;;; <unknown-location>: warning: shadows previous definition of `repro-configuration-location' at <unknown-location>

scheme@(guix-user)> (serialize-configuration (repro-configuration) repro-configuration-fields)

ice-9/boot-9.scm:1685:16: In procedure raise-exception:

In procedure number->string: Wrong type argument in position 1: #f

Entering a new prompt. Type `,bt' for a backtrace or `,q' to continue.

scheme@(guix-user) [1]> ,q

scheme@(guix-user)> (repro-configuration)

$1 = #<<repro-configuration> location: 2 %location: #f>

Note the warning about location being shadowed. And it looks like

%location is trying to use `serialize-integer` but failing as it is #f.

Here is what `guix describe` reports on what guix version I'm running:

Generation 13 nov 26 2022 15:05:06 (current)

guix 68925b5

repository URL: https://git.savannah.gnu.org/git/guix.git

branch: master

commit: 68925b5ee7e0d96b0c84ae98a633eea5097bf511

I hope this helps with debugging this.

s/Fred[re]+i[ck]+/Fredrik/g

Maxim Cournoyer wrote on 27 Nov 2022 03:33

Recipients:(name . Pierre Langlois)(address . pierre.langlois@gmx.com)

Message-ID:87o7stw2gz.fsf@gmail.com

Hi Pierre,

Pierre Langlois <pierre.langlois@gmx.com> writes:

Toggle quote (4 lines)

> Hi Maxim,

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

[...]

Toggle quote (55 lines)>> The change was reinstated as part of the mcron update, in
>> 44554e7133aa60e1b453436be1e80394189cabd9.  The bit that seems to cause
>> the issue here (still not clearly understood) is probably this one:
>>
>> 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
>>
>>
>> Reverting it would likely fix the issue (haven't tried), but it'd be
>> nice to have a clear understanding of what's going on.  It may have
>> unmasked a bug waiting to bite.
>>
>> The issue seems to be with the serialization of the
>> <namespace-configuration> object nested in the <dovecot-configuration>
>> record.  I tried this at the REPL:
>>
>> scheme@(guile-user)> ,m (gnu services mail)
>> scheme@(gnu services mail)> (namespace-configuration (name "inbox"))
>> $8 = #<<namespace-configuration> name: "inbox" type: "private" separator: "" prefix: "" location: "" inbox?: #f hidden?: #f list?: #t subscriptions?: #t mailboxes: () %location: #f>
>> scheme@(gnu services mail)> (serialize-configuration $8 namespace-configuration-fields)
>> name=inbox
>> type=private
>> separator=
>> prefix=
>> location=#f
>
> The location here should probably be empty rather than `#f' no? It looks
> as though the value is coming from the internal %location, rather than
> the user-provided location.

Good eye! Perhaps my earlier simple session was able to reproduce after

all! #f sure doesn't read as a successfully serialized value :-). It

probably came from %location, which is set to #f when working at the

REPL.

Toggle quote (6 lines)

> If the two fields can shadow each other,

> then indeed, that looks like an existing bug that was exposed by the

> reordering, rather than a bug with the reorder itself.

> I'll if I can find anything the macro, it looks quite complex to me :-).

It's not only to you, if that helps. It's rather... intimidating ^^'.

Looking at it again, the problem is not so mysterious after all... The

%location field has its accessor set to be:

Toggle snippet (6 lines)

(%location #,(id #'stem #'stem #'-location)

(default (and=> (current-source-location)

source-properties->location))

(innate)))

#,(id #'stem #'stem #'-location)

which gets expanded to namespace-configuration-location, which shadows

that of the now preceding "location" field.

The bug in the previous condition would have been reversed; the source

location would have been shadowed by the "location" field value.

Ludovic, would you have an idea of where the %location field or its

CONFIGURATION-location accessor come into play? I tried tracing it in

the source, but I only see it being set and the location being pulled

from the sanitizer via "(datum->syntax #'value (syntax-source #'value)"

in (gnu services configuration) around line 227, which is the location

that would get printed in the error handler CALL-WITH-ERROR-HANDLING

from (guix ui):

Toggle snippet (10 lines)

((formatted-message? c)

(apply report-error

(and (error-location? c) (error-location c))

(gettext (formatted-message-string c) %gettext-domain)

(formatted-message-arguments c))

(when (fix-hint? c)

(display-hint (condition-fix-hint c)))

(exit 1))

I could be wrong, but since the "location" of a field appears to be an
"intrinsic" property rather than something explicitly attached to it,
perhaps there's no need for a "location" accessor?  Or it could be named
differently, such as "%location" to reduce the risk of clashing with
user-defined fields.

Does that make sense?

-- 
Thanks,
Maxim

Ludovic Courtès wrote on 28 Nov 2022 16:01

Recipients:(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)

Message-ID:87pmd7ceck.fsf@gnu.org

Hi,

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

Toggle quote (18 lines)>>> The issue seems to be with the serialization of the
>>> <namespace-configuration> object nested in the <dovecot-configuration>
>>> record.  I tried this at the REPL:
>>>
>>> scheme@(guile-user)> ,m (gnu services mail)
>>> scheme@(gnu services mail)> (namespace-configuration (name "inbox"))
>>> $8 = #<<namespace-configuration> name: "inbox" type: "private" separator: "" prefix: "" location: "" inbox?: #f hidden?: #f list?: #t subscriptions?: #t mailboxes: () %location: #f>
>>> scheme@(gnu services mail)> (serialize-configuration $8 namespace-configuration-fields)
>>> name=inbox
>>> type=private
>>> separator=
>>> prefix=
>>> location=#f
>>
>> The location here should probably be empty rather than `#f' no? It looks
>> as though the value is coming from the internal %location, rather than
>> the user-provided location.

Uh.

Toggle quote (4 lines)

>> I'll if I can find anything the macro, it looks quite complex to me :-).

> It's not only to you, if that helps. It's rather... intimidating ^^'.

[...]

Toggle quote (3 lines)

> Ludovic, would you have an idea of where the %location field or its

> CONFIGURATION-location accessor come into play?

We have this:

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

That generates two accessors called ‘namespace-configuration-location’.
The second one shadows the first one.  With commit
44554e7133aa60e1b453436be1e80394189cabd9, the second one is the “wrong”
one: ‘namespace-configuration-location’ now returns the ‘%location’
field, not the user-specified ‘location’ field.  (I reported that issue
in https://issues.guix.gnu.org/48284.)

What do you think of reverting 44554e7133aa60e1b453436be1e80394189cabd9?

After that we can work on renaming the ‘location’ field of
<namespace-configuration> while preserving backward compatibility.

Thanks,
Ludo’.

Maxim Cournoyer wrote on 28 Nov 2022 21:00

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)

Message-ID:87sfi2svb9.fsf@gmail.com

Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (56 lines)> Hi,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>>>> The issue seems to be with the serialization of the
>>>> <namespace-configuration> object nested in the <dovecot-configuration>
>>>> record.  I tried this at the REPL:
>>>>
>>>> scheme@(guile-user)> ,m (gnu services mail)
>>>> scheme@(gnu services mail)> (namespace-configuration (name "inbox"))
>>>> $8 = #<<namespace-configuration> name: "inbox" type: "private"
>>>> separator: "" prefix: "" location: "" inbox?: #f hidden?: #f
>>>> list?: #t subscriptions?: #t mailboxes: () %location: #f>
>>>> scheme@(gnu services mail)> (serialize-configuration $8 namespace-configuration-fields)
>>>> name=inbox
>>>> type=private
>>>> separator=
>>>> prefix=
>>>> location=#f
>>>
>>> The location here should probably be empty rather than `#f' no? It looks
>>> as though the value is coming from the internal %location, rather than
>>> the user-provided location.
>
> Uh.
>
>>> I'll if I can find anything the macro, it looks quite complex to me :-).
>>
>> It's not only to you, if that helps.  It's rather... intimidating ^^'.
>
> [...]
>
>> Ludovic, would you have an idea of where the %location field or its
>> CONFIGURATION-location accessor come into play?
>
> We have this:
>
>          (define-record-type* #,(id #'stem #'< #'stem #'>)
>            stem
>            #,(id #'stem #'make- #'stem)
>            #,(id #'stem #'stem #'?)
>            #,@(map (lambda (name getter def)
>                      #`(#,name #,getter (default #,def)
>                                (sanitize
>                                 #,(id #'stem #'validate- #'stem #'- name))))
>                    #'(field ...)
>                    #'(field-getter ...)
>                    #'(field-default ...))
>            (%location #,(id #'stem #'stem #'-location)
>                       (default (and=> (current-source-location)
>                                       source-properties->location))
>                       (innate)))
>
> That generates two accessors called ‘namespace-configuration-location’.
> The second one shadows the first one.

Yes.  You didn't address my question directly though, so let me ask it
again: where is this %location field access (named "location") used?  It
seems nowhere.  Thus, we can simply rename it without impacting
anything, right (except theoretical usages in the wild, since in the
API).

Toggle quote (7 lines)

> With commit 44554e7133aa60e1b453436be1e80394189cabd9, the second one

> is the “wrong” one: ‘namespace-configuration-location’ now returns the

> ‘%location’ field, not the user-specified ‘location’ field. (I

> reported that issue in <https://issues.guix.gnu.org/48284>.)

> What do you think of reverting 44554e7133aa60e1b453436be1e80394189cabd9?

No. If we revert something, it won't be that whole commit, but just the

moving of the field in the define-configuration produced record.

Toggle quote (3 lines)

> After that we can work on renaming the ‘location’ field of

> <namespace-configuration> while preserving backward compatibility.

Why do we have to massage the user facing record
(namespace-configuration) instead of the underlying mechanics?  The
macro should serve us, not the other way around :-).  See my idea to
simply rename/remove that automatically produced "location" accessor
which appears unused to me.  Would that work?

-- 
Thanks,
Maxim

Ludovic Courtès wrote on 28 Nov 2022 22:33

Recipients:(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)

Message-ID:8735a2ahmi.fsf@gnu.org

Hi Maxim,

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

Toggle quote (24 lines)>> We have this:
>>
>>          (define-record-type* #,(id #'stem #'< #'stem #'>)
>>            stem
>>            #,(id #'stem #'make- #'stem)
>>            #,(id #'stem #'stem #'?)
>>            #,@(map (lambda (name getter def)
>>                      #`(#,name #,getter (default #,def)
>>                                (sanitize
>>                                 #,(id #'stem #'validate- #'stem #'- name))))
>>                    #'(field ...)
>>                    #'(field-getter ...)
>>                    #'(field-default ...))
>>            (%location #,(id #'stem #'stem #'-location)
>>                       (default (and=> (current-source-location)
>>                                       source-properties->location))
>>                       (innate)))
>>
>> That generates two accessors called ‘namespace-configuration-location’.
>> The second one shadows the first one.
>
> Yes.  You didn't address my question directly though, so let me ask it
> again: where is this %location field access (named "location") used?

When doing something like this:

Toggle snippet (16 lines)scheme@(guix-user)> ,m(gnu services mail)
scheme@(gnu services mail)> (namespace-configuration (name "inbox"))
$1 = #<<namespace-configuration> name: "inbox" type: "private" separator: "" prefix: "" location: "" inbox?: #f hidden?: #f list?: #t subscriptions?: #t mailboxes: () %location: #f>
scheme@(gnu services mail)> (serialize-configuration $1 namespace-configuration-fields)
name=inbox
type=private
separator=
prefix=
location=#f
inbox=no
hidden=no
list=yes
subscriptions=yes
$2 = #<gexp  gnu/services/configuration.scm:123:2 7f196470ac00>

… the field is accessed via its accessor,

‘name-configuration-location’. We can kinda see it here:

Toggle snippet (8 lines)

scheme@(gnu services mail)> ,pp namespace-configuration-fields

$1 = (#<<configuration-field> name: name type: string getter: #<procedure %namespace-configuration-name-procedure (s)> predicate: #<procedure string? (_)> serializer: #<procedure serialize-string (field-name val)> default-value-thunk: #<procedure 7fce8d866478 at gnu/services/mail.scm:433:0 ()> documentation: "Name for this namespace.">

#<<configuration-field> name: type type: string getter: #<procedure %namespace-configuration-type-procedure (s)> predicate: #<procedure string? (_)> serializer: #<procedure serialize-string (field-name val)> default-value-thunk: #<procedure 7fce8d8664a8 at gnu/services/mail.scm:433:0 ()> documentation: "Namespace type: @samp{private}, @samp{shared} or @samp{public}.">

[…]

#<<configuration-field> name: location type: string getter: #<procedure %namespace-configuration-location-procedure (s)> predicate: #<procedure string? (_)> serializer: #<procedure serialize-string (field-name val)> default-value-thunk: #<procedure 7fce8d866538 at gnu/services/mail.scm:433:0 ()> documentation: "Physical location of the mailbox. This is in same format as\nmail_location, which is also the default for it.">

[…]

Each <configuration-field> record has a ‘getter’ field that refers to

the accessor. In the case of ‘location’, that’s the “wrong”

accessor—the accessor of ‘%location’.

I hope that addresses your question!

Toggle quote (5 lines)

>> What do you think of reverting 44554e7133aa60e1b453436be1e80394189cabd9?

> No. If we revert something, it won't be that whole commit, but just the

> moving of the field in the define-configuration produced record.

Yes, that’s what I meant; sorry for the confusion.

Toggle quote (9 lines)

>> After that we can work on renaming the ‘location’ field of

>> <namespace-configuration> while preserving backward compatibility.

> Why do we have to massage the user facing record

> (namespace-configuration) instead of the underlying mechanics? The

> macro should serve us, not the other way around :-). See my idea to

> simply rename/remove that automatically produced "location" accessor

> which appears unused to me. Would that work?

What would need renaming in this case is the accessor, not the field.

In https://issues.guix.gnu.org/48284 you proposed renaming the

accessor to *-source-location instead of *-location. That sounds like a

good idea to me. We should get unbound-variable warnings in modules

that use the previous name, so we’ll see if that breaks something.

The only downside is that the convention elsewhere in the code is

-location, not -source-location.

So the other option is to rename ‘location’ in

<namespace-configuration>. That also has the advantage of being less

intrusive.

WDYT?

Ludo’.

Maxim Cournoyer wrote on 29 Nov 2022 02:58

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)

Message-ID:87a64aseqm.fsf@gmail.com

Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (28 lines)> Hi Maxim,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>>> We have this:
>>>
>>>          (define-record-type* #,(id #'stem #'< #'stem #'>)
>>>            stem
>>>            #,(id #'stem #'make- #'stem)
>>>            #,(id #'stem #'stem #'?)
>>>            #,@(map (lambda (name getter def)
>>>                      #`(#,name #,getter (default #,def)
>>>                                (sanitize
>>>                                 #,(id #'stem #'validate- #'stem #'- name))))
>>>                    #'(field ...)
>>>                    #'(field-getter ...)
>>>                    #'(field-default ...))
>>>            (%location #,(id #'stem #'stem #'-location)
>>>                       (default (and=> (current-source-location)
>>>                                       source-properties->location))
>>>                       (innate)))
>>>
>>> That generates two accessors called ‘namespace-configuration-location’.
>>> The second one shadows the first one.
>>
>> Yes.  You didn't address my question directly though, so let me ask it
>> again: where is this %location field access (named "location") used?

[...]

Toggle quote (3 lines)

> … the field is accessed via its accessor,

> ‘name-configuration-location’. We can kinda see it here:

[...]

Toggle quote (6 lines)

> Each <configuration-field> record has a ‘getter’ field that refers to

> the accessor. In the case of ‘location’, that’s the “wrong”

> accessor—the accessor of ‘%location’.

> I hope that addresses your question!

No :-). I meant why do we even set a default accessor for the *source

location* information (in the (gnu service configuration) macros); it's

that one that doesn't seem to get used (or I'm blind to it!), at least

via this accessor. If it's not strictly necessary, we can stop

producing it, and that would solve the problem.

Does that make sense? I'm not talking about namespace-location; that

one has the right to be! I'm talking about the auto-generated

x-location or y-location, where x and y are configuration records that

were specified via 'define-configuration'.

[...]

Toggle quote (9 lines)

> What would need renaming in this case is the accessor, not the field.

> In <https://issues.guix.gnu.org/48284> you proposed renaming the

> accessor to *-source-location instead of *-location. That sounds like a

> good idea to me. We should get unbound-variable warnings in modules

> that use the previous name, so we’ll see if that breaks something.

> The only downside is that the convention elsewhere in the code is

> -location, not -source-location.

What about giving it an even more cryptic accessor name like -%location

or dropping it entirely as suggested above?

Toggle quote (4 lines)

> So the other option is to rename ‘location’ in

> <namespace-configuration>. That also has the advantage of being less

> intrusive.

I don't like that alternative, because 'location' seems likely to be
used for future services and reintroduce the same name clash problem.

-- 
Thanks,
Maxim

Pierre Langlois wrote on 1 Dec 2022 21:29

Recipients:(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)

Message-ID:878rjqg8zh.fsf@gmx.com

Hi Maxim,

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

Toggle quote (76 lines)> Hi Ludovic,
>
> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Hi,
>>
>> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>>
>>>>> The issue seems to be with the serialization of the
>>>>> <namespace-configuration> object nested in the <dovecot-configuration>
>>>>> record.  I tried this at the REPL:
>>>>>
>>>>> scheme@(guile-user)> ,m (gnu services mail)
>>>>> scheme@(gnu services mail)> (namespace-configuration (name "inbox"))
>>>>> $8 = #<<namespace-configuration> name: "inbox" type: "private"
>>>>> separator: "" prefix: "" location: "" inbox?: #f hidden?: #f
>>>>> list?: #t subscriptions?: #t mailboxes: () %location: #f>
>>>>> scheme@(gnu services mail)> (serialize-configuration $8 namespace-configuration-fields)
>>>>> name=inbox
>>>>> type=private
>>>>> separator=
>>>>> prefix=
>>>>> location=#f
>>>>
>>>> The location here should probably be empty rather than `#f' no? It looks
>>>> as though the value is coming from the internal %location, rather than
>>>> the user-provided location.
>>
>> Uh.
>>
>>>> I'll if I can find anything the macro, it looks quite complex to me :-).
>>>
>>> It's not only to you, if that helps.  It's rather... intimidating ^^'.
>>
>> [...]
>>
>>> Ludovic, would you have an idea of where the %location field or its
>>> CONFIGURATION-location accessor come into play?
>>
>> We have this:
>>
>>          (define-record-type* #,(id #'stem #'< #'stem #'>)
>>            stem
>>            #,(id #'stem #'make- #'stem)
>>            #,(id #'stem #'stem #'?)
>>            #,@(map (lambda (name getter def)
>>                      #`(#,name #,getter (default #,def)
>>                                (sanitize
>>                                 #,(id #'stem #'validate- #'stem #'- name))))
>>                    #'(field ...)
>>                    #'(field-getter ...)
>>                    #'(field-default ...))
>>            (%location #,(id #'stem #'stem #'-location)
>>                       (default (and=> (current-source-location)
>>                                       source-properties->location))
>>                       (innate)))
>>
>> That generates two accessors called ‘namespace-configuration-location’.
>> The second one shadows the first one.
>
> Yes.  You didn't address my question directly though, so let me ask it
> again: where is this %location field access (named "location") used?  It
> seems nowhere.  Thus, we can simply rename it without impacting
> anything, right (except theoretical usages in the wild, since in the
> API).
>
>> With commit 44554e7133aa60e1b453436be1e80394189cabd9, the second one
>> is the “wrong” one: ‘namespace-configuration-location’ now returns the
>> ‘%location’ field, not the user-specified ‘location’ field.  (I
>> reported that issue in <https://issues.guix.gnu.org/48284>.)
>>
>> What do you think of reverting 44554e7133aa60e1b453436be1e80394189cabd9?
>
> No.  If we revert something, it won't be that whole commit, but just the
> moving of the field in the define-configuration produced record.

If we don't have an obvious solution to the issue and it may need more
time, how do you feel about reverting the changes? Unless there is a
work around that could be applied until a nicer more permanent solution
is found (although those temporary fixes do tend to stick around
sometimes :-) ).

Thanks,
Pierre

Pierre Langlois wrote on 1 Dec 2022 22:55

Re: Invalid 'location' field generated in dovecot configuration

Recipients:(address . bug-guix@gnu.org)(name . mirai)(address . mirai@makinata.eu)

Message-ID:87359ydbnq.fsf@gmx.com

Hi all!

As suggested by mirai off-list, it would be nice to have a test that
would have caught the issue. How do people feel about something along
the following patch? The idea is to use namespaces in the dovecot config
to declare the INBOX and another additional mailbox.

The bug is quite obscure and not really about dovecot itself, so I don't
think adding such a test is strictly necessary, maybe more tests for the
configuration macro would be good instead. But I figured expanding the
dovecot system test can't hurt. Let me know if you agree and I'll format
it properly.

Toggle diff (114 lines)diff --git a/gnu/tests/mail.scm b/gnu/tests/mail.scm
index f13751b72f..8a2dbd798f 100644
--- a/gnu/tests/mail.scm
+++ b/gnu/tests/mail.scm
@@ -301,8 +301,19 @@ (define %dovecot-os
                      (auth-anonymous-username "alice")
                      (mail-location
                       (string-append "maildir:~/Maildir"
-                                     ":INBOX=~/Maildir/INBOX"
-                                     ":LAYOUT=fs"))))))
+                                     ":LAYOUT=fs"))
+                     (namespaces
+                      (list
+                       (namespace-configuration
+                        (name "INBOX")
+                        (inbox? #t)
+                        (separator "/")
+                        (location "maildir:~/Maildir/INBOX"))
+                       (namespace-configuration
+                        (name "guix-devel")
+                        (separator "/")
+                        (prefix "guix-devel/")
+                        (location "maildir:~/Maildir/guix-devel"))))))))
 
 (define (run-dovecot-test)
   "Return a test of an OS running Dovecot service."
@@ -351,9 +362,10 @@ (define message "From: test@example.com\n\
               (marionette-eval `(file-exists? (string-append "/proc/" ,pid))
                                marionette)))
 
-          (test-assert "accept an email"
+          (define-syntax-rule (with-imap-connection imap exp ...)
             (let ((imap (socket AF_INET SOCK_STREAM 0))
-                  (addr (make-socket-address AF_INET INADDR_LOOPBACK 8143)))
+                  (addr (make-socket-address AF_INET INADDR_LOOPBACK 8143))
+                  (body (lambda (imap) exp ...)))
               (connect imap addr)
               ;; Be greeted.
               (read-line imap) ;OK
@@ -362,6 +374,43 @@ (define message "From: test@example.com\n\
               (read-line imap) ;+
               (write-line "c2lyaGM=" imap)
               (read-line imap) ;OK
+
+              (let ((ok (body imap)))
+                ;; Logout
+                (write-line "a LOGOUT" imap)
+                (close imap)
+                ok)))
+
+          (define (marionette-read-new-mail mailbox marionette)
+            (marionette-eval
+             `(begin
+                (use-modules (ice-9 ftw)
+                             (ice-9 match))
+                (match (scandir ,mailbox)
+                  (("." ".." message-file)
+                   (call-with-input-file
+                       (string-append ,mailbox message-file)
+                     get-string-all))))
+             marionette))
+
+          (test-assert "accept an email"
+            (with-imap-connection imap
+              ;; Append a message to the INBOX mailbox
+              (write-line (format #f "a APPEND INBOX {~a}"
+                                  (number->string (message-length message)))
+                          imap)
+              (read-line imap) ;+
+              (write-line message imap)
+              (read-line imap) ;OK
+              #t))
+
+          (test-equal "mail arrived"
+            message
+            (marionette-read-new-mail "/home/alice/Maildir/INBOX/new/"
+                                      marionette))
+
+          (test-assert "accept an email in fresh mailbox"
+            (with-imap-connection imap
               ;; Create a TESTBOX mailbox
               (write-line "a CREATE TESTBOX" imap)
               (read-line imap) ;OK
@@ -372,24 +421,16 @@ (define message "From: test@example.com\n\
               (read-line imap) ;+
               (write-line message imap)
               (read-line imap) ;OK
-              ;; Logout
-              (write-line "a LOGOUT" imap)
-              (close imap)
               #t))
 
-          (test-equal "mail arrived"
+          (test-equal "mail arrived in fresh mailbox"
             message
-            (marionette-eval
-             '(begin
-                (use-modules (ice-9 ftw)
-                             (ice-9 match))
-                (let ((TESTBOX/new "/home/alice/Maildir/TESTBOX/new/"))
-                  (match (scandir TESTBOX/new)
-                    (("." ".." message-file)
-                     (call-with-input-file
-                         (string-append TESTBOX/new message-file)
-                       get-string-all)))))
-             marionette))
+            (marionette-read-new-mail "/home/alice/Maildir/TESTBOX/new/"
+                                      marionette))
+
+          (test-assert "mailbox exists"
+            (marionette-eval `(file-exists? "/home/alice/Maildir/guix-devel")
+                             marionette))
 
           (test-end))))

Thanks,

Pierre

Ludovic Courtès wrote on 2 Dec 2022 10:30

Re: bug#59423: Invalid 'location' field generated in dovecot configuration

Recipients:(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)

Message-ID:87iliuup80.fsf@gnu.org

Hi Maxim,

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

Toggle quote (2 lines)

> Ludovic Courtès <ludo@gnu.org> writes:

[...]

Toggle quote (25 lines)>>>> That generates two accessors called ‘namespace-configuration-location’.
>>>> The second one shadows the first one.
>>>
>>> Yes.  You didn't address my question directly though, so let me ask it
>>> again: where is this %location field access (named "location") used?
>
> [...]
>
>> … the field is accessed via its accessor,
>> ‘name-configuration-location’.  We can kinda see it here:
>
> [...]
>
>> Each <configuration-field> record has a ‘getter’ field that refers to
>> the accessor.  In the case of ‘location’, that’s the “wrong”
>> accessor—the accessor of ‘%location’.
>>
>> I hope that addresses your question!
>
> No :-).  I meant why do we even set a default accessor for the *source
> location* information (in the (gnu service configuration) macros); it's
> that one that doesn't seem to get used (or I'm blind to it!), at least
> via this accessor.  If it's not strictly necessary, we can stop
> producing it, and that would solve the problem.

Like I wrote, I think it’s necessary, even if not used now.

We’ve been knowingly shipping a broken Dovecot for two weeks now. As I

wrote, and as Pierre suggested, can we just revert the ‘%location’ field

shuffling for now? I can even do it on your behalf.

After that we can continue that discussion (though I don’t have much to

add to be honest).

Thanks in advance!

Ludo’.

Ludovic Courtès wrote on 2 Dec 2022 10:34

control message for bug #59423

Recipients:(address . control@debbugs.gnu.org)

Message-ID:87edtiup2m.fsf@gnu.org

severity 59423 important

quit

Ludovic Courtès wrote on 2 Dec 2022 22:18

Re: bug#59423: Invalid 'location' field generated in dovecot configuration

Recipients:(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)

Message-ID:878rjpqzbi.fsf@gnu.org

Ludovic Courtès <ludo@gnu.org> skribis:

Toggle quote (2 lines)

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

[...]

Toggle quote (8 lines)

>> No :-). I meant why do we even set a default accessor for the *source

>> location* information (in the (gnu service configuration) macros); it's

>> that one that doesn't seem to get used (or I'm blind to it!), at least

>> via this accessor. If it's not strictly necessary, we can stop

>> producing it, and that would solve the problem.

> Like I wrote, I think it’s necessary, even if not used now.

To complement this answer: key high-level record types usually have a
‘location’ field: <package>, <channel>, <mapped-device>, <file-system>,
<service-type>, etc.  The rationale is that it allows us to report
accurate location info for errors and warnings, to jump to their
definition, and so on.

For configuration records this is not a common pattern, but the
rationale holds.  ‘zabbix-front-end-config’ uses the ‘%location’ field,
but it seems to be the only one so far.

Ludo’.

Maxim Cournoyer wrote on 3 Dec 2022 03:24

Recipients:(name . Pierre Langlois)(address . pierre.langlois@gmx.com)

Message-ID:878rjpgr6q.fsf@gmail.com

Hi Pierre,

Pierre Langlois <pierre.langlois@gmx.com> writes:

Toggle quote (13 lines)> Hi all!
>
> As suggested by mirai off-list, it would be nice to have a test that
> would have caught the issue. How do people feel about something along
> the following patch? The idea is to use namespaces in the dovecot config
> to declare the INBOX and another additional mailbox.
>
> The bug is quite obscure and not really about dovecot itself, so I don't
> think adding such a test is strictly necessary, maybe more tests for the
> configuration macro would be good instead. But I figured expanding the
> dovecot system test can't hurt. Let me know if you agree and I'll format
> it properly.

Thanks for the diff!  I've turned it into a patch and was ready to apply
it, but 2 tests are always failing, even after addressing this issue;
could you look into why?  It looks good otherwise.

Attachment: 0001-tests-Expound-Dovecot-test-suite.patch

-- 
Thanks,
Maxim

Maxim Cournoyer wrote on 3 Dec 2022 04:05

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)

Message-ID:874judgpav.fsf@gmail.com

Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (24 lines)> Ludovic Courtès <ludo@gnu.org> skribis:
>
>> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
> [...]
>
>>> No :-).  I meant why do we even set a default accessor for the *source
>>> location* information (in the (gnu service configuration) macros); it's
>>> that one that doesn't seem to get used (or I'm blind to it!), at least
>>> via this accessor.  If it's not strictly necessary, we can stop
>>> producing it, and that would solve the problem.
>>
>> Like I wrote, I think it’s necessary, even if not used now.
>
> To complement this answer: key high-level record types usually have a
> ‘location’ field: <package>, <channel>, <mapped-device>, <file-system>,
> <service-type>, etc.  The rationale is that it allows us to report
> accurate location info for errors and warnings, to jump to their
> definition, and so on.
>
> For configuration records this is not a common pattern, but the
> rationale holds.  ‘zabbix-front-end-config’ uses the ‘%location’ field,
> but it seems to be the only one so far.

Thanks for this extra bit of information and for spotting this usage.  I
think "location" is likely to conflict for the general purpose
'define-configuration' generated records, so I've renamed the "location"
*accessor* to "source-location".

In the near future I want to migrate more service configurations to the
'define-configuration' machinery, to benefit from its useful
self-validating property.  For now I wouldn't feel at ease doing so
unless raw record matching (not yet using 'match-record') works the same
way, since we still have many occurrences making use of that (often via
match-lambda).  For that reason, I prefer to not revert the record
layout until we've gotten rid of all the match-lambda matching record
fields directly (which will take some time).

I've applied the rename fix to master.

-- 
Thanks,
Maxim

Closed

Ludovic Courtès wrote on 4 Dec 2022 17:53

Recipients:(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)

Message-ID:87h6ybm7pk.fsf@gnu.org

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

Toggle quote (5 lines)

> Thanks for this extra bit of information and for spotting this usage. I

> think "location" is likely to conflict for the general purpose

> 'define-configuration' generated records, so I've renamed the "location"

> *accessor* to "source-location".

Thank you.

It wasn’t my preferred solution¹ but I think it’s a good one.

¹ https://issues.guix.gnu.org/59423#15-lineno81

Toggle quote (9 lines)

> In the near future I want to migrate more service configurations to the

> 'define-configuration' machinery, to benefit from its useful

> self-validating property. For now I wouldn't feel at ease doing so

> unless raw record matching (not yet using 'match-record') works the same

> way, since we still have many occurrences making use of that (often via

> match-lambda). For that reason, I prefer to not revert the record

> layout until we've gotten rid of all the match-lambda matching record

> fields directly (which will take some time).

Right, especially given that ‘match-record’ was added in 2017. :-)

We’ll have to discuss the implications of a possible move to

‘define-configuration’. For example, ‘define-configuration’ cannot

report missing field values (for fields that lack a default value) at

macro-expansion time, contrary to plain ‘define-record-type*’. Anyway,

future work!

Ludo’.

Closed

Maxim Cournoyer wrote on 4 Dec 2022 22:43

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)

Message-ID:87tu2a6e0d.fsf@gmail.com

Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (24 lines)> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> Thanks for this extra bit of information and for spotting this usage.  I
>> think "location" is likely to conflict for the general purpose
>> 'define-configuration' generated records, so I've renamed the "location"
>> *accessor* to "source-location".
>
> Thank you.
>
> It wasn’t my preferred solution¹ but I think it’s a good one.
>
> ¹ https://issues.guix.gnu.org/59423#15-lineno81
>
>> In the near future I want to migrate more service configurations to the
>> 'define-configuration' machinery, to benefit from its useful
>> self-validating property.  For now I wouldn't feel at ease doing so
>> unless raw record matching (not yet using 'match-record') works the same
>> way, since we still have many occurrences making use of that (often via
>> match-lambda).  For that reason, I prefer to not revert the record
>> layout until we've gotten rid of all the match-lambda matching record
>> fields directly (which will take some time).
>
> Right, especially given that ‘match-record’ was added in 2017.  :-)

Hehe! I had started adapting all the match-lambda, and got overwhelmed

by the changes needed. So I think progressively (i.e., small) be easier

to review or revert in case of errors.

Toggle quote (6 lines)

> We’ll have to discuss the implications of a possible move to

> ‘define-configuration’. For example, ‘define-configuration’ cannot

> report missing field values (for fields that lack a default value) at

> macro-expansion time, contrary to plain ‘define-record-type*’. Anyway,

> future work!

OK.  That's optimization work rather than an impediment to migrate
though, right?  If so, I think the value for users of having errors on
invalid field types outweighs run time efficiency :-).

-- 
Thanks,
Maxim

Closed

Ludovic Courtès wrote on 6 Dec 2022 09:53

Recipients:(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)

Message-ID:875yeooqvc.fsf@gnu.org

Hi,

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

Toggle quote (10 lines)>> We’ll have to discuss the implications of a possible move to
>> ‘define-configuration’.  For example, ‘define-configuration’ cannot
>> report missing field values (for fields that lack a default value) at
>> macro-expansion time, contrary to plain ‘define-record-type*’.  Anyway,
>> future work!
>
> OK.  That's optimization work rather than an impediment to migrate
> though, right?  If so, I think the value for users of having errors on
> invalid field types outweighs run time efficiency :-).

I guess my point is “we’ll have to discuss”. It has non-obvious

implications such as this one that have a visible impact on users.

Ludo’.

Closed

Your comment

This issue is archived.

To comment on this conversation send an email to 59423@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it

mumi current 59423

Then, you may apply the latest patchset in this issue (with sign off)

mumi am -- -s

Or, compose a reply to this issue

mumi compose

Or, send patches to this issue

mumi send-email *.patch

is:open	open issues
is:done	closed issues
submitter:<who>	search issue submitter
author:<who>	search by message author
date:yesterday..now	search by issue date
mdate:3m..2d	search by message date