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

[Maxim Cournoyer]

Hi Ludovic,

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

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

[...]

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

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

[...]

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

> 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