* [bug#27977] [PATCH] services: herd: Fix matching ok responses and add stop service procedure @ 2017-08-05 21:26 Christopher Baines 2017-08-05 21:30 ` [bug#27977] [PATCH 1/2] services: herd: Fix matching ok responses from shepherd service Christopher Baines 0 siblings, 1 reply; 10+ messages in thread From: Christopher Baines @ 2017-08-05 21:26 UTC (permalink / raw) To: 27977 [-- Attachment #1: Type: text/plain, Size: 21 bytes --] Patches incoming... [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 963 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* [bug#27977] [PATCH 1/2] services: herd: Fix matching ok responses from shepherd service. 2017-08-05 21:26 [bug#27977] [PATCH] services: herd: Fix matching ok responses and add stop service procedure Christopher Baines @ 2017-08-05 21:30 ` Christopher Baines 2017-08-05 21:30 ` [bug#27977] [PATCH 2/2] services: herd: Add a stop-service procedure Christopher Baines ` (3 more replies) 0 siblings, 4 replies; 10+ messages in thread From: Christopher Baines @ 2017-08-05 21:30 UTC (permalink / raw) To: 27977 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)) (for-each display-message messages) (cont result)) -- 2.13.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [bug#27977] [PATCH 2/2] services: herd: Add a stop-service procedure. 2017-08-05 21:30 ` [bug#27977] [PATCH 1/2] services: herd: Fix matching ok responses from shepherd service Christopher Baines @ 2017-08-05 21:30 ` Christopher Baines 2017-08-08 8:14 ` Danny Milosavljevic 2017-08-08 8:16 ` [bug#27977] [PATCH 1/2] services: herd: Fix matching ok responses from shepherd service Danny Milosavljevic ` (2 subsequent siblings) 3 siblings, 1 reply; 10+ messages in thread From: Christopher Baines @ 2017-08-05 21:30 UTC (permalink / raw) To: 27977 * gnu/services/herd.scm (stop-service): New procedure. --- gnu/services/herd.scm | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/gnu/services/herd.scm b/gnu/services/herd.scm index 49400aba4..e16d51b9d 100644 --- a/gnu/services/herd.scm +++ b/gnu/services/herd.scm @@ -49,7 +49,8 @@ unload-services unload-service load-services - start-service)) + start-service + stop-service)) ;;; Commentary: ;;; @@ -222,6 +223,10 @@ returns a shepherd <service> object." (with-shepherd-action name ('start) result result)) +(define (stop-service name) + (with-shepherd-action name ('stop) result + result)) + ;; Local Variables: ;; eval: (put 'alist-let* 'scheme-indent-function 2) ;; eval: (put 'with-shepherd 'scheme-indent-function 1) -- 2.13.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [bug#27977] [PATCH 2/2] services: herd: Add a stop-service procedure. 2017-08-05 21:30 ` [bug#27977] [PATCH 2/2] services: herd: Add a stop-service procedure Christopher Baines @ 2017-08-08 8:14 ` Danny Milosavljevic 2017-08-08 19:53 ` bug#27977: " Christopher Baines 0 siblings, 1 reply; 10+ messages in thread From: Danny Milosavljevic @ 2017-08-08 8:14 UTC (permalink / raw) To: Christopher Baines; +Cc: 27977 LGTM! ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#27977: [PATCH 2/2] services: herd: Add a stop-service procedure. 2017-08-08 8:14 ` Danny Milosavljevic @ 2017-08-08 19:53 ` Christopher Baines 0 siblings, 0 replies; 10+ messages in thread From: Christopher Baines @ 2017-08-08 19:53 UTC (permalink / raw) To: Danny Milosavljevic; +Cc: 27977-done [-- Attachment #1: Type: text/plain, Size: 154 bytes --] On Tue, 8 Aug 2017 10:14:01 +0200 Danny Milosavljevic <dannym@scratchpost.org> wrote: > LGTM! Thanks for your review Danny. I've now pushed :) [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 963 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* [bug#27977] [PATCH 1/2] services: herd: Fix matching ok responses from shepherd service. 2017-08-05 21:30 ` [bug#27977] [PATCH 1/2] services: herd: Fix matching ok responses from shepherd service Christopher Baines 2017-08-05 21:30 ` [bug#27977] [PATCH 2/2] services: herd: Add a stop-service procedure Christopher Baines @ 2017-08-08 8:16 ` Danny Milosavljevic 2017-08-22 12:39 ` Ludovic Courtès 2017-08-22 15:52 ` Ludovic Courtès 3 siblings, 0 replies; 10+ messages in thread From: Danny Milosavljevic @ 2017-08-08 8:16 UTC (permalink / raw) To: Christopher Baines; +Cc: 27977 LGTM! ^ permalink raw reply [flat|nested] 10+ messages in thread
* [bug#27977] [PATCH 1/2] services: herd: Fix matching ok responses from shepherd service. 2017-08-05 21:30 ` [bug#27977] [PATCH 1/2] services: herd: Fix matching ok responses from shepherd service Christopher Baines 2017-08-05 21:30 ` [bug#27977] [PATCH 2/2] services: herd: Add a stop-service procedure Christopher Baines 2017-08-08 8:16 ` [bug#27977] [PATCH 1/2] services: herd: Fix matching ok responses from shepherd service Danny Milosavljevic @ 2017-08-22 12:39 ` Ludovic Courtès 2017-08-22 15:52 ` Ludovic Courtès 3 siblings, 0 replies; 10+ messages in thread From: Ludovic Courtès @ 2017-08-22 12:39 UTC (permalink / raw) To: Christopher Baines; +Cc: 27977 Christopher Baines <mail@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. Good catch! Ludo’. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [bug#27977] [PATCH 1/2] services: herd: Fix matching ok responses from shepherd service. 2017-08-05 21:30 ` [bug#27977] [PATCH 1/2] services: herd: Fix matching ok responses from shepherd service Christopher Baines ` (2 preceding siblings ...) 2017-08-22 12:39 ` Ludovic Courtès @ 2017-08-22 15:52 ` Ludovic Courtès 2017-08-22 16:44 ` Christopher Baines 3 siblings, 1 reply; 10+ messages in thread From: Ludovic Courtès @ 2017-08-22 15:52 UTC (permalink / raw) To: Christopher Baines; +Cc: 27977 [-- Attachment #1: Type: text/plain, Size: 2223 bytes --] Christopher Baines <mail@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: [-- Attachment #2: Type: text/x-patch, Size: 1868 bytes --] 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@gnu.org> +;;; Copyright © 2016, 2017 Ludovic Courtès <ludo@gnu.org> ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@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 [-- Attachment #3: Type: text/plain, Size: 74 bytes --] Probably this patch is better than reverting. Thoughts? Ludo’. ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [bug#27977] [PATCH 1/2] services: herd: Fix matching ok responses from shepherd service. 2017-08-22 15:52 ` Ludovic Courtès @ 2017-08-22 16:44 ` Christopher Baines 2017-08-22 22:30 ` Ludovic Courtès 0 siblings, 1 reply; 10+ messages in thread From: Christopher Baines @ 2017-08-22 16:44 UTC (permalink / raw) To: Ludovic Courtès; +Cc: 27977 [-- Attachment #1: Type: text/plain, Size: 754 bytes --] On Tue, 22 Aug 2017 17:52:44 +0200 ludo@gnu.org (Ludovic Courtès) wrote: > Probably this patch is better than reverting. > > Thoughts? I had to apply that patch with --ignore-whitespace-change, as the code in the middle of (current-services) has been indented outside of that patch. I think I get what is going on. As far as I understand it, the (match results ((services _ ...) ... bit is equivilent to the use of first in the other procedures, which suggests to me that you could use first in (current-services)? I'm guessing that the only difference is that they will fail differently on the empty list? Also, I've successfully ran the memcached service test with this change, so there is no regression there which is good :) [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 963 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* [bug#27977] [PATCH 1/2] services: herd: Fix matching ok responses from shepherd service. 2017-08-22 16:44 ` Christopher Baines @ 2017-08-22 22:30 ` Ludovic Courtès 0 siblings, 0 replies; 10+ messages in thread From: Ludovic Courtès @ 2017-08-22 22:30 UTC (permalink / raw) To: Christopher Baines; +Cc: 27977-done Christopher Baines <mail@cbaines.net> skribis: > On Tue, 22 Aug 2017 17:52:44 +0200 > ludo@gnu.org (Ludovic Courtès) wrote: > >> Probably this patch is better than reverting. >> >> Thoughts? > > I had to apply that patch with --ignore-whitespace-change, as the code > in the middle of (current-services) has been indented outside of that > patch. Right, sorry about that (I have -bB in ‘vc-diff-switches’). > I think I get what is going on. As far as I understand it, the (match > results ((services _ ...) ... bit is equivilent to the use of first in > the other procedures, which suggests to me that you could use first in > (current-services)? I'm guessing that the only difference is that they > will fail differently on the empty list? Yes. I’ve pushed it as 7d14082d56462f7bef4254d65a21fd265fbce471. Thanks, Ludo’. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-08-22 22:31 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-08-05 21:26 [bug#27977] [PATCH] services: herd: Fix matching ok responses and add stop service procedure Christopher Baines 2017-08-05 21:30 ` [bug#27977] [PATCH 1/2] services: herd: Fix matching ok responses from shepherd service Christopher Baines 2017-08-05 21:30 ` [bug#27977] [PATCH 2/2] services: herd: Add a stop-service procedure Christopher Baines 2017-08-08 8:14 ` Danny Milosavljevic 2017-08-08 19:53 ` bug#27977: " Christopher Baines 2017-08-08 8:16 ` [bug#27977] [PATCH 1/2] services: herd: Fix matching ok responses from shepherd service Danny Milosavljevic 2017-08-22 12:39 ` Ludovic Courtès 2017-08-22 15:52 ` Ludovic Courtès 2017-08-22 16:44 ` Christopher Baines 2017-08-22 22:30 ` Ludovic Courtès
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/guix.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.