[PATCH 0/1] services: home: Use pairs instead of lists.

  • Done
  • quality assurance status badge
Details
6 participants
  • Andrew Tropin
  • Efraim Flashner
  • Ludovic Courtès
  • Maxim Cournoyer
  • Richard Sent
  • Zheng Junjie
Owner
unassigned
Submitted by
Andrew Tropin
Severity
normal
A
A
Andrew Tropin wrote on 22 May 12:02 +0200
(address . guix-patches@gnu.org)
cover.1716372146.git.andrew@trop.in
After rewriting from car/cdr to match-lambda in v2 of this patch:

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.

Andrew Tropin (1):
services: home: Use pairs instead of lists.

doc/guix.texi | 4 ++--
gnu/services/guix.scm | 2 +-
gnu/tests/guix.scm | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)


base-commit: b06a460bf5236a9d52f936f2023451051b3e622a
--
2.41.0
A
A
Andrew Tropin wrote on 22 May 13:06 +0200
[PATCH 1/1] services: home: Use pairs instead of lists.
(address . guix-patches@gnu.org)
878r02f6ls.fsf@trop.in
* gnu/services/guix.scm: Use pairs instead of lists.
* doc/guix.texi: Update accordingly.
* gnu/tests/guix.scm: Update accordingly.

Change-Id: I0b8d3fa5b214add89bdb84a11fa20d1b319435f0
---
doc/guix.texi | 4 ++--
gnu/services/guix.scm | 2 +-
gnu/tests/guix.scm | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)

