Subject: [bug#74425] [PATCH] ui: Include channels in load path before searching for commands.

From: Carlo Zancanaro <[email-masked]>
Date: Fri, 22 Nov 2024 10:25:15 +1100

On Thu, Nov 21 2024, Ludovic Courtès wrote:
> I think you could do:
>
>   #:autoload (guix describe) (package-path-entries)
>
> instead of doing the trick above.

I tried that, and it breaks "guix pull" (as mentioned in the comment in
the code). I don't have the specific error easily accessible, but I
think it was "no code for module: (git)". I assume it was an issue with
how build-program in (build-self) puts things together, but I wasn't
able to investigate further than that.

>   1. The ‘%load-path’ and ‘%load-compiled-path’ modifications can now be
>      removed from (gnu packages), after checking that GUIX_PACKAGE_PATH
>      still takes precedence.

With this change, (extension-directories) does not include the paths in
GUIX_PACKAGE_PATH. Perhaps it should, but I wasn't sure given the
distinction between GUIX_PACKAGE_PATH and GUIX_EXTENSIONS_PATH.

Do we want to run commands from GUIX_PACKAGE_PATH? Or load packages from
GUIX_EXTENSIONS_PATH? At the moment I think the code says "no" to both.

My other concern with removing the code in (gnu packages) is that it may
break code that is using Guix as a library. That is, something like
this:

--8<---------------cut here---------------start------------->8---
guix shell guix guile -- guile -c '(use-modules (gnu packages)) (pk (find-packages-by-name "guile"))'
--8<---------------cut here---------------end--------------->8---

might not add the right things to the load path.

That said, the current patch will add duplicate entries to the load path
when using the Guix CLI, which also isn't good. Perhaps this needs to be
factored out into a single place where we can keep track of whether
we've already added to the load paths.

>   2. The performance impact of this change must be tested, in particular
>      the startup time of ‘guix’ commands since they’ll now be loading
>      (guix channels) & co. unconditionally.
>
>      One way to do that might be:
>
>        PROFILE=$(guix time-machine -q --url=/path/to/checkout)
>        guix shell time -- time $PROFILE/bin/guix build --help
>        strace -c $PROFILE/bin/guix build --help
>      
> Could you give it a try?

Sure!

With the change in my patch, I'm seeing:

strace: number of calls and errors is roughly the same both with and
without my patch.

time: all the numbers look pretty similar with and without my patch.

I haven't done any statistical analysis of the various values recorded
by strace and time, but the numbers don't vary *too* much between runs,
so I'm fairly confident this isn't having too much of an effect.

I also tested leaving off the "-q" in the time machine call (so my other
channels were also included), but this didn't make any meaningful
difference to the numbers when running "guix build --help".

Carlo