Subject: bug#59423: closed (Re: bug#59423: Invalid 'location' field generated in dovecot configuration)

From: [email-masked] (GNU bug Tracking System)
Date: Sat, 03 Dec 2022 03:06:02 +0000

Your bug report

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

which was filed against the guix package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
59423: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=59423
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#59423: Invalid 'location' field generated in dovecot configuration
From: Maxim Cournoyer <[email-masked]>
To: Ludovic Courtès <[email-masked]>
Cc: Pierre Langlois <[email-masked]>, [email-masked]
Date: Fri, 02 Dec 2022 22:05:12 -0500
List-Id: <debbugs-submit.debbugs.gnu.org>

Hi Ludovic,

Ludovic Courtès <[email-masked]> writes:

> Ludovic Courtès <[email-masked]> skribis:
>
>> Maxim Cournoyer <[email-masked]> 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

Subject: Invalid 'location' field generated in dovecot configuration
From: Pierre Langlois <[email-masked]>
To: [email-masked]
Date: Sun, 20 Nov 2022 21:53:01 +0000
List-Id: <debbugs-submit.debbugs.gnu.org>

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:

--8<---------------cut here---------------start------------->8---
location=#<<location> file: "path/to/config.scm" line: 297 column: 20>
--8<---------------cut here---------------end--------------->8---

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:

--8<---------------cut here---------------start------------->8---
(format #t "~a\n" (wait-for-file "/etc/dovecot/dovecot.conf"
                                 marionette
                                 #:read 'get-string-all))
--8<---------------cut here---------------end--------------->8---

Giving us the snippets like this in the config:

--8<---------------cut here---------------start------------->8---
$ 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
}
}
...
--8<---------------cut here---------------end--------------->8---

I did a `git bisect` with `guix time-machine` (this tool is invaluable)
and found the issue started with this commit:

--8<---------------cut here---------------start------------->8---
commit 543d971ed2a1d9eb934af1f51930741d7cc4e7ef
Author: Maxim Cournoyer <[email-masked]>
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.
--8<---------------cut here---------------end--------------->8---

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