GNU bug report logs - #74633
[PATCH] ui: Search channels for guix extensions

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 74633 in the body.
You can then email your comments to 74633 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Brian Kubisiak <brian <at> kubisiak.com>
To: guix-patches <at> gnu.org
Subject: [PATCH] ui: Search channels for guix extensions
Date: Sun, 1 Dec 2024 06:53:03 -0800

* guix/describe.scm (add-channels-to-load-path!): New function.
* gnu/packages.scm (%package-module-path): Call new function. Remove
the code that the function call replaces.
* guix/ui.scm (extension-directories): Call new function. Search
channels for guix extensions.
* guix/self.scm (compiled-guix)[*core-modules*]: Add 'guile-git' to
the list of extensions.

Change-Id: I53af828dc554485ca28389c9e2653ea6b4fb6b7e
---
 gnu/packages.scm  |  7 ++++---
 guix/describe.scm | 13 +++++++++++++
 guix/self.scm     |  1 +
 guix/ui.scm       | 13 ++++++++++---
 4 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/gnu/packages.scm b/gnu/packages.scm
index 80c22d1d7f..05b8bf8e6d 100644
--- a/gnu/packages.scm
+++ b/gnu/packages.scm
@@ -147,15 +147,16 @@ (define %package-module-path
   (let* ((not-colon   (char-set-complement (char-set #\:)))
          (environment (string-tokenize (or (getenv "GUIX_PACKAGE_PATH") "")
                                        not-colon))
-         (channels-scm channels-go (package-path-entries)))
+         (channels-scm (package-path-entries)))
     ;; Automatically add channels and items from $GUIX_PACKAGE_PATH to Guile's
     ;; search path.  For historical reasons, $GUIX_PACKAGE_PATH goes to the
     ;; front; channels go to the back so that they don't override Guix' own
     ;; modules.
+    (add-channels-to-load-path!)
     (set! %load-path
-      (append environment %load-path channels-scm))
+      (append environment %load-path))
     (set! %load-compiled-path
-      (append environment %load-compiled-path channels-go))
+      (append environment %load-compiled-path))
 
     (make-parameter
      (append environment
diff --git a/guix/describe.scm b/guix/describe.scm
index a4ca2462f4..3bbda15db5 100644
--- a/guix/describe.scm
+++ b/guix/describe.scm
@@ -27,6 +27,7 @@ (define-module (guix describe)
                                 sexp->channel
                                 manifest-entry-channel)
   #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-34)
   #:use-module (ice-9 match)
   #:export (current-profile
@@ -34,6 +35,7 @@ (define-module (guix describe)
             current-profile-entries
             current-channels
             package-path-entries
+            add-channels-to-load-path!
 
             package-provenance
             package-channels
@@ -190,6 +192,17 @@ (define (package-path-entries)
                                       "/site-ccache")))
                (current-channel-entries))))
 
+(define add-channels-to-load-path!
+  (let ((promise
+         (delay
+           (let-values (((channels-scm channels-go) (package-path-entries)))
+             (set! %load-path
+                   (append %load-path channels-scm))
+             (set! %load-compiled-path
+                   (append %load-compiled-path channels-go))))))
+    (lambda ()
+      (force promise))))
+
 (define (package-channels package)
   "Return the list of channels providing PACKAGE or an empty list if it could
 not be determined."
diff --git a/guix/self.scm b/guix/self.scm
index 2652688c71..28239d53f5 100644
--- a/guix/self.scm
+++ b/guix/self.scm
@@ -882,6 +882,7 @@ (define* (compiled-guix source #:key
                     ,(local-file "../guix/store/schema.sql")))
 
                  #:extensions (list guile-gcrypt
+                                    guile-git     ;for (guix git)
                                     guile-json)   ;for (guix swh)
                  #:guile-for-build guile-for-build))
 
