[Shepherd] Export deregister-service.

  • Done
  • quality assurance status badge
Details
2 participants
  • Ludovic Courtès
  • Bruno Victal
Owner
unassigned
Submitted by
Bruno Victal
Severity
normal
B
B
Bruno Victal wrote on 30 Jun 2023 16:30
(name . bug-guix)(address . bug-guix@gnu.org)
0e13eeb6-ad8f-7955-acd2-0a189034cff9@makinata.eu
Although 'register-services' is exported its counterpart
'deregister-service' is not.

Some notes:
* 'register-services' is in its plural form while its counterpart
is not, maybe one of them should be renamed for consistency sake?

* From the docstring and code the interface of 'deregister-service'
seems to differ from 'register-services' though I haven't
confirmed it. Perhaps add some documented examples for reference?


--
Furthermore, I consider that nonfree software must be eradicated.

Cheers,
Bruno.
L
L
Ludovic Courtès wrote on 12 Jul 2023 22:52
(name . Bruno Victal)(address . mirai@makinata.eu)(address . 64365@debbugs.gnu.org)
87lefk26po.fsf@gnu.org
Hi,

Bruno Victal <mirai@makinata.eu> skribis:

Toggle quote (11 lines)
> Although 'register-services' is exported its counterpart
> 'deregister-service' is not.
>
> Some notes:
> * 'register-services' is in its plural form while its counterpart
> is not, maybe one of them should be renamed for consistency sake?
>
> * From the docstring and code the interface of 'deregister-service'
> seems to differ from 'register-services' though I haven't
> confirmed it. Perhaps add some documented examples for reference?

All good points. Since ‘deregister-service’ is an internal helper with
a clunky interface (it takes a string, special-cases "all", prints
messages), I propose exposing ‘unregister-services’ as defined below.

WDYT?

Thanks,
Ludo’.
Toggle diff (55 lines)
diff --git a/modules/shepherd/service.scm b/modules/shepherd/service.scm
index 6b8c562..e44cbef 100644
--- a/modules/shepherd/service.scm
+++ b/modules/shepherd/service.scm
@@ -95,6 +95,7 @@
spawn-shell-command
%precious-signals
register-services
+ unregister-services
default-respawn-limit
default-service-termination-handler
@@ -2554,6 +2555,19 @@ If it is currently stopped, replace it immediately."
(warn-deprecated-form)
(register-services services)))))
+(define (unregister-services services)
+ "Remove all of @var{services} from the registry, stopping them if they are not
+already stopped."
+ (for-each (lambda (service)
+ (unless (service-stopped? service)
+ (stop-service service)))
+ services)
+
+ ;; Remove SERVICE from the registry.
+ (put-message (current-registry-channel)
+ `(unregister ,services))
+ #t)
+
(define (deregister-service service-name)
"For each string in SERVICE-NAME, stop the associated service if
necessary and remove it from the services table. If SERVICE-NAME is
@@ -2562,13 +2576,6 @@ the special string 'all', remove all services except of 'root'.
This will remove a service either if it is identified by its canonical
name, or if it is the only service providing the service that is
requested to be removed."
- (define (deregister service)
- (when (service-running? service)
- (stop-service service))
- ;; Remove services provided by service from the hash table.
- (put-message (current-registry-channel)
- `(unregister ,(list service))))
-
(let ((name (string->symbol service-name)))
(cond ((eq? name 'all)
;; Special 'remove all' case.
@@ -2587,7 +2594,7 @@ requested to be removed."
(local-output
"Removing service '~a' providing '~a'..."
(service-canonical-name service) name))
- (deregister service)
+ (unregister-services (list service))
(local-output (l10n "Done."))))))))
(define (load-config file-name)
B
B
Bruno Victal wrote on 13 Jul 2023 05:10
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 64365@debbugs.gnu.org)
ed74ef56-f8a4-d040-c307-b8fdef570259@makinata.eu
Hi Ludo’,

On 2023-07-12 21:52, Ludovic Courtès wrote:
Toggle quote (21 lines)
> Hi,
>
> Bruno Victal <mirai@makinata.eu> skribis:
>
>> Although 'register-services' is exported its counterpart
>> 'deregister-service' is not.
>>
>> Some notes:
>> * 'register-services' is in its plural form while its counterpart
>> is not, maybe one of them should be renamed for consistency sake?
>>
>> * From the docstring and code the interface of 'deregister-service'
>> seems to differ from 'register-services' though I haven't
>> confirmed it. Perhaps add some documented examples for reference?
>
> All good points. Since ‘deregister-service’ is an internal helper with
> a clunky interface (it takes a string, special-cases "all", prints
> messages), I propose exposing ‘unregister-services’ as defined below.
>
> WDYT?

Neat, LGTM.


--
Furthermore, I consider that nonfree software must be eradicated.

Cheers,
Bruno.
L
L
Ludovic Courtès wrote on 15 Jul 2023 22:11
(name . Bruno Victal)(address . mirai@makinata.eu)(address . 64365-done@debbugs.gnu.org)
87lefhue8n.fsf@gnu.org
Hi Bruno,

Bruno Victal <mirai@makinata.eu> skribis:

Toggle quote (2 lines)
> Neat, LGTM.

Pushed as b0eee0d037ccceb79c6db810063d129bba67f3f4.

Thanks!

Ludo’.
Closed
?
Your comment

This issue is archived.

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

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