unofficial mirror of bug-guix@gnu.org 
 help / color / mirror / code / Atom feed
From: "Ludovic Courtès" <ludo@gnu.org>
To: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Cc: Pierre Langlois <pierre.langlois@gmx.com>, 59423@debbugs.gnu.org
Subject: bug#59423: Invalid 'location' field generated in dovecot configuration
Date: Mon, 28 Nov 2022 22:33:57 +0100	[thread overview]
Message-ID: <8735a2ahmi.fsf@gnu.org> (raw)
In-Reply-To: <87sfi2svb9.fsf@gmail.com> (Maxim Cournoyer's message of "Mon, 28 Nov 2022 15:00:58 -0500")

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?

When doing something like this:

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

… the field is accessed via its accessor,
‘name-configuration-location’.  We can kinda see it here:

--8<---------------cut here---------------start------------->8---
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.">
[…]
--8<---------------cut here---------------end--------------->8---

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!

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

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




  reply	other threads:[~2022-11-28 21:35 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-20 21:53 bug#59423: Invalid 'location' field generated in dovecot configuration Pierre Langlois
2022-11-22  8:10 ` Ludovic Courtès
2022-11-25 15:36   ` Maxim Cournoyer
2022-11-25 20:19     ` Pierre Langlois
2022-11-25 19:17 ` mirai
2022-11-25 20:06 ` Maxim Cournoyer
2022-11-25 20:25   ` Pierre Langlois
2022-11-25 20:50     ` Pierre Langlois
2022-11-25 21:09       ` Pierre Langlois
2022-11-26  2:54         ` Maxim Cournoyer
2022-11-26 19:32           ` Pierre Langlois
2022-11-27  2:33             ` Maxim Cournoyer
2022-11-28 15:01               ` Ludovic Courtès
2022-11-28 20:00                 ` Maxim Cournoyer
2022-11-28 21:33                   ` Ludovic Courtès [this message]
2022-11-29  1:58                     ` Maxim Cournoyer
2022-12-02  9:30                       ` Ludovic Courtès
2022-12-02 21:18                         ` Ludovic Courtès
2022-12-03  3:05                           ` Maxim Cournoyer
2022-12-04 16:53                             ` Ludovic Courtès
2022-12-04 21:43                               ` Maxim Cournoyer
2022-12-06  8:53                                 ` Ludovic Courtès
2022-12-01 20:29                   ` Pierre Langlois
2022-11-26 23:17 ` Fredrik Salomonsson
2022-12-01 21:55 ` Pierre Langlois
2022-12-03  2:24   ` Maxim Cournoyer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8735a2ahmi.fsf@gnu.org \
    --to=ludo@gnu.org \
    --cc=59423@debbugs.gnu.org \
    --cc=maxim.cournoyer@gmail.com \
    --cc=pierre.langlois@gmx.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).