diff --git a/guix/ui.scm b/guix/ui.scm
index eba12c8616..28690b22bc 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -38,6 +38,7 @@
 (define-module (guix ui)                       ;import in user interfaces only
   #:use-module (guix i18n)
   #:use-module (guix colors)
+  #:use-module (guix describe)
   #:use-module (guix diagnostics)
   #:use-module (guix gexp)
   #:use-module (guix sets)
@@ -2192,9 +2193,15 @@ (define* (command-files #:optional directory)
 
 (define (extension-directories)
   "Return the list of directories containing Guix extensions."
-  (filter file-exists?
-          (parse-path
-           (getenv "GUIX_EXTENSIONS_PATH"))))
+  (add-channels-to-load-path!)
+  (let ((channels (package-path-entries)))
+    (filter file-exists?
+            (append
+             (parse-path
+              (getenv "GUIX_EXTENSIONS_PATH"))
+             (map
+              (cut string-append <> "/guix/extensions")
+              channels)))))
 
 (define (commands)
   "Return the list of commands, alphabetically sorted."

base-commit: 369d2698b0bfc3726f8e6d232d43d0dda832225f
prerequisite-patch-id: cd0707c90e1d321f3f16f2f861313dd330e9f0b4
-- 
2.46.0

Message #8 received at 74633 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Brian Kubisiak <brian <at> kubisiak.com>
Cc: 74633 <at> debbugs.gnu.org, Carlo Zancanaro <carlo <at> zancanaro.id.au>
Subject: Re: [bug#74633] [PATCH] ui: Search channels for guix extensions
Date: Mon, 02 Dec 2024 18:25:43 +0100

Hello,

Brian Kubisiak <brian <at> kubisiak.com> skribis:

> * guix/describe.scm (add-channels-to-load-path!): New function.
> * gnu/packages.scm (%package-module-path): Call new function. Remove
> the code that the function call replaces.
> * guix/ui.scm (extension-directories): Call new function. Search
> channels for guix extensions.
> * guix/self.scm (compiled-guix)[*core-modules*]: Add 'guile-git' to
> the list of extensions.
>
> Change-Id: I53af828dc554485ca28389c9e2653ea6b4fb6b7e

Before looking further, note that Carlo (Cc’d) has been looking into
this as well:

  https://issues.guix.gnu.org/74425

So there’s a real need :-) and perhaps ways both can be merged.

Ludo’.

Message #11 received at 74633 <at> debbugs.gnu.org (full text, mbox):

From: Carlo Zancanaro <carlo <at> zancanaro.id.au>
To: Brian Kubisiak <brian <at> kubisiak.com>
Cc: 74633 <at> debbugs.gnu.org
Subject: Re: [bug#74633] [PATCH] ui: Search channels for guix extensions
Date: Tue, 03 Dec 2024 22:29:46 +1100

Hi Brian!

As Ludo mentioned, I've recently sent a patch to solve the same problem,
but mine is a bit different. I think your change is better in most ways
to mine, but I'll comment on some things inline below.

On Sun, Dec 01 2024, Brian Kubisiak wrote:
> diff --git a/gnu/packages.scm b/gnu/packages.scm
> index 80c22d1d7f..05b8bf8e6d 100644
> --- a/gnu/packages.scm
> +++ b/gnu/packages.scm
> @@ -147,15 +147,16 @@ (define %package-module-path
>    (let* ((not-colon   (char-set-complement (char-set #\:)))
>           (environment (string-tokenize (or (getenv "GUIX_PACKAGE_PATH") "")
>                                         not-colon))
> -         (channels-scm channels-go (package-path-entries)))
> +         (channels-scm (package-path-entries)))
>      ;; Automatically add channels and items from $GUIX_PACKAGE_PATH to Guile's
>      ;; search path.  For historical reasons, $GUIX_PACKAGE_PATH goes to the
>      ;; front; channels go to the back so that they don't override Guix' own
>      ;; modules.

This comment should be moved onto add-channels-to-load-path!. Possibly
even as a docstring.

> diff --git a/guix/describe.scm b/guix/describe.scm
> index a4ca2462f4..3bbda15db5 100644
> --- a/guix/describe.scm
> +++ b/guix/describe.scm
> @@ -190,6 +192,17 @@ (define (package-path-entries)
>                                        "/site-ccache")))
>                 (current-channel-entries))))
>  
> +(define add-channels-to-load-path!
> +  (let ((promise
> +         (delay
> +           (let-values (((channels-scm channels-go) (package-path-entries)))
> +             (set! %load-path
> +                   (append %load-path channels-scm))
> +             (set! %load-compiled-path
> +                   (append %load-compiled-path channels-go))))))
> +    (lambda ()
> +      (force promise))))

I like that this is avoiding adding things to the path multiple times
(which is a problem with my proposed change).

> diff --git a/guix/self.scm b/guix/self.scm
> index 2652688c71..28239d53f5 100644
> --- a/guix/self.scm
> +++ b/guix/self.scm
> @@ -882,6 +882,7 @@ (define* (compiled-guix source #:key
>                      ,(local-file "../guix/store/schema.sql")))
>  
>                   #:extensions (list guile-gcrypt
> +                                    guile-git     ;for (guix git)

I don't know enough to know if this is a problem, but it's a shame to
have to add this, given I don't think this change actually ends up using
any git functionality.

I solved the same issue by lazily resolving the (guix describe) module.
Using an autoload wasn't enough (i.e. it still broke "guix pull"), but
using (@ (guix describe) package-path-entries) worked.

> diff --git a/guix/ui.scm b/guix/ui.scm
> index eba12c8616..28690b22bc 100644
> --- a/guix/ui.scm
> +++ b/guix/ui.scm
> @@ -2192,9 +2193,15 @@ (define* (command-files #:optional directory)
>  
>  (define (extension-directories)
>    "Return the list of directories containing Guix extensions."
> -  (filter file-exists?
> -          (parse-path
> -           (getenv "GUIX_EXTENSIONS_PATH"))))
> +  (add-channels-to-load-path!)
> +  (let ((channels (package-path-entries)))
> +    (filter file-exists?
> +            (append
> +             (parse-path
> +              (getenv "GUIX_EXTENSIONS_PATH"))
> +             (map
> +              (cut string-append <> "/guix/extensions")
> +              channels)))))

I don't think you need the (append ...). According to the manual,
parse-path takes another argument as a "tail" for the resulting list, so
(parse-path (getenv "GUIX_EXTENSIONS_PATH") (map ... channels)) should
be enough here.

Am I right in thinking that this will look inside the channels at
/guix/extensions to try to find things? That is, if I wanted to add a
command "guix foo" I would need to define a module in
$channel_dir/guix/extensions/guix/scripts/foo.scm?

If I'm reading that correctly, this feels unnecessary. Channels can
already specify a directory in their .guix-channel file. In my change it
just looks for $channel_dir/guix/scripts/foo.scm, which seems more
straightforward. (I'm not worried about people accidentally defining
something /guix/scripts/, because the name is pretty obvious.)

All together, I'm proposing simplifying things to:

--8<---------------cut here---------------start------------->8---
(define (extension-directories)
  "Return the list of directories containing Guix extensions."
  (add-channels-to-load-path!)
  (let ((channels (package-path-entries)))
    (filter file-exists?
            (parse-path (getenv "GUIX_EXTENSIONS_PATH") channels))))
--8<---------------cut here---------------end--------------->8---

We may also want to something into the docstring to mention that it adds
to %load-path.

Carlo

Message #14 received at 74633 <at> debbugs.gnu.org (full text, mbox):

From: Brian Kubisiak <brian <at> kubisiak.com>
To: Carlo Zancanaro <carlo <at> zancanaro.id.au>
Cc: 74633 <at> debbugs.gnu.org
Subject: Re: [bug#74633] [PATCH] ui: Search channels for guix extensions
Date: Tue, 3 Dec 2024 16:59:47 -0800

Hi Carlo,

> On Sun, Dec 01 2024, Brian Kubisiak wrote:
> > diff --git a/gnu/packages.scm b/gnu/packages.scm
> > index 80c22d1d7f..05b8bf8e6d 100644
> > --- a/gnu/packages.scm
> > +++ b/gnu/packages.scm
> > @@ -147,15 +147,16 @@ (define %package-module-path
> >    (let* ((not-colon   (char-set-complement (char-set #\:)))
> >           (environment (string-tokenize (or (getenv "GUIX_PACKAGE_PATH") "")
> >                                         not-colon))
> > -         (channels-scm channels-go (package-path-entries)))
> > +         (channels-scm (package-path-entries)))
> >      ;; Automatically add channels and items from $GUIX_PACKAGE_PATH to Guile's
> >      ;; search path.  For historical reasons, $GUIX_PACKAGE_PATH goes to the
> >      ;; front; channels go to the back so that they don't override Guix' own
> >      ;; modules.
>
> This comment should be moved onto add-channels-to-load-path!. Possibly
> even as a docstring.

Will do.

> > diff --git a/guix/self.scm b/guix/self.scm
> > index 2652688c71..28239d53f5 100644
> > --- a/guix/self.scm
> > +++ b/guix/self.scm
> > @@ -882,6 +882,7 @@ (define* (compiled-guix source #:key
> >                      ,(local-file "../guix/store/schema.sql")))
> >
> >                   #:extensions (list guile-gcrypt
> > +                                    guile-git     ;for (guix git)
>
> I don't know enough to know if this is a problem, but it's a shame to
> have to add this, given I don't think this change actually ends up using
> any git functionality.

Honestly I'm not sure what the are the downsides of this, since I
don't see how *core-module* would get used without *extra-modules* or
why *core-modules* doesn't include all the dependencies. Maybe Ludo
knows more?

> > diff --git a/guix/ui.scm b/guix/ui.scm
> > index eba12c8616..28690b22bc 100644
> > --- a/guix/ui.scm
> > +++ b/guix/ui.scm
> > @@ -2192,9 +2193,15 @@ (define* (command-files #:optional directory)
> >
> >  (define (extension-directories)
> >    "Return the list of directories containing Guix extensions."
> > -  (filter file-exists?
> > -          (parse-path
> > -           (getenv "GUIX_EXTENSIONS_PATH"))))
> > +  (add-channels-to-load-path!)
> > +  (let ((channels (package-path-entries)))
> > +    (filter file-exists?
> > +            (append
> > +             (parse-path
> > +              (getenv "GUIX_EXTENSIONS_PATH"))
> > +             (map
> > +              (cut string-append <> "/guix/extensions")
> > +              channels)))))
>
> I don't think you need the (append ...). According to the manual,
> parse-path takes another argument as a "tail" for the resulting list, so
> (parse-path (getenv "GUIX_EXTENSIONS_PATH") (map ... channels)) should
> be enough here.

I saw that on your patch, good to know!

> Am I right in thinking that this will look inside the channels at
> /guix/extensions to try to find things?

Correct. I chose this to match existing behavior, since
$GUIX_EXTENSIONS_PATH points at this directory for an extension
installed as a package. (I was able to tkae an existing extension and
pull it in as a channel and everything "just worked" without changing
any code or directory structure in the extension)

> That is, if I wanted to add a command "guix foo" I would need to
> define a module in $channel_dir/guix/extensions/guix/scripts/foo.scm?

This is not required, the command "guix foo" could be defined in
$channel_dir/guix/extensions/foo.scm. The command-files function
doesn't add /guix/scripts if a directory is passed as an argument and
 the run-guix-command function will resolve '(guix extensions foo)
if foo.scm exists in an extension directory
(i.e. $channel_dir/guix/extensions/foo.scm exists).

Thanks for the feedback,
Brian

Message #17 received at 74633 <at> debbugs.gnu.org (full text, mbox):

From: Carlo Zancanaro <carlo_zancanaro <at> sil.org>
To: Brian Kubisiak <brian <at> kubisiak.com>
Cc: 74633 <at> debbugs.gnu.org
Subject: Re: [bug#74633] [PATCH] ui: Search channels for guix extensions
Date: Thu, 05 Dec 2024 11:27:05 +1100

On Tue, Dec 03 2024, Brian Kubisiak wrote:
>> That is, if I wanted to add a command "guix foo" I would need to
>> define a module in $channel_dir/guix/extensions/guix/scripts/foo.scm?
>
> This is not required, the command "guix foo" could be defined in
> $channel_dir/guix/extensions/foo.scm. The command-files function
> doesn't add /guix/scripts if a directory is passed as an argument and
>  the run-guix-command function will resolve '(guix extensions foo)
> if foo.scm exists in an extension directory
> (i.e. $channel_dir/guix/extensions/foo.scm exists).

Ah, I see. That sounds good to me.

Given the channel is also being added to %load-path, I expect you can
add a command by defining (guix scripts foo) or (guix extensions foo)
and it will work either way (finding the former first).

We should document this method of extension in the manual. I think a new
section in the "(guix) Channels" chapter for "Declaring Custom Commands"
would be helpful. I might have some time to write something in the next
few days, but if you get a chance before then go ahead.

Thanks for your work on this,

Carlo

Message #20 received at 74633 <at> debbugs.gnu.org (full text, mbox):

From: Brian Kubisiak <brian <at> kubisiak.com>
To: 74633 <at> debbugs.gnu.org
Subject: [PATCH v2] ui: Search channels for guix extensions
Date: Sun, 8 Dec 2024 14:03:45 -0800

* guix/describe.scm (add-channels-to-load-path!): New function.
* gnu/packages.scm (%package-module-path): Call new function. Remove
the code that the function call replaces.
* guix/ui.scm (extension-directories): Call new function. Search
channels for guix extensions.
* guix/self.scm (compiled-guix)[*core-modules*]: Add 'guile-git' to
the list of extensions.

Change-Id: I53af828dc554485ca28389c9e2653ea6b4fb6b7e
---
 gnu/packages.scm  |  7 ++++---
 guix/describe.scm | 17 +++++++++++++++++
 guix/self.scm     |  1 +
 guix/ui.scm       | 12 +++++++++---
 4 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/gnu/packages.scm b/gnu/packages.scm
index 1af3b8d440..42f3546606 100644
--- a/gnu/packages.scm
+++ b/gnu/packages.scm
@@ -148,15 +148,16 @@ (define %package-module-path
   (let* ((not-colon   (char-set-complement (char-set #\:)))
          (environment (string-tokenize (or (getenv "GUIX_PACKAGE_PATH") "")
                                        not-colon))
-         (channels-scm channels-go (package-path-entries)))
+         (channels-scm (package-path-entries)))
     ;; Automatically add channels and items from $GUIX_PACKAGE_PATH to Guile's
     ;; search path.  For historical reasons, $GUIX_PACKAGE_PATH goes to the
     ;; front; channels go to the back so that they don't override Guix' own
     ;; modules.
+    (add-channels-to-load-path!)
     (set! %load-path
-      (append environment %load-path channels-scm))
+      (append environment %load-path))
     (set! %load-compiled-path
-      (append environment %load-compiled-path channels-go))
+      (append environment %load-compiled-path))

     (make-parameter
      (append environment
diff --git a/guix/describe.scm b/guix/describe.scm
index a4ca2462f4..6694b1eaf7 100644
--- a/guix/describe.scm
+++ b/guix/describe.scm
@@ -27,6 +27,7 @@ (define-module (guix describe)
                                 sexp->channel
                                 manifest-entry-channel)
   #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-34)
   #:use-module (ice-9 match)
   #:export (current-profile
@@ -34,6 +35,7 @@ (define-module (guix describe)
             current-profile-entries
             current-channels
             package-path-entries
+            add-channels-to-load-path!

             package-provenance
             package-channels
@@ -190,6 +192,21 @@ (define (package-path-entries)
                                       "/site-ccache")))
                (current-channel-entries))))

+(define add-channels-to-load-path!
+  (let ((promise
+         (delay
+           (let-values (((channels-scm channels-go) (package-path-entries)))
+             (set! %load-path
+                   (append %load-path channels-scm))
+             (set! %load-compiled-path
+                   (append %load-compiled-path channels-go))))))
+    (lambda ()
+      "Automatically add channels to Guile's search path.  Channels are added
+to the end of the path so they don't override Guix' own modules.  This
+function ensures that channels are only added to the search path once even if
+it is called multiple times."
+      (force promise))))
+
 (define (package-channels package)
   "Return the list of channels providing PACKAGE or an empty list if it could
 not be determined."
diff --git a/guix/self.scm b/guix/self.scm
index 2652688c71..28239d53f5 100644
--- a/guix/self.scm
+++ b/guix/self.scm
@@ -882,6 +882,7 @@ (define* (compiled-guix source #:key
                     ,(local-file "../guix/store/schema.sql")))

                  #:extensions (list guile-gcrypt
+                                    guile-git     ;for (guix git)
                                     guile-json)   ;for (guix swh)
                  #:guile-for-build guile-for-build))

diff --git a/guix/ui.scm b/guix/ui.scm
index eba12c8616..87ae24ea55 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -38,6 +38,7 @@
 (define-module (guix ui)                       ;import in user interfaces only
   #:use-module (guix i18n)
   #:use-module (guix colors)
+  #:use-module (guix describe)
   #:use-module (guix diagnostics)
   #:use-module (guix gexp)
   #:use-module (guix sets)
@@ -2192,9 +2193,14 @@ (define* (command-files #:optional directory)

 (define (extension-directories)
   "Return the list of directories containing Guix extensions."
-  (filter file-exists?
-          (parse-path
-           (getenv "GUIX_EXTENSIONS_PATH"))))
+  (add-channels-to-load-path!)
+  (let ((channels (package-path-entries)))
+    (filter file-exists?
+            (parse-path
+             (getenv "GUIX_EXTENSIONS_PATH")
+             (map
+              (cut string-append <> "/guix/extensions")
+              channels)))))

 (define (commands)
   "Return the list of commands, alphabetically sorted."

base-commit: 9001514e242ad15c190588439930b0fa4f6782e3
prerequisite-patch-id: cd0707c90e1d321f3f16f2f861313dd330e9f0b4
--
2.46.0

Message #23 received at 74633 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Brian Kubisiak <brian <at> kubisiak.com>
Cc: 74633 <at> debbugs.gnu.org, Carlo Zancanaro <carlo <at> zancanaro.id.au>
Subject: Re: [bug#74633] [PATCH v2] ui: Search channels for guix extensions
Date: Thu, 26 Dec 2024 22:08:52 +0100

Hi Brian,

Brian Kubisiak <brian <at> kubisiak.com> skribis:

> * guix/describe.scm (add-channels-to-load-path!): New function.
> * gnu/packages.scm (%package-module-path): Call new function. Remove
> the code that the function call replaces.
> * guix/ui.scm (extension-directories): Call new function. Search
> channels for guix extensions.
> * guix/self.scm (compiled-guix)[*core-modules*]: Add 'guile-git' to
> the list of extensions.
>
> Change-Id: I53af828dc554485ca28389c9e2653ea6b4fb6b7e

Overall LGTM.  I tested it with ‘make as-derivation’ and it works as
advertised; ‘strace -c’ shows that the number of syscalls is comparable
to that we currently have.

A couple of minor comments:

> +++ b/gnu/packages.scm
> @@ -148,15 +148,16 @@ (define %package-module-path
>    (let* ((not-colon   (char-set-complement (char-set #\:)))
>           (environment (string-tokenize (or (getenv "GUIX_PACKAGE_PATH") "")
>                                         not-colon))
> -         (channels-scm channels-go (package-path-entries)))
> +         (channels-scm (package-path-entries)))

This variable is now unused; I think it can be removed.

> +(define add-channels-to-load-path!
> +  (let ((promise
> +         (delay
> +           (let-values (((channels-scm channels-go) (package-path-entries)))
> +             (set! %load-path
> +                   (append %load-path channels-scm))
> +             (set! %load-compiled-path
> +                   (append %load-compiled-path channels-go))))))
> +    (lambda ()
> +      "Automatically add channels to Guile's search path.  Channels are added
> +to the end of the path so they don't override Guix' own modules.  This
> +function ensures that channels are only added to the search path once even if
> +it is called multiple times."
> +      (force promise))))

For clarity, I would call this ‘append-channels-to-load-path!’.

Using a promise here works, but I find it slightly misleading (because
we’re using it for side effects) and a bit heavyweight (promises are
thread-safe, so there’s a mutex etc.).

How about this:

  (define (append-channels-to-load-path!)
    (let-values (…)
      …)
    (set! append-channels-to-load-path! (lambda () #t)))

?

Could you send a v3 along these lines, if you think that makes sense?

Thanks,
Ludo’.

Message #28 received at 74633 <at> debbugs.gnu.org (full text, mbox):

From: Brian Kubisiak <brian <at> kubisiak.com>
To: 74633 <at> debbugs.gnu.org
Subject: [PATCH v3] ui: Search channels for guix extensions
Date: Tue, 7 Jan 2025 17:22:35 -0800

* guix/describe.scm (append-channels-to-load-path!): New function.
* gnu/packages.scm (%package-module-path): Call new function. Remove
the code that the function call replaces.
* guix/ui.scm (extension-directories): Call new function. Search
channels for guix extensions.
* guix/self.scm (compiled-guix)[*core-modules*]: Add 'guile-git' to
the list of extensions.

Change-Id: I53af828dc554485ca28389c9e2653ea6b4fb6b7e
---
 gnu/packages.scm     |  7 ++++---
 gnu/system/image.scm |  2 +-
 guix/describe.scm    | 14 ++++++++++++++
 guix/self.scm        |  1 +
 guix/ui.scm          | 12 +++++++++---
 5 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/gnu/packages.scm b/gnu/packages.scm
index bdd5d21940..ee99dea2ca 100644
--- a/gnu/packages.scm
+++ b/gnu/packages.scm
@@ -148,15 +148,16 @@ (define %package-module-path
   (let* ((not-colon   (char-set-complement (char-set #\:)))
          (environment (string-tokenize (or (getenv "GUIX_PACKAGE_PATH") "")
                                        not-colon))
-         (channels-scm channels-go (package-path-entries)))
+         (channels-scm (package-path-entries)))
     ;; Automatically add channels and items from $GUIX_PACKAGE_PATH to Guile's
     ;; search path.  For historical reasons, $GUIX_PACKAGE_PATH goes to the
     ;; front; channels go to the back so that they don't override Guix' own
     ;; modules.
+    (append-channels-to-load-path!)
     (set! %load-path
-      (append environment %load-path channels-scm))
+      (append environment %load-path))
     (set! %load-compiled-path
-      (append environment %load-compiled-path channels-go))
+      (append environment %load-compiled-path))
 
     (make-parameter
      (append environment
diff --git a/gnu/system/image.scm b/gnu/system/image.scm
index af0f3eb354..e4fdbab634 100644
--- a/gnu/system/image.scm
+++ b/gnu/system/image.scm
@@ -319,7 +319,7 @@ (define gcrypt-sqlite3&co
            (match (package-transitive-propagated-inputs package)
              (((labels packages) ...)
               packages))))
-   (list guile-gcrypt guile-sqlite3)))
+   (list guile-gcrypt guile-git guile-json-4 guile-sqlite3)))
 
 (define-syntax-rule (with-imported-modules* gexp* ...)
   (with-extensions gcrypt-sqlite3&co
diff --git a/guix/describe.scm b/guix/describe.scm
index a4ca2462f4..90c17084d1 100644
--- a/guix/describe.scm
+++ b/guix/describe.scm
@@ -27,6 +27,7 @@ (define-module (guix describe)
                                 sexp->channel
                                 manifest-entry-channel)
   #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-34)
   #:use-module (ice-9 match)
   #:export (current-profile
@@ -34,6 +35,7 @@ (define-module (guix describe)
             current-profile-entries
             current-channels
             package-path-entries
+            append-channels-to-load-path!
 
             package-provenance
             package-channels
@@ -190,6 +192,18 @@ (define (package-path-entries)
                                       "/site-ccache")))
                (current-channel-entries))))
 
+(define (append-channels-to-load-path!)
+  "Automatically add channels to Guile's search path.  Channels are added to the
+end of the path so they don't override Guix' own modules.  This function ensures
+that channels are only added to the search path once even if it is called
+multiple times."
+  (let-values (((channels-scm channels-go) (package-path-entries)))
+    (set! %load-path
+          (append %load-path channels-scm))
+    (set! %load-compiled-path
+          (append %load-compiled-path channels-go)))
+  (set! append-channels-to-load-path! (lambda () #t)))
+
 (define (package-channels package)
   "Return the list of channels providing PACKAGE or an empty list if it could
 not be determined."
diff --git a/guix/self.scm b/guix/self.scm
index 2652688c71..28239d53f5 100644
--- a/guix/self.scm
+++ b/guix/self.scm
@@ -882,6 +882,7 @@ (define* (compiled-guix source #:key
                     ,(local-file "../guix/store/schema.sql")))
 
                  #:extensions (list guile-gcrypt
+                                    guile-git     ;for (guix git)
                                     guile-json)   ;for (guix swh)
                  #:guile-for-build guile-for-build))
 
diff --git a/guix/ui.scm b/guix/ui.scm
index 87a448bf72..05bc99a7e3 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -38,6 +38,7 @@
 (define-module (guix ui)                       ;import in user interfaces only
   #:use-module (guix i18n)
   #:use-module (guix colors)
+  #:use-module (guix describe)
   #:use-module (guix diagnostics)
   #:use-module (guix gexp)
   #:use-module (guix sets)
@@ -2194,9 +2195,14 @@ (define* (command-files #:optional directory)
 
 (define (extension-directories)
   "Return the list of directories containing Guix extensions."
-  (filter file-exists?
-          (parse-path
-           (getenv "GUIX_EXTENSIONS_PATH"))))
+  (append-channels-to-load-path!)
+  (let ((channels (package-path-entries)))
+    (filter file-exists?
+            (parse-path
+             (getenv "GUIX_EXTENSIONS_PATH")
+             (map
+              (cut string-append <> "/guix/extensions")
+              channels)))))
 
 (define (commands)
   "Return the list of commands, alphabetically sorted."

base-commit: a76f2d5927c86e4a76a1d3b49c1a37054612f6c0
prerequisite-patch-id: 7835a4f0fbc5e19dcb65195bc9fb69c656915ee1
-- 
2.47.1

Message #31 received at 74633 <at> debbugs.gnu.org (full text, mbox):

From: Brian Kubisiak <brian <at> kubisiak.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 74633 <at> debbugs.gnu.org, Carlo Zancanaro <carlo <at> zancanaro.id.au>
Subject: Re: [bug#74633] [PATCH v2] ui: Search channels for guix extensions
Date: Tue, 7 Jan 2025 17:29:07 -0800

> Could you send a v3 along these lines, if you think that makes sense?

Sent. Note that during testing I noticed that guile-git (and
guile-json-4) needed to be added as dependencies to a g-expression
that imported guix/build/utils.scm due to the additional dependencies
introduced in this patch. I'm probably missing more places where this
is required and Carlo's lazy import may be a better option to avoid
issues like this. Let me know what you think, I can send another
version if you think that's better.

> > +++ b/gnu/packages.scm
> > @@ -148,15 +148,16 @@ (define %package-module-path
> >    (let* ((not-colon   (char-set-complement (char-set #\:)))
> >           (environment (string-tokenize (or (getenv "GUIX_PACKAGE_PATH") "")
> >                                         not-colon))
> > -         (channels-scm channels-go (package-path-entries)))
> > +         (channels-scm (package-path-entries)))
>
> This variable is now unused; I think it can be removed.

This is still used in the below make-parameter call (not shown in the
patch), so I've left it in.

Thanks,
Brian

Message #34 received at 74633 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Brian Kubisiak <brian <at> kubisiak.com>
Cc: 74633 <at> debbugs.gnu.org
Subject: Re: [bug#74633] [PATCH v3] ui: Search channels for guix extensions
Date: Fri, 10 Jan 2025 20:03:03 +0100

[Message part 1 (text/plain, inline)]

Hello,

Overall it LGTM.  I propose the mostly-cosmetic changes below.

Once thing I overlooked before is that commands will have to live under
/guix/extensions, right?

  (define (commands)
    "Return the list of commands, alphabetically sorted."
    (filter-map source-file-command
                (append (command-files)
                        (append-map command-files
                                    (extension-directories)))))

And likewise in ‘run-guix-command’.

But now, if a channel provides ‘guix/scripts/foo.scm’, the ‘guix help’
command will not show ‘foo’ but the ‘guix foo’ command will effectively
work (which wasn’t the case until now).

Maybe it’s fine actually, I don’t know, but I thought this is worth
mentioning and thinking though.

WDYT?

Ludo’.

[Message part 2 (text/x-patch, inline)]

diff --git a/guix/describe.scm b/guix/describe.scm
index 90c17084d1..819f0fef74 100644
--- a/guix/describe.scm
+++ b/guix/describe.scm
@@ -27,8 +27,8 @@ (define-module (guix describe)
                                 sexp->channel
                                 manifest-entry-channel)
   #:use-module (srfi srfi-1)
-  #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-34)
+  #:use-module (srfi srfi-71)
   #:use-module (ice-9 match)
   #:export (current-profile
             current-profile-date
@@ -194,10 +194,11 @@ (define (package-path-entries)
 
 (define (append-channels-to-load-path!)
   "Automatically add channels to Guile's search path.  Channels are added to the
-end of the path so they don't override Guix' own modules.  This function ensures
-that channels are only added to the search path once even if it is called
-multiple times."
-  (let-values (((channels-scm channels-go) (package-path-entries)))
+end of the path so they don't override Guix' own modules.
+
+This procedure ensures that channels are only added to the search path once
+even if it is called multiple times."
+  (let ((channels-scm channels-go (package-path-entries)))
     (set! %load-path
           (append %load-path channels-scm))
     (set! %load-compiled-path
diff --git a/guix/ui.scm b/guix/ui.scm
index 05bc99a7e3..d9d7c8469f 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -38,7 +38,8 @@
 (define-module (guix ui)                       ;import in user interfaces only
   #:use-module (guix i18n)
   #:use-module (guix colors)
-  #:use-module (guix describe)
+  #:autoload   (guix describe) (append-channels-to-load-path!
+                                package-path-entries)
   #:use-module (guix diagnostics)
   #:use-module (guix gexp)
   #:use-module (guix sets)
@@ -2200,9 +2201,8 @@ (define (extension-directories)
     (filter file-exists?
             (parse-path
              (getenv "GUIX_EXTENSIONS_PATH")
-             (map
-              (cut string-append <> "/guix/extensions")
-              channels)))))
+             (map (cut string-append <> "/guix/extensions")
+                  channels)))))
 
 (define (commands)
   "Return the list of commands, alphabetically sorted."

Message #37 received at 74633 <at> debbugs.gnu.org (full text, mbox):

From: Brian Kubisiak <brian <at> kubisiak.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 74633 <at> debbugs.gnu.org
Subject: Re: [bug#74633] [PATCH v3] ui: Search channels for guix extensions
Date: Mon, 13 Jan 2025 15:53:25 -0800

> Overall it LGTM.  I propose the mostly-cosmetic changes below.

LGTM

> Once thing I overlooked before is that commands will have to live under
> /guix/extensions, right?
>
>   (define (commands)
>     "Return the list of commands, alphabetically sorted."
>     (filter-map source-file-command
>                 (append (command-files)
>                         (append-map command-files
>                                     (extension-directories)))))
>
> And likewise in ‘run-guix-command’.
>
> But now, if a channel provides ‘guix/scripts/foo.scm’, the ‘guix help’
> command will not show ‘foo’ but the ‘guix foo’ command will effectively
> work (which wasn’t the case until now).
>
> Maybe it’s fine actually, I don’t know, but I thought this is worth
> mentioning and thinking though.

My intention was to use /guix/extensions since it's the same directory
structure as existing extensions. I don't have a strong preference
here, though I think Carlo intended to support /guix/scripts in
#74425. Maybe it's a good idea to support both?

Brian

Message #40 received at 74633 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Brian Kubisiak <brian <at> kubisiak.com>
Cc: 74633 <at> debbugs.gnu.org, Carlo Zancanaro <carlo <at> zancanaro.id.au>
Subject: Re: [bug#74633] [PATCH v3] ui: Search channels for guix extensions
Date: Fri, 24 Jan 2025 23:32:01 +0100

Hi,

Brian Kubisiak <brian <at> kubisiak.com> skribis:

>> Once thing I overlooked before is that commands will have to live under
>> /guix/extensions, right?
>>
>>   (define (commands)
>>     "Return the list of commands, alphabetically sorted."
>>     (filter-map source-file-command
>>                 (append (command-files)
>>                         (append-map command-files
>>                                     (extension-directories)))))
>>
>> And likewise in ‘run-guix-command’.
>>
>> But now, if a channel provides ‘guix/scripts/foo.scm’, the ‘guix help’
>> command will not show ‘foo’ but the ‘guix foo’ command will effectively
>> work (which wasn’t the case until now).
>>
>> Maybe it’s fine actually, I don’t know, but I thought this is worth
>> mentioning and thinking though.
>
> My intention was to use /guix/extensions since it's the same directory
> structure as existing extensions. I don't have a strong preference
> here, though I think Carlo intended to support /guix/scripts in
> #74425. Maybe it's a good idea to support both?

Yes, it’s probably a good idea to support both.  In that case, the only
thing missing here I think is for ‘commands’ to somehow visit
guix/scripts/*.scm coming from channels.

Thanks,
Ludo’.

Message #43 received at 74633 <at> debbugs.gnu.org (full text, mbox):

From: Brian Kubisiak <brian <at> kubisiak.com>
To: 74633 <at> debbugs.gnu.org
Subject: [PATCH v4] ui: Search channels for guix extensions
Date: Mon, 17 Feb 2025 07:41:46 -0800

* guix/describe.scm (append-channels-to-load-path!): New function.
* gnu/packages.scm (%package-module-path): Call new function. Remove
the code that the function call replaces.
* guix/ui.scm (extension-directories): Call new function. Search
channels for guix extensions.
* guix/self.scm (compiled-guix)[*core-modules*]: Add 'guile-git' to
the list of extensions.

Change-Id: I53af828dc554485ca28389c9e2653ea6b4fb6b7e
---
 gnu/packages.scm  |  7 ++++---
 guix/describe.scm | 15 +++++++++++++++
 guix/self.scm     |  1 +
 guix/ui.scm       | 27 +++++++++++++++++++++++----
 4 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/gnu/packages.scm b/gnu/packages.scm
index bdd5d21940..ee99dea2ca 100644
--- a/gnu/packages.scm
+++ b/gnu/packages.scm
@@ -148,15 +148,16 @@ (define %package-module-path
   (let* ((not-colon   (char-set-complement (char-set #\:)))
          (environment (string-tokenize (or (getenv "GUIX_PACKAGE_PATH") "")
                                        not-colon))
-         (channels-scm channels-go (package-path-entries)))
+         (channels-scm (package-path-entries)))
     ;; Automatically add channels and items from $GUIX_PACKAGE_PATH to Guile's
     ;; search path.  For historical reasons, $GUIX_PACKAGE_PATH goes to the
     ;; front; channels go to the back so that they don't override Guix' own
     ;; modules.
+    (append-channels-to-load-path!)
     (set! %load-path
-      (append environment %load-path channels-scm))
+      (append environment %load-path))
     (set! %load-compiled-path
-      (append environment %load-compiled-path channels-go))
+      (append environment %load-compiled-path))
 
     (make-parameter
      (append environment
diff --git a/guix/describe.scm b/guix/describe.scm
index a4ca2462f4..819f0fef74 100644
--- a/guix/describe.scm
+++ b/guix/describe.scm
@@ -28,12 +28,14 @@ (define-module (guix describe)
                                 manifest-entry-channel)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-34)
+  #:use-module (srfi srfi-71)
   #:use-module (ice-9 match)
   #:export (current-profile
             current-profile-date
             current-profile-entries
             current-channels
             package-path-entries
+            append-channels-to-load-path!
 
             package-provenance
             package-channels
@@ -190,6 +192,19 @@ (define (package-path-entries)
                                       "/site-ccache")))
                (current-channel-entries))))
 
+(define (append-channels-to-load-path!)
+  "Automatically add channels to Guile's search path.  Channels are added to the
+end of the path so they don't override Guix' own modules.
+
+This procedure ensures that channels are only added to the search path once
+even if it is called multiple times."
+  (let ((channels-scm channels-go (package-path-entries)))
+    (set! %load-path
+          (append %load-path channels-scm))
+    (set! %load-compiled-path
+          (append %load-compiled-path channels-go)))
+  (set! append-channels-to-load-path! (lambda () #t)))
+
 (define (package-channels package)
   "Return the list of channels providing PACKAGE or an empty list if it could
 not be determined."
diff --git a/guix/self.scm b/guix/self.scm
index 2652688c71..28239d53f5 100644
--- a/guix/self.scm
+++ b/guix/self.scm
@@ -882,6 +882,7 @@ (define* (compiled-guix source #:key
                     ,(local-file "../guix/store/schema.sql")))
 
                  #:extensions (list guile-gcrypt
+                                    guile-git     ;for (guix git)
                                     guile-json)   ;for (guix swh)
                  #:guile-for-build guile-for-build))
 
diff --git a/guix/ui.scm b/guix/ui.scm
index 87a448bf72..3cc15b05fc 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -2194,9 +2194,24 @@ (define* (command-files #:optional directory)
 
 (define (extension-directories)
   "Return the list of directories containing Guix extensions."
-  (filter file-exists?
-          (parse-path
-           (getenv "GUIX_EXTENSIONS_PATH"))))
+  ;; We need to resolve these lazily, because even using an #:autoload is too
+  ;; much and breaks compilation during "guix pull".
+  (define append-channels-to-load-path!
+    (module-ref (resolve-module `(guix describe))
+                (symbol-append 'append-channels-to-load-path!)))
+  (define package-path-entries
+    (module-ref (resolve-module `(guix describe))
+                (symbol-append 'package-path-entries)))
+  (append-channels-to-load-path!)
+  (let ((channels (package-path-entries)))
+    (filter file-exists?
+            (parse-path
+             (getenv "GUIX_EXTENSIONS_PATH")
+             (append
+              (map (cut string-append <> "/guix/scripts")
+                   channels)
+              (map (cut string-append <> "/guix/extensions")
+                   channels))))))
 
 (define (commands)
   "Return the list of commands, alphabetically sorted."
@@ -2284,7 +2299,11 @@ (define (run-guix-command command . args)
              (show-guix-usage)))))
       (file
        (load file)
-       (resolve-interface `(guix extensions ,command)))))
+       (let ((maybe-extension-path
+              (format #f "/guix/extensions/~a.scm" command)))
+         (if (string-suffix? maybe-extension-path file)
+             (resolve-interface `(guix extensions ,command))
+             (resolve-interface `(guix scripts ,command)))))))
 
   (let ((command-main (module-ref module
                                   (symbol-append 'guix- command))))

base-commit: 0cd02f5e083abac280f6478cb8fda16a0a02e789
-- 
2.48.1

Message #46 received at 74633 <at> debbugs.gnu.org (full text, mbox):

From: Brian Kubisiak <brian <at> kubisiak.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 74633 <at> debbugs.gnu.org, Carlo Zancanaro <carlo <at> zancanaro.id.au>
Subject: Re: [bug#74633] [PATCH v3] ui: Search channels for guix extensions
Date: Mon, 17 Feb 2025 07:53:07 -0800

> > My intention was to use /guix/extensions since it's the same directory
> > structure as existing extensions. I don't have a strong preference
> > here, though I think Carlo intended to support /guix/scripts in
> > #74425. Maybe it's a good idea to support both?
>
> Yes, it’s probably a good idea to support both.  In that case, the only
> thing missing here I think is for ‘commands’ to somehow visit
> guix/scripts/*.scm coming from channels.

I've sent an updated v4 (sorry for the long delay!) that addresses
this as well as the style fixes you suggested above.

I've also continued to run into issues with trying to import (or
autoload) the 'guix describe' module from guix/ui.scm and have
resorted to Carlo's original method of resolving the symbols lazily to
avoid the issue.

Thanks,
Brian

Message #51 received at 74633-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Brian Kubisiak <brian <at> kubisiak.com>
Cc: 74633-done <at> debbugs.gnu.org
Subject: Re: [bug#74633] [PATCH v4] ui: Search channels for guix extensions
Date: Sat, 08 Mar 2025 17:39:03 +0100

[Message part 1 (text/plain, inline)]

Hi Brian,

Brian Kubisiak <brian <at> kubisiak.com> skribis:

> * guix/describe.scm (append-channels-to-load-path!): New function.
> * gnu/packages.scm (%package-module-path): Call new function. Remove
> the code that the function call replaces.
> * guix/ui.scm (extension-directories): Call new function. Search
> channels for guix extensions.
> * guix/self.scm (compiled-guix)[*core-modules*]: Add 'guile-git' to
> the list of extensions.
>
> Change-Id: I53af828dc554485ca28389c9e2653ea6b4fb6b7e

Finally applied, with the cosmetic changes below.

Thanks!

Ludo’.

[Message part 2 (text/x-patch, inline)]

diff --git a/guix/ui.scm b/guix/ui.scm
index 3cc15b05fc..d462f7133e 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -2197,11 +2197,12 @@ (define (extension-directories)
   ;; We need to resolve these lazily, because even using an #:autoload is too
   ;; much and breaks compilation during "guix pull".
   (define append-channels-to-load-path!
-    (module-ref (resolve-module `(guix describe))
-                (symbol-append 'append-channels-to-load-path!)))
+    (module-ref (resolve-interface '(guix describe))
+                'append-channels-to-load-path!))
   (define package-path-entries
-    (module-ref (resolve-module `(guix describe))
-                (symbol-append 'package-path-entries)))
+    (module-ref (resolve-interface '(guix describe))
+                'package-path-entries))
+
   (append-channels-to-load-path!)
   (let ((channels (package-path-entries)))
     (filter file-exists?

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.