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

[Andrew Tropin]

On 2023-08-25 10:28, Andrew Tropin wrote:

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

To support my previous suggestion to revert this patch series I will
document the example of a consequence:

https://git.savannah.gnu.org/cgit/guix.git/commit/?h=cf6abf50dbbbd95fef465ab4bb3b608843ac47e1
https://issues.guix.gnu.org/65510

It highlights this:

> Now the result of evaluation depends on the order source files are
> read.
>>> Collecting those values in unknown order and applying this
>>> implicit transformation making a good room for foot shooting.

to say it differently this change introduced the implicit dependency on
the fact of the evaluation of the form, which is not explicitly
referenced anywhere.

> 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