[PATCH] gnu: services: Error in MODIFY-SERVICES when services don't exist

  • Done
  • quality assurance status badge
Details
2 participants
  • Brian Cully
  • Ludovic Courtès
Owner
unassigned
Submitted by
Brian Cully
Severity
normal
B
B
Brian Cully wrote on 16 May 2023 17:39
[PATCH 1/3] tests: Add tests for MODIFY-SERVICES procedure
(address . guix-patches@gnu.org)(name . Brian Cully)(address . bjc@spork.org)
999ea3ff3ea32e4e1bb8b7b8abd4e0e29d1f2395.1684251578.git.bjc@spork.org
* tests/services.scm ("modify-services: do nothing")
("modify-services: delete service")
("modify-services: change value"): New tests.
---
tests/services.scm | 50 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)

Toggle diff (67 lines)
diff --git a/tests/services.scm b/tests/services.scm
index 8e35758209..435f39e59b 100644
--- a/tests/services.scm
+++ b/tests/services.scm
@@ -286,4 +286,54 @@ (define-module (test-services)
((one) one)
(x x))))
+(test-equal "modify-services: do nothing"
+ '(1 2 3)
+ (let* ((t1 (service-type (name 't1)
+ (extensions '())
+ (description "")))
+ (t2 (service-type (name 't2)
+ (extensions '())
+ (description "")))
+ (t3 (service-type (name 't3)
+ (extensions '())
+ (description "")))
+ (services (list (service t1 1) (service t2 2) (service t3 3))))
+ (sort (map service-value
+ (modify-services services))
+ <)))
+
+(test-equal "modify-services: delete service"
+ '(1 3)
+ (let* ((t1 (service-type (name 't1)
+ (extensions '())
+ (description "")))
+ (t2 (service-type (name 't2)
+ (extensions '())
+ (description "")))
+ (t3 (service-type (name 't3)
+ (extensions '())
+ (description "")))
+ (services (list (service t1 1) (service t2 2) (service t3 3))))
+ (sort (map service-value
+ (modify-services services
+ (delete t2)))
+ <)))
+
+(test-equal "modify-services: change value"
+ '(1 2 33)
+ (let* ((t1 (service-type (name 't1)
+ (extensions '())
+ (description "")))
+ (t2 (service-type (name 't2)
+ (extensions '())
+ (description "")))
+ (t3 (service-type (name 't3)
+ (extensions '())
+ (description "")))
+ (services (list (service t1 1) (service t2 2) (service t3 3))))
+ (sort (map service-value
+ (modify-services services
+ (t3 value => 33)))
+ <)))
+
(test-end)

base-commit: b363fab46f5af42b3f653e2fee1834477bd5aacd
prerequisite-patch-id: 8a03c5e8bcd4c526b93c558d550725887f932e41
prerequisite-patch-id: 89400c29b4c30dfbe8492aff1751ca583397b4f0
prerequisite-patch-id: a1963f772e753239b80e6a7b0d9f55e0ab4d662b
prerequisite-patch-id: b047430c30ba9ea274aea33a467cdb49d769884e
--
2.40.1
B
B
Brian Cully wrote on 16 May 2023 17:41
[PATCH 2/3] tests: Check for service existence in MODIFY-SERVICES
(address . 63538@debbugs.gnu.org)(name . Brian Cully)(address . bjc@spork.org)
4de40059943bdd7fa6cc45a70a9cd1087edad57d.1684251702.git.bjc@spork.org
* tests/services.scm ("modify-services: delete non-existing service")
("modify-services: change value for non-existing service"): New tests.
---
tests/services.scm | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)

Toggle diff (49 lines)
diff --git a/tests/services.scm b/tests/services.scm
index 435f39e59b..5a9cd47489 100644
--- a/tests/services.scm
+++ b/tests/services.scm
@@ -319,6 +319,21 @@ (define-module (test-services)
(delete t2)))
<)))
+(test-error "modify-services: delete non-existing service"
+ #t
+ (let* ((t1 (service-type (name 't1)
+ (extensions '())
+ (description "")))
+ (t2 (service-type (name 't2)
+ (extensions '())
+ (description "")))
+ (t3 (service-type (name 't2)
+ (extensions '())
+ (description "")))
+ (services (list (service t1 1) (service t2 2))))
+ (modify-services services
+ (delete t3))))
+
(test-equal "modify-services: change value"
'(1 2 33)
(let* ((t1 (service-type (name 't1)
@@ -336,4 +351,20 @@ (define-module (test-services)
(t3 value => 33)))
<)))
+(test-error "modify-services: change value for non-existing service"
+ #t
+ (let* ((t1 (service-type (name 't1)
+ (extensions '())
+ (description "")))
+ (t2 (service-type (name 't2)
+ (extensions '())
+ (description "")))
+ (t3 (service-type (name 't3)
+ (extensions '())
+ (description "")))
+ (services (list (service t1 1) (service t3 3))))
+ (map service-value
+ (modify-services services
+ (t2 value => 22)))))
+
(test-end)
--
2.40.1
B
B
Brian Cully wrote on 16 May 2023 17:41
[PATCH 3/3] gnu: services: Error in MODIFY-SERVICES when services don't exist
(address . 63538@debbugs.gnu.org)(name . Brian Cully)(address . bjc@spork.org)
bf7a7c9f0750842c7bfc440920e7ff52caa78729.1684251702.git.bjc@spork.org
This patch causes MODIFY-SERVICES to raise an error if a reference is made to
a service which isn't in its service list. This it to help users notice if
they have an invalid rule, which is currently silently ignored.

* gnu/services.scm (%delete-service): new procedure
(%apply-clauses): new syntax rule
(%modify-service): remove syntax rule
---
gnu/services.scm | 47 ++++++++++++++++++++++++++++++-----------------
1 file changed, 30 insertions(+), 17 deletions(-)

Toggle diff (68 lines)
diff --git a/gnu/services.scm b/gnu/services.scm
index d6c7ad0553..988325b253 100644
--- a/gnu/services.scm
+++ b/gnu/services.scm
@@ -296,20 +296,35 @@ (define (simple-service name target value)
(description "This is a simple service."))))
(service type value)))
-(define-syntax %modify-service
+(define (%delete-service kind services)
+ (let loop ((found #f)
+ (return '())
+ (services services))
+ (match services
+ ('()
+ (if found
+ (values return found)
+ (raise (formatted-message
+ (G_ "modify-services: service '~a' not found in service list")
+ (service-type-name kind)))))
+ ((svc . rest)
+ (if (eq? (service-kind svc) kind)
+ (loop svc return rest)
+ (loop found (cons svc return) rest))))))
+
+(define-syntax %apply-clauses
(syntax-rules (=> delete)
- ((_ svc (delete kind) clauses ...)
- (if (eq? (service-kind svc) kind)
- #f
- (%modify-service svc clauses ...)))
- ((_ service)
- service)
- ((_ svc (kind param => exp ...) clauses ...)
- (if (eq? (service-kind svc) kind)
- (let ((param (service-value svc)))
- (service (service-kind svc)
- (begin exp ...)))
- (%modify-service svc clauses ...)))))
+ ((_ ((delete kind) . rest) services)
+ (%apply-clauses rest (%delete-service kind services)))
+ ((_ ((kind param => exp ...) . rest) services)
+ (call-with-values (lambda () (%delete-service kind services))
+ (lambda (svcs found)
+ (let ((param (service-value found)))
+ (cons (service (service-kind found)
+ (begin exp ...))
+ svcs)))))
+ ((_ () services)
+ services)))
(define-syntax modify-services
(syntax-rules ()
@@ -345,10 +360,8 @@ (define-syntax modify-services
UDEV-SERVICE-TYPE.
This is a shorthand for (filter-map (lambda (svc) ...) %base-services)."
- ((_ services clauses ...)
- (filter-map (lambda (service)
- (%modify-service service clauses ...))
- services))))
+ ((_ services . clauses)
+ (%apply-clauses clauses services))))
;;;
--
2.40.1
B
B
Brian Cully wrote on 16 May 2023 19:54
control message for bug #63538
(address . control@debbugs.gnu.org)
878rdoqi0u.fsf@psyduck.jhoto.kublai.com
retitle 63538 [PATCH] gnu: services: Error in MODIFY-SERVICES when services don't exist
quit
B
B
Brian Cully wrote on 27 May 2023 00:27
Re: bug#63538: [PATCH] gnu: services: Error in MODIFY-SERVICES when services don't exist
(address . 63538@debbugs.gnu.org)
87o7m6ybgw.fsf_-_@psyduck.jhoto.kublai.com
The previous patch is incorrect, because it didn't continue recursing
after a modify rule. The next set fixes that, and adds a test for it.
B
B
Brian Cully wrote on 27 May 2023 00:30
[PATCH v2 1/3] tests: Add tests for MODIFY-SERVICES procedure
(address . 63538@debbugs.gnu.org)(name . Brian Cully)(address . bjc@spork.org)
d289f6bcc270806e2384f2443703ed135dc4c111.1685140217.git.bjc@spork.org
* tests/services.scm ("modify-services: do nothing")
("modify-services: delete service")
("modify-services: change value"): New tests.
---
tests/services.scm | 50 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)

Toggle diff (63 lines)
diff --git a/tests/services.scm b/tests/services.scm
index 8e35758209..435f39e59b 100644
--- a/tests/services.scm
+++ b/tests/services.scm
@@ -286,4 +286,54 @@ (define-module (test-services)
((one) one)
(x x))))
+(test-equal "modify-services: do nothing"
+ '(1 2 3)
+ (let* ((t1 (service-type (name 't1)
+ (extensions '())
+ (description "")))
+ (t2 (service-type (name 't2)
+ (extensions '())
+ (description "")))
+ (t3 (service-type (name 't3)
+ (extensions '())
+ (description "")))
+ (services (list (service t1 1) (service t2 2) (service t3 3))))
+ (sort (map service-value
+ (modify-services services))
+ <)))
+
+(test-equal "modify-services: delete service"
+ '(1 3)
+ (let* ((t1 (service-type (name 't1)
+ (extensions '())
+ (description "")))
+ (t2 (service-type (name 't2)
+ (extensions '())
+ (description "")))
+ (t3 (service-type (name 't3)
+ (extensions '())
+ (description "")))
+ (services (list (service t1 1) (service t2 2) (service t3 3))))
+ (sort (map service-value
+ (modify-services services
+ (delete t2)))
+ <)))
+
+(test-equal "modify-services: change value"
+ '(1 2 33)
+ (let* ((t1 (service-type (name 't1)
+ (extensions '())
+ (description "")))
+ (t2 (service-type (name 't2)
+ (extensions '())
+ (description "")))
+ (t3 (service-type (name 't3)
+ (extensions '())
+ (description "")))
+ (services (list (service t1 1) (service t2 2) (service t3 3))))
+ (sort (map service-value
+ (modify-services services
+ (t3 value => 33)))
+ <)))
+
(test-end)

base-commit: 1be6baed2b58a75868cdcc9f51b78624c2fefc4f
--
2.40.1
B
B
Brian Cully wrote on 27 May 2023 00:30
[PATCH v2 2/3] tests: Check for service existence in MODIFY-SERVICES
(address . 63538@debbugs.gnu.org)(name . Brian Cully)(address . bjc@spork.org)
d98b767a2772bcdc4e09de87e19612dea5424862.1685140217.git.bjc@spork.org
* tests/services.scm ("modify-services: delete non-existing service")
("modify-services: change value for non-existing service"): New tests.
---
tests/services.scm | 37 +++++++++++++++++++++++++++++++++++--
1 file changed, 35 insertions(+), 2 deletions(-)

Toggle diff (69 lines)
diff --git a/tests/services.scm b/tests/services.scm
index 435f39e59b..8cdb1b2a31 100644
--- a/tests/services.scm
+++ b/tests/services.scm
@@ -303,7 +303,7 @@ (define-module (test-services)
<)))
(test-equal "modify-services: delete service"
- '(1 3)
+ '(1)
(let* ((t1 (service-type (name 't1)
(extensions '())
(description "")))
@@ -316,11 +316,27 @@ (define-module (test-services)
(services (list (service t1 1) (service t2 2) (service t3 3))))
(sort (map service-value
(modify-services services
+ (delete t3)
(delete t2)))
<)))
+(test-error "modify-services: delete non-existing service"
+ #t
+ (let* ((t1 (service-type (name 't1)
+ (extensions '())
+ (description "")))
+ (t2 (service-type (name 't2)
+ (extensions '())
+ (description "")))
+ (t3 (service-type (name 't2)
+ (extensions '())
+ (description "")))
+ (services (list (service t1 1) (service t2 2))))
+ (modify-services services
+ (delete t3))))
+
(test-equal "modify-services: change value"
- '(1 2 33)
+ '(2 11 33)
(let* ((t1 (service-type (name 't1)
(extensions '())
(description "")))
@@ -333,7 +349,24 @@ (define-module (test-services)
(services (list (service t1 1) (service t2 2) (service t3 3))))
(sort (map service-value
(modify-services services
+ (t1 value => 11)
(t3 value => 33)))
<)))
+(test-error "modify-services: change value for non-existing service"
+ #t
+ (let* ((t1 (service-type (name 't1)
+ (extensions '())
+ (description "")))
+ (t2 (service-type (name 't2)
+ (extensions '())
+ (description "")))
+ (t3 (service-type (name 't3)
+ (extensions '())
+ (description "")))
+ (services (list (service t1 1) (service t3 3))))
+ (map service-value
+ (modify-services services
+ (t2 value => 22)))))
+
(test-end)
--
2.40.1
B
B
Brian Cully wrote on 27 May 2023 00:30
[PATCH v2 3/3] gnu: services: Error in MODIFY-SERVICES when services don't exist
(address . 63538@debbugs.gnu.org)(name . Brian Cully)(address . bjc@spork.org)
13ab94bbed475a933d8376ad8fc166c6ad124b67.1685140217.git.bjc@spork.org
This patch causes MODIFY-SERVICES to raise an error if a reference is made to
a service which isn't in its service list. This it to help users notice if
they have an invalid rule, which is currently silently ignored.

* gnu/services.scm (%delete-service): new procedure
(%apply-clauses): new syntax rule
(%modify-service): remove syntax rule
---
gnu/services.scm | 47 ++++++++++++++++++++++++++++++-----------------
1 file changed, 30 insertions(+), 17 deletions(-)

Toggle diff (68 lines)
diff --git a/gnu/services.scm b/gnu/services.scm
index 31eba9f035..a58cffe536 100644
--- a/gnu/services.scm
+++ b/gnu/services.scm
@@ -296,20 +296,35 @@ (define (simple-service name target value)
(description "This is a simple service."))))
(service type value)))
-(define-syntax %modify-service
+(define (%delete-service kind services)
+ (let loop ((found #f)
+ (return '())
+ (services services))
+ (match services
+ ('()
+ (if found
+ (values return found)
+ (raise (formatted-message
+ (G_ "modify-services: service '~a' not found in service list")
+ (service-type-name kind)))))
+ ((svc . rest)
+ (if (eq? (service-kind svc) kind)
+ (loop svc return rest)
+ (loop found (cons svc return) rest))))))
+
+(define-syntax %apply-clauses
(syntax-rules (=> delete)
- ((_ svc (delete kind) clauses ...)
- (if (eq? (service-kind svc) kind)
- #f
- (%modify-service svc clauses ...)))
- ((_ service)
- service)
- ((_ svc (kind param => exp ...) clauses ...)
- (if (eq? (service-kind svc) kind)
- (let ((param (service-value svc)))
- (service (service-kind svc)
- (begin exp ...)))
- (%modify-service svc clauses ...)))))
+ ((_ ((delete kind) . rest) services)
+ (%apply-clauses rest (%delete-service kind services)))
+ ((_ ((kind param => exp ...) . rest) services)
+ (call-with-values (lambda () (%delete-service kind services))
+ (lambda (svcs found)
+ (let ((param (service-value found)))
+ (cons (service (service-kind found)
+ (begin exp ...))
+ (%apply-clauses rest svcs))))))
+ ((_ () services)
+ services)))
(define-syntax modify-services
(syntax-rules ()
@@ -345,10 +360,8 @@ (define-syntax modify-services
UDEV-SERVICE-TYPE.
This is a shorthand for (filter-map (lambda (svc) ...) %base-services)."
- ((_ services clauses ...)
- (filter-map (lambda (service)
- (%modify-service service clauses ...))
- services))))
+ ((_ services . clauses)
+ (%apply-clauses clauses services))))
;;;
--
2.40.1
L
L
Ludovic Courtès wrote on 2 Jun 2023 16:22
Re: bug#63538: [PATCH] gnu: services: Error in MODIFY-SERVICES when services don't exist
(name . Brian Cully)(address . bjc@spork.org)(address . 63538-done@debbugs.gnu.org)
87wn0m3pvv.fsf_-_@gnu.org
Hi Brian,

Applied with the minor change below. Thanks for working on this!

Ludo’.
Toggle diff (27 lines)
diff --git a/gnu/services.scm b/gnu/services.scm
index a58cffe536..a990d297c9 100644
--- a/gnu/services.scm
+++ b/gnu/services.scm
@@ -6,6 +6,7 @@
;;; Copyright © 2021 raid5atemyhomework <raid5atemyhomework@protonmail.com>
;;; Copyright © 2020 Christine Lemmer-Webber <cwebber@dustycloud.org>
;;; Copyright © 2020, 2021 Brice Waegeneire <brice@waegenei.re>
+;;; Copyright © 2023 Brian Cully <bjc@spork.org>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -307,10 +308,10 @@ (define (%delete-service kind services)
(raise (formatted-message
(G_ "modify-services: service '~a' not found in service list")
(service-type-name kind)))))
- ((svc . rest)
- (if (eq? (service-kind svc) kind)
- (loop svc return rest)
- (loop found (cons svc return) rest))))))
+ ((service . rest)
+ (if (eq? (service-kind service) kind)
+ (loop service return rest)
+ (loop found (cons service return) rest))))))
(define-syntax %apply-clauses
(syntax-rules (=> delete)
Closed
?
Your comment

This issue is archived.

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

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