bug#71817: 29.3; Sub-directory handling of ELPA package

bug#71817: 29.3; Sub-directory handling of ELPA package

[Xiyue Deng]

Hi,

I'm opening a bug to continue the discussion from the emacs-devel
mailing list thread[1].

TL;DR I would like to understand how upstream would like to handle
sub-directories with ELisp source files as to whether to add them to
`load-path' recursively or not to determine the path forward for source
code organization.

Currently as observed, ELPA packages only get their root path added to
`load-path', but source code in sub-directories will still get
byte-compiled.  That is, for an ELPA package elpafoo with a nested
sub-directory of the following structure (installed through package.el):

,----
| ~/.config/emacs/elpa/elpafoo/
| ~/.config/emacs/elpa/elpafoo/elpafoo.el
| ~/.config/emacs/elpa/elpafoo/elpabar
| ~/.config/emacs/elpa/elpafoo/elpabar/elpabar.el
`----

only `~/.config/emacs/elpa/elpafoo' is added to `load-path', while both
elpafoo.el and elpabar.el will get byte-compiled.  Currently I don't
seem to find where this behavior is documented, so do give me a pointer
if it exists.

If this is not yet a policy, I wonder whether this will be the path
forward for `load-path' handling.  I see some pros of adding
sub-directories recursively, such as better source code organization so
that not all source code files need to reside in the root directory.
However, the downside is also obvious, such as unnecessary loading of
test sources as Michael pointed out in [2], or loading vendored code
that could cause conflicts (as I observed in the case of auctex), etc.

So, what would be the upstream or package.el maintainers' opinions on
how this should be handled?

Thanks in advance.

[1] https://lists.gnu.org/archive/html/emacs-devel/2024-05/msg00658.html
[2] https://lists.gnu.org/archive/html/emacs-devel/2024-05/msg01039.html


In GNU Emacs 29.3 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.38,
 cairo version 1.16.0) of 2024-05-20, modified by Debian built on sbuild
System Description: Debian GNU/Linux 12 (bookworm)

Configured using:
 'configure --build x86_64-linux-gnu --prefix=/usr
 --sharedstatedir=/var/lib --libexecdir=/usr/libexec
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --with-libsystemd --with-pop=yes
 --enable-locallisppath=/etc/emacs:/usr/local/share/emacs/29.3/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/29.3/site-lisp:/usr/share/emacs/site-lisp
 --with-sound=alsa --without-gconf --with-mailutils
 --with-native-compilation --build x86_64-linux-gnu --prefix=/usr
 --sharedstatedir=/var/lib --libexecdir=/usr/libexec
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --with-libsystemd --with-pop=yes
 --enable-locallisppath=/etc/emacs:/usr/local/share/emacs/29.3/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/29.3/site-lisp:/usr/share/emacs/site-lisp
 --with-sound=alsa --without-gconf --with-mailutils
 --with-native-compilation --with-cairo --with-x=yes
 --with-x-toolkit=gtk3 --with-toolkit-scroll-bars 'CFLAGS=-g -O2
 -ffile-prefix-map=/build/reproducible-path/emacs-29.3+1=. -fstack-protector-strong
 -Wformat -Werror=format-security -Wall' 'CPPFLAGS=-Wdate-time
 -D_FORTIFY_SOURCE=2' LDFLAGS=-Wl,-z,relro'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES
NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3
THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER WEBP X11 XDBE XIM XINPUT2
XPM GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  shell-dirtrack-mode: t
  mu4e-modeline-mode: t
  windmove-mode: t
  rcirc-track-minor-mode: t
  server-mode: t
  subword-mode: t
  bug-reference-prog-mode: t
  whitespace-mode: t
  yas-minor-mode: t
  xclip-mode: t
  global-treesit-auto-mode: t
  treemacs-project-follow-mode: t
  treemacs-follow-mode: t
  treemacs-git-mode: t
  treemacs-fringe-indicator-mode: t
  corfu-terminal-mode: t
  corfu-popupinfo-mode: t
  corfu-echo-mode: t
  global-corfu-mode: t
  corfu-mode: t
  activities-tabs-mode: t
  activities-mode: t
  fido-vertical-mode: t
  icomplete-vertical-mode: t
  icomplete-mode: t
  fido-mode: t
  override-global-mode: t
  global-display-line-numbers-mode: t
  display-line-numbers-mode: t
  global-auto-revert-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  tab-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
/usr/share/emacs/site-lisp/elpa/debian-el-37.13/debian-autoloads hides /usr/share/emacs/site-lisp/elpa/gnuplot-0.8.1/debian-autoloads
/usr/share/emacs/site-lisp/elpa/magit-3.3.0/magit-section hides /usr/share/emacs/site-lisp/elpa/magit-section-3.3.0/magit-section

Features:
(shadow emacsbug mailalias vterm tramp tramp-loaddefs trampver
tramp-integration tramp-compat term ehelp vterm-module debian-copyright
goto-addr oc-basic org-element org-persist org-id avl-tree ol-eww eww
xdg mm-url ol-rmail ol-mhe ol-irc ol-info ol-gnus nnselect ol-docview
doc-view image-mode exif ol-bibtex bibtex ol-bbdb ol-w3m ol-doi
org-link-doi shortdoc cl-print mu4e-speedbar speedbar ezimage dframe
mu4e-contrib eshell esh-cmd generator esh-ext esh-opt esh-proc esh-io
esh-arg esh-module esh-groups esh-util files-x mu4e-icalendar
gnus-icalendar org-capture org-refile icalendar diary-lib diary-loaddefs
url-http url-gw url-auth url-queue url-cache shr-color magit-extras
eglot external-completion array jsonrpc ert ewoc debug backtrace xref
pcase debian-changelog-mode debian-bug face-remap magit-bookmark
magit-submodule magit-obsolete magit-blame magit-stash magit-reflog
magit-bisect magit-push magit-pull magit-fetch magit-clone magit-remote
magit-commit magit-sequence magit-notes magit-worktree magit-tag
magit-merge magit-branch magit-reset magit-files magit-refs magit-status
magit magit-repos magit-apply magit-wip magit-log which-func imenu
magit-diff smerge-mode diff git-commit log-edit add-log magit-core
magit-autorevert magit-margin magit-transient magit-process with-editor
shell magit-mode transient magit-git magit-section gnus-async gnus-bcklg
gnus-ml gnus-topic cursor-sensor utf-7 dired-aux misearch multi-isearch
qp magit-utils crm jka-compr sort gnus-cite matlab matlab-scan
matlab-syntax matlab-compat mm-archive mail-extr textsec uni-scripts
idna-mapping ucs-normalize uni-confusable textsec-check help-fns
radix-tree nnfolder gnus-demon nnml ezgnus gnus-delay gnus-draft
gnus-agent gnus-srvr gnus-score score-mode nnvirtual nntp gnus-cache
nndraft nnmh mu4e-debian-hx90 mu4e mu4e-org org ob ob-tangle ob-ref
ob-lob ob-table ob-exp org-macro org-src ob-comint org-pcomplete
pcomplete org-list org-footnote org-faces org-entities noutline outline
ob-emacs-lisp ob-core ob-eval org-cycle org-table ol org-fold
org-fold-core org-keys oc org-loaddefs find-func org-version org-compat
org-macs format-spec mu4e-notification notifications mu4e-main smtpmail
mu4e-view mu4e-mime-parts cal-menu calendar cal-loaddefs mu4e-headers
mu4e-thread mu4e-actions mu4e-compose mu4e-draft gnus-msg gnus-art mm-uu
mml2015 mm-view mml-smime smime dig gnus-sum gnus-group gnus-undo
gnus-start gnus-dbus dbus gnus-cloud nnimap nnmail mail-source utf7 nnoo
gnus-spec gnus-int gnus-range gnus-win gnus nnheader range mu4e-search
mu4e-lists mu4e-bookmarks mu4e-mark mu4e-message shr pixel-fill kinsoku
url-file svg xml dom flow-fill mule-util mu4e-contacts mu4e-update
mu4e-folders mu4e-context mu4e-query-items mu4e-server mu4e-modeline
mu4e-vars mu4e-helpers mu4e-config mu4e-window ido message sendmail
yank-media rfc822 mml mml-sec gnus-util mm-decode mm-bodies mm-encode
mail-parse rfc2231 rfc2047 rfc2045 mm-util ietf-drums mail-prsvr
mailabbrev mail-utils gmm-utils mailheader mu4e-obsolete windmove
flyspell ispell gnutls network-stream puny nsm epa-file epa derived epg
rfc6068 epg-config rcirc parse-time iso8601 time-date term/xterm xterm
comp comp-cstr server cap-words superword subword vc-hg vc-git diff-mode
vc-bzr vc-src vc-sccs vc-svn vc-cvs vc-rcs log-view pcvs-util vc
vc-dispatcher bug-reference disp-table whitespace yasnippet-snippets
yasnippet cus-edit cus-start wid-edit init zenburn-theme xclip
treesit-auto treesit treemacs-project-follow-mode treemacs-follow-mode
treemacs-rendering treemacs-annotations treemacs-async treemacs-visuals
treemacs-fringe-indicator pulse color treemacs-workspaces treemacs-dom
treemacs-icons treemacs-themes treemacs-scope treemacs-core-utils
treemacs-logging treemacs-customization pfuture inline ht s hl-line dash
keychain-environment exec-path-from-shell corfu-terminal popon
corfu-popupinfo corfu-echo corfu compat activities-tabs activities
persist bookmark pp edmacro kmacro advice icomplete cus-load
flymake-proc flymake project compile text-property-search comint
ansi-osc ansi-color ring warnings icons thingatpt cl-extra help-mode
use-package use-package-ensure use-package-delight use-package-diminish
use-package-bind-key bind-key easy-mmode use-package-core
display-line-numbers autorevert filenotify
keychain-environment-autoloads treesit-auto-autoloads xclip-autoloads rx
info debian-el dired dired-loaddefs finder-inf package browse-url url
url-proxy url-privacy url-expand url-methods url-history url-cookie
generate-lisp-file url-domsuf url-util mailcap url-handlers url-parse
auth-source cl-seq eieio eieio-core cl-macs password-cache json subr-x
map byte-opt gv bytecomp byte-compile url-vars cl-loaddefs cl-lib rmc
iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook
vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode register
page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind inotify lcms2 dynamic-setting system-font-setting
font-render-setting cairo move-toolbar gtk x-toolkit xinput2 x multi-tty
make-network-process native-compile emacs)

Memory information:
((conses 16 1552547 193601)
 (symbols 48 45285 63)
 (strings 32 201941 25320)
 (string-bytes 1 6355104)
 (vectors 16 124914)
 (vector-slots 8 2943117 153722)
 (floats 8 1149 2222)
 (intervals 56 55044 8833)
 (buffers 984 122))

-- 
Xiyue Deng

bug#71817: 29.3; Sub-directory handling of ELPA package

[Eli Zaretskii]

> From: Xiyue Deng <manphiz@gmail.com>
> Date: Fri, 28 Jun 2024 03:13:57 -0700
> 
> I'm opening a bug to continue the discussion from the emacs-devel
> mailing list thread[1].
> 
> TL;DR I would like to understand how upstream would like to handle
> sub-directories with ELisp source files as to whether to add them to
> `load-path' recursively or not to determine the path forward for source
> code organization.
> 
> Currently as observed, ELPA packages only get their root path added to
> `load-path', but source code in sub-directories will still get
> byte-compiled.  That is, for an ELPA package elpafoo with a nested
> sub-directory of the following structure (installed through package.el):
> 
> ,----
> | ~/.config/emacs/elpa/elpafoo/
> | ~/.config/emacs/elpa/elpafoo/elpafoo.el
> | ~/.config/emacs/elpa/elpafoo/elpabar
> | ~/.config/emacs/elpa/elpafoo/elpabar/elpabar.el
> `----
> 
> only `~/.config/emacs/elpa/elpafoo' is added to `load-path', while both
> elpafoo.el and elpabar.el will get byte-compiled.  Currently I don't
> seem to find where this behavior is documented, so do give me a pointer
> if it exists.
> 
> If this is not yet a policy, I wonder whether this will be the path
> forward for `load-path' handling.  I see some pros of adding
> sub-directories recursively, such as better source code organization so
> that not all source code files need to reside in the root directory.
> However, the downside is also obvious, such as unnecessary loading of
> test sources as Michael pointed out in [2], or loading vendored code
> that could cause conflicts (as I observed in the case of auctex), etc.
> 
> So, what would be the upstream or package.el maintainers' opinions on
> how this should be handled?
> 
> Thanks in advance.
> 
> [1] https://lists.gnu.org/archive/html/emacs-devel/2024-05/msg00658.html
> [2] https://lists.gnu.org/archive/html/emacs-devel/2024-05/msg01039.html

Adding Stefan.

bug#71817: 29.3; Sub-directory handling of ELPA package

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Currently as observed, ELPA packages only get their root path added to
> `load-path', but source code in sub-directories will still get
> byte-compiled.  That is, for an ELPA package elpafoo with a nested
> sub-directory of the following structure (installed through package.el):

