unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: "Ludovic Courtès" <ludo@gnu.org>
To: Brian Kubisiak <brian@kubisiak.com>
Cc: 74633@debbugs.gnu.org, Carlo Zancanaro <carlo@zancanaro.id.au>
Subject: [bug#74633] [PATCH v2] ui: Search channels for guix extensions
Date: Thu, 26 Dec 2024 22:08:52 +0100	[thread overview]
Message-ID: <87plle17a3.fsf@gnu.org> (raw)
In-Reply-To: <2c078699007e847611b7a20679621813691d9db3.1733695255.git.brian@kubisiak.com> (Brian Kubisiak's message of "Sun, 8 Dec 2024 14:03:45 -0800")

Hi Brian,

Brian Kubisiak <brian@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’.




      reply	other threads:[~2024-12-26 21:10 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-01 14:53 [bug#74633] [PATCH] ui: Search channels for guix extensions Brian Kubisiak
2024-12-02 17:25 ` Ludovic Courtès
2024-12-03 11:29 ` Carlo Zancanaro
2024-12-04  0:59   ` Brian Kubisiak
2024-12-05  0:27     ` Carlo Zancanaro via Guix-patches via
2024-12-08 22:03 ` [bug#74633] [PATCH v2] " Brian Kubisiak
2024-12-26 21:08   ` Ludovic Courtès [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87plle17a3.fsf@gnu.org \
    --to=ludo@gnu.org \
    --cc=74633@debbugs.gnu.org \
    --cc=brian@kubisiak.com \
    --cc=carlo@zancanaro.id.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).