On 2023-08-22 18:25, Ludovic Courtès wrote: > Hi Andrew, > > Andrew Tropin 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