The recursive compilation is somewhat of an "accident": it was the
easiest to implement (and seemed like a good idea anyway).

The `load-path` behavior is conscious: it's easy for a package to add
more subdirectories to the `load-path` but it would be much harder to
remove undesired ones.  [ And of course, the current behavior is also
the easiest one to implement.  ]

> If this is not yet a policy, I wonder whether this will be the path
> forward for `load-path' handling.

In `elpafoo.el`, include something like:

    ;;;###autoload
    (add-to-list 'load-path
                 (expand-file-name
                  "elpabar" (file-name-directory load-file-name)))

This assumes that you want `elpabar` to be in your `load-path` right
from the start (i.e. that an entry point to your package is in the
`elpabar` subdirectory).  If `elpabar` can only ever be used from code
that's in the `elpafoo` directory, there are other options (such as
`(require 'elpabar/elpabar)` or using an auxiliary `elpafoo-loaddefs.el`
which you load when `elpafoo.el` is loaded, ...

> I see some pros of adding sub-directories recursively,

I don't.  Most of the packages which use subdirectories have a complex
enough layout that some of those directories should not be in
`load-path`: it's better to let them add entries "manually" at the
appropriate time than to try and do it automatically.

The more real problem is that the way `elpafoo-autoloads.el` is created
does *not* scan ELisp files in subdirectories.  The way this is handled
typically in that the ELPA tarball comes with its own
`elpabar/elpabar-autoloads.el` file and `elpafoo.el` then needs to
contain something like

    ;;;###autoload
    (require 'elpabar/elpabar-autoloads)

The main downside here is that the current elpa.gnu.org scripts don't
know how to build such a `elpabar/elpabar-autoloads.el`, so you either
need to store it in the Git (which is ugly since it's a generated file),
or use an ad-hoc `:make` rule.

IMO we should change the ELPA protocol so that the
`elpafoo-autoloads.el` is not created during installation but is instead
included in the tarball, so it can be generated any way we like.


        Stefan

bug#71817: 29.3; Sub-directory handling of ELPA package

[Xiyue Deng]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Currently as observed, ELPA packages only get their root path added to
>> `load-path', but source code in sub-directories will still get
>> byte-compiled.  That is, for an ELPA package elpafoo with a nested
>> sub-directory of the following structure (installed through package.el):
>
> The recursive compilation is somewhat of an "accident": it was the
> easiest to implement (and seemed like a good idea anyway).
>
> The `load-path` behavior is conscious: it's easy for a package to add
> more subdirectories to the `load-path` but it would be much harder to
> remove undesired ones.  [ And of course, the current behavior is also
> the easiest one to implement.  ]
>
>> If this is not yet a policy, I wonder whether this will be the path
>> forward for `load-path' handling.
>
> In `elpafoo.el`, include something like:
>
>     ;;;###autoload
>     (add-to-list 'load-path
>                  (expand-file-name
>                   "elpabar" (file-name-directory load-file-name)))
>
> This assumes that you want `elpabar` to be in your `load-path` right
> from the start (i.e. that an entry point to your package is in the
> `elpabar` subdirectory).  If `elpabar` can only ever be used from code
> that's in the `elpafoo` directory, there are other options (such as
> `(require 'elpabar/elpabar)` or using an auxiliary `elpafoo-loaddefs.el`
> which you load when `elpafoo.el` is loaded, ...
>
>> I see some pros of adding sub-directories recursively,
>
> I don't.  Most of the packages which use subdirectories have a complex
> enough layout that some of those directories should not be in
> `load-path`: it's better to let them add entries "manually" at the
> appropriate time than to try and do it automatically.
>
> The more real problem is that the way `elpafoo-autoloads.el` is created
> does *not* scan ELisp files in subdirectories.  The way this is handled
> typically in that the ELPA tarball comes with its own
> `elpabar/elpabar-autoloads.el` file and `elpafoo.el` then needs to
> contain something like
>
>     ;;;###autoload
>     (require 'elpabar/elpabar-autoloads)
>
> The main downside here is that the current elpa.gnu.org scripts don't
> know how to build such a `elpabar/elpabar-autoloads.el`, so you either
> need to store it in the Git (which is ugly since it's a generated file),
> or use an ad-hoc `:make` rule.
>
> IMO we should change the ELPA protocol so that the
> `elpafoo-autoloads.el` is not created during installation but is instead
> included in the tarball, so it can be generated any way we like.
>
>
>         Stefan
>

Thanks for the explanations and for exploring options to load sources in
sub-directories without recursive loading by default.  I take that the
current status - byte-compile recursively, only add source root path to
`load-path' - will continue to be the path forward.

For now, it makes sense that loading sources from sub-directories
requires some manual `load-path' handling.  Maybe when using
sub-directories to organize source files becomes more common upstream
may consider adding some convenient functions/macros to make it easier,
but it will be for the future.

Thanks again!

-- 
Xiyue Deng

bug#71817: 29.3; Sub-directory handling of ELPA package

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Thanks for the explanations and for exploring options to load sources in
> sub-directories without recursive loading by default.  I take that the
> current status - byte-compile recursively, only add source root path to
> `load-path' - will continue to be the path forward.

Yes.

> For now, it makes sense that loading sources from sub-directories
> requires some manual `load-path' handling.  Maybe when using
> sub-directories to organize source files becomes more common upstream
> may consider adding some convenient functions/macros to make it easier,
> but it will be for the future.

The direction I'd like to move into is to do fewer things automatically
when installing Emacs and instead move the work to the moment when we
build the tarball.


        Stefan