[bug#74633,v3] ui: Search channels for guix extensions

Message ID 87v7umjxug.fsf@gnu.org
State New
Headers
Series [bug#74633,v3] ui: Search channels for guix extensions |

Commit Message

Ludovic Courtès Jan. 10, 2025, 7:03 p.m. UTC
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’.
  

Comments

Brian Kubisiak Jan. 13, 2025, 11:53 p.m. UTC | #1
> 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
  
Ludovic Courtès Jan. 24, 2025, 10:32 p.m. UTC | #2
Hi,

Brian Kubisiak <brian@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’.
  
Brian Kubisiak Feb. 17, 2025, 3:53 p.m. UTC | #3
> > 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
  

Patch

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."