Raise an error when extending a service type that doesn't support extensions

  • Open
  • quality assurance status badge
Details
One participant
  • Richard Sent
Owner
unassigned
Submitted by
Richard Sent
Severity
normal
R
R
Richard Sent wrote on 2 Jul 00:10 +0200
(address . bug-guix@gnu.org)
877ce4dar0.fsf@freakingpenguin.com
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.

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



--
Take it easy,
Richard Sent
Making my computer weirder one commit at a time.
R
R
Richard Sent wrote on 2 Jul 05:36 +0200
(address . 71887@debbugs.gnu.org)
87cynwbh36.fsf@freakingpenguin.com
I've started looking into this issue and came up with the following
diff:

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

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,

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

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.
?
Your comment

Commenting via the web interface is currently disabled.

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

To respond to this issue using the mumi CLI, first switch to it
mumi current 71887
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