bug#71887: Raise an error when extending a service type that doesn't support extensions

bug#71887: Raise an error when extending a service type that doesn't support extensions

[Richard Sent]

Hi Guix,

At present it is possible to extend service-types that do not implement
compose or extend methods, resulting in surprising behavior that's hard
to debug [1]. We should throw an error when a service-type that does not
have compose or extend fields is extended.

--8<---------------cut here---------------start------------->8---
(use-modules (gnu home)
             (gnu home services)
             (gnu home services desktop)
             (gnu services))

(home-environment
 (services
  (list
   ;; home-redshift-service-type does not support extensions
   (simple-service 'this-should-error home-redshift-service-type
                   "According to all known laws of aviation..."))))
--8<---------------cut here---------------end--------------->8---


[1]: https://lists.gnu.org/archive/html/help-guix/2024-07/msg00000.html

-- 
Take it easy,
Richard Sent
Making my computer weirder one commit at a time.

bug#71887: Raise an error when extending a service type that doesn't support extensions

[Richard Sent]

I've started looking into this issue and came up with the following
diff:

--8<---------------cut here---------------start------------->8---
diff --git a/gnu/services.scm b/gnu/services.scm
index 88593e8091..e7e2da6ad5 100644
--- a/gnu/services.scm
+++ b/gnu/services.scm
@@ -1225,10 +1225,17 @@ (define* (fold-services services
                       ->
                       ;; Distinguish COMPOSE and EXTEND because PARAMS typically
                       ;; has a different type than the elements of EXTENSIONS.
-                      (if extend
+                      (begin
+                        (unless (or (null? extensions)
+                                    (and extend compose))
+                          (error (format #f "Extensions are not supported in ~a \
+yet the following extensions were found: ~a~%"
+                                         (service-kind sink)
+                                         dependents)))
+                        (if extend
                           (service (service-kind sink)
                                    (extend params (compose extensions)))
-                          sink)))
+                          sink))))
                   (mbegin %state-monad
                     (set-current-state (vhash-consq sink service visited))
                     (return service))))
--8<---------------cut here---------------end--------------->8---

Unfortunately this approach isn't sufficient. In certain cases services
intentionally extend other services to ensure they're instantiated even
when the service is unused. For example,

--8<---------------cut here---------------start------------->8---
(define cgit-service-type
  (service-type
   (name 'cgit)
   (extensions
    (list ...
          ;; Make sure fcgiwrap is instantiated.
          (service-extension fcgiwrap-service-type
                             (const #t))))
   ...
   (default-value (cgit-configuration))))
--8<---------------cut here---------------end--------------->8---

If we all agree that "make sure X is instantiated" service-extensions
must use (const #t), then the above diff can work with minimal
modifications (if every extension value is #t, valid). I'll see if I can
find where to document this when I create the full patch.

If not, I do not believe this bug is solvable. Any service can be
extended with any value regardless of whether that value is used or
coherent. Ergo there's no way to check if any particular extension is
valid.

$ make check-system TESTS=cgit is a good way to verify behavior in this
context.

-- 
Take it easy,
Richard Sent
Making my computer weirder one commit at a time.