GNU bug report logs - #71928
[PATCH] gnu: Throw error when extending services that do not use extensions.

Previous Next

To reply to this bug, email your comments to 71928 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Richard Sent <richard <at> freakingpenguin.com>
To: guix-patches <at> gnu.org
Cc: Richard Sent <richard <at> freakingpenguin.com>
Subject: [PATCH] gnu: Throw error when extending services that do not use
 extensions.
Date: Wed,  3 Jul 2024 18:12:48 -0400

* gnu/services.scm (fold-services): Add error handling when using service
extensions.

Fixes: https://issues.guix.gnu.org/71887
Change-Id: Ic8d631674bfddde495c93952d9e6cd5649bb287d
---
 gnu/services.scm | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/gnu/services.scm b/gnu/services.scm
index 88593e8091..a7a9e00478 100644
--- a/gnu/services.scm
+++ b/gnu/services.scm
@@ -1223,12 +1223,22 @@ (define* (fold-services services
                      (params     -> (service-value sink))
                      (service
                       ->
-                      ;; Distinguish COMPOSE and EXTEND because PARAMS typically
-                      ;; has a different type than the elements of EXTENSIONS.
-                      (if extend
-                          (service (service-kind sink)
-                                   (extend params (compose extensions)))
-                          sink)))
+                      (begin
+                        (unless (or (null? extensions)
+                                    ;; A value of #t is a convention for "make
+                                    ;; sure the service is present."
+                                    (every (cut eq? #t <>) extensions)
+                                    (and extend compose))
+                          (error (format #f "Extensions are not supported in \
+~a yet the following service types extend it:~{~%   ~a~}"
+                                         (service-kind sink)
+                                         (map service-kind dependents))))
+                        ;; Distinguish COMPOSE and EXTEND because PARAMS typically
+                        ;; has a different type than the elements of EXTENSIONS.
+                        (if extend
+                            (service (service-kind sink)
+                                     (extend params (compose extensions)))
+                            sink))))
                   (mbegin %state-monad
                     (set-current-state (vhash-consq sink service visited))
                     (return service))))

base-commit: 85012e64819b39fd6112038134548b415fd5daff
-- 
2.45.2

Message #8 received at 71928 <at> debbugs.gnu.org (full text, mbox):

From: Richard Sent <richard <at> freakingpenguin.com>
To: 71928 <at> debbugs.gnu.org
Cc: Richard Sent <richard <at> freakingpenguin.com>
Subject: [PATCH v2] gnu: Throw error when extending services that do not use
 extensions.
Date: Wed,  3 Jul 2024 18:21:01 -0400

* gnu/services.scm (fold-services): Add error handling when using service
extensions.

Fixes: https://issues.guix.gnu.org/71887
Change-Id: Ic8d631674bfddde495c93952d9e6cd5649bb287d
---
 gnu/services.scm | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/gnu/services.scm b/gnu/services.scm
index 88593e8091..f3a1041f14 100644
--- a/gnu/services.scm
+++ b/gnu/services.scm
@@ -53,6 +53,7 @@ (define-module (gnu services)
   #:use-module (srfi srfi-34)
   #:use-module (srfi srfi-35)
   #:use-module (srfi srfi-71)
+  #:use-module (ice-9 format)
   #:use-module (ice-9 vlist)
   #:use-module (ice-9 match)
   #:autoload   (ice-9 pretty-print) (pretty-print)
@@ -1223,12 +1224,22 @@ (define* (fold-services services
                      (params     -> (service-value sink))
                      (service
                       ->
-                      ;; Distinguish COMPOSE and EXTEND because PARAMS typically
-                      ;; has a different type than the elements of EXTENSIONS.
-                      (if extend
-                          (service (service-kind sink)
-                                   (extend params (compose extensions)))
-                          sink)))
+                      (begin
+                        (unless (or (null? extensions)
+                                    ;; A value of #t is a convention for "make
+                                    ;; sure the service is present."
+                                    (every (cut eq? #t <>) extensions)
+                                    (and extend compose))
+                          (error (format #f "Extensions are not supported in \
+~a yet the following service types extend it:~{~%   ~a~}"
+                                         (service-kind sink)
+                                         (map service-kind dependents))))
+                        ;; Distinguish COMPOSE and EXTEND because PARAMS typically
+                        ;; has a different type than the elements of EXTENSIONS.
+                        (if extend
+                            (service (service-kind sink)
+                                     (extend params (compose extensions)))
+                            sink))))
                   (mbegin %state-monad
                     (set-current-state (vhash-consq sink service visited))
                     (return service))))

base-commit: 85012e64819b39fd6112038134548b415fd5daff
-- 
2.45.2

