bug#45316: [PATCH]: Re-introduce Emacs packages specific installation prefix.

[Maxim Cournoyer <maxim.cournoyer@gmail.com>]

Hi Leo,

Leo Prikler <leo.prikler@student.tugraz.at> writes:

> Hello Maxim,
>
> As someone, who lobbied for the current status quo, I have some
> thoughts to share.

I'm happy you're commenting :-).

> Am Montag, den 21.12.2020, 22:28 -0500 schrieb Maxim Cournoyer:
>> The Emacs packages built with the Emacs built system used to be
>> installed in a sub-directory under the share/emacs/guix.d/ directory,
>> but this was changed in commit
>> 65a7dd2950ca13a8b942b2836260a2192351b271
>> shortly after having accommodated the site-start.el machinery to
>> enable
>> loading packages from any profile (via the EMACSLOADPATH search path
>> specification).
> Won't this reintroduce <https://bugs.gnu.org/38309> then?

This bug was caused by the EMACSLOADPATH environment variable being too
long.  This was because the original search path specification was
recursively adding any directories under site-lisp/ so that package
installed under a guix.d sub-directory could were directly added to the
load path.

In the current situation, the EMACSLOADPATH contains only the site-lisp/
directory of any profile, plus the versioned lisp/ directory of the
installed Emacs package.  This patch series doesn't change that, so no,
that bug wouldn't be reintroduced.

>> While this change allowed to expose simply and directly the packages
>> found in EMACSLOADPATH, it does introduce the risk of file name
>> collisions when multiple Emacs packages are joined in the same
>> profile,
>> especially with Emacs packages increasing in complexity (e.g., using
>> more than a single .el file!) and expecting to have both their
>> sources
>> and resources extracted under their own nested directory rather than
>> as
>> a flat collection (ELPA, MELPA).
>
>> One recent example I stumbled on was attempting to use the
>> emacs-yasnippet-snippets package along with emacs-elpy; both wanted
>> to
>> install a 'snippets' directory to share/emacs/site-lisp/snippets,
>> collided and resulted in problems that prove difficult to understand.

> I believe that to be a problem in those packages.  Data should not be
> installed into share/emacs/site-lisp, but share/emacs/etc.  See for
> instance also emacs-telega, which – while not quite adhering to the
> above – installs its data in share/emacs/{telega-contrib,telega-data}.
>
> Regardless of what you intend to do with site-lisp otherwise, data
> files should *not*, I repeat *not* be installed there.  I do believe
> standardizing share/emacs/etc is the way to go, however.

While I agree that it would be more elegant the way you propose, that's
not the way Emacs packages have been standardized.  So going against the
single "content directory" (c.f. info "(elisp) Packaging Basics") would
break many Elisp library assumptions about where there files are,
causing more friction (thus work) to adapt them in Guix.  The content
directory of an Elisp package can contain any kind of files (code,
images, etc.), according to info "(elisp) Multi-file Packages".

>> This is what motivated this patch series, where the site-start.el
>> auxiliary code used for package discovery is extended to support
>> packages installed in their own directory under a 'share/emacs/guix'
>> installation prefix, via Emacs' own package library!
>>
>> The emacs-build-system is updated for this new installation prefix,
>> as
>> well as existing packages and documentation.  Parting with a directly
>> usable EMACSLOADPATH means that site-start.el *must* run for packages
>> to
>> appear in the load-path; that means for running a test suite, the -Q
>> or
>> --quick Emacs options cannot be used, since it implies --no-site-
>> file.
> Having no usable EMACSLOADPATH sounds like a dealbreaker to me.  Could
> one be restored by using direct subdirectories to share/emacs/site-lisp
> or would that merely be a cosmetic change?

We have two schemes to accommodate for our Emacs packages:

1. Those installed via their own mean, e.g. make install, using the
gnu-build-system for example.  These would still typically install their
packages directly under site-lisp, possibly multiple files (that could
still collide).

