bug#41140: “guix system” suggests wrong module import when using “remove”

bug#41140: “guix system” suggests wrong module import when using “remove”

[Ricardo Wurmus]

I want to delete a service from %desktop-services, so I write

--8<---------------cut here---------------start------------->8---
…
(services (remove
           (lambda (service)
             (eq? (service-kind service) that-annoying-service-type))
           %desktop-services))
…
--8<---------------cut here---------------end--------------->8---

Then I run “guix system build config.scm” and Guix tells me that it
doesn’t know what “remove” is:

--8<---------------cut here---------------start------------->8---
config.scm:56:18: error: remove: unbound variable
hint: Did you forget `(use-modules (rnrs lists))'?
--8<---------------cut here---------------end--------------->8---

I add the suggested module import to my file and it no longer complains,
but that-annoying-service-type has not actually been removed.  Oh no!

Of course, the Guix manual says this:

--8<---------------cut here---------------start------------->8---
   Again, ‘%desktop-services’ is just a list of service objects.  If you
want to remove services from there, you can do so using the procedures
for list filtering (*note (guile)SRFI-1 Filtering and Partitioning::).
For instance, the following expression returns a list that contains all
the services in ‘%desktop-services’ minus the Avahi service:

     (remove (lambda (service)
               (eq? (service-kind service) avahi-service-type))
             %desktop-services)
--8<---------------cut here---------------end--------------->8---

But maybe I don’t know what SRFI-1 is and I trust that the hint Guix
provides will be the right thing to do.

But now I’m curious and I look at the documentation for “remove” from
(rnrs lists):

--8<---------------cut here---------------start------------->8---
 -- Scheme Procedure: remp proc list
 -- Scheme Procedure: remove obj list
 -- Scheme Procedure: remv obj list
 -- Scheme Procedure: remq obj list
     ‘remove’, ‘remv’, and ‘remq’ are identical to the ‘delete’, ‘delv’,
     and ‘delq’ procedures provided by Guile’s core library, (*note List
     Modification::).  ‘remp’ is identical to the alternate ‘remove’
     procedure provided by SRFI-1; *Note SRFI-1 Deleting::.
--8<---------------cut here---------------end--------------->8---

Oh.

So here are my questions:

* can we prefer (srfi srfi-1) over (rnrs lists) in the suggestions for “remove”?

* can we avoid this by extending modify-services to support “delete”
  much like modify-phases, and suggesting to use that instead of
  “remove”?

--
Ricardo

bug#41140: “guix system” suggests wrong module import when using “remove”

[Danny Milosavljevic]

[-- Attachment #1: Type: text/plain, Size: 888 bytes --]

Hi Ricardo,

On Sat, 09 May 2020 00:00:05 +0200
Ricardo Wurmus <rekado@elephly.net> wrote:

> * can we prefer (srfi srfi-1) over (rnrs lists) in the suggestions for “remove”?

If possible, I'd just make it search all the modules with that binding that
are available in the search path.  It's okay if that's slow--since it's a
fatal error case anyway.

Then it could say "either (rnrs lists) or (srfi srfi-1), but probably not
both".

Better than hardcoding preferences...

So in short, known-variable-definition shouldn't find the "best" module but
rather all modules with such a binding.  Also, it sounds like that just
searches the currently-loaded modules.  Aren't that too few?

If we don't want that for some reason, we can just add "srfi srfi-1" to the
last "match" form where it already prefers "(gnu ...)", but that's gonna
turn into whack-a-mole fast.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

bug#41140: “guix system” suggests wrong module import when using “remove”

[Ricardo Wurmus]

[-- Attachment #1: Type: text/plain, Size: 285 bytes --]


Ricardo Wurmus <rekado@elephly.net> writes:

> * can we avoid this by extending modify-services to support “delete”
>   much like modify-phases, and suggesting to use that instead of
>   “remove”?

The attached patch does this.

What do you think?

-- 
Ricardo


[-- Attachment #2: 0001-services-Support-DELETE-in-MODIFY-SERVICES-macro.patch --]
[-- Type: text/x-patch, Size: 3919 bytes --]

From 40c1208cbe9cbfa58ee385ef6ee06b775d309753 Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <rekado@elephly.net>
Date: Sun, 10 May 2020 23:29:38 +0200
Subject: [PATCH] services: Support DELETE in MODIFY-SERVICES macro.

* gnu/services.scm (%modify-service): Add clause for DELETE syntax.
(modify-services): Use FILTER-MAP; adjust docstring.
* doc/guix.texi (System Services): Mention alternative syntax.
(X Window): Use MODIFY-SERVICES syntax.
---
 doc/guix.texi    | 13 ++++++++++---
 gnu/services.scm | 23 +++++++++++++++--------
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 14a42e7070..25274a8539 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -11230,6 +11230,14 @@ following expression returns a list that contains all the services in
         %desktop-services)
 @end lisp
 
+Alternatively, the @code{modify-services} macro can be used:
+
+@lisp
+(modify-services %desktop-services
+  (delete avahi-service-type))
+@end lisp
+
+
 @unnumberedsubsec Instantiating the System
 
 Assuming the @code{operating-system} declaration
@@ -14732,9 +14740,8 @@ and tty8.
                    (service slim-service-type (slim-configuration
                                                (display ":1")
                                                (vt "vt8")))
-                   (remove (lambda (service)
-                             (eq? (service-kind service) gdm-service-type))
-                           %desktop-services))))
+                   (modify-services %desktop-services
+                     (delete gdm-service-type)))))
 @end lisp
 
 @end defvr
diff --git a/gnu/services.scm b/gnu/services.scm
index 2e4648bf78..ac614a7317 100644
--- a/gnu/services.scm
+++ b/gnu/services.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2015, 2016, 2017, 2018, 2019, 2020 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2016 Chris Marusich <cmmarusich@gmail.com>
+;;; Copyright © 2020 Ricardo Wurmus <rekado@elephly.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -33,7 +34,7 @@
   #:use-module (guix modules)
   #:use-module (gnu packages base)
   #:use-module (gnu packages bash)
-  #:use-module (srfi srfi-1)
+  #:use-module ((srfi srfi-1) #:hide (delete))
   #:use-module (srfi srfi-9)
   #:use-module (srfi srfi-9 gnu)
   #:use-module (srfi srfi-26)
@@ -272,7 +273,11 @@ singleton service type NAME, of which the returned service is an instance."
     (service type value)))
 
 (define-syntax %modify-service
-  (syntax-rules (=>)
+  (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 ...)
@@ -302,16 +307,18 @@ TYPE.  Consider this example:
     (mingetty-service-type config =>
                            (mingetty-configuration
                             (inherit config)
-                            (motd (plain-file \"motd\" \"Hi there!\")))))
+                            (motd (plain-file \"motd\" \"Hi there!\"))))
+    (delete udev-service-type))
 
 It changes the configuration of the GUIX-SERVICE-TYPE instance, and that of
-all the MINGETTY-SERVICE-TYPE instances.
+all the MINGETTY-SERVICE-TYPE instances, and it deletes instances of the
+UDEV-SERVICE-TYPE.
 
-This is a shorthand for (map (lambda (svc) ...) %base-services)."
+This is a shorthand for (filter-map (lambda (svc) ...) %base-services)."
     ((_ services clauses ...)
-     (map (lambda (service)
-            (%modify-service service clauses ...))
-          services))))
+     (filter-map (lambda (service)
+                   (%modify-service service clauses ...))
+                 services))))
 
 \f
 ;;;
-- 
2.25.1

bug#41140: “guix system” suggests wrong module import when using “remove”

[Ludovic Courtès]

Hi,

Ricardo Wurmus <rekado@elephly.net> skribis:

>>From 40c1208cbe9cbfa58ee385ef6ee06b775d309753 Mon Sep 17 00:00:00 2001
> From: Ricardo Wurmus <rekado@elephly.net>
> Date: Sun, 10 May 2020 23:29:38 +0200
> Subject: [PATCH] services: Support DELETE in MODIFY-SERVICES macro.
>
> * gnu/services.scm (%modify-service): Add clause for DELETE syntax.
> (modify-services): Use FILTER-MAP; adjust docstring.
> * doc/guix.texi (System Services): Mention alternative syntax.
> (X Window): Use MODIFY-SERVICES syntax.

I like it!

> -  #:use-module (srfi srfi-1)
> +  #:use-module ((srfi srfi-1) #:hide (delete))
>    #:use-module (srfi srfi-9)
>    #:use-module (srfi srfi-9 gnu)
>    #:use-module (srfi srfi-26)
> @@ -272,7 +273,11 @@ singleton service type NAME, of which the returned service is an instance."
>      (service type value)))
>  
>  (define-syntax %modify-service
> -  (syntax-rules (=>)
> +  (syntax-rules (=> delete)
> +    ((_ svc (delete kind) clauses ...)
> +     (if (eq? (service-kind svc) kind)
> +         #f
> +         (%modify-service svc clauses ...)))

Best practice suggests that ‘delete’ should be bound (info "(guile)
Syntax Rules"):

--8<---------------cut here---------------start------------->8---
   Although literals can be unbound, usually they are bound to allow
them to be imported, exported, and renamed.  *Note Modules::, for more
information on imports and exports.  In Guile there are a few standard
auxiliary syntax definitions, as specified by R6RS and R7RS:
--8<---------------cut here---------------end--------------->8---

Now, if we export a new ‘delete’ binding from here, it’ll annoy
everyone.  So perhaps we can keep the srfi-1 ‘delete’ and re-export it,
as done in (guix build utils)… though that situation is also annoying
because we get warnings saying that it collides with core ‘delete’.

Dunno, give it a try!

Ludo’.

bug#41140: “guix system” suggests wrong module import when using “remove”

[Ricardo Wurmus]

Ludovic Courtès <ludo@gnu.org> writes:

> Hi,
>
> Ricardo Wurmus <rekado@elephly.net> skribis:
>
>>>From 40c1208cbe9cbfa58ee385ef6ee06b775d309753 Mon Sep 17 00:00:00 2001
>> From: Ricardo Wurmus <rekado@elephly.net>
>> Date: Sun, 10 May 2020 23:29:38 +0200
>> Subject: [PATCH] services: Support DELETE in MODIFY-SERVICES macro.
>>
>> * gnu/services.scm (%modify-service): Add clause for DELETE syntax.
>> (modify-services): Use FILTER-MAP; adjust docstring.
>> * doc/guix.texi (System Services): Mention alternative syntax.
>> (X Window): Use MODIFY-SERVICES syntax.
>
> I like it!
>
>> -  #:use-module (srfi srfi-1)
>> +  #:use-module ((srfi srfi-1) #:hide (delete))
>>    #:use-module (srfi srfi-9)
>>    #:use-module (srfi srfi-9 gnu)
>>    #:use-module (srfi srfi-26)
>> @@ -272,7 +273,11 @@ singleton service type NAME, of which the returned service is an instance."
>>      (service type value)))
>>  
>>  (define-syntax %modify-service
>> -  (syntax-rules (=>)
>> +  (syntax-rules (=> delete)
>> +    ((_ svc (delete kind) clauses ...)
>> +     (if (eq? (service-kind svc) kind)
>> +         #f
>> +         (%modify-service svc clauses ...)))
>
> Best practice suggests that ‘delete’ should be bound (info "(guile)
> Syntax Rules"):
>
> --8<---------------cut here---------------start------------->8---
>    Although literals can be unbound, usually they are bound to allow
> them to be imported, exported, and renamed.  *Note Modules::, for more
> information on imports and exports.  In Guile there are a few standard
> auxiliary syntax definitions, as specified by R6RS and R7RS:
> --8<---------------cut here---------------end--------------->8---
>
> Now, if we export a new ‘delete’ binding from here, it’ll annoy
> everyone.  So perhaps we can keep the srfi-1 ‘delete’ and re-export it,
> as done in (guix build utils)… though that situation is also annoying
> because we get warnings saying that it collides with core ‘delete’.
>
> Dunno, give it a try!

I finally did give it a try.  I’m re-exporting “delete” and I don’t get
any warnings.

I pushed this with commit a247f5c7537df7e0c09051ba22d5c95eb08f48b9.
Thank you for the review and your comments!

-- 
Ricardo

bug#41140: “guix system” suggests wrong module import when using “remove”

[Ludovic Courtès]

Ricardo Wurmus <rekado@elephly.net> skribis:

> But now I’m curious and I look at the documentation for “remove” from
> (rnrs lists):
>
>  -- Scheme Procedure: remp proc list
>  -- Scheme Procedure: remove obj list
>  -- Scheme Procedure: remv obj list
>  -- Scheme Procedure: remq obj list
>      ‘remove’, ‘remv’, and ‘remq’ are identical to the ‘delete’, ‘delv’,
>      and ‘delq’ procedures provided by Guile’s core library, (*note List
>      Modification::).  ‘remp’ is identical to the alternate ‘remove’
>      procedure provided by SRFI-1; *Note SRFI-1 Deleting::.
>
> Oh.

Bah, R6 is terrible in that respect.

> So here are my questions:
>
> * can we prefer (srfi srfi-1) over (rnrs lists) in the suggestions for “remove”?

I don’t think we should do that.  However, listing all the
possibilities, as Danny suggests, would be nice.

> * can we avoid this by extending modify-services to support “delete”
>   much like modify-phases, and suggesting to use that instead of
>   “remove”?

That’s the better option!

Ludo’.