Subject: `modify-services` no longer affects multiple instances of the same service

From: "David Wilson" <[email-masked]>
Date: Fri, 16 Jun 2023 15:52:30 +0300

Hi Guix!

Recently there was a change to the behavior of `modify-services` that adds logic to check for any unused clauses so that an exception can be raised to alert the user of this case.

https://git.savannah.gnu.org/cgit/guix.git/commit/?id=181951207339508789b28ba7cb914f983319920f

It seems that the new logic has a bug that prevents a used clause from being executed on more than one instance of a compatible service in a single execution of `modify-services`.  Here's a new test case for `gnu/tests/services.scm` that exhibits the issue:

```
(test-equal "modify-services: delete multiple services of the same type"
  '(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 t2 2) (service t3 3))))
    (map service-value
         (modify-services services
           (delete t2)))))
```

Here's the output of the test:

```
test-name: modify-services: delete multiple services of the same type
location: /home/daviwil/Projects/Code/guix/tests/services.scm:325
source:
+ (test-equal
+   "modify-services: delete multiple services of the same type"
+   '(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 t2 2)
+                  (service t3 3))))
+     (map service-value
+          (modify-services services (delete t2)))))
expected-value: (1 3)
actual-value: (1 2 3)
result: FAIL
```

The problem occurs because of this `fold2` logic in `apply-clauses` of gnu/services.scm`:

```
                      (fold2 (lambda (clause service remainder)
                               (if service
                                   (match clause
                                     ((kind proc properties)
                                      (if (eq? kind (service-kind service))
                                          (values (proc service) remainder)
                                          (values service
                                                  (cons clause remainder)))))
                                   (values #f (cons clause remainder))))
                             head
                             '()
                             clauses)))
```

In the #t case of checking the service kind, `(values (proc service remainder)` is returned, meaning the successful clause is not being added back to the list of clauses as `fold2` continues.  Any subsequent items of the service list will no longer be tested against the removed clause.

I believe this function's logic needs to be updated to keep a list of successful clauses to be diffed against the full clause list at the end of `apply-clauses` so that the unapplied clause list can be determined without having to remove successful clauses in-flight.

If anyone has any pointers on the best way to approach this, I'll be happy to submit a patch!

David