Message #11 received at 71928 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Richard Sent <richard <at> freakingpenguin.com>
Cc: 71928 <at> debbugs.gnu.org
Subject: Re: [bug#71928] [PATCH v2] gnu: Throw error when extending services
 that do not use extensions.
Date: Mon, 30 Sep 2024 22:43:36 +0200

Hello,

Richard Sent <richard <at> freakingpenguin.com> skribis:

> * gnu/services.scm (fold-services): Add error handling when using service
> extensions.
>
> Fixes: https://issues.guix.gnu.org/71887
> Change-Id: Ic8d631674bfddde495c93952d9e6cd5649bb287d

[...]

> @@ -1223,12 +1224,22 @@ (define* (fold-services services
>                       (params     -> (service-value sink))
>                       (service
>                        ->
> -                      ;; Distinguish COMPOSE and EXTEND because PARAMS typically
> -                      ;; has a different type than the elements of EXTENSIONS.
> -                      (if extend
> -                          (service (service-kind sink)
> -                                   (extend params (compose extensions)))
> -                          sink)))
> +                      (begin
> +                        (unless (or (null? extensions)
> +                                    ;; A value of #t is a convention for "make
> +                                    ;; sure the service is present."
> +                                    (every (cut eq? #t <>) extensions)
> +                                    (and extend compose))
> +                          (error (format #f "Extensions are not supported in \
> +~a yet the following service types extend it:~{~%   ~a~}"
> +                                         (service-kind sink)
> +                                         (map service-kind dependents))))
> +                        ;; Distinguish COMPOSE and EXTEND because PARAMS typically
> +                        ;; has a different type than the elements of EXTENSIONS.
> +                        (if extend
> +                            (service (service-kind sink)
> +                                     (extend params (compose extensions)))
> +                            sink))))

I understand the current situation where extensions to a non-extensible
service are silently swallowed is suboptimal, but I wonder about the
implications of the semantic change here.

Currently the “#t means ensure the service is present” convention is not
enforced; is it even widely followed in the Guix repo?  But then, what
about repos out there that provide services?

It would seem to me that this is something that cannot be changed
overnight without running the risk to break people’s code.  So perhaps
the first step (and maybe last step?) would be to print a warning.

WDYT?

Thanks,
Ludo’.

Message #16 received at 71928 <at> debbugs.gnu.org (full text, mbox):

From: Richard Sent <richard <at> freakingpenguin.com>
To: 71928 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org, Richard Sent <richard <at> freakingpenguin.com>
Subject: [PATCH v2] services: Warn when extending services that do not use
 extensions.
Date: Wed, 15 Jan 2025 12:08:00 -0500

* gnu/services.scm (fold-services): Add warning when extending a service that
does not use extensions.

Fixes: https://issues.guix.gnu.org/71887
Change-Id: Ic8d631674bfddde495c93952d9e6cd5649bb287d
---
Hi all. Better late than never, right?

> Currently the “#t means ensure the service is present” convention is not
> enforced; is it even widely followed in the Guix repo?  But then, what
> about repos out there that provide services?

This convention is used for zabbix-front-end-service-type,
cuirass-service-type, and cgit-service-type. There might be
other services that use a different value in the Guix repo, but I
haven't found them yet.

> It would seem to me that this is something that cannot be changed
> overnight without running the risk to break people’s code.  So perhaps
> the first step (and maybe last step?) would be to print a warning.

Done.

This patch has the following limitations:

1. If services A, B, and C all extend unextendable service D, and one
of the services doesn't use #t as the value, the warning is printed
for A, B, and C. I can't imagine this situation being common and it
seems awkward to resolve without needlessly recomputing the extenion
value.

2. Extendable services can not be extended with #t to ensure the
service is instantiated because #t will be passed to compose/extend
and likely throw an error. Not sure of how this should be resolved
because a service could always expect to receive a #t from an
extension and do some special behavior. In my opinion this is a
separate issue.

3. The "#t to ensure an unextendable service is instantiated"
convention isn't documented. Given 2 I couldn't find a way to phrase
it without it turning into word salad.

 gnu/services.scm | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/gnu/services.scm b/gnu/services.scm
index 8a4002e072..2e012ff028 100644
--- a/gnu/services.scm
+++ b/gnu/services.scm
@@ -9,6 +9,7 @@
 ;;; Copyright © 2022 Tobias Geerinckx-Rice <me <at> tobias.gr>
 ;;; Copyright © 2023 Brian Cully <bjc <at> spork.org>
 ;;; Copyright © 2024 Nicolas Graves <ngraves <at> ngraves.fr>
+;;; Copyright © 2025 Richard Sent <richard <at> freakingpenguin.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1250,12 +1251,26 @@ (define* (fold-services services
                      (params     -> (service-value sink))
                      (service
                       ->
-                      ;; Distinguish COMPOSE and EXTEND because PARAMS typically
-                      ;; has a different type than the elements of EXTENSIONS.
-                      (if extend
-                          (service (service-kind sink)
-                                   (extend params (compose extensions)))
-                          sink)))
+                      (begin
+                        (unless (or (null? extensions)
+                                    ;; Extending an unextendable service with
+                                    ;; a value of #t is a convention for "make
+                                    ;; sure the service is present."
+                                    (every (cut eq? #t <>) extensions)
+                                    (and extend compose))
+                          (warning
+                           ;; XXX: This warning lists all dependents even if
+                           ;; only one dependent extends with a non-#t value.
+                           (G_ "Extensions are not supported in \
+~a yet the following service types extend it:~{~%   ~a~}~%")
+                           (service-kind sink)
+                           (map service-kind dependents)))
+                        ;; Distinguish COMPOSE and EXTEND because PARAMS typically
+                        ;; has a different type than the elements of EXTENSIONS.
+                        (if extend
+                            (service (service-kind sink)
+                                     (extend params (compose extensions)))
+                            sink))))
                   (mbegin %state-monad
                     (set-current-state (vhash-consq sink service visited))
                     (return service))))

base-commit: b696658ee8e0655b17f5d26e024956b5148e36d6
-- 
2.47.1

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.