[bug#65119] [PATCH 0/8] Sharing service code between Home and System

[Andrew Tropin]

On 2023-08-22 18:25, Ludovic Courtès wrote:

> Hi Andrew,
>
> Andrew Tropin <andrew@trop.in> skribis:
>
>> Sorry for comming late to the party, I saw this message only a week ago
>> and didn't have time to make an extensive reply yet, so I will share my
>> quick thought on the most problematic part and maybe later will
>> formulate others thoughts in more details.
>>
>> define-service-type-mapping looks imperative and potentially very
>> problematic.  Collecting those values in unknown order and applying this
>> implicit transformation making a good room for foot shooting.  Imagine
>> someone would like to use his own (let's say) shepherd home service
>> implementation and will add this to one of the source files of their
>> channel:
>>
>> (define-service-type-mapping
>>   shepherd-root-service-type => my-home-shepherd-service-type)
>>
>> What happens if somebody will use his channel just for getting some
>> package?  Very likely it would break the build or in the worst case it
>> will build with unexpected service implementation under the hood.
>
> Yes, this is always possible.  Though it’s not very different from:
>
>   (set! home-shepherd-service-type …)

Yes, and and this is exactly what this solution does and in addition to
that it hides it under the sweet define-service-type-mapping interface.
`set!` explicitly communicates the danger of usage,
define-service-type-mapping does not.


1. We introduce a global state here and infect potentially all the
modules from all channels with it + mask it with nice interface. => Now
the result of evaluation depends on the order source files are read.
Every channel can break valid user's configurations with perfectly legal
public guix API.  We make reloading of the modules and REPL-driven
development in general a huge pain in the lower back.


2. The service extension mechanism is already quite complex to understand
on its own, in addition to that the devirsity of record creation macros
/ DSLs (define-record-type, define-record-type*, define-configuration)
doesn't make the situation better.  We introduce one more DSL on top of
couple existing. => Learning curve raises even higher. Inspecting,
navigating and debugging such code becomes harder. It prevents future
extension with for-container or for-development-environment.  It saves
couple of lines and avoids some minor repetions at the moment, but not
sure that it's the right way to reuse the stuff between home and system
services.

>
> Maybe the unintended effect is more likely to happen unwillingly though,
> maybe.
>
> Do you have other solutions in mind, be it for this specific issue or
> for system/home service mapping in general?
>
>> I had [1][2] and still have concerns about macros and records
>> composability and reusability.  I personally don't like excessive usage
>> of them in general.  By adding more macros, already quite complex guix
>> services mechanism becomes even more harder to learn, inspect, reason
>> about and work with.  In addition to that it has a major technical issue
>> mentioned above.  I'm strongly against this change and would suggest to
>> revert it.
>>
>> I hope it doesn't sound rude and I'm really thankful for your work on
>> this, but I just think it's not the right solution, at least yet, in its
>> current form.
>
> It does sound a bit rude.  :-)  I would have loved to get any feedback
> from you while we were discussing this in the course of reviewing the
> Syncthing and Dicod patches a couple of months ago (which I believe you
> were Cc’d on, as member of the Home team).
>
> I won’t argue about macros and records, it’s off-topic: macros and
> records are part of the Schemer’s toolbox, we try and use them wisely,
> and Guix code has always used macros and records.  We can discuss
> whether a specific macro or record type is suitable, of course, but
> general statements about them are unhelpful.
>

Actually, it's quite on-topic IMO and it seems as a consequence of the
abandoned discussions I linked earlier, but it's a long story and we can
keep it aside for now, because despite the reasons lead to this solution
it has fundamental problems on its own we better to focus on.  Maybe
later I'll elaborate on my thought process in more details.

> Was it ‘for-home’ that triggered your comment?
>
> The patches do not introduce any new record type IIRC; what triggered
> your comment regarding records?
>
> I’m all for making changes to improve on this patch series.  I’m against
> reverting the patch series: the conditions for reverting are not met
> (info "(guix) Commit Access").
>
> Thanks for your feedback,
> Ludo’.

Please treat it with adjustment to my not very proficient level of
english and lacking ability to express nuances.  It's not a personal
critique in any way, it's my roughly formulated concerns on 1. technical
aspects of the solution and 2. possible consequences for the future of
guix and its user-facing API.

-- 
Best regards,
Andrew Tropin

signature.asc
Description: PGP signature