GNU bug report logs - #27977
[PATCH] services: herd: Fix matching ok responses and add stop service procedure

Previous Next

Full log

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

From: ludo <at> gnu.org (Ludovic Courtès)
To: Christopher Baines <mail <at> cbaines.net>
Cc: 27977 <at> debbugs.gnu.org
Subject: Re: [bug#27977] [PATCH 1/2] services: herd: Fix matching ok responses
 from shepherd service.
Date: Tue, 22 Aug 2017 17:52:44 +0200

[Message part 1 (text/plain, inline)]

Christopher Baines <mail <at> cbaines.net> skribis:

> Previously the match expression case for a successful response
> (where error is #f) required that the result component contained a list with a
> single element.
>
> As far as I see when looking at the responses from the shepherd, this is not
> normally the case. Therefore, to avoid treating successful responses as
> errors, make the match requirement more permissive, accepting any value.
>
> * gnu/services/herd.scm (invoke-action): Change match condition for ok responses.
> ---
>  gnu/services/herd.scm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gnu/services/herd.scm b/gnu/services/herd.scm
> index f8d60a480..49400aba4 100644
> --- a/gnu/services/herd.scm
> +++ b/gnu/services/herd.scm
> @@ -146,7 +146,7 @@ result.  Otherwise return #f."
>      (force-output sock)
>  
>      (match (read sock)
> -      (('reply ('version 0 _ ...) ('result (result)) ('error #f)
> +      (('reply ('version 0 _ ...) ('result result) ('error #f)
>                 ('messages messages))

Actually this is not OK (it broke system tests because
‘current-services’ was now getting a single-element list instead of the
list of service sexps.)

The reason for this is that the ‘action’ method in the Shepherd, when
invoked on a symbol, returns a list of results, one for each service of
that name:

  (define-method (action (obj <symbol>) the-action . args)
    "Perform THE-ACTION on all the services named OBJ.  Return the list of
  results."
    (let ((which-services (lookup-running-or-providing obj)))
      (if (null? which-services)
          (let ((unknown (lookup-running 'unknown)))
            (if (and unknown
                     (defines-action? unknown 'action))
                (apply action unknown 'action the-action args)
                (raise (condition (&missing-service-error (name obj))))))
          (map (lambda (service)
                 (apply action service the-action args))
               which-services))))

(With the exception of actions called on the ‘unknown’ service, which we
should probably get rid of.)

So either we revert the change, or we do this:

[Message part 2 (text/x-patch, inline)]

diff --git a/gnu/services/herd.scm b/gnu/services/herd.scm
index e16d51b9d..7614c7f9f 100644
--- a/gnu/services/herd.scm
+++ b/gnu/services/herd.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2016 Ludovic Courtès <ludo <at> gnu.org>
+;;; Copyright © 2016, 2017 Ludovic Courtès <ludo <at> gnu.org>
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe <at> gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -186,7 +186,11 @@ of pairs."
   "Return the list of currently defined Shepherd services, represented as
 <live-service> objects.  Return #f if the list of services could not be
 obtained."
-  (with-shepherd-action 'root ('status) services
+  (with-shepherd-action 'root ('status) results
+    ;; We get a list of results, one for each service with the name 'root'.
+    ;; In practice there's only one such service though.
+    (match results
+      ((services _ ...)
        (match services
          ((('service ('version 0 _ ...) _ ...) ...)
           (map (lambda (service)
@@ -194,22 +198,22 @@ obtained."
                    (live-service provides requires running)))
                services))
          (x
-       #f))))
+          #f))))))
 
 (define (unload-service service)
   "Unload SERVICE, a symbol name; return #t on success."
   (with-shepherd-action 'root ('unload (symbol->string service)) result
-    result))
+    (first result)))
 
 (define (%load-file file)
   "Load FILE in the Shepherd."
   (with-shepherd-action 'root ('load file) result
-    result))
+    (first result)))
 
 (define (eval-there exp)
   "Eval EXP in the Shepherd."
   (with-shepherd-action 'root ('eval (object->string exp)) result
-    result))
+    (first result)))
 
 (define (load-services files)
   "Load and register the services from FILES, where FILES contain code that

[Message part 3 (text/plain, inline)]

Probably this patch is better than reverting.

Thoughts?

Ludo’.

Previous Next

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