bug#65575: [PATCH 3/3] gnu: emacs: Reload subdirs.el files in 'guix-emacs-autoload-packages'.

[Maxim Cournoyer]

Hi Liliana,

Liliana Marie Prikler <liliana.prikler@ist.tugraz.at> writes:

> Hi Maxim,
>
> Am Montag, dem 28.08.2023 um 01:11 -0400 schrieb Maxim Cournoyer:
>> This fixes a regression introduced with 79cfe30f3 ("build-system:
>> emacs: Use subdirectories again.") which caused the
>> 'guix-emacs-autoload-packages' to no
>> longer be able to autoload all packages.
>>
>> * gnu/packages/aux-files/emacs/guix-emacs.el
>> (guix-emacs-autoload-packages-called): New variable.
>> (guix-emacs-autoload-packages): Reload subdirs.el files an all but
>> the first call of this procedure.
>> * doc/guix.texi (Application Setup): Document that
>> 'guix-emacs-autoload-packages' can be invoked interactively to auto-
>> reload newly installed Emacs packages.
>>
>> ---
> Thank you for looking at this.  I agree that even if we don't want to
> generally load new packages into existing Emacs processes for the
> breakages they might induce, not being able to do so at all is also not
> a great option.  However, I don't think that tracking is the right
> solution here, see below.

[...]

>> diff --git a/gnu/packages/aux-files/emacs/guix-emacs.el
>> b/gnu/packages/aux-files/emacs/guix-emacs.el
>> index ed0c913163..b4a4fd1d91 100644
>> --- a/gnu/packages/aux-files/emacs/guix-emacs.el
>> +++ b/gnu/packages/aux-files/emacs/guix-emacs.el
>> @@ -54,6 +54,9 @@ The files in the list do not have extensions (.el,
>> .elc)."
>>                          (expand-file-name "subdirs.el" dir))
>>                        (guix-emacs--non-core-load-path))))
>>  
>> +(defvar guix-emacs-autoload-packages-called nil
>> +  "True if `guix-emacs-autoload-packages' was already run.")
>> +
>>  ;;;###autoload
>>  (defun guix-emacs-autoload-packages ()
>>    "Autoload Emacs packages found in EMACSLOADPATH.
>> @@ -61,6 +64,15 @@ The files in the list do not have extensions (.el,
>> .elc)."
>>  'Autoload' means to load the 'autoloads' files matching
>>  `guix-emacs-autoloads-regexp'."
>>    (interactive)
>> +  ;; Reload the subdirs.el files such as the one generated by the
>> Guix profile
>> +  ;; hook, so that newly installed Emacs packages located under
>> +  ;; sub-directories are put on the load-path without having to
>> restart Emacs.
>> +  (if guix-emacs-autoload-packages-called
>> +      (progn
>> +        (mapc #'load-file (guix-emacs--subdirs-files))
>> +        (setq load-path (delete-dups load-path)))
>> +    (setq guix-emacs-autoload-packages-called t))
>> +
> Rather than keeping track of whether the function was already called
> (which would make debugging more tedious if you also have e.g. broken
> packages from another source on your EMACSLOADPATH), I think the user
> ought to signal their intent to reload the subdirs files via the
> universal argument.
>
> e.g. 
> (defun guix-emacs-autoload-packages (&optional reload)
>   "..."
>   (interactive "P")
>   (when reload (mapc #'load-file (guix-emacs--subdirs-files)))
>   ...)
>
> WDYT?

The reason for avoiding loading the subdirs.el files on the first call
is just an optimization, since it would at this time duplicate work
already done by Emacs itself when it first executes.  This shouldn't
fail; I've now employed the same 'noerror strategy as used for autoloads
to ensure that.

There's one edge case I've just thought though, which is if a user
invoked emacs with the documented '--no-site-file' option disabling
loading autoloads; this would cause guix-emacs-autoload-packages-called
to be nil.

To balance between making things both convenient and flexible, I've
preserved the tracking but also added the reload override you suggested.
Let me know what you think.

Thank you for the review!

-- 
Thanks,
Maxim