(modify-services … (delete …)) should delete all matching service types

  • Done
  • quality assurance status badge
Details
3 participants
  • Felix Lechner
  • Maxim Cournoyer
  • Tobias Geerinckx-Rice
Owner
unassigned
Submitted by
Tobias Geerinckx-Rice
Severity
important
Merged with
T
T
Tobias Geerinckx-Rice wrote on 9 Aug 2023 19:40
(modify-services … (delete …)) should delete all matching service types
(address . bug-guix@gnu.org)
786c3ff6d991719575b26a82cb7a39cb@tobias.gr
TODO: the snippet

(modify-services %base-services
(delete mingetty-service-type))

deletes only the first (tty1) instance of the mingetty service. I can't
think of a scenario where this is likely to reflect the user's
intention. It should delete all matching services.

A delete-first variant could be added iff there's demand.

Kind regards,

T G-R

Sent from a Web browser. Excuse or enjoy my brevity.
T
T
Tobias Geerinckx-Rice wrote on 9 Aug 2023 19:49
(no subject)
(address . control@debbugs.gnu.org)
7381c9980c40714ebb8255e7ac7f0931@tobias.gr
severity 65184 important
merge 64106 65184
thanks
sigh
M
M
Maxim Cournoyer wrote on 1 Sep 2023 05:49
Re: bug#65184: (modify-services … (delete …)) should delete all matching service types
(name . Brian Cully)(address . bjc@spork.org)
87msy65z8f.fsf_-_@gmail.com
Hi Brian!

Brian Cully <bjc@spork.org> writes:

Toggle quote (21 lines)
> This patch reverts the behavior introduced in
> 181951207339508789b28ba7cb914f983319920f which caused ‘modify-services’
> clauses to only match a single instance of a service.
>
> We will now match all service instances when doing a deletion or update, while
> still raising an exception when trying to match against a service that does
> not exist in the services list, or which was deleted explicitly by a ‘delete’
> clause (or an update clause that returns ‘#f’ for the service).
>
> Fixes: #64106
>
> * gnu/services.scm (%modify-services): New procedure.
> (modify-services): Use it.
> (apply-clauses): Add DELETED-SERVICES argument, change to modify one service
> at a time.
> * tests/services.scm
> ("modify-services: delete then modify"),
> ("modify-services: modify then delete"),
> ("modify-services: delete multiple services of the same type"),
> ("modify-services: modify multiple services of the same type"): New tests.

[...]

I've applied the following cosmetic changes:

Toggle snippet (66 lines)
1 file changed, 20 insertions(+), 18 deletions(-)
gnu/services.scm | 38 ++++++++++++++++++++------------------

modified gnu/services.scm
@@ -325,11 +325,13 @@ (define-syntax clause-alist
'())))
(define (apply-clauses clauses service deleted-services)
+ "Apply CLAUSES, an alist as returned by 'clause-alist', to SERVICE. An
+exception is raised if a clause attempts to modify a service
+present in DELETED-SERVICES."
(define (raise-if-deleted kind properties)
- (match (find (lambda (deleted)
- (match deleted
- ((deleted-kind _)
- (eq? kind deleted-kind))))
+ (match (find (match-lambda
+ ((deleted-kind _)
+ (eq? kind deleted-kind)))
deleted-services)
((_ deleted-properties)
(raise (make-compound-condition
@@ -344,27 +346,27 @@ (define (apply-clauses clauses service deleted-services)
(match clauses
(((kind proc properties) . rest)
- (begin
- (raise-if-deleted kind properties)
- (if (eq? (and service (service-kind service))
- kind)
- (let ((new-service (proc service)))
- (apply-clauses rest new-service
- (if new-service
- deleted-services
- (cons (list kind properties)
- deleted-services))))
- (apply-clauses rest service deleted-services))))
+ (raise-if-deleted kind properties)
+ (if (eq? (and service (service-kind service)) kind)
+ (let ((new-service (proc service)))
+ (apply-clauses rest new-service
+ (if new-service
+ deleted-services
+ (cons (list kind properties)
+ deleted-services))))
+ (apply-clauses rest service deleted-services)))
(()
service)))
(define (%modify-services services clauses)
+ "Apply CLAUSES, an alist as returned by 'clause-alist', to SERVICES. An
+exception is raised if a clause attempts to modify a missing service."
(define (raise-if-not-found clause)
(match clause
((kind _ properties)
- (when (not (find (lambda (service)
- (eq? kind (service-kind service)))
- services))
+ (unless (find (lambda (service)
+ (eq? kind (service-kind service)))
+ services)
(raise (make-compound-condition
(condition
(&error-location

and installed it. Thanks for contributing to Guix!

--
Thanks,
Maxim
Closed
F
F
Felix Lechner wrote on 1 Sep 2023 06:00
Re: bug#65184: (modify-services … (delete …)) sh ould delete all matching service types
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
CAFHYt56hE1WNRudv94bwTPJX8XHL9_F_QPtY19EOrpiVdufz2w@mail.gmail.com
Hi Maxim,

On Thu, Aug 31, 2023 at 8:49?PM Maxim Cournoyer
<maxim.cournoyer@gmail.com> wrote:
Toggle quote (3 lines)
>
> > Fixes: #64106

Thanks for taking action. Can Bug#63921 also be closed?

Kind regards
Felix
?
Your comment

This issue is archived.

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

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