Message ID | c89139f9177259e04e17df162eb6020a41b58294.1716443624.git.andrew@trop.in |
---|---|
State | New |
Headers | show |
Series | [bug#71111,v2,1/1] services: home: Use pairs instead of lists. | expand |
Hi Andrew, Andrew Tropin <andrew@trop.in> skribis: > (operating-system > (services (append (list (service guix-home-service-type > - `(("alice" ,my-home)))) > + `(("alice" . ,my-home)))) What’s the rationale for this? In general I think we should avoid gratuitous incompatible changes. Thanks, Ludo’.
On 2024-05-23 11:16, Ludovic Courtès wrote: > Hi Andrew, > > Andrew Tropin <andrew@trop.in> skribis: > >> (operating-system >> (services (append (list (service guix-home-service-type >> - `(("alice" ,my-home)))) >> + `(("alice" . ,my-home)))) > > What’s the rationale for this? --8<---------------cut here---------------start------------->8--- After rewriting from car/cdr to match-lambda in v2 of this patch: https://yhetil.org/guix-patches/3394b0b51f6a5a608ebcfb7a63fdc34e52fe928e.1711046203.git.richard@freakingpenguin.com/ the format changed from pairs to lists, I didn't noticed this nuance during review because the documentation still says that service should be configured and extended with pairs. Also, pairs are more apropriate data type here. And this match-lambda rewrite will break downstream RDE user's setups after migrating to upstreamed version of service. That's why I propose to go back to pairs. --8<---------------cut here---------------end--------------->8--- > > In general I think we should avoid gratuitous incompatible changes. Agree. This API is very young, so I think it make sense to update it in this particular case, considering rationale above.
diff --git a/doc/guix.texi b/doc/guix.texi index 8073e3f6d4..8cc5edc805 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -39576,7 +39576,7 @@ Guix Services (operating-system (services (append (list (service guix-home-service-type - `(("alice" ,my-home)))) + `(("alice" . ,my-home)))) %base-services))) @end lisp @@ -39585,7 +39585,7 @@ Guix Services @lisp (simple-service 'my-extra-home home-service-type - `(("bob" ,my-extra-home)))) + `(("bob" . ,my-extra-home)))) @end lisp @end defvar diff --git a/gnu/services/guix.scm b/gnu/services/guix.scm index 96f5ecaac0..3818749baa 100644 --- a/gnu/services/guix.scm +++ b/gnu/services/guix.scm @@ -696,7 +696,7 @@ (define guix-data-service-type (define (guix-home-shepherd-service config) (map (match-lambda - ((user he) + (((? string? user) . (? home-environment? he)) (shepherd-service (documentation "Activate Guix Home.") (requirement '(user-processes)) @@ -710,7 +710,9 @@ (define (guix-home-shepherd-service config) (list (string-append "HOME=" (passwd:dir (getpw #$user))) "GUIX_SYSTEM_IS_RUNNING_HOME_ACTIVATE=t") #:group (group:name (getgrgid (passwd:gid (getpw #$user)))))) - (stop #~(make-kill-destructor))))) + (stop #~(make-kill-destructor)))) + (e (error "Invalid value for guix-home, it should be in a form of +(\"user-name\" . home-environment), but the following value is provided:\n" e))) config)) (define guix-home-service-type diff --git a/gnu/tests/guix.scm b/gnu/tests/guix.scm index 12ad1bf255..6071cb018e 100644 --- a/gnu/tests/guix.scm +++ b/gnu/tests/guix.scm @@ -271,7 +271,7 @@ (define %guix-home-service-he (define %guix-home-service-os (simple-operating-system (service guix-home-service-type - `(("alice" ,%guix-home-service-he))))) + `(("alice" . ,%guix-home-service-he))))) (define (run-guix-home-service-test) (define os