2. Those installed via the emacs-build-system.  With the proposed
changes, those now go to site-lisp/guix/.  The 'guix' sub-directory
makes it unambiguous that anything found under is to be loaded by
package.el; the `package-directory-list' variable can be pointed to it
to have the Emacs' package library discover these self-contained
packages.

>> + Avoid inter-package file name collisions.
> Note: This regards data, which should not be stored in site-lisp to
> begin with.  You don't get any benefits doing this for code, because
> Emacs has no strong module system.

True.

>> + Better integration with user installed packages via M-x
>> package-install.  The Guix-installed packages are listed in M-x
>> package-list as 'external'.
> That does sounds like a genuine pro to me.
>> - Slightly more complex loader (although much of it is offloaded to
>>   package.el), thus slightly slower (see the comparison below).
> That should be bearable.
>> - Requires to ensure every package's test suite doesn't make use of
>> -Q.
> That not so much.  Note, that test cases are not the only use for -Q!

Currently if you use -Q, the Elisp libraries are in the load-path, but
not loaded (you need manually do M-x load-library before you can use
it), or call M-x guix-emacs-autoload-packages to load their autoloads.
For programs, this mean (require 'some-library) works, but M-x
some-library-autoloaded-function doesn't.

After this change, if you use -Q the new style Emacs packages are not
visible at all until you activate them with 'M-x load-library
guix-emacs' then 'M-x guix-package-initialize', or (require 'guix-emacs)
then (guix-package-initialize), programmatically.

I don't see this as a problem.  -Q is simply an alias for
"--no-init-file" "--no-site-lisp" "--no-splash" "--no-x-resources"
"--no-site-file".

>> In my opinion the benefits outweighs the cons by a comfortable
>> margin,
>> especially with the boring work of adapting the package collection
>> already done.
>>
>> To test the performance of the new approach, the following manifest
>> file
>> was used to test the rebuild of the ~900 Emacs packages making use of
>> the Emacs build system:
>>
>> A simple benchmark testing the performance of the activation of the
>> hundreds of Emacs packages was then run using:
>>
>> --8<---------------cut here---------------start------------->8---
>> $ ./pre-inst-env guix environment --pure -m emacs-packages-
>> manifest.scm \
>>                                   --ad-hoc emacs
>> [env]$ /run/setuid-programs/sudo /bin/sh -c 'echo 3 >
>> /proc/sys/vm/drop_caches'
>> [env]$ emacs --batch --no-site-file \
>>     --eval="(progn (require 'guix-emacs) \
>>                    (require 'benchmark) \
>>                    (message \"(total gc-count gc-time) = %s\" \
>>                             (benchmark-run 1 (guix-emacs-autoload-
>> packages))))"
>> --8<---------------cut here---------------end--------------->8---
>>
>> On the master branch:
>>
>> --8<---------------cut here---------------start------------->8---
>> [...]
>> Loading /gnu/store/qajc70c7nqycs1301ram8s3x7k9ibg5f-
>> profile/share/emacs/site-lisp/zotxt-autoloads...
>> Loading /gnu/store/qajc70c7nqycs1301ram8s3x7k9ibg5f-
>> profile/share/emacs/site-lisp/zoutline-autoloads...
>> Loading /gnu/store/qajc70c7nqycs1301ram8s3x7k9ibg5f-
>> profile/share/emacs/site-lisp/ztree-autoloads...
>> (total gc-count gc-time) = (25.242400751 13 0.189669369)
>> --8<---------------cut here---------------end--------------->8---
>>
>> Or about 0.65 s on a warm cache.
>>
>> On a branch with these changes:
>>
>> --8<---------------cut here---------------start------------->8---
>> Error loading autoloads: (file-missing Cannot open load file No such
>> file or directory kotl/kotl-autoloads)
>> Error loading autoloads: (file-missing Cannot open load file No such
>> file or directory helm-easymenu)
>> Error loading autoloads: (file-missing Cannot open load file No such
>> file or directory /gnu/store/ryh0rasi9frm98dkd3kbck6hya6hn2qr-
>> profile/share/emacs/site-lisp/guix/flycheck-cpplint-0.1-
>> 1.1d8a090/flycheck-cpplint-autoloads)
>> Error loading autoloads: (file-missing Cannot open load file No such
>> file or directory /gnu/store/ryh0rasi9frm98dkd3kbck6hya6hn2qr-
>> profile/share/emacs/site-lisp/guix/evil-anzu-0.03/evil-anzu-
>> autoloads)
>> Error loading autoloads: (file-missing Cannot open load file No such
>> file or directory /gnu/store/ryh0rasi9frm98dkd3kbck6hya6hn2qr-
>> profile/share/emacs/site-lisp/guix/erc-image-0-3.82fb387/erc-image-
>> autoloads)
>> ad-handle-definition: `ido-completing-read' got redefined
>> Error loading autoloads: (file-missing Cannot open load file No such
>> file or directory tex-site)
>> (total gc-count gc-time) = (26.175704339 47 0.783184412)
>> --8<---------------cut here---------------end--------------->8---
>>
>> Or about 3 seconds on a warm cache.
> Looking at these, total does not seem to have changed much, but gc-
> count and gc-time more than tripled.  Any idea on how to mitigate this
> or do we have to live with that?

I don't know enough about Elisp to offer a guess.  We'd have to profile
it.  When put in perspective, that's 3 seconds to refresh about 900
packages.  I'm guessing most users must have less than 100 Emacs
packages in their collections (I have about 35). I think the current
performance is good enough to not worry optimizing it at this stage.  In
any case, performance optimizations would need to be made to Emacs'
package.el.

>> There a 6 errors that would need to be looked into, but I these look
>> like actual packaging problems rather than new issues.  The
>> previously
>> used way to load the autoloads, '(load f 'noerror)' would have masked
>> them.

> Regardless of how we handle site-lisp going forward, please do look
> into those issues and perhaps file them against the individual packages
> if they also cause (other) trouble using the current setup.

I'll try having a look, thanks for the reminder.

Thank you for commenting!

Maxim