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

Previous Next

Full log

View this message in rfc822 format

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: [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’.

Previous Next

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