Toggle diff (50 lines)
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..1f0e2a310d 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)
+ ((user . he)
(shepherd-service
(documentation "Activate Guix Home.")
(requirement '(user-processes))
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
--
2.41.0
R
R
Richard Sent wrote on 22 May 23:33 +0200
Re: [PATCH 0/1] services: home: Use pairs instead of lists.
(name . Andrew Tropin)(address . andrew@trop.in)
87cypdwmyi.fsf@freakingpenguin.com
Andrew Tropin <andrew@trop.in> writes:

Toggle quote (13 lines)
> 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.
>

I'm not opposed to going back to cons cell pairs. I didn't put too much
thought in a "list of two elements" vs. "cons cell" besides the match
statement being easier to handle with a list.

Would this patch have unintended side effects? I thought the . in match
conditions had a different behavior.

Toggle snippet (11 lines)
scheme@(guile-user)> (match '(1 2 3 4)
((a . b) b))
$5 = (2 3 4)
scheme@(guile-user)> (match '(1 2)
((a . b) b))
$6 = (2)
scheme@(guile-user)> (match '(1 . 2)
((a . b) b))
$7 = 2

So changing to this would allow for a home-service entry like the
following to match:

Toggle snippet (4 lines)
(simple-service 'my-extra-home home-service-type
`(("bob" "bill" "bertha" "billyanne" ,my-extra-home ,my-home-2)))

From my testing, this /will/ error (yay), but not as soon as the match
would with the current code. Instead of being caught before being passed
to the daemon, it seems to be caught while lowering the invalid
file-append object.

Personally I would prefer to catch as many errors as possible before
beginning to pass code off to the daemon where possible. Generally
speaking it feels like pre-daemon errors are easier to mentally parse.

In fairness, the current code also isn't trying particularly hard to
check that "user" is a string and "he" is a home-environment or printing
a fancy error message.

Perhaps this would work?

Toggle snippet (14 lines)
(define-module (gnu services guix)
...
#:use-module (gnu home))
...
(define (guix-home-shepherd-service config)
(map (match-lambda
(((? string? user) . (? home-environment? he))
(shepherd-service
...
))
(e (error "Invalid value for guix-home-shepherd-service: " e)))
config))

Maybe I'm just being silly. ?

--
Take it easy,
Richard Sent
Making my computer weirder one commit at a time.
Z
Z
Zheng Junjie wrote on 23 May 05:38 +0200
Re: [bug#71111] [PATCH 0/1] services: home: Use pairs instead of lists.
(name . Andrew Tropin via Guix-patches via)(address . guix-patches@gnu.org)
87sey9rye0.fsf@iscas.ac.cn
Andrew Tropin via Guix-patches via <guix-patches@gnu.org> writes:

Toggle quote (12 lines)
> 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.

Maybe we can support pairs and list of length two at same time?

Toggle quote (11 lines)
>
> Andrew Tropin (1):
> services: home: Use pairs instead of lists.
>
> doc/guix.texi | 4 ++--
> gnu/services/guix.scm | 2 +-
> gnu/tests/guix.scm | 2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
>
> base-commit: b06a460bf5236a9d52f936f2023451051b3e622a
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEfr6klGDOXiwIdX/bO1qpk+Gi3/AFAmZOuicACgkQO1qpk+Gi
3/AlDQ/+OnphQ185KNByjj4Qah07Eyi/khK7+iJopOtZ8TGgiOyTp3VoxFFn7H9v
fxM9XLqmG63juqlmJo25ZkW50AeFGpCPe+FCObIWh5J632OeFovVe4m+yJHIt/QL
DYir92kEqdpN5cgwhN2pYQODWLmS0iyPh9BWuOBn8OV7UENpBY5h/eWbdEnYvalx
KTOgJdP2aRXBqrqdH6eoCatAcXQt3lhsa3j2zVaNTjl97ZsJWT8bjchWYO+9JBo5
4DAPQ+fXlVIDXU1CQvJt5lzPiY8fUN+FYYyTCs3yz8sycyJ7yBFzSCaPYtQ/Q6XF
BVSv2sFLGjU/oX8ZGbfr32lWD4j93XpOWritDCxCUfYKp8pcJ2WZfeZkEK55aw3E
ua8OP2rgPpfpC5dR//7WcvdQFnCPPcWH8BSKKwDJ38MWjVpXdXXKMW0NEd4T+3RK
Bg0bndGcZ3UCYpWbzwu54WidkM6cAEpJIns50iXqEJYeEnZEF7DpwKQ6zc8WceBZ
WOgKEFEVE9DU1LfI755A0P0JLbggTm8TjKB0UOihcOWogNIfrTarhYaiXzj4SZFX
SdWeFpU517fXZy1Nq3cQNqcM5sscGCOPigcBrm6OSPloJX+tQs+4msEdR9NSOlTx
y0eKb3jpV9VKtGpP+qnw6uU+csUyIDf/3sqDItX1OaSjGVVxUAM=
=Q4nC
-----END PGP SIGNATURE-----

A
A
Andrew Tropin wrote on 23 May 07:43 +0200
877cfl13sc.fsf@trop.in
On 2024-05-23 11:38, Zheng Junjie wrote:

Toggle quote (16 lines)
> Andrew Tropin via Guix-patches via <guix-patches@gnu.org> writes:
>
>> 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.
>
> Maybe we can support pairs and list of length two at same time?

Thank you for the idea, however I think ambiguity is a bad practice,
from my early experience with guix it's more confusing rather than
helpful. I still don't know why profile-service-type accepts list of
lists rather than alist (list of pairs).

Toggle quote (13 lines)
>
>>
>> Andrew Tropin (1):
>> services: home: Use pairs instead of lists.
>>
>> doc/guix.texi | 4 ++--
>> gnu/services/guix.scm | 2 +-
>> gnu/tests/guix.scm | 2 +-
>> 3 files changed, 4 insertions(+), 4 deletions(-)
>>
>>
>> base-commit: b06a460bf5236a9d52f936f2023451051b3e622a

--
Best regards,
Andrew Tropin
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEKEGaxlA4dEDH6S/6IgjSCVjB3rAFAmZO15MACgkQIgjSCVjB
3rAq8Q//Y0vo3tShXCOsgPB75ylEGhYMEUlF18+L2noFVYcMI1BR2hclw2LJ9+T7
2eDd7wdN3dWCnhDYZf3KTVDlg6/w84MBAj4MUrvJWcK96lcEYwCKxN7+jspYmVMC
j6sxt4n5/dU2NkMwKuyQv+8ysmOj/TYX+EkbV8YrpA1JVWp0c0d2ad/URbfbyEOq
uloe1JZh2VBDXrpbYUiIQeG/YU6DAwgXdmS/Ja0REkV7AhkJ5a30l7GVnuwcOv0+
j0/LjpLkMej9F7lISTJ1uBbIqF66gCPqrsYLjuZjNOHANQwvuBtQimGkRmKxOwfn
wOVzCXV1/VmtUb4HIPdiXJw2QivWAPBvGOksYgi8LA0BuiCXGfBoSuMLYkEKmHjQ
62d4uYY7S2jAl53K8P/HA+p9zS5uDSybk8VjWr7PGRH/nNgmqykoiCLTYUFGCHmp
Jf0w/Gvz7C304FObHlLT+7gcSdc07yv1sgeJy7mcd9piUO67+tl20N3m6KZGianV
MXWo+cm3Eli/S/ZnKjgCZAlAcNCzjSt0A2XhgWzh7RGJMkzjyarEL9bm6YeJLyY3
tAR7B39c6XpBRwbxhvfHNn4jmqpaRoQh8LTsW0jtIDUYPtQ4cbSOX0RLf+7VNbNF
jWJ6QV5TmKw2taQoYiC9Edl9SunS8o1vaWAQMdvJlbu/3rYfi/0=
=jLbC
-----END PGP SIGNATURE-----

A
A
Andrew Tropin wrote on 23 May 07:45 +0200
Re: [PATCH 0/1] services: home: Use pairs instead of lists.
(name . Richard Sent)(address . richard@freakingpenguin.com)
874jap13q9.fsf@trop.in
On 2024-05-22 17:33, Richard Sent wrote:

Toggle quote (72 lines)
> Andrew Tropin <andrew@trop.in> writes:
>
>> 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.
>>
>
> I'm not opposed to going back to cons cell pairs. I didn't put too much
> thought in a "list of two elements" vs. "cons cell" besides the match
> statement being easier to handle with a list.
>
> Would this patch have unintended side effects? I thought the . in match
> conditions had a different behavior.
>
> --8<---------------cut here---------------start------------->8---
> scheme@(guile-user)> (match '(1 2 3 4)
> ((a . b) b))
> $5 = (2 3 4)
> scheme@(guile-user)> (match '(1 2)
> ((a . b) b))
> $6 = (2)
> scheme@(guile-user)> (match '(1 . 2)
> ((a . b) b))
> $7 = 2
> --8<---------------cut here---------------end--------------->8---
>
> So changing to this would allow for a home-service entry like the
> following to match:
>
> --8<---------------cut here---------------start------------->8---
> (simple-service 'my-extra-home home-service-type
> `(("bob" "bill" "bertha" "billyanne" ,my-extra-home ,my-home-2)))
> --8<---------------cut here---------------end--------------->8---
>
> From my testing, this /will/ error (yay), but not as soon as the match
> would with the current code. Instead of being caught before being passed
> to the daemon, it seems to be caught while lowering the invalid
> file-append object.
>
> Personally I would prefer to catch as many errors as possible before
> beginning to pass code off to the daemon where possible. Generally
> speaking it feels like pre-daemon errors are easier to mentally parse.
>
> In fairness, the current code also isn't trying particularly hard to
> check that "user" is a string and "he" is a home-environment or printing
> a fancy error message.
>
> Perhaps this would work?
>
> --8<---------------cut here---------------start------------->8---
> (define-module (gnu services guix)
> ...
> #:use-module (gnu home))
> ...
> (define (guix-home-shepherd-service config)
> (map (match-lambda
> (((? string? user) . (? home-environment? he))
> (shepherd-service
> ...
> ))
> (e (error "Invalid value for guix-home-shepherd-service: " e)))
> config))
> --8<---------------cut here---------------end--------------->8---

This idea is good, I'll incorporate this into v2.

Toggle quote (3 lines)
>
> Maybe I'm just being silly. ?

--
Best regards,
Andrew Tropin
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEKEGaxlA4dEDH6S/6IgjSCVjB3rAFAmZO194ACgkQIgjSCVjB
3rB5Hg//YInm4obk2a3gC1MigegfEoYJAxJmAdHO7523zmfyo12LB//knGFiGn6U
VqPCtjbPEWLXVE+YpFm5AI5wOAe3a3oXSId07ozHdocEZoDgVFE8GZJD4hnmZ7te
dRftmcckEKIntpNZzJQzub0tC2tmRvV6sd/3mVnhETcyf71YElWOsmHj3iQhD5CB
POwxDKO+H4FgjE0fw949aS5C+SCDyV2WiFyUTJ2qanWyb3WASOf+rtQz7M+5SvN2
RXVCxO8nzY9nH293fOreeQp7AQtLkRu9wcdSOQIGrJhy1p00wc1HePYw/Q+3AEOq
+3gwVZVafqK6XPjeYJxWMtq/SVx/6vjGg8ivZLgszO1f8I+XtZHcPcA89fP4FcMK
fgemyhgs30eR6JgP+r7fPfSkOBoXbex9ieOV4i2So+Kwf+ncy2Tyztnqpra9NPqs
4j7ypIW2uts6aY+pQkn8t+0CBJ8s24xnM9y5h5wNn0cYXPlPIsK7+Gjn8xzzSU8L
5rQfFElWN2puQD/yg3jP7R7Esu0VNX1G5E3zaB2GPcW+gBLsGzlHHPH5zUbhbmWD
id+f89nGGM1JJFSGtg69UjObGCmOe8N3Ze/q1ceNl31GZXu54pJazntSaJYm9m8m
lvgftrK69cFf3Zk3Ij1v3nNP1f2kVoIjCjJ+RiJCy3Mb+mU2xoY=
=ujiz
-----END PGP SIGNATURE-----

A
A
Andrew Tropin wrote on 23 May 07:53 +0200
[PATCH v2 0/1] services: home: Use pairs instead of lists.
(address . guix-patches@gnu.org)(name . Andrew Tropin)(address . andrew@trop.in)
cover.1716443624.git.andrew@trop.in
Use more precise pattern matching and signal error if it is not a pair
of string/home-environment.

Andrew Tropin (1):
services: home: Use pairs instead of lists.

doc/guix.texi | 4 ++--
gnu/services/guix.scm | 6 ++++--
gnu/tests/guix.scm | 2 +-
3 files changed, 7 insertions(+), 5 deletions(-)


base-commit: b06a460bf5236a9d52f936f2023451051b3e622a
--
2.41.0
A
A
Andrew Tropin wrote on 23 May 07:53 +0200
[PATCH v2 1/1] services: home: Use pairs instead of lists.
(address . guix-patches@gnu.org)
c89139f9177259e04e17df162eb6020a41b58294.1716443624.git.andrew@trop.in
* gnu/services/guix.scm: Use pairs instead of lists.
* doc/guix.texi: Update accordingly.
* gnu/tests/guix.scm: Update accordingly.

Change-Id: I0b8d3fa5b214add89bdb84a11fa20d1b319435f0
---
doc/guix.texi | 4 ++--
gnu/services/guix.scm | 6 ++++--
gnu/tests/guix.scm | 2 +-
3 files changed, 7 insertions(+), 5 deletions(-)

Toggle diff (61 lines)
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
--
2.41.0
L
L
Ludovic Courtès wrote on 23 May 11:16 +0200
(name . Andrew Tropin)(address . andrew@trop.in)
87h6eoc2h4.fsf@gnu.org
Hi Andrew,

Andrew Tropin <andrew@trop.in> skribis:

Toggle quote (5 lines)
> (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’.
A
A
Andrew Tropin wrote on 23 May 15:06 +0200
(name . Ludovic Courtès)(address . ludo@gnu.org)
87pltc65jr.fsf@trop.in
On 2024-05-23 11:16, Ludovic Courtès wrote:

Toggle quote (11 lines)
> 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?

Toggle snippet (13 lines)
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.

Toggle quote (3 lines)
>
> 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.

--
Best regards,
Andrew Tropin
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEKEGaxlA4dEDH6S/6IgjSCVjB3rAFAmZPP2kACgkQIgjSCVjB
3rA69g//YmEbGvo+X+g+Wh5Y7HEbe43cz8YjpAJDbR43B1Fzzcsfq7+VrR8Nxbu6
gbJlY304kFPVmLeERrvTWDirIw6uJ9X8/zBwyES0nR88or2w2XnJLEhylp+4nb5E
Z7QvYr1Zk/CnsQfyACdqAFnDfBTVMAN93R3zv4+YPGWbD0nCFNtB4uk2qVmJbPg6
n8+XapLDrBdTD7Dd+jsZgwyXRbm9GAdHlw0MoYCmaZOpuFralMpiywQajN2Wez4d
H3VqYjTsuLr4xkj9nUdJ4vMauHTFK04YVztdHgQfXw7rdGiGdockeWwu9bLXfV3v
jubu+hVxs8ckeryVL3ZJFq/aDfBbMuveFnmF9WpZTAg/KuMm0dcY1oYcEXoXBuiA
w5AA1J52eBEupQDoo7KEsfd6hMZ6lPD2k81ygZ999D2PDXaM4VQ0gJjE0IqZibDQ
FITqlTG3Z+z2dUNVA7k6JvDI/2sjByfVRc4ZlhQm7UahPKDrT2OHcnaNdTtyTaJQ
lkpKQEK6n0dZ7TBoiMgAcTI49blmejKW+N7iw+56JAoIkN6o9PwgPfgzQjLAXubG
BlPEaoNmIh0tzCZGRrjjH9zhbplBp6kvuR1L3ZxNwl1p5uTDnsmxDEr1g59fAAZO
EWu5VNWre6jY5z4DZt0BS8SVT6ReIYQVw+8h0vsAIS+36iknPAE=
=n9tj
-----END PGP SIGNATURE-----

M
M
Maxim Cournoyer wrote on 23 May 18:02 +0200
Re: [bug#71111] [PATCH 0/1] services: home: Use pairs instead of lists.
(name . Andrew Tropin)(address . andrew@trop.in)
874jaojz31.fsf@gmail.com
Hi,

Andrew Tropin <andrew@trop.in> writes:

Toggle quote (22 lines)
> On 2024-05-23 11:38, Zheng Junjie wrote:
>
>> Andrew Tropin via Guix-patches via <guix-patches@gnu.org> writes:
>>
>>> 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.
>>
>> Maybe we can support pairs and list of length two at same time?
>
> Thank you for the idea, however I think ambiguity is a bad practice,
> from my early experience with guix it's more confusing rather than
> helpful.

I agree.

--
Thanks,
Maxim
A
A
Andrew Tropin wrote on 2 Jun 11:50 +0200
87ttib1xmz.fsf@trop.in
On 2024-05-22 14:02, Andrew Tropin via Guix-patches via wrote:

Toggle quote (23 lines)
> 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.
>
> Andrew Tropin (1):
> services: home: Use pairs instead of lists.
>
> doc/guix.texi | 4 ++--
> gnu/services/guix.scm | 2 +-
> gnu/tests/guix.scm | 2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
>
> base-commit: b06a460bf5236a9d52f936f2023451051b3e622a

Merged v2 with updated API and additional type checks.

--
Best regards,
Andrew Tropin
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEKEGaxlA4dEDH6S/6IgjSCVjB3rAFAmZcQHQACgkQIgjSCVjB
3rDn/xAAhFOYGJlwoGSaVfDexvszA9dIZx9MXxipUJmv/tVwTfxZxkz+4HNGi8Mb
3O9XqJgJfQt5McnqDdQNTDtdgAp1Tml5TeCL2edWiQbqCj4zSZQuodKmOMC2MzJw
w/fYnIUp/oB91Q3jx5wtRmVnLyGmx76HwcIDHkgU9r8xlNDhSCJNivu2kLSCc8bP
yAqm4ewKxbZlm9SD3T5ZOQp6yFlNqVZDDhQ1lZpmkmoioMoJtZ1mt9zR+cXpoUZO
ySy+7DTTEh6H1aBrKSuxuTAzofEMLmI6yjtFC6ZtuHZrD0ZuA+9LDCfXieGPkEMD
RYgPNxhM1uoif7Sp805j8vImqQk9HwxGjdqaF386vdcOTyNqrOA53Mett0IxUNCu
/BiBD1cwkM8vyT5/mC+0d1WHBWNRjOHrnXU/KvCI8RymESxaAFaIfzuJZzBREJ/R
AQWUGTKV44lBGTyPOJlqKZRdAyAAQ0VgC1GhnjCexXGLKj+y3zkY3sobZIKpPZ25
T99OwHZX6X/hZh27dpJFjQoXu+RdupEBM+3EkRpEaAaP3l3A7NMtCgUUwG0GocGl
OOGxE+PRWFDIaKAaTaTedc2ywgzsBp/zYqe9SKQBQ0bkIVgFbcbehmuw/YKsk1XT
RbflDrhftdBJhqrYmzvPlqgjK7SE8wSYtYqjhSVDAwYf6sZIkb8=
=PlU9
-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 2 Jun 12:15 +0200
(name . Andrew Tropin)(address . andrew@trop.in)
87ikyrlkgd.fsf@gnu.org
Hi Andrew,

Andrew Tropin <andrew@trop.in> skribis:

Toggle quote (27 lines)
> On 2024-05-22 14:02, Andrew Tropin via Guix-patches via wrote:
>
>> 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.
>>
>> Andrew Tropin (1):
>> services: home: Use pairs instead of lists.
>>
>> doc/guix.texi | 4 ++--
>> gnu/services/guix.scm | 2 +-
>> gnu/tests/guix.scm | 2 +-
>> 3 files changed, 4 insertions(+), 4 deletions(-)
>>
>>
>> base-commit: b06a460bf5236a9d52f936f2023451051b3e622a
>
> Merged v2 with updated API and additional type checks.

Perhaps I wasn’t clear enough when asking for clarifications¹, but I
think this change shouldn’t happen: first because it’s an incompatible
change that will break user configs, and second because it’s
inconsistent with other similar interfaces (such as ‘authorized-keys’
and <openssh-configuration>).

For these reasons, I’m in favor of reverting this change.

What do others think?

Aside, it’s unfortunate that you weren’t around to review this patch
initially, despite being one of the recipients:
https://issues.guix.gnu.org/69781. I think it’s important to not give
the impression that you chime in just when an rde incompatibility comes
up.

Thanks,
Ludo’.

E
E
Efraim Flashner wrote on 2 Jun 12:37 +0200
(name . Ludovic Courtès)(address . ludo@gnu.org)
ZlxLbtNpW1puijYR@3900XT
On Sun, Jun 02, 2024 at 12:15:14PM +0200, Ludovic Courtès wrote:
Toggle quote (41 lines)
> Hi Andrew,
>
> Andrew Tropin <andrew@trop.in> skribis:
>
> > On 2024-05-22 14:02, Andrew Tropin via Guix-patches via wrote:
> >
> >> 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.
> >>
> >> Andrew Tropin (1):
> >> services: home: Use pairs instead of lists.
> >>
> >> doc/guix.texi | 4 ++--
> >> gnu/services/guix.scm | 2 +-
> >> gnu/tests/guix.scm | 2 +-
> >> 3 files changed, 4 insertions(+), 4 deletions(-)
> >>
> >>
> >> base-commit: b06a460bf5236a9d52f936f2023451051b3e622a
> >
> > Merged v2 with updated API and additional type checks.
>
> Perhaps I wasn’t clear enough when asking for clarifications¹, but I
> think this change shouldn’t happen: first because it’s an incompatible
> change that will break user configs, and second because it’s
> inconsistent with other similar interfaces (such as ‘authorized-keys’
> and <openssh-configuration>).
>
> For these reasons, I’m in favor of reverting this change.
>
> What do others think?

This patch also added home-environment? without adding an import of
(gnu home).

It's unfortunate that the wording for the manual says 'pair' when it's a
list, but IMO that's more of a typo in the manual than a mistake in the
code.

With a quick look I didn't see in any of my OS configs configurations
with pair notations, even with simple-service or extra-special-file,
where it would have been most likely.

I think it would be best to roll this back.

Toggle quote (14 lines)
> Aside, it’s unfortunate that you weren’t around to review this patch
> initially, despite being one of the recipients:
> <https://issues.guix.gnu.org/69781>. I think it’s important to not give
> the impression that you chime in just when an rde incompatibility comes
> up.
>
> Thanks,
> Ludo’.
>
> ¹ https://issues.guix.gnu.org/71111#8
>
>
>

--
Efraim Flashner <efraim@flashner.co.il> ????? ?????
GPG key = A28B F40C 3E55 1372 662D 14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEoov0DD5VE3JmLRT3Qarn3Mo9g1EFAmZcS2oACgkQQarn3Mo9
g1EJYg//R5YFdIHEINWk6msWDDNBxoGnC+mLmTPxkYYmM2mfEh2z1cFsTLVnmHny
O4FBC4A8qVTa1f2M/9HLzdn3EEzQEueLwQlo39ZrJpraJcpgr5IE5Cbyr45+cMiP
5ekfNesdxe4epdNJisJlFJ8YCt++bodKDxQh5XiQpN7LXz+lVxZAkRAfxUqrwF7O
OR3GS+XXN4NcRLsfwnP3IpogybklzulN+nkb8AyFpIvkqNME9hiLHJZ8dyC5yd3T
UZI73xFGlzhyQsN0oHj6NscLhzUD2RUQRTlfcoVbeYTUhZgs/L2cH7cZnaUFZf8Z
3wdRR/uegSe1il7iaYoWumL1U3n83lV7o4qIWC0pyvlrY1w8lQ5g8d6PucvRf8Wa
LlKf78415FEDsO7Ym6r/jmG0Llz/zqRBBdKEn0kVsxVTOVy3/yy1c2L4xkk6Pimv
9NQfX+IiTq4N6SQGKTFM6vMTvPbMlGr3/ZJBVOVwpMWsQh33XhfGjw6iXBXWi99o
phBbeJXFKId/7XK1hQVw1EahaoWqdT+T7iueYeABJGnER17OMwhrvdc9gS7ysDQW
idOtx9JuGHZYh/QQIqHFIi5MeZKYvMCDBqruB04l4bLJku4bip/bC38pdChj/5s2
1KIvDPwMQl3JGUVLeHqjUfMHa+dtroNKcEBspyh1Hi/PvU3Zuh8=
=ERny
-----END PGP SIGNATURE-----


A
A
Andrew Tropin wrote on 2 Jun 12:57 +0200
(name . Ludovic Courtès)(address . ludo@gnu.org)
871q5f1ujr.fsf@trop.in
On 2024-06-02 12:15, Ludovic Courtès wrote:

Toggle quote (45 lines)
> Hi Andrew,
>
> Andrew Tropin <andrew@trop.in> skribis:
>
>> On 2024-05-22 14:02, Andrew Tropin via Guix-patches via wrote:
>>
>>> 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.
>>>
>>> Andrew Tropin (1):
>>> services: home: Use pairs instead of lists.
>>>
>>> doc/guix.texi | 4 ++--
>>> gnu/services/guix.scm | 2 +-
>>> gnu/tests/guix.scm | 2 +-
>>> 3 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>>
>>> base-commit: b06a460bf5236a9d52f936f2023451051b3e622a
>>
>> Merged v2 with updated API and additional type checks.
>
> Perhaps I wasn’t clear enough when asking for clarifications¹, but I
> think this change shouldn’t happen: first because it’s an incompatible
> change that will break user configs, and second because it’s
> inconsistent with other similar interfaces (such as ‘authorized-keys’
> and <openssh-configuration>).
>
> For these reasons, I’m in favor of reverting this change.
>
> What do others think?
>
> Aside, it’s unfortunate that you weren’t around to review this patch
> initially, despite being one of the recipients:
> <https://issues.guix.gnu.org/69781>.

We discussed the upstreaming of this service with Richard and I was
following the thread above, so I was around. I didn't merge or comment
on it because it is literally code written by me, so it make sense to
let someone else to review and merge it.

I didn't realise that in the second revision API was changed from pairs
to lists, when destructuring was rewritten from car/cdr to match. I
skimmed through the docs and was satisfyed and didn't wrote anything.
It came up only now, when people started reporting problems.

Toggle quote (3 lines)
> I think it’s important to not give the impression that you chime in
> just when an rde incompatibility comes up.

Not sure what you mean.

--
Best regards,
Andrew Tropin
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEKEGaxlA4dEDH6S/6IgjSCVjB3rAFAmZcUBgACgkQIgjSCVjB
3rAV/w/+NyCz1opMXSYAnWjJvQwCdMoEkbn5LZ98NAxF8qPfBluA1wmoC8xpNNUE
OZ3o9QUayi1cJITT0lPiKy/Jp7651BBlJ9enI8xghGPnYCxlpZKLcXS56ncnx9LY
O+UbCkUzPVhpr0tEdiyoecn8Jgfo+cAJ8FKQSrXlK9zqY159qG6an4xJBUSogKVl
QTOyF0G1dB6z56fQ8o+wI5rKHSOJu/VJuY3b4gp7DvB5EqatzKs2pxXbZPnRkx6p
OJM56RdigcOQotZ3Sbp6ZyIDitMUDsFAgPiw1A2l49noI8exMBnSiNjOQATkoB7g
wsEmrlqFpmuKQERwvOLPnholF1tdE0OA5I5CU3NKwnZaiCwjHWI3pDQQVJ3mkLoT
DrzAwu+eQaFy8UHs1ga1kmnibFwVvy06zfjX9cuCMJwni/flJ6FQG3/g/JeqzDtU
TQJfnJhOygyTH04lLbviedC6JSERsCPNHXfCxQ+0mInLeQNpzEsWf0e8o3gJYudl
MgQktdv3K5+JktRlY4scHDbporPx4qG1UWJoGN2fncPPmJPoTx6GgENlPGVeKb4R
6kE1sMpxb5vuAO4V6w5KWr6mji9TfoAw9I41vk0sBA5cn2CkdOABbUH/Iv7dvoFc
1dQSZD13eICiFBy1eqJsa+6ku+Oma0ru7WUuGhxraitOv0u1ncY=
=tTkx
-----END PGP SIGNATURE-----

A
A
Andrew Tropin wrote on 2 Jun 13:12 +0200
87y17nzjgu.fsf@trop.in
On 2024-06-02 13:37, Efraim Flashner wrote:

Toggle quote (55 lines)
> On Sun, Jun 02, 2024 at 12:15:14PM +0200, Ludovic Courtès wrote:
>> Hi Andrew,
>>
>> Andrew Tropin <andrew@trop.in> skribis:
>>
>> > On 2024-05-22 14:02, Andrew Tropin via Guix-patches via wrote:
>> >
>> >> 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.
>> >>
>> >> Andrew Tropin (1):
>> >> services: home: Use pairs instead of lists.
>> >>
>> >> doc/guix.texi | 4 ++--
>> >> gnu/services/guix.scm | 2 +-
>> >> gnu/tests/guix.scm | 2 +-
>> >> 3 files changed, 4 insertions(+), 4 deletions(-)
>> >>
>> >>
>> >> base-commit: b06a460bf5236a9d52f936f2023451051b3e622a
>> >
>> > Merged v2 with updated API and additional type checks.
>>
>> Perhaps I wasn’t clear enough when asking for clarifications¹, but I
>> think this change shouldn’t happen: first because it’s an incompatible
>> change that will break user configs, and second because it’s
>> inconsistent with other similar interfaces (such as ‘authorized-keys’
>> and <openssh-configuration>).
>>
>> For these reasons, I’m in favor of reverting this change.
>>
>> What do others think?
>
> This patch also added home-environment? without adding an import of
> (gnu home).
>
> It's unfortunate that the wording for the manual says 'pair' when it's a
> list, but IMO that's more of a typo in the manual than a mistake in the
> code.
>
> With a quick look I didn't see in any of my OS configs configurations
> with pair notations, even with simple-service or extra-special-file,
> where it would have been most likely.
>
> I think it would be best to roll this back.

ok, reverted.

Toggle quote (15 lines)
>
>> Aside, it’s unfortunate that you weren’t around to review this patch
>> initially, despite being one of the recipients:
>> <https://issues.guix.gnu.org/69781>. I think it’s important to not give
>> the impression that you chime in just when an rde incompatibility comes
>> up.
>>
>> Thanks,
>> Ludo’.
>>
>> ¹ https://issues.guix.gnu.org/71111#8
>>
>>
>>

--
Best regards,
Andrew Tropin
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEKEGaxlA4dEDH6S/6IgjSCVjB3rAFAmZcU7EACgkQIgjSCVjB
3rDPyxAAhcfw528oP9X46rqdujoSoKAz3mEuAnL8j/J4o1Eg5ccBhWG4cak0Pe6h
TlbnNlGAuwFuRGFZ0eEzFCzTOapwYEvgCcxphbIQCy3QgalhLz0S5JtJEap0GTod
3plsPHEnQ8vHWYRvCrKDEXSZFiNN6zAgnzIev3KG+MM9QnQwCi2TIKZOAq1cL7Nr
D4ulhQ+YH0F6/lnRGaJw2v7ra5w0fxCtaOYVSEBVSwHDarpDpLv68oLDJvjD7LMo
StE10HZ0dEVoPPJvKZcYRlOJ7tF5XEozY5bKbf+yzz/vaBxwt/Hox/4RNhG3NE1q
VtBt5mRubRj0zr/dSa07rVuj6i8UYJzX9B98xpxPUDnGlPlc4iPoPvvq4o2f8cTS
5Uv4kxo5lRxugdXa5RpKHuCL+969UYz32IXYZoz+y29Tey7HzSP1QFrLDbfLnfUc
fvEFZNok2Ndr9hnI5cgSoUHBO4723fMxO1lG3Uf0XqZJThgq6vrBuJ9YgD+U/J6q
UPZgfILU1aiHvAYhUzCREy2IRSrAq+XJeL6Yo22hjqzswcl9Y6zFvomBMa7GoOc5
ct1VgdJtFg54iWTeovZPnH1PidOm/yw85cm7qhnIkp14+6BCupAGWfo6amSfRz4q
PXzRYPhh1biI5Rf0fUR0TIhN3LZsMh0+jiOixIyL3yM4g00TgRA=
=x0nJ
-----END PGP SIGNATURE-----

?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 71111@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 71111
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch