Package: emacs;
Reported by: Aritro Sen <1.sen.aritro <at> gmail.com>
Date: Mon, 15 Dec 2025 20:49:01 UTC
Severity: normal
Tags: patch
To reply to this bug, email your comments to 80016 AT debbugs.gnu.org.
Toggle the display of automated, internal messages from the tracker.
View this report as an mbox folder, status mbox, maintainer mbox
bug-gnu-emacs <at> gnu.org:bug#80016; Package emacs.
(Mon, 15 Dec 2025 20:49:02 GMT) Full text and rfc822 format available.Aritro Sen <1.sen.aritro <at> gmail.com>:bug-gnu-emacs <at> gnu.org.
(Mon, 15 Dec 2025 20:49:02 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Aritro Sen <1.sen.aritro <at> gmail.com> To: bug-gnu-emacs <at> gnu.org Cc: Eli Zaretskii <eliz <at> gnu.org>, Philip Kaludercic <philipk <at> posteo.net> Subject: [PATCH] Load lisp sub-directories in addition to the lisp-dir Date: Tue, 16 Dec 2025 02:18:00 +0530
[Message part 1 (text/plain, inline)]
Tags: patch This patch implements the feature request described in bug #79874. It introduces the keyword lisp-subdirs to use-package-vc. This allows subdirectories of the PKG-DIR to be added to the Emacs load-path when installing packages directly via VC (e.g., Git), in addition to the PKG-DIR or lisp-dir itself. The subdirectory paths must be specified relative to lisp-dir (which defaults to PKG-DIR). This patch depends on bug#80015 being solved. This patch is written based on the discussions in bug#79873.
[0001-use-package-lisp-subdirs.patch (text/x-patch, attachment)]
bug-gnu-emacs <at> gnu.org:bug#80016; Package emacs.
(Thu, 18 Dec 2025 09:22:02 GMT) Full text and rfc822 format available.Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Philip Kaludercic <philipk <at> posteo.net> To: Aritro Sen <1.sen.aritro <at> gmail.com> Cc: bug-gnu-emacs <at> gnu.org, Eli Zaretskii <eliz <at> gnu.org> Subject: Re: [PATCH] Load lisp sub-directories in addition to the lisp-dir Date: Thu, 18 Dec 2025 09:21:09 +0000
Aritro Sen <1.sen.aritro <at> gmail.com> writes:
> Tags: patch
>
> This patch implements the feature request described in bug #79874. It
> introduces the keyword lisp-subdirs to use-package-vc. This allows
> subdirectories of the PKG-DIR to be added to the Emacs load-path when
> installing packages directly via VC (e.g., Git), in addition to the
> PKG-DIR or lisp-dir itself. The subdirectory paths must be specified
> relative to lisp-dir (which defaults to PKG-DIR).
>
> This patch depends on bug#80015 being solved. This patch is written
> based on the discussions in bug#79873.
>
> From 70748db9a20b0c9649ba54f457e4a4ce547d62f1 Mon Sep 17 00:00:00 2001
> From: Aritro Sen <1.sen.aritro <at> gmail.com>
> Date: Tue, 16 Dec 2025 01:10:12 +0530
> Subject: [PATCH] Load lisp-subdirs in addition to pkg-dir
>
> ---
> doc/emacs/package.texi | 8 ++++++
> lisp/emacs-lisp/package-vc.el | 32 +++++++++++++----------
> lisp/emacs-lisp/package.el | 39 ++++++++++++++++++----------
> lisp/use-package/use-package-core.el | 2 +-
> 4 files changed, 53 insertions(+), 28 deletions(-)
>
> diff --git a/doc/emacs/package.texi b/doc/emacs/package.texi
> index fd8a79aa922..34a867e9505 100644
> --- a/doc/emacs/package.texi
> +++ b/doc/emacs/package.texi
> @@ -715,6 +715,14 @@ Fetching Package Sources
> use for loading the Lisp sources, which defaults to the root directory
> of the repository.
>
> +@item :lisp-subdirs
> +A string or a list of strings providing the names of subdirectories
> +containing additional Lisp sources. These names are interpreted
> +relative to the directory specified by @code{:lisp-dir} (or the root
> +directory of the repository if @code{:lisp-dir} is unspecified). The
> +specified subdirectories are added to the load path and scanned for
> +library dependencies and autoloads.
I have responded to this issue in bug#79874 (specifically
<87qzsvmvax.fsf <at> posteo.net>), I am not sure if you saw the message,
because IIRC I sent it to you before you sent this message?
> @item :main-file
> A string providing the main file of the project, from which to gather
> package metadata. If not given, the default is the package name with
> diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
> index 7cb4bfde51a..f3d96904da6 100644
> --- a/lisp/emacs-lisp/package-vc.el
> +++ b/lisp/emacs-lisp/package-vc.el
> @@ -530,6 +530,11 @@ package-vc--unpack-1
> ;; Directory where package's Lisp code resides. It may be
> ;; equal to `checkout-dir' or be a subdirectory of it.
> (lisp-dir (package-vc--checkout-dir pkg-desc 'lisp-dir))
> + (lisp-subdirs (ensure-list (plist-get pkg-spec :lisp-subdirs)))
> + (all-dirs (cons lisp-dir
> + (mapcar
> + (lambda (dir) (expand-file-name dir lisp-dir))
> + lisp-subdirs)))
> missing)
>
> ;; In case the package was installed directly from source, the
> @@ -547,17 +552,19 @@ package-vc--unpack-1
> "\\|")
> regexp-unmatchable))
> (deps '()))
> - (dolist (file (directory-files lisp-dir t "\\.el\\'" t))
> - (unless (string-match-p ignored-files file)
> - (with-temp-buffer
> - (insert-file-contents file)
> - (when-let* ((require-lines (lm-header-multiline "package-requires")))
> - (thread-last
> - (mapconcat #'identity require-lines " ")
> - package-read-from-string
> - lm--prepare-package-dependencies
> - (nconc deps)
> - (setq deps))))))
> + (dolist (dir all-dirs)
> + (dolist (file (directory-files dir t "\\.el\\'" t))
We can reduce the number of lines changed here by replacing the
(directory-files ...) call with something like
--8<---------------cut here---------------start------------->8---
(mapcan
(lambda (dir)
(directory-files dir t "\\.el\\'" t))
all-dirs)
--8<---------------cut here---------------end--------------->8---
> + (unless (string-match-p ignored-files file)
> + (with-temp-buffer
> + (insert-file-contents file)
> + (when-let* ((require-lines (lm-header-multiline "package-requires")))
> + (thread-last
> + (mapconcat #'identity require-lines " ")
> + package-read-from-string
> + lm--prepare-package-dependencies
> + (nconc deps)
> + (setq deps)))))))
> + (setq deps (assq-delete-all (package-desc-name pkg-desc) deps))
> (dolist (dep deps)
> (cl-callf version-to-list (cadr dep)))
> (setf (package-desc-reqs pkg-desc) deps)
> @@ -568,8 +575,7 @@ package-vc--unpack-1
>
> ;; Generate autoloads
> (let* ((name (package-desc-name pkg-desc))
> - (auto-name (format "%s-autoloads.el" name)))
> - (package-generate-autoloads name lisp-dir)
> + (auto-name (package-generate-autoloads name lisp-dir lisp-subdirs)))
> ;; There are two cases when we wish to "indirect" the loading of
> ;; autoload files:
> ;;
> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index 993216c6881..6f4f493eae7 100644
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -1127,29 +1127,40 @@ package-autoload-ensure-default-file
> (defvar autoload-timestamps)
> (defvar version-control)
>
> -(defun package-generate-autoloads (name pkg-dir)
> - "Generate autoloads in PKG-DIR for package named NAME."
> +(defun package-generate-autoloads (name pkg-dir &optional pkg-subdirs)
> + "Generate autoloads in PKG-DIR for package named NAME.
> +If LISP-SUBDIRS is provided, it should be a list of subdirectory names
> +relative to PKG-DIR."
> (let* ((auto-name (format "%s-autoloads.el" name))
> ;;(ignore-name (concat name "-pkg.el"))
> (output-file (expand-file-name auto-name pkg-dir))
> + (all-dirs (cons pkg-dir
> + (mapcar
> + (lambda (dir) (expand-file-name dir pkg-dir))
> + pkg-subdirs)))
> ;; We don't need 'em, and this makes the output reproducible.
> (autoload-timestamps nil)
> (backup-inhibited t)
> (version-control 'never))
> (loaddefs-generate
> - pkg-dir output-file nil
> + all-dirs output-file nil
> (prin1-to-string
> - '(add-to-list
> - 'load-path
> - ;; Add the directory that will contain the autoload file to
> - ;; the load path. We don't hard-code `pkg-dir', to avoid
> - ;; issues if the package directory is moved around.
> - ;; `loaddefs-generate' has code to do this for us, but it's
> - ;; not currently exposed. (Bug#63625)
> - (or (and load-file-name
> - (directory-file-name
> - (file-name-directory load-file-name)))
> - (car load-path)))))
> + `(let ((dir (or (and load-file-name
> + (directory-file-name
> + (file-name-directory load-file-name)))
> + (car load-path))))
> + ;; Add the directory that will contain the autoload file to
> + ;; the load path. We don't hard-code `pkg-dir', to avoid
> + ;; issues if the package directory is moved around.
> + ;; `loaddefs-generate' has code to do this for us, but it's
> + ;; not currently exposed. (Bug#63625)
> + (add-to-list 'load-path dir)
> + ,@(mapcar
> + (lambda (subdir)
> + `(add-to-list
> + 'load-path
> + (expand-file-name ,subdir dir)))
> + pkg-subdirs))))
This is related to a point I may or may not have raised at some point:
Why couldn't we add a function (or probably a macro like
`package-get-version') to package.el that a package developer can
invoke/autoload that would take care of just this? This would then
automatically cover both tarball and VC packages, without having to
adjust package specifications?
For backwards compatibility, we can then add the definition to Compat as
it shouldn't depend on any new functionality in the core.
> (let ((buf (find-buffer-visiting output-file)))
> (when buf (kill-buffer buf)))
> auto-name))
> diff --git a/lisp/use-package/use-package-core.el b/lisp/use-package/use-package-core.el
> index 0f049a6f86b..a6c92d663cc 100644
> --- a/lisp/use-package/use-package-core.el
> +++ b/lisp/use-package/use-package-core.el
> @@ -1735,7 +1735,7 @@ use-package-handler/:vc
> body))
>
> (defconst use-package-vc-valid-keywords
> - '( :url :branch :lisp-dir :main-file :vc-backend :rev
> + '( :url :branch :lisp-dir :lisp-subdirs :main-file :vc-backend :rev
> :shell-command :make :ignored-files)
> "Valid keywords for the `:vc' keyword.
> See Info node `(emacs)Fetching Package Sources'.")
bug-gnu-emacs <at> gnu.org:bug#80016; Package emacs.
(Mon, 22 Dec 2025 01:24:02 GMT) Full text and rfc822 format available.Message #11 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Aritro Sen <1.sen.aritro <at> gmail.com> To: Philip Kaludercic <philipk <at> posteo.net> Cc: bug-gnu-emacs <at> gnu.org, Eli Zaretskii <eliz <at> gnu.org> Subject: Re: [PATCH] Load lisp sub-directories in addition to the lisp-dir Date: Mon, 22 Dec 2025 06:53:02 +0530
[Message part 1 (text/plain, inline)]
On 12/18/25 2:51 PM, Philip Kaludercic wrote: > Aritro Sen <1.sen.aritro <at> gmail.com> writes: > >> >> This patch implements the feature request described in bug #79874. It >> introduces the keyword lisp-subdirs to use-package-vc. This allows >> subdirectories of the PKG-DIR to be added to the Emacs load-path when >> installing packages directly via VC (e.g., Git), in addition to the >> PKG-DIR or lisp-dir itself. The subdirectory paths must be specified >> relative to lisp-dir (which defaults to PKG-DIR). >> >> This patch depends on bug#80015 being solved. This patch is written >> based on the discussions in bug#79873. > I have responded to this issue in bug#79874 (specifically > <87qzsvmvax.fsf <at> posteo.net>), I am not sure if you saw the message, > because IIRC I sent it to you before you sent this message? Yeah, I saw the message after I sent the patch. I was already composing my mail and dealing with some text formatting and attachment issues that happen when sending via the mail client after composing in Emacs, so I guess I missed the notification. I haven't been using Emacs for too long, so I'm still discovering all the quirks. Anyway, I digress. > We should turn this into a discussion about package-vc instead of > use-package. The configuration macro doesn't add any real > functionality, after all. > > Secondly, a goal of package-vc is to maintain compatibility with > specifications as used by the ELPA build server. Before implementing > something like this, I would like to be sure how elpa-admin.el would > handle the additional properties and how they would be translated into > tarballs. When I wrote this patch, I was focusing on a narrowly defined VC specific problem: some packages work perfectly fine when installed as a tarball via MELPA/ELPA, but break when installed directly via VC. This is because (and please correct me if I'm wrong) with the :files property of MELPA or :renames property of ELPA recipes, the packages were getting "flattened," for lack of a better term. This isn't the case when installing via VC, so when loaddefs are generated and the package is loaded, there are some "missing" files, which causes major issues. So I planned for the sub-directories to be user-specified. Now, this isn't exactly the same as :files in MELPA recipes, which takes a glob expression IIRC, but I think specifying sub-directories combined with using the :ignored-files property does the same job anyways. > This is related to a point I may or may not have raised at some point: > Why couldn't we add a function (or probably a macro like > `package-get-version') to package.el that a package developer can > invoke/autoload that would take care of just this? This would then > automatically cover both tarball and VC packages, without having to > adjust package specifications? > > For backwards compatibility, we can then add the definition to Compat as > it shouldn't depend on any new functionality in the core. But now, if I'm understanding you correctly, you're proposing that instead of this being user-specified, package authors would specify this directly in their code with a macro—something like (package-load-subdirs "extensions" "clients")—which would eliminate the need for flattening entirely, working identically for both VC and tarball installations. This takes a far broader view, but yes, I do agree that this shouldn't depend on any new functionality in the core, so implementing it shouldn't be much of a hassle, and yes, it would be a good idea for the list of sub-directories to be maintained by package authors, rather than being managed by users. Let me know if I understood the whole thing correctly, or whether I've misunderstood something. >> diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el >> index 7cb4bfde51a..f3d96904da6 100644 >> --- a/lisp/emacs-lisp/package-vc.el >> +++ b/lisp/emacs-lisp/package-vc.el >> - (dolist (file (directory-files lisp-dir t "\\.el\\'" t)) >> - (unless (string-match-p ignored-files file) >> - (with-temp-buffer >> - (insert-file-contents file) >> - (when-let* ((require-lines (lm-header-multiline "package-requires"))) >> - (thread-last >> - (mapconcat #'identity require-lines " ") >> - package-read-from-string >> - lm--prepare-package-dependencies >> - (nconc deps) >> - (setq deps)))))) >> + (dolist (dir all-dirs) >> + (dolist (file (directory-files dir t "\\.el\\'" t)) > > We can reduce the number of lines changed here by replacing the > (directory-files ...) call with something like > > --8<---------------cut here---------------start------------->8--- > (mapcan > (lambda (dir) > (directory-files dir t "\\.el\\'" t)) > all-dirs) > --8<---------------cut here---------------end--------------->8--- Thanks, I have made the suggested changes to the patch.
[0001-use-package-lisp-subdirs.patch (text/x-patch, attachment)]
bug-gnu-emacs <at> gnu.org:bug#80016; Package emacs.
(Tue, 23 Dec 2025 00:07:02 GMT) Full text and rfc822 format available.Message #14 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Philip Kaludercic <philipk <at> posteo.net> To: Aritro Sen <1.sen.aritro <at> gmail.com> Cc: bug-gnu-emacs <at> gnu.org, Eli Zaretskii <eliz <at> gnu.org> Subject: Re: [PATCH] Load lisp sub-directories in addition to the lisp-dir Date: Tue, 23 Dec 2025 00:05:39 +0000
[Message part 1 (text/plain, inline)]
Aritro Sen <1.sen.aritro <at> gmail.com> writes: > On 12/18/25 2:51 PM, Philip Kaludercic wrote: >> Aritro Sen <1.sen.aritro <at> gmail.com> writes: >> >>> >>> This patch implements the feature request described in bug #79874. It >>> introduces the keyword lisp-subdirs to use-package-vc. This allows >>> subdirectories of the PKG-DIR to be added to the Emacs load-path when >>> installing packages directly via VC (e.g., Git), in addition to the >>> PKG-DIR or lisp-dir itself. The subdirectory paths must be specified >>> relative to lisp-dir (which defaults to PKG-DIR). >>> >>> This patch depends on bug#80015 being solved. This patch is written >>> based on the discussions in bug#79873. >> I have responded to this issue in bug#79874 (specifically >> <87qzsvmvax.fsf <at> posteo.net>), I am not sure if you saw the message, >> because IIRC I sent it to you before you sent this message? > > Yeah, I saw the message after I sent the patch. I was already composing > my mail and dealing with some text formatting and attachment issues that > happen when sending via the mail client after composing in Emacs, so I > guess I missed the notification. I haven't been using Emacs for too > long, so I'm still discovering all the quirks. Anyway, I digress. No problem! >> We should turn this into a discussion about package-vc instead of >> use-package. The configuration macro doesn't add any real >> functionality, after all. >> >> Secondly, a goal of package-vc is to maintain compatibility with >> specifications as used by the ELPA build server. Before implementing >> something like this, I would like to be sure how elpa-admin.el would >> handle the additional properties and how they would be translated into >> tarballs. > > When I wrote this patch, I was focusing on a narrowly defined > VC specific problem: some packages work perfectly fine when installed as > a tarball via MELPA/ELPA, but break when installed directly via VC. This > is because (and please correct me if I'm wrong) with the :files property > of MELPA or :renames property of ELPA recipes, the packages were getting > "flattened," for lack of a better term. This isn't the case when > installing via VC, so when loaddefs are generated and the package is > loaded, there are some "missing" files, which causes major issues. So I > planned for the sub-directories to be user-specified. > > Now, this isn't exactly the same as :files in MELPA recipes, which takes > a glob expression IIRC, but I think specifying sub-directories combined > with using the :ignored-files property does the same job anyways. This seems like a fair summary of the situation. And I think that "flattening" is a fair description of what we are doing, as tarball packages only add a single directory to `load-path', and for ELPA we use :renames (IMO as a hack) to that end. >> This is related to a point I may or may not have raised at some point: >> Why couldn't we add a function (or probably a macro like >> `package-get-version') to package.el that a package developer can >> invoke/autoload that would take care of just this? This would then >> automatically cover both tarball and VC packages, without having to >> adjust package specifications? >> >> For backwards compatibility, we can then add the definition to Compat as >> it shouldn't depend on any new functionality in the core. > > But now, if I'm understanding you correctly, you're proposing that > instead of this being user-specified, package authors would specify this > directly in their code with a macro—something like (package-load-subdirs > "extensions" "clients")—which would eliminate the need for flattening > entirely, working identically for both VC and tarball installations. > This takes a far broader view, but yes, I do agree that this shouldn't > depend on any new functionality in the core, so implementing it > shouldn't be much of a hassle, and yes, it would be a good idea for the > list of sub-directories to be maintained by package authors, rather than > being managed by users. > > Let me know if I understood the whole thing correctly, or whether I've > misunderstood something. Yes, that is about it. Here is a sketch of how this could look like:
[Message part 2 (text/x-patch, inline)]
diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 70e88d081fa..504187bf9f1 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -1127,6 +1127,9 @@ package-autoload-ensure-default-file
(defvar autoload-timestamps)
(defvar version-control)
+(defconst package-add-subdir-regexp
+ (rx bol ";;;###autoload" (+ space) "(package-add-subdir" (+ space) (group ?\" (+ nonl) ?\") (* space) ")" (* space) eol))
+
(defun package-generate-autoloads (name pkg-dir)
"Generate autoloads in PKG-DIR for package named NAME."
(let* ((auto-name (format "%s-autoloads.el" name))
@@ -1137,7 +1140,27 @@ package-generate-autoloads
(backup-inhibited t)
(version-control 'never))
(loaddefs-generate
- pkg-dir output-file nil
+ ;; Search all Lisp files in `pkg-dir' for `package-add-subdir'
+ ;; calls that we should also scrape for autoload cookies.
+ (let ((dirs (list pkg-dir)))
+ (dolist (file (directory-files pkg-dir t ".el\\'" t))
+ (when (and (file-regular-p file) (file-readable-p file)
+ (not (file-equal-p file output-file)))
+ (with-temp-buffer
+ (insert-file-contents file)
+ (goto-char (point-min))
+ (condition-case nil
+ (while t
+ (let ((exp (read (current-buffer))))
+ (when (eq (car-safe exp) 'package-add-subdir)
+ (push (expand-file-name (cadr exp) pkg-dir)
+ dirs))))
+ (end-of-file))
+ (goto-char (point-min))
+ (while (search-forward-regexp package-add-subdir-regexp nil t)
+ (push (read (match-string 1)) dirs)))))
+ dirs)
+ output-file nil
(prin1-to-string
'(add-to-list
'load-path
@@ -4769,6 +4792,23 @@ package-get-version
(require 'lisp-mnt)
(lm-package-version mainfile)))))))
+;;;###autoload
+(progn
+ (defmacro package-add-subdir (dir)
+ "Manually instruct the package manager to load DIR.
+DIR is a string constant denoting a directory relative to the package
+root. This macro may only occur in files within the package root,
+ideally the main file. It is recommended to autoload calls to this
+function."
+ `(add-to-list
+ 'load-path
+ (expand-file-name
+ ,dir
+ (or (and load-file-name
+ (directory-file-name
+ (file-name-directory load-file-name)))
+ (car load-path))))))
+
;;;; Quickstart: precompute activation actions for faster start up.
[Message part 3 (text/plain, inline)]
I have mainly tested it with vertico and it seems to work well (but of course currently lacks the documentation that we have to update as well). If you think that this is an acceptable solution, I'd appreciate it if you could try testing it a bit as well!
bug-gnu-emacs <at> gnu.org:bug#80016; Package emacs.
(Fri, 26 Dec 2025 13:45:02 GMT) Full text and rfc822 format available.Message #17 received at 80016 <at> debbugs.gnu.org (full text, mbox):
From: Philip Kaludercic <philipk <at> posteo.net> To: Aritro Sen <1.sen.aritro <at> gmail.com>, 80016 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org> Subject: Re: [PATCH] Load lisp sub-directories in addition to the lisp-dir Date: Fri, 26 Dec 2025 13:43:54 +0000
It seems that Aritro accidentally fell out of the CC list, I have added him again and hope to hear opinions on the suggestion I made. Philip Kaludercic <philipk <at> posteo.net> writes: > Aritro Sen <1.sen.aritro <at> gmail.com> writes: > >> On 12/18/25 2:51 PM, Philip Kaludercic wrote: >>> Aritro Sen <1.sen.aritro <at> gmail.com> writes: >>> >>>> >>>> This patch implements the feature request described in bug #79874. It >>>> introduces the keyword lisp-subdirs to use-package-vc. This allows >>>> subdirectories of the PKG-DIR to be added to the Emacs load-path when >>>> installing packages directly via VC (e.g., Git), in addition to the >>>> PKG-DIR or lisp-dir itself. The subdirectory paths must be specified >>>> relative to lisp-dir (which defaults to PKG-DIR). >>>> >>>> This patch depends on bug#80015 being solved. This patch is written >>>> based on the discussions in bug#79873. >>> I have responded to this issue in bug#79874 (specifically >>> <87qzsvmvax.fsf <at> posteo.net>), I am not sure if you saw the message, >>> because IIRC I sent it to you before you sent this message? >> >> Yeah, I saw the message after I sent the patch. I was already composing >> my mail and dealing with some text formatting and attachment issues that >> happen when sending via the mail client after composing in Emacs, so I >> guess I missed the notification. I haven't been using Emacs for too >> long, so I'm still discovering all the quirks. Anyway, I digress. > > No problem! > >>> We should turn this into a discussion about package-vc instead of >>> use-package. The configuration macro doesn't add any real >>> functionality, after all. >>> >>> Secondly, a goal of package-vc is to maintain compatibility with >>> specifications as used by the ELPA build server. Before implementing >>> something like this, I would like to be sure how elpa-admin.el would >>> handle the additional properties and how they would be translated into >>> tarballs. >> >> When I wrote this patch, I was focusing on a narrowly defined >> VC specific problem: some packages work perfectly fine when installed as >> a tarball via MELPA/ELPA, but break when installed directly via VC. This >> is because (and please correct me if I'm wrong) with the :files property >> of MELPA or :renames property of ELPA recipes, the packages were getting >> "flattened," for lack of a better term. This isn't the case when >> installing via VC, so when loaddefs are generated and the package is >> loaded, there are some "missing" files, which causes major issues. So I >> planned for the sub-directories to be user-specified. >> >> Now, this isn't exactly the same as :files in MELPA recipes, which takes >> a glob expression IIRC, but I think specifying sub-directories combined >> with using the :ignored-files property does the same job anyways. > > This seems like a fair summary of the situation. And I think that > "flattening" is a fair description of what we are doing, as tarball > packages only add a single directory to `load-path', and for ELPA we use > :renames (IMO as a hack) to that end. > >>> This is related to a point I may or may not have raised at some point: >>> Why couldn't we add a function (or probably a macro like >>> `package-get-version') to package.el that a package developer can >>> invoke/autoload that would take care of just this? This would then >>> automatically cover both tarball and VC packages, without having to >>> adjust package specifications? >>> >>> For backwards compatibility, we can then add the definition to Compat as >>> it shouldn't depend on any new functionality in the core. >> >> But now, if I'm understanding you correctly, you're proposing that >> instead of this being user-specified, package authors would specify this >> directly in their code with a macro—something like (package-load-subdirs >> "extensions" "clients")—which would eliminate the need for flattening >> entirely, working identically for both VC and tarball installations. >> This takes a far broader view, but yes, I do agree that this shouldn't >> depend on any new functionality in the core, so implementing it >> shouldn't be much of a hassle, and yes, it would be a good idea for the >> list of sub-directories to be maintained by package authors, rather than >> being managed by users. >> >> Let me know if I understood the whole thing correctly, or whether I've >> misunderstood something. > > Yes, that is about it. Here is a sketch of how this could look like: > > diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el > index 70e88d081fa..504187bf9f1 100644 > --- a/lisp/emacs-lisp/package.el > +++ b/lisp/emacs-lisp/package.el > @@ -1127,6 +1127,9 @@ package-autoload-ensure-default-file > (defvar autoload-timestamps) > (defvar version-control) > > +(defconst package-add-subdir-regexp > + (rx bol ";;;###autoload" (+ space) "(package-add-subdir" (+ space) (group ?\" (+ nonl) ?\") (* space) ")" (* space) eol)) > + > (defun package-generate-autoloads (name pkg-dir) > "Generate autoloads in PKG-DIR for package named NAME." > (let* ((auto-name (format "%s-autoloads.el" name)) > @@ -1137,7 +1140,27 @@ package-generate-autoloads > (backup-inhibited t) > (version-control 'never)) > (loaddefs-generate > - pkg-dir output-file nil > + ;; Search all Lisp files in `pkg-dir' for `package-add-subdir' > + ;; calls that we should also scrape for autoload cookies. > + (let ((dirs (list pkg-dir))) > + (dolist (file (directory-files pkg-dir t ".el\\'" t)) > + (when (and (file-regular-p file) (file-readable-p file) > + (not (file-equal-p file output-file))) > + (with-temp-buffer > + (insert-file-contents file) > + (goto-char (point-min)) > + (condition-case nil > + (while t > + (let ((exp (read (current-buffer)))) > + (when (eq (car-safe exp) 'package-add-subdir) > + (push (expand-file-name (cadr exp) pkg-dir) > + dirs)))) > + (end-of-file)) > + (goto-char (point-min)) > + (while (search-forward-regexp package-add-subdir-regexp nil t) > + (push (read (match-string 1)) dirs))))) > + dirs) > + output-file nil > (prin1-to-string > '(add-to-list > 'load-path > @@ -4769,6 +4792,23 @@ package-get-version > (require 'lisp-mnt) > (lm-package-version mainfile))))))) > > +;;;###autoload > +(progn > + (defmacro package-add-subdir (dir) > + "Manually instruct the package manager to load DIR. > +DIR is a string constant denoting a directory relative to the package > +root. This macro may only occur in files within the package root, > +ideally the main file. It is recommended to autoload calls to this > +function." > + `(add-to-list > + 'load-path > + (expand-file-name > + ,dir > + (or (and load-file-name > + (directory-file-name > + (file-name-directory load-file-name))) > + (car load-path)))))) > + > > ;;;; Quickstart: precompute activation actions for faster start up. > > > > I have mainly tested it with vertico and it seems to work well (but of > course currently lacks the documentation that we have to update as > well). If you think that this is an acceptable solution, I'd appreciate > it if you could try testing it a bit as well!
bug-gnu-emacs <at> gnu.org:bug#80016; Package emacs.
(Fri, 26 Dec 2025 15:35:02 GMT) Full text and rfc822 format available.Message #20 received at 80016 <at> debbugs.gnu.org (full text, mbox):
From: Aritro Sen <1.sen.aritro <at> gmail.com> To: Philip Kaludercic <philipk <at> posteo.net>, 80016 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org> Subject: Re: [PATCH] Load lisp sub-directories in addition to the lisp-dir Date: Fri, 26 Dec 2025 21:04:13 +0530
On 12/26/25 7:13 PM, Philip Kaludercic wrote:
>
> It seems that Aritro accidentally fell out of the CC list, I have added
> him again and hope to hear opinions on the suggestion I made.
>
> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> Aritro Sen <1.sen.aritro <at> gmail.com> writes:
>>
>>> On 12/18/25 2:51 PM, Philip Kaludercic wrote:
>>>> Aritro Sen <1.sen.aritro <at> gmail.com> writes:
>>>>
>>>>>
>>>>> This patch implements the feature request described in bug #79874. It
>>>>> introduces the keyword lisp-subdirs to use-package-vc. This allows
>>>>> subdirectories of the PKG-DIR to be added to the Emacs load-path when
>>>>> installing packages directly via VC (e.g., Git), in addition to the
>>>>> PKG-DIR or lisp-dir itself. The subdirectory paths must be specified
>>>>> relative to lisp-dir (which defaults to PKG-DIR).
>>>>>
>>>>> This patch depends on bug#80015 being solved. This patch is written
>>>>> based on the discussions in bug#79873.
>>>> I have responded to this issue in bug#79874 (specifically
>>>> <87qzsvmvax.fsf <at> posteo.net>), I am not sure if you saw the message,
>>>> because IIRC I sent it to you before you sent this message?
>>>
>>> Yeah, I saw the message after I sent the patch. I was already composing
>>> my mail and dealing with some text formatting and attachment issues that
>>> happen when sending via the mail client after composing in Emacs, so I
>>> guess I missed the notification. I haven't been using Emacs for too
>>> long, so I'm still discovering all the quirks. Anyway, I digress.
>>
>> No problem!
>>
>>>> We should turn this into a discussion about package-vc instead of
>>>> use-package. The configuration macro doesn't add any real
>>>> functionality, after all.
>>>>
>>>> Secondly, a goal of package-vc is to maintain compatibility with
>>>> specifications as used by the ELPA build server. Before implementing
>>>> something like this, I would like to be sure how elpa-admin.el would
>>>> handle the additional properties and how they would be translated into
>>>> tarballs.
>>>
>>> When I wrote this patch, I was focusing on a narrowly defined
>>> VC specific problem: some packages work perfectly fine when installed as
>>> a tarball via MELPA/ELPA, but break when installed directly via VC. This
>>> is because (and please correct me if I'm wrong) with the :files property
>>> of MELPA or :renames property of ELPA recipes, the packages were getting
>>> "flattened," for lack of a better term. This isn't the case when
>>> installing via VC, so when loaddefs are generated and the package is
>>> loaded, there are some "missing" files, which causes major issues. So I
>>> planned for the sub-directories to be user-specified.
>>>
>>> Now, this isn't exactly the same as :files in MELPA recipes, which takes
>>> a glob expression IIRC, but I think specifying sub-directories combined
>>> with using the :ignored-files property does the same job anyways.
>>
>> This seems like a fair summary of the situation. And I think that
>> "flattening" is a fair description of what we are doing, as tarball
>> packages only add a single directory to `load-path', and for ELPA we use
>> :renames (IMO as a hack) to that end.
>>
>>>> This is related to a point I may or may not have raised at some point:
>>>> Why couldn't we add a function (or probably a macro like
>>>> `package-get-version') to package.el that a package developer can
>>>> invoke/autoload that would take care of just this? This would then
>>>> automatically cover both tarball and VC packages, without having to
>>>> adjust package specifications?
>>>>
>>>> For backwards compatibility, we can then add the definition to Compat as
>>>> it shouldn't depend on any new functionality in the core.
>>>
>>> But now, if I'm understanding you correctly, you're proposing that
>>> instead of this being user-specified, package authors would specify this
>>> directly in their code with a macro—something like (package-load-subdirs
>>> "extensions" "clients")—which would eliminate the need for flattening
>>> entirely, working identically for both VC and tarball installations.
>>> This takes a far broader view, but yes, I do agree that this shouldn't
>>> depend on any new functionality in the core, so implementing it
>>> shouldn't be much of a hassle, and yes, it would be a good idea for the
>>> list of sub-directories to be maintained by package authors, rather than
>>> being managed by users.
>>>
>>> Let me know if I understood the whole thing correctly, or whether I've
>>> misunderstood something.
>>
>> Yes, that is about it. Here is a sketch of how this could look like:
>>
>> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
>> index 70e88d081fa..504187bf9f1 100644
>> --- a/lisp/emacs-lisp/package.el
>> +++ b/lisp/emacs-lisp/package.el
>> @@ -1127,6 +1127,9 @@ package-autoload-ensure-default-file
>> (defvar autoload-timestamps)
>> (defvar version-control)
>>
>> +(defconst package-add-subdir-regexp
>> + (rx bol ";;;###autoload" (+ space) "(package-add-subdir" (+ space) (group ?\" (+ nonl) ?\") (* space) ")" (* space) eol))
>> +
>> (defun package-generate-autoloads (name pkg-dir)
>> "Generate autoloads in PKG-DIR for package named NAME."
>> (let* ((auto-name (format "%s-autoloads.el" name))
>> @@ -1137,7 +1140,27 @@ package-generate-autoloads
>> (backup-inhibited t)
>> (version-control 'never))
>> (loaddefs-generate
>> - pkg-dir output-file nil
>> + ;; Search all Lisp files in `pkg-dir' for `package-add-subdir'
>> + ;; calls that we should also scrape for autoload cookies.
>> + (let ((dirs (list pkg-dir)))
>> + (dolist (file (directory-files pkg-dir t ".el\\'" t))
>> + (when (and (file-regular-p file) (file-readable-p file)
>> + (not (file-equal-p file output-file)))
>> + (with-temp-buffer
>> + (insert-file-contents file)
>> + (goto-char (point-min))
>> + (condition-case nil
>> + (while t
>> + (let ((exp (read (current-buffer))))
>> + (when (eq (car-safe exp) 'package-add-subdir)
>> + (push (expand-file-name (cadr exp) pkg-dir)
>> + dirs))))
>> + (end-of-file))
>> + (goto-char (point-min))
>> + (while (search-forward-regexp package-add-subdir-regexp nil t)
>> + (push (read (match-string 1)) dirs)))))
>> + dirs)
>> + output-file nil
>> (prin1-to-string
>> '(add-to-list
>> 'load-path
>> @@ -4769,6 +4792,23 @@ package-get-version
>> (require 'lisp-mnt)
>> (lm-package-version mainfile)))))))
>>
>> +;;;###autoload
>> +(progn
>> + (defmacro package-add-subdir (dir)
>> + "Manually instruct the package manager to load DIR.
>> +DIR is a string constant denoting a directory relative to the package
>> +root. This macro may only occur in files within the package root,
>> +ideally the main file. It is recommended to autoload calls to this
>> +function."
>> + `(add-to-list
>> + 'load-path
>> + (expand-file-name
>> + ,dir
>> + (or (and load-file-name
>> + (directory-file-name
>> + (file-name-directory load-file-name)))
>> + (car load-path))))))
>> +
>>
>> ;;;; Quickstart: precompute activation actions for faster start up.
>>
>>
>>
>> I have mainly tested it with vertico and it seems to work well (but of
>> course currently lacks the documentation that we have to update as
>> well). If you think that this is an acceptable solution, I'd appreciate
>> it if you could try testing it a bit as well!
This works for me too, but I can see a potential problem with scanning
and collection of the package-requires. Specifically, when flattened, we
will be scanning all the files in the pkg-dir, and the files in the
sub-directories we just flattened, but in this case, we are not scanning
the .el files in the sub-directories. In the recent patch, all .el files
in all directories (pkg-dir + sub-dirs) were being scanned for
package-requires. I am referring to this (patched) snippet of code:
(dolist (file (mapcan (lambda (dir)
(directory-files dir t "\\.el\\'" t))
all-dirs))
(unless (string-match-p ignored-files file)
(with-temp-buffer
(insert-file-contents file)
(when-let* ((require-lines (lm-header-multiline "package-requires")))
(thread-last
(mapconcat #'identity require-lines " ")
package-read-from-string
lm--prepare-package-dependencies
(nconc deps)
(setq deps))))))
Btw, I was thinking of a different approach with an lm-header
'Package-Subdirectories' or something like that. I also think we do not
need to scan all the .el files in pkg-dir either. Instead, we can opt
for the package-get-version solution of only scanning the 'mainfile',
i.e., (concat pkgname ".el") for the lm-header. This 'mainfile' should
presumably be in the lisp-dir of pkg-dir, and all the other
sub-directories mentioned should be relative to this lisp-dir. This way,
we can extract the header line in package-vc--unpack-1, and then call
'package-generate-autoloads' with the directories as an optional
parameter to avoid double computation. What do you think of this
approach?
bug-gnu-emacs <at> gnu.org:bug#80016; Package emacs.
(Fri, 26 Dec 2025 16:03:01 GMT) Full text and rfc822 format available.Message #23 received at 80016 <at> debbugs.gnu.org (full text, mbox):
From: Philip Kaludercic <philipk <at> posteo.net> To: Aritro Sen <1.sen.aritro <at> gmail.com> Cc: 80016 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org> Subject: Re: bug#80016: [PATCH] Load lisp sub-directories in addition to the lisp-dir Date: Fri, 26 Dec 2025 16:02:35 +0000
Aritro Sen <1.sen.aritro <at> gmail.com> writes: > On 12/26/25 7:13 PM, Philip Kaludercic wrote: >> It seems that Aritro accidentally fell out of the CC list, I have >> added >> him again and hope to hear opinions on the suggestion I made. >> Philip Kaludercic <philipk <at> posteo.net> writes: >> >>> Aritro Sen <1.sen.aritro <at> gmail.com> writes: >>> >>>> On 12/18/25 2:51 PM, Philip Kaludercic wrote: >>>>> Aritro Sen <1.sen.aritro <at> gmail.com> writes: >>>>> >>>>>> >>>>>> This patch implements the feature request described in bug #79874. It >>>>>> introduces the keyword lisp-subdirs to use-package-vc. This allows >>>>>> subdirectories of the PKG-DIR to be added to the Emacs load-path when >>>>>> installing packages directly via VC (e.g., Git), in addition to the >>>>>> PKG-DIR or lisp-dir itself. The subdirectory paths must be specified >>>>>> relative to lisp-dir (which defaults to PKG-DIR). >>>>>> >>>>>> This patch depends on bug#80015 being solved. This patch is written >>>>>> based on the discussions in bug#79873. >>>>> I have responded to this issue in bug#79874 (specifically >>>>> <87qzsvmvax.fsf <at> posteo.net>), I am not sure if you saw the message, >>>>> because IIRC I sent it to you before you sent this message? >>>> >>>> Yeah, I saw the message after I sent the patch. I was already composing >>>> my mail and dealing with some text formatting and attachment issues that >>>> happen when sending via the mail client after composing in Emacs, so I >>>> guess I missed the notification. I haven't been using Emacs for too >>>> long, so I'm still discovering all the quirks. Anyway, I digress. >>> >>> No problem! >>> >>>>> We should turn this into a discussion about package-vc instead of >>>>> use-package. The configuration macro doesn't add any real >>>>> functionality, after all. >>>>> >>>>> Secondly, a goal of package-vc is to maintain compatibility with >>>>> specifications as used by the ELPA build server. Before implementing >>>>> something like this, I would like to be sure how elpa-admin.el would >>>>> handle the additional properties and how they would be translated into >>>>> tarballs. >>>> >>>> When I wrote this patch, I was focusing on a narrowly defined >>>> VC specific problem: some packages work perfectly fine when installed as >>>> a tarball via MELPA/ELPA, but break when installed directly via VC. This >>>> is because (and please correct me if I'm wrong) with the :files property >>>> of MELPA or :renames property of ELPA recipes, the packages were getting >>>> "flattened," for lack of a better term. This isn't the case when >>>> installing via VC, so when loaddefs are generated and the package is >>>> loaded, there are some "missing" files, which causes major issues. So I >>>> planned for the sub-directories to be user-specified. >>>> >>>> Now, this isn't exactly the same as :files in MELPA recipes, which takes >>>> a glob expression IIRC, but I think specifying sub-directories combined >>>> with using the :ignored-files property does the same job anyways. >>> >>> This seems like a fair summary of the situation. And I think that >>> "flattening" is a fair description of what we are doing, as tarball >>> packages only add a single directory to `load-path', and for ELPA we use >>> :renames (IMO as a hack) to that end. >>> >>>>> This is related to a point I may or may not have raised at some point: >>>>> Why couldn't we add a function (or probably a macro like >>>>> `package-get-version') to package.el that a package developer can >>>>> invoke/autoload that would take care of just this? This would then >>>>> automatically cover both tarball and VC packages, without having to >>>>> adjust package specifications? >>>>> >>>>> For backwards compatibility, we can then add the definition to Compat as >>>>> it shouldn't depend on any new functionality in the core. >>>> >>>> But now, if I'm understanding you correctly, you're proposing that >>>> instead of this being user-specified, package authors would specify this >>>> directly in their code with a macro—something like (package-load-subdirs >>>> "extensions" "clients")—which would eliminate the need for flattening >>>> entirely, working identically for both VC and tarball installations. >>>> This takes a far broader view, but yes, I do agree that this shouldn't >>>> depend on any new functionality in the core, so implementing it >>>> shouldn't be much of a hassle, and yes, it would be a good idea for the >>>> list of sub-directories to be maintained by package authors, rather than >>>> being managed by users. >>>> >>>> Let me know if I understood the whole thing correctly, or whether I've >>>> misunderstood something. >>> >>> Yes, that is about it. Here is a sketch of how this could look like: >>> >>> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el >>> index 70e88d081fa..504187bf9f1 100644 >>> --- a/lisp/emacs-lisp/package.el >>> +++ b/lisp/emacs-lisp/package.el >>> @@ -1127,6 +1127,9 @@ package-autoload-ensure-default-file >>> (defvar autoload-timestamps) >>> (defvar version-control) >>> +(defconst package-add-subdir-regexp >>> + (rx bol ";;;###autoload" (+ space) "(package-add-subdir" (+ space) (group ?\" (+ nonl) ?\") (* space) ")" (* space) eol)) >>> + >>> (defun package-generate-autoloads (name pkg-dir) >>> "Generate autoloads in PKG-DIR for package named NAME." >>> (let* ((auto-name (format "%s-autoloads.el" name)) >>> @@ -1137,7 +1140,27 @@ package-generate-autoloads >>> (backup-inhibited t) >>> (version-control 'never)) >>> (loaddefs-generate >>> - pkg-dir output-file nil >>> + ;; Search all Lisp files in `pkg-dir' for `package-add-subdir' >>> + ;; calls that we should also scrape for autoload cookies. >>> + (let ((dirs (list pkg-dir))) >>> + (dolist (file (directory-files pkg-dir t ".el\\'" t)) >>> + (when (and (file-regular-p file) (file-readable-p file) >>> + (not (file-equal-p file output-file))) >>> + (with-temp-buffer >>> + (insert-file-contents file) >>> + (goto-char (point-min)) >>> + (condition-case nil >>> + (while t >>> + (let ((exp (read (current-buffer)))) >>> + (when (eq (car-safe exp) 'package-add-subdir) >>> + (push (expand-file-name (cadr exp) pkg-dir) >>> + dirs)))) >>> + (end-of-file)) >>> + (goto-char (point-min)) >>> + (while (search-forward-regexp package-add-subdir-regexp nil t) >>> + (push (read (match-string 1)) dirs))))) >>> + dirs) >>> + output-file nil >>> (prin1-to-string >>> '(add-to-list >>> 'load-path >>> @@ -4769,6 +4792,23 @@ package-get-version >>> (require 'lisp-mnt) >>> (lm-package-version mainfile))))))) >>> +;;;###autoload >>> +(progn >>> + (defmacro package-add-subdir (dir) >>> + "Manually instruct the package manager to load DIR. >>> +DIR is a string constant denoting a directory relative to the package >>> +root. This macro may only occur in files within the package root, >>> +ideally the main file. It is recommended to autoload calls to this >>> +function." >>> + `(add-to-list >>> + 'load-path >>> + (expand-file-name >>> + ,dir >>> + (or (and load-file-name >>> + (directory-file-name >>> + (file-name-directory load-file-name))) >>> + (car load-path)))))) >>> + >>> >>> ;;;; Quickstart: precompute activation actions for faster start up. >>> I have mainly tested it with vertico and it seems to work well >>> (but of >>> course currently lacks the documentation that we have to update as >>> well). If you think that this is an acceptable solution, I'd appreciate >>> it if you could try testing it a bit as well! > > This works for me too, but I can see a potential problem with scanning > and collection of the package-requires. Specifically, when flattened, (I think we should avoid using the phrase "flattened", since that is explicitly not what we are doing.) > we > will be scanning all the files in the pkg-dir, and the files in the > sub-directories we just flattened, but in this case, we are not scanning > the .el files in the sub-directories. In the recent patch, all .el files > in all directories (pkg-dir + sub-dirs) were being scanned for > package-requires. I am referring to this (patched) snippet of code: > > (dolist (file (mapcan (lambda (dir) > (directory-files dir t "\\.el\\'" t)) > all-dirs)) > (unless (string-match-p ignored-files file) > (with-temp-buffer > (insert-file-contents file) > (when-let* ((require-lines (lm-header-multiline "package-requires"))) > (thread-last > (mapconcat #'identity require-lines " ") > package-read-from-string > lm--prepare-package-dependencies > (nconc deps) > (setq deps)))))) > > Btw, I was thinking of a different approach with an lm-header > 'Package-Subdirectories' or something like that. I also think we do not > need to scan all the .el files in pkg-dir either. Instead, we can opt > for the package-get-version solution of only scanning the 'mainfile', > i.e., (concat pkgname ".el") for the lm-header. This 'mainfile' should > presumably be in the lisp-dir of pkg-dir, and all the other > sub-directories mentioned should be relative to this lisp-dir. This way, > we can extract the header line in package-vc--unpack-1, and then call > 'package-generate-autoloads' with the directories as an optional > parameter to avoid double computation. What do you think of this > approach? I was thinking about something like this as well after sending the patch a few days ago, but I haven't thought too much about it since. The main issues is that the main file is not always a reliable heuristic and (much less important) that the header can become long, but package maintainers should be able to split that if the format of 'Package-Subdirectories' uses a list syntax like 'Package-Requires'. Do you want to take a stab at it and combine the two approaches? Otherwise it shouldn't be difficult to address the issue you raised within 'package-vc--unpack-1' based on my proposal. Also, I am working on a refactoring of package.el on the feature/package-refactor, and am adding some features on top of that branch in feature/package-revamp. If you could prepare the patch on top of the latter, that would be great, because that would avoid merge conflicts in the future.
bug-gnu-emacs <at> gnu.org:bug#80016; Package emacs.
(Sat, 27 Dec 2025 17:00:02 GMT) Full text and rfc822 format available.Message #26 received at 80016 <at> debbugs.gnu.org (full text, mbox):
From: Aritro Sen <1.sen.aritro <at> gmail.com> To: Philip Kaludercic <philipk <at> posteo.net> Cc: 80016 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org> Subject: Re: bug#80016: [PATCH] Load lisp sub-directories in addition to the lisp-dir Date: Sat, 27 Dec 2025 22:29:24 +0530
On 12/26/25 9:32 PM, Philip Kaludercic wrote:
> Aritro Sen <1.sen.aritro <at> gmail.com> writes:
>
>> On 12/26/25 7:13 PM, Philip Kaludercic wrote:
>>> It seems that Aritro accidentally fell out of the CC list, I have
>>> added
>>> him again and hope to hear opinions on the suggestion I made.
>>> Philip Kaludercic <philipk <at> posteo.net> writes:
>>>
>>>> Aritro Sen <1.sen.aritro <at> gmail.com> writes:
>>>>
>>>>> On 12/18/25 2:51 PM, Philip Kaludercic wrote:
>>>>>> Aritro Sen <1.sen.aritro <at> gmail.com> writes:
>>>>>>
>>>>>>>
>>>>>>> This patch implements the feature request described in bug #79874. It
>>>>>>> introduces the keyword lisp-subdirs to use-package-vc. This allows
>>>>>>> subdirectories of the PKG-DIR to be added to the Emacs load-path when
>>>>>>> installing packages directly via VC (e.g., Git), in addition to the
>>>>>>> PKG-DIR or lisp-dir itself. The subdirectory paths must be specified
>>>>>>> relative to lisp-dir (which defaults to PKG-DIR).
>>>>>>>
>>>>>>> This patch depends on bug#80015 being solved. This patch is written
>>>>>>> based on the discussions in bug#79873.
>>>>>> I have responded to this issue in bug#79874 (specifically
>>>>>> <87qzsvmvax.fsf <at> posteo.net>), I am not sure if you saw the message,
>>>>>> because IIRC I sent it to you before you sent this message?
>>>>>
>>>>> Yeah, I saw the message after I sent the patch. I was already composing
>>>>> my mail and dealing with some text formatting and attachment issues that
>>>>> happen when sending via the mail client after composing in Emacs, so I
>>>>> guess I missed the notification. I haven't been using Emacs for too
>>>>> long, so I'm still discovering all the quirks. Anyway, I digress.
>>>>
>>>> No problem!
>>>>
>>>>>> We should turn this into a discussion about package-vc instead of
>>>>>> use-package. The configuration macro doesn't add any real
>>>>>> functionality, after all.
>>>>>>
>>>>>> Secondly, a goal of package-vc is to maintain compatibility with
>>>>>> specifications as used by the ELPA build server. Before implementing
>>>>>> something like this, I would like to be sure how elpa-admin.el would
>>>>>> handle the additional properties and how they would be translated into
>>>>>> tarballs.
>>>>>
>>>>> When I wrote this patch, I was focusing on a narrowly defined
>>>>> VC specific problem: some packages work perfectly fine when installed as
>>>>> a tarball via MELPA/ELPA, but break when installed directly via VC. This
>>>>> is because (and please correct me if I'm wrong) with the :files property
>>>>> of MELPA or :renames property of ELPA recipes, the packages were getting
>>>>> "flattened," for lack of a better term. This isn't the case when
>>>>> installing via VC, so when loaddefs are generated and the package is
>>>>> loaded, there are some "missing" files, which causes major issues. So I
>>>>> planned for the sub-directories to be user-specified.
>>>>>
>>>>> Now, this isn't exactly the same as :files in MELPA recipes, which takes
>>>>> a glob expression IIRC, but I think specifying sub-directories combined
>>>>> with using the :ignored-files property does the same job anyways.
>>>>
>>>> This seems like a fair summary of the situation. And I think that
>>>> "flattening" is a fair description of what we are doing, as tarball
>>>> packages only add a single directory to `load-path', and for ELPA we use
>>>> :renames (IMO as a hack) to that end.
>>>>
>>>>>> This is related to a point I may or may not have raised at some point:
>>>>>> Why couldn't we add a function (or probably a macro like
>>>>>> `package-get-version') to package.el that a package developer can
>>>>>> invoke/autoload that would take care of just this? This would then
>>>>>> automatically cover both tarball and VC packages, without having to
>>>>>> adjust package specifications?
>>>>>>
>>>>>> For backwards compatibility, we can then add the definition to Compat as
>>>>>> it shouldn't depend on any new functionality in the core.
>>>>>
>>>>> But now, if I'm understanding you correctly, you're proposing that
>>>>> instead of this being user-specified, package authors would specify this
>>>>> directly in their code with a macro—something like (package-load-subdirs
>>>>> "extensions" "clients")—which would eliminate the need for flattening
>>>>> entirely, working identically for both VC and tarball installations.
>>>>> This takes a far broader view, but yes, I do agree that this shouldn't
>>>>> depend on any new functionality in the core, so implementing it
>>>>> shouldn't be much of a hassle, and yes, it would be a good idea for the
>>>>> list of sub-directories to be maintained by package authors, rather than
>>>>> being managed by users.
>>>>>
>>>>> Let me know if I understood the whole thing correctly, or whether I've
>>>>> misunderstood something.
>>>>
>>>> Yes, that is about it. Here is a sketch of how this could look like:
>>>>
>>>> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
>>>> index 70e88d081fa..504187bf9f1 100644
>>>> --- a/lisp/emacs-lisp/package.el
>>>> +++ b/lisp/emacs-lisp/package.el
>>>> @@ -1127,6 +1127,9 @@ package-autoload-ensure-default-file
>>>> (defvar autoload-timestamps)
>>>> (defvar version-control)
>>>> +(defconst package-add-subdir-regexp
>>>> + (rx bol ";;;###autoload" (+ space) "(package-add-subdir" (+ space) (group ?\" (+ nonl) ?\") (* space) ")" (* space) eol))
>>>> +
>>>> (defun package-generate-autoloads (name pkg-dir)
>>>> "Generate autoloads in PKG-DIR for package named NAME."
>>>> (let* ((auto-name (format "%s-autoloads.el" name))
>>>> @@ -1137,7 +1140,27 @@ package-generate-autoloads
>>>> (backup-inhibited t)
>>>> (version-control 'never))
>>>> (loaddefs-generate
>>>> - pkg-dir output-file nil
>>>> + ;; Search all Lisp files in `pkg-dir' for `package-add-subdir'
>>>> + ;; calls that we should also scrape for autoload cookies.
>>>> + (let ((dirs (list pkg-dir)))
>>>> + (dolist (file (directory-files pkg-dir t ".el\\'" t))
>>>> + (when (and (file-regular-p file) (file-readable-p file)
>>>> + (not (file-equal-p file output-file)))
>>>> + (with-temp-buffer
>>>> + (insert-file-contents file)
>>>> + (goto-char (point-min))
>>>> + (condition-case nil
>>>> + (while t
>>>> + (let ((exp (read (current-buffer))))
>>>> + (when (eq (car-safe exp) 'package-add-subdir)
>>>> + (push (expand-file-name (cadr exp) pkg-dir)
>>>> + dirs))))
>>>> + (end-of-file))
>>>> + (goto-char (point-min))
>>>> + (while (search-forward-regexp package-add-subdir-regexp nil t)
>>>> + (push (read (match-string 1)) dirs)))))
>>>> + dirs)
>>>> + output-file nil
>>>> (prin1-to-string
>>>> '(add-to-list
>>>> 'load-path
>>>> @@ -4769,6 +4792,23 @@ package-get-version
>>>> (require 'lisp-mnt)
>>>> (lm-package-version mainfile)))))))
>>>> +;;;###autoload
>>>> +(progn
>>>> + (defmacro package-add-subdir (dir)
>>>> + "Manually instruct the package manager to load DIR.
>>>> +DIR is a string constant denoting a directory relative to the package
>>>> +root. This macro may only occur in files within the package root,
>>>> +ideally the main file. It is recommended to autoload calls to this
>>>> +function."
>>>> + `(add-to-list
>>>> + 'load-path
>>>> + (expand-file-name
>>>> + ,dir
>>>> + (or (and load-file-name
>>>> + (directory-file-name
>>>> + (file-name-directory load-file-name)))
>>>> + (car load-path))))))
>>>> +
>>>>
>>>> ;;;; Quickstart: precompute activation actions for faster start up.
>>>> I have mainly tested it with vertico and it seems to work well
>>>> (but of
>>>> course currently lacks the documentation that we have to update as
>>>> well). If you think that this is an acceptable solution, I'd appreciate
>>>> it if you could try testing it a bit as well!
>>
>> This works for me too, but I can see a potential problem with scanning
>> and collection of the package-requires. Specifically, when flattened,
>
> (I think we should avoid using the phrase "flattened", since that is
> explicitly not what we are doing.)
>
>> we
>> will be scanning all the files in the pkg-dir, and the files in the
>> sub-directories we just flattened, but in this case, we are not scanning
>> the .el files in the sub-directories. In the recent patch, all .el files
>> in all directories (pkg-dir + sub-dirs) were being scanned for
>> package-requires. I am referring to this (patched) snippet of code:
>>
>> (dolist (file (mapcan (lambda (dir)
>> (directory-files dir t "\\.el\\'" t))
>> all-dirs))
>> (unless (string-match-p ignored-files file)
>> (with-temp-buffer
>> (insert-file-contents file)
>> (when-let* ((require-lines (lm-header-multiline "package-requires")))
>> (thread-last
>> (mapconcat #'identity require-lines " ")
>> package-read-from-string
>> lm--prepare-package-dependencies
>> (nconc deps)
>> (setq deps))))))
>>
>> Btw, I was thinking of a different approach with an lm-header
>> 'Package-Subdirectories' or something like that. I also think we do not
>> need to scan all the .el files in pkg-dir either. Instead, we can opt
>> for the package-get-version solution of only scanning the 'mainfile',
>> i.e., (concat pkgname ".el") for the lm-header. This 'mainfile' should
>> presumably be in the lisp-dir of pkg-dir, and all the other
>> sub-directories mentioned should be relative to this lisp-dir. This way,
>> we can extract the header line in package-vc--unpack-1, and then call
>> 'package-generate-autoloads' with the directories as an optional
>> parameter to avoid double computation. What do you think of this
>> approach?
>
> I was thinking about something like this as well after sending the patch
> a few days ago, but I haven't thought too much about it since. The main
> issues is that the main file is not always a reliable heuristic and
> (much less important) that the header can become long, but package
> maintainers should be able to split that if the format of
> 'Package-Subdirectories' uses a list syntax like 'Package-Requires'.
>
> Do you want to take a stab at it and combine the two approaches?
> Otherwise it shouldn't be difficult to address the issue you raised
> within 'package-vc--unpack-1' based on my proposal.
>
> Also, I am working on a refactoring of package.el on the
> feature/package-refactor, and am adding some features on top of that
> branch in feature/package-revamp. If you could prepare the patch on top
> of the latter, that would be great, because that would avoid merge
> conflicts in the future.
This is the revised patch on the feature/package-refactor branch:
diff --git a/lisp/emacs-lisp/lisp-mnt.el b/lisp/emacs-lisp/lisp-mnt.el
index 111d512ef59..169bbbd7f0a 100644
--- a/lisp/emacs-lisp/lisp-mnt.el
+++ b/lisp/emacs-lisp/lisp-mnt.el
@@ -468,6 +468,13 @@ lm-package-requires
(lm--prepare-package-dependencies
(package-read-from-string (mapconcat #'identity require-lines "
"))))))
+(defun lm-package-subdirectories (&optional file)
+ "Return \"Package-Subdirectories\" header."
+ (require 'package)
+ (lm-with-file file
+ (and-let* ((subdir-lines (lm-header-multiline
"package-subdirectories")))
+ (package-read-from-string (mapconcat #'identity subdir-lines " ")))))
+
(defun lm-package-version (&optional file)
"Return \"Package-Version\" or \"Version\" header.
Prefer Package-Version; if defined, the package author
diff --git a/lisp/package/package-install.el
b/lisp/package/package-install.el
index cec08e60ac3..eee39f79899 100644
--- a/lisp/package/package-install.el
+++ b/lisp/package/package-install.el
@@ -976,29 +976,40 @@ package-autoload-ensure-default-file
(defvar autoload-timestamps)
(defvar version-control)
-(defun package-generate-autoloads (name pkg-dir)
- "Generate autoloads in PKG-DIR for package named NAME."
+(defun package-generate-autoloads (name pkg-dir &optional pkg-subdirs)
+ "Generate autoloads in PKG-DIR for package named NAME.
+If LISP-SUBDIRS is provided, it should be a list of subdirectory names
+relative to PKG-DIR."
(let* ((auto-name (format "%s-autoloads.el" name))
;;(ignore-name (concat name "-pkg.el"))
(output-file (expand-file-name auto-name pkg-dir))
+ (all-dirs (cons pkg-dir
+ (mapcar
+ (lambda (dir) (expand-file-name dir pkg-dir))
+ pkg-subdirs)))
;; We don't need 'em, and this makes the output reproducible.
(autoload-timestamps nil)
(backup-inhibited t)
(version-control 'never))
(loaddefs-generate
- pkg-dir output-file nil
+ all-dirs output-file nil
(prin1-to-string
- '(add-to-list
- 'load-path
- ;; Add the directory that will contain the autoload file to
- ;; the load path. We don't hard-code `pkg-dir', to avoid
- ;; issues if the package directory is moved around.
- ;; `loaddefs-generate' has code to do this for us, but it's
- ;; not currently exposed. (Bug#63625)
- (or (and load-file-name
- (directory-file-name
- (file-name-directory load-file-name)))
- (car load-path)))))
+ `(let ((dir (or (and load-file-name
+ (directory-file-name
+ (file-name-directory load-file-name)))
+ (car load-path))))
+ ;; Add the directory that will contain the autoload file to
+ ;; the load path. We don't hard-code `pkg-dir', to avoid
+ ;; issues if the package directory is moved around.
+ ;; `loaddefs-generate' has code to do this for us, but it's
+ ;; not currently exposed. (Bug#63625)
+ (add-to-list 'load-path dir)
+ ,@(mapcar
+ (lambda (subdir)
+ `(add-to-list
+ 'load-path
+ (expand-file-name ,subdir dir)))
+ pkg-subdirs))))
(let ((buf (find-buffer-visiting output-file)))
(when buf (kill-buffer buf)))
auto-name))
diff --git a/lisp/package/package-vc.el b/lisp/package/package-vc.el
index d6b84d896f9..d7d6f4ea586 100644
--- a/lisp/package/package-vc.el
+++ b/lisp/package/package-vc.el
@@ -533,6 +533,7 @@ package-vc--unpack-1
;; Directory where package's Lisp code resides. It may be
;; equal to `checkout-dir' or be a subdirectory of it.
(lisp-dir (package-vc--checkout-dir pkg-desc 'lisp-dir))
+ subdirs
missing)
;; In case the package was installed directly from source, the
@@ -549,30 +550,29 @@ package-vc--unpack-1
(plist-get pkg-spec :ignored-files)
"\\|")
regexp-unmatchable))
- (deps '()))
+ deps)
(dolist (file (directory-files lisp-dir t "\\.el\\'" t))
(unless (string-match-p ignored-files file)
- (with-temp-buffer
- (insert-file-contents file)
- (when-let* ((require-lines (lm-header-multiline
"package-requires")))
- (setq deps
- (nconc deps
- (lm--prepare-package-dependencies
- (package-read-from-string
- (mapconcat (function identity)
- require-lines " ")))))))))
+ (setq deps (nconc deps (lm-package-requires file)))
+ (setq subdirs (nconc subdirs (lm-package-subdirectories file)))))
+ (dolist (file (mapcan (lambda (dir)
+ (directory-files
+ (expand-file-name dir lisp-dir) t
"\\.el\\'" t))
+ subdirs))
+ (unless (string-match-p ignored-files file)
+ (setq deps (nconc deps (lm-package-requires file)))))
+ (setq deps (assq-delete-all (package-desc-name pkg-desc)
(delete-dups deps)))
(dolist (dep deps)
(cl-callf version-to-list (cadr dep)))
(setf (package-desc-reqs pkg-desc) deps)
- (setf missing (package-vc-install-dependencies (delete-dups deps)))
+ (setf missing (package-vc-install-dependencies deps))
(setf missing (delq (assq (package-desc-name pkg-desc)
missing)
missing)))
;; Generate autoloads
(let* ((name (package-desc-name pkg-desc))
- (auto-name (format "%s-autoloads.el" name)))
- (package-generate-autoloads name lisp-dir)
+ (auto-name (package-generate-autoloads name lisp-dir subdirs)))
;; There are two cases when we wish to "indirect" the loading of
;; autoload files:
;;
@@ -777,7 +777,7 @@ package-vc--unpack
(user-error "Installation aborted")))
;; Ensure we have a copy of the package specification
- (when (null (package-vc--desc->spec pkg-desc name))
+ (unless (equal (package-vc--desc->spec pkg-desc name) pkg-spec)
(customize-save-variable
'package-vc-selected-packages
(cons (cons name pkg-spec)
Some explanations and design choices are in order. I have introduced a
new function to extract the 'Package-Subdirectories' header, named
'lm-package-subdirectories', in line with the other "lm-package-..."
functions. It uses the same logic as lm-package-requires, extracting the
header with lm-header-multiline. The package-generate-autoloads logic
remains unchanged from the previous patch.
There are multiple changes in package-vc--unpack-1. First, the previous
(with-temp-buffer ...) logic for collecting package-requires was
essentially a duplication of the lm-package-requires function, so I have
removed that code and used lm-package-requires instead. In the same
loop, Package-Subdirectories headers are being collected. This assumes
that the headers should be in the .el files of lisp-dir (following your
logic of scanning all '.el' files, rather than relying on a single
mainfile). Then, with the collected list of subdirectories, I am
scanning through each of their .el files to collect other
package-requires.
Since dependencies are installed before the VC package itself, all
mentions of the package itself as a dependency is removed. I am also
deleting duplicate dependencies here rather than waiting for the
delete-dups call here: (setf missing (package-vc-install-dependencies
(delete-dups deps))). This is because we are setting package-desc-reqs
to deps, where duplicates and self-references don't make any sense, nor
does calling version-to-list on the duplicates.
Lastly, in package-vc--unpack, I have changed the when null check to
unless equal. I have raised the point in bug#80015, but in case you have
missed that patch, here is a summary. In commit
573acd97e54ceead6d11b330909ffb8e744247cc this change was made:
- (unless (seq-some (lambda (alist) (equal (alist-get name (cdr
alist)) pkg-spec))
- package-vc--archive-spec-alists)
+ (when (null (package-vc--desc->spec pkg-desc name))
I believe that the logic here is faulty - we have to check if the user
provided pkg-spec can be found, and only then should the block be
ignored. Otherwise, for most (all?) packages, since they do have a
pkg-spec in the archives (MELPA, ELPA, etc.), any user-provided pkg-spec
will effectively be ignored. We should check for equality to ensure the
user specified pkg-spec is found, rather than checking for the existence
of any pkg-spec.
However, this brings forth a interesting question: suppose in the
archives the authors ignores a file, and when installing from VC the
user ignores some more files - shouldn't these lists be merged? Maybe we
should use the same logic as the subdirectories, where we "embed" the
author-specified ignored-files directly in the code itself, possibly
with another header?
BTW, I have removed the lisp-subdirs keyword from the first diff, but I
have included a second diff for that feature, since I believe it may be
necessary for certain packages. For example, treemacs has a number of
"extra" packages, which are not packed with treemacs itself, but rather
each of these files are a package unto itself. For example, this is the
recipe for one such "extra" package:
(treemacs-all-the-icons
:fetcher github
:repo "Alexander-Miller/treemacs"
:files ("src/extra/treemacs-all-the-icons.el"))
In the main treemacs recipe, however, src/extra/* is explicitly
excluded. Since we don't support treating a single file in a VC package
as a full-package (and I don't think we should), and for treemacs
package maintainers, it makes no sense to include the "extras" directory
in Package-Subdirectories header, the user is stuck with installing
those extra packages from the archives, unless we give them a way to
include other subdirectories themselves. They can then prune the
unneeded "extras" files with ignored-files. Anyways, here is the patch
that keeps lisp-subdirs keyword too:
diff --git a/doc/emacs/package.texi b/doc/emacs/package.texi
index 53b339bd606..a66ed9b1e78 100644
--- a/doc/emacs/package.texi
+++ b/doc/emacs/package.texi
@@ -736,6 +736,14 @@ Fetching Package Sources
use for loading the Lisp sources, which defaults to the root directory
of the repository.
+@item :lisp-subdirs
+A string or a list of strings providing the names of subdirectories
+containing additional Lisp sources. These names are interpreted
+relative to the directory specified by @code{:lisp-dir} (or the root
+directory of the repository if @code{:lisp-dir} is unspecified). The
+specified subdirectories are added to the load path and scanned for
+library dependencies and autoloads.
+
@item :main-file
A string providing the main file of the project, from which to gather
package metadata. If not given, the default is the package name with
diff --git a/lisp/package/package-vc.el b/lisp/package/package-vc.el
index d7d6f4ea586..2b5e984fb40 100644
--- a/lisp/package/package-vc.el
+++ b/lisp/package/package-vc.el
@@ -533,7 +533,7 @@ package-vc--unpack-1
;; Directory where package's Lisp code resides. It may be
;; equal to `checkout-dir' or be a subdirectory of it.
(lisp-dir (package-vc--checkout-dir pkg-desc 'lisp-dir))
- subdirs
+ (subdirs (ensure-list (plist-get pkg-spec :lisp-subdirs)))
missing)
;; In case the package was installed directly from source, the
diff --git a/lisp/use-package/use-package-core.el
b/lisp/use-package/use-package-core.el
index 0f049a6f86b..a6c92d663cc 100644
--- a/lisp/use-package/use-package-core.el
+++ b/lisp/use-package/use-package-core.el
@@ -1735,7 +1735,7 @@ use-package-handler/:vc
body))
(defconst use-package-vc-valid-keywords
- '( :url :branch :lisp-dir :main-file :vc-backend :rev
+ '( :url :branch :lisp-dir :lisp-subdirs :main-file :vc-backend :rev
:shell-command :make :ignored-files)
"Valid keywords for the `:vc' keyword.
See Info node `(emacs)Fetching Package Sources'.")
Lastly - and this is slightly off-topic - is there a reason we use regex
to detect ignored-files? Why is it not something like this:
diff --git a/lisp/package/package-vc.el b/lisp/package/package-vc.el
index 2b5e984fb40..dad9e1037c9 100644
--- a/lisp/package/package-vc.el
+++ b/lisp/package/package-vc.el
@@ -540,26 +540,19 @@ package-vc--unpack-1
;; dependency list wasn't know beforehand, and they might have
;; to be installed explicitly.
(let ((ignored-files
- (if (plist-get pkg-spec :ignored-files)
- (mapconcat
- (lambda (ignore)
- (wildcard-to-regexp
- (if (string-match-p "\\`/" ignore)
- (concat checkout-dir ignore)
- (concat "*/" ignore))))
- (plist-get pkg-spec :ignored-files)
- "\\|")
- regexp-unmatchable))
+ (mapcar (lambda (file)
+ (expand-file-name file lisp-dir))
+ (plist-get pkg-spec :ignored-files)))
deps)
(dolist (file (directory-files lisp-dir t "\\.el\\'" t))
- (unless (string-match-p ignored-files file)
+ (unless (member file ignored-files)
(setq deps (nconc deps (lm-package-requires file)))
(setq subdirs (nconc subdirs (lm-package-subdirectories
file)))))
(dolist (file (mapcan (lambda (dir)
(directory-files
(expand-file-name dir lisp-dir) t
"\\.el\\'" t))
subdirs))
- (unless (string-match-p ignored-files file)
+ (unless (member file ignored-files)
(setq deps (nconc deps (lm-package-requires file)))))
(setq deps (assq-delete-all (package-desc-name pkg-desc)
(delete-dups deps)))
(dolist (dep deps)
This seems to be more robust, but I'm unsure whether it'd introduce any
performance bottlenecks.
bug-gnu-emacs <at> gnu.org:bug#80016; Package emacs.
(Sat, 27 Dec 2025 19:49:01 GMT) Full text and rfc822 format available.Message #29 received at 80016 <at> debbugs.gnu.org (full text, mbox):
From: Philip Kaludercic <philipk <at> posteo.net> To: Aritro Sen <1.sen.aritro <at> gmail.com> Cc: 80016 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org> Subject: Re: bug#80016: [PATCH] Load lisp sub-directories in addition to the lisp-dir Date: Sat, 27 Dec 2025 19:48:26 +0000
Aritro Sen <1.sen.aritro <at> gmail.com> writes:
[...]
>>> Btw, I was thinking of a different approach with an lm-header
>>> 'Package-Subdirectories' or something like that. I also think we do not
>>> need to scan all the .el files in pkg-dir either. Instead, we can opt
>>> for the package-get-version solution of only scanning the 'mainfile',
>>> i.e., (concat pkgname ".el") for the lm-header. This 'mainfile' should
>>> presumably be in the lisp-dir of pkg-dir, and all the other
>>> sub-directories mentioned should be relative to this lisp-dir. This way,
>>> we can extract the header line in package-vc--unpack-1, and then call
>>> 'package-generate-autoloads' with the directories as an optional
>>> parameter to avoid double computation. What do you think of this
>>> approach?
>>
>> I was thinking about something like this as well after sending the patch
>> a few days ago, but I haven't thought too much about it since. The main
>> issues is that the main file is not always a reliable heuristic and
>> (much less important) that the header can become long, but package
>> maintainers should be able to split that if the format of
>> 'Package-Subdirectories' uses a list syntax like 'Package-Requires'.
>>
>> Do you want to take a stab at it and combine the two approaches?
>> Otherwise it shouldn't be difficult to address the issue you raised
>> within 'package-vc--unpack-1' based on my proposal.
>>
>> Also, I am working on a refactoring of package.el on the
>> feature/package-refactor, and am adding some features on top of that
>> branch in feature/package-revamp. If you could prepare the patch on top
>> of the latter, that would be great, because that would avoid merge
>> conflicts in the future.
>
> This is the revised patch on the feature/package-refactor branch:
>
> diff --git a/lisp/emacs-lisp/lisp-mnt.el b/lisp/emacs-lisp/lisp-mnt.el
> index 111d512ef59..169bbbd7f0a 100644
> --- a/lisp/emacs-lisp/lisp-mnt.el
> +++ b/lisp/emacs-lisp/lisp-mnt.el
> @@ -468,6 +468,13 @@ lm-package-requires
> (lm--prepare-package-dependencies
> (package-read-from-string (mapconcat #'identity require-lines "
> "))))))
>
> +(defun lm-package-subdirectories (&optional file)
> + "Return \"Package-Subdirectories\" header."
> + (require 'package)
> + (lm-with-file file
> + (and-let* ((subdir-lines (lm-header-multiline
> "package-subdirectories")))
> + (package-read-from-string (mapconcat #'identity subdir-lines " ")))))
> +
> (defun lm-package-version (&optional file)
> "Return \"Package-Version\" or \"Version\" header.
> Prefer Package-Version; if defined, the package author
>
> diff --git a/lisp/package/package-install.el
> b/lisp/package/package-install.el
> index cec08e60ac3..eee39f79899 100644
> --- a/lisp/package/package-install.el
> +++ b/lisp/package/package-install.el
> @@ -976,29 +976,40 @@ package-autoload-ensure-default-file
> (defvar autoload-timestamps)
> (defvar version-control)
>
> -(defun package-generate-autoloads (name pkg-dir)
> - "Generate autoloads in PKG-DIR for package named NAME."
> +(defun package-generate-autoloads (name pkg-dir &optional pkg-subdirs)
> + "Generate autoloads in PKG-DIR for package named NAME.
> +If LISP-SUBDIRS is provided, it should be a list of subdirectory names
> +relative to PKG-DIR."
> (let* ((auto-name (format "%s-autoloads.el" name))
> ;;(ignore-name (concat name "-pkg.el"))
> (output-file (expand-file-name auto-name pkg-dir))
> + (all-dirs (cons pkg-dir
> + (mapcar
> + (lambda (dir) (expand-file-name dir pkg-dir))
> + pkg-subdirs)))
> ;; We don't need 'em, and this makes the output reproducible.
> (autoload-timestamps nil)
> (backup-inhibited t)
> (version-control 'never))
> (loaddefs-generate
> - pkg-dir output-file nil
> + all-dirs output-file nil
> (prin1-to-string
> - '(add-to-list
> - 'load-path
> - ;; Add the directory that will contain the autoload file to
> - ;; the load path. We don't hard-code `pkg-dir', to avoid
> - ;; issues if the package directory is moved around.
> - ;; `loaddefs-generate' has code to do this for us, but it's
> - ;; not currently exposed. (Bug#63625)
> - (or (and load-file-name
> - (directory-file-name
> - (file-name-directory load-file-name)))
> - (car load-path)))))
> + `(let ((dir (or (and load-file-name
> + (directory-file-name
> + (file-name-directory load-file-name)))
> + (car load-path))))
> + ;; Add the directory that will contain the autoload file to
> + ;; the load path. We don't hard-code `pkg-dir', to avoid
> + ;; issues if the package directory is moved around.
> + ;; `loaddefs-generate' has code to do this for us, but it's
> + ;; not currently exposed. (Bug#63625)
> + (add-to-list 'load-path dir)
> + ,@(mapcar
> + (lambda (subdir)
> + `(add-to-list
> + 'load-path
> + (expand-file-name ,subdir dir)))
> + pkg-subdirs))))
> (let ((buf (find-buffer-visiting output-file)))
> (when buf (kill-buffer buf)))
> auto-name))
>
> diff --git a/lisp/package/package-vc.el b/lisp/package/package-vc.el
> index d6b84d896f9..d7d6f4ea586 100644
> --- a/lisp/package/package-vc.el
> +++ b/lisp/package/package-vc.el
> @@ -533,6 +533,7 @@ package-vc--unpack-1
> ;; Directory where package's Lisp code resides. It may be
> ;; equal to `checkout-dir' or be a subdirectory of it.
> (lisp-dir (package-vc--checkout-dir pkg-desc 'lisp-dir))
> + subdirs
> missing)
I'd merge these two into a single line.
>
> ;; In case the package was installed directly from source, the
> @@ -549,30 +550,29 @@ package-vc--unpack-1
> (plist-get pkg-spec :ignored-files)
> "\\|")
> regexp-unmatchable))
> - (deps '()))
> + deps)
And I wouldn't revert this change. Perhaps this is just personal
preference, but I prefer to be explicit when binding nil or '() when
initialising a variable. I think about it like in Scheme, where an
undefined value, a boolean or an empty list are all distinct.
> (dolist (file (directory-files lisp-dir t "\\.el\\'" t))
> (unless (string-match-p ignored-files file)
> - (with-temp-buffer
> - (insert-file-contents file)
> - (when-let* ((require-lines (lm-header-multiline
> "package-requires")))
> - (setq deps
> - (nconc deps
> - (lm--prepare-package-dependencies
> - (package-read-from-string
> - (mapconcat (function identity)
> - require-lines " ")))))))))
> + (setq deps (nconc deps (lm-package-requires file)))
> + (setq subdirs (nconc subdirs (lm-package-subdirectories file)))))
> + (dolist (file (mapcan (lambda (dir)
> + (directory-files
> + (expand-file-name dir lisp-dir) t
> "\\.el\\'" t))
> + subdirs))
> + (unless (string-match-p ignored-files file)
> + (setq deps (nconc deps (lm-package-requires file)))))
> + (setq deps (assq-delete-all (package-desc-name pkg-desc)
> (delete-dups deps)))
> (dolist (dep deps)
> (cl-callf version-to-list (cadr dep)))
> (setf (package-desc-reqs pkg-desc) deps)
> - (setf missing (package-vc-install-dependencies (delete-dups deps)))
> + (setf missing (package-vc-install-dependencies deps))
> (setf missing (delq (assq (package-desc-name pkg-desc)
> missing)
> missing)))
>
> ;; Generate autoloads
> (let* ((name (package-desc-name pkg-desc))
> - (auto-name (format "%s-autoloads.el" name)))
> - (package-generate-autoloads name lisp-dir)
> + (auto-name (package-generate-autoloads name lisp-dir subdirs)))
> ;; There are two cases when we wish to "indirect" the loading of
> ;; autoload files:
> ;;
> @@ -777,7 +777,7 @@ package-vc--unpack
> (user-error "Installation aborted")))
>
> ;; Ensure we have a copy of the package specification
> - (when (null (package-vc--desc->spec pkg-desc name))
> + (unless (equal (package-vc--desc->spec pkg-desc name) pkg-spec)
> (customize-save-variable
> 'package-vc-selected-packages
> (cons (cons name pkg-spec)
>
> Some explanations and design choices are in order. I have introduced a
> new function to extract the 'Package-Subdirectories' header, named
> 'lm-package-subdirectories', in line with the other "lm-package-..."
> functions. It uses the same logic as lm-package-requires, extracting the
> header with lm-header-multiline. The package-generate-autoloads logic
> remains unchanged from the previous patch.
>
> There are multiple changes in package-vc--unpack-1. First, the previous
> (with-temp-buffer ...) logic for collecting package-requires was
> essentially a duplication of the lm-package-requires function, so I have
> removed that code and used lm-package-requires instead. In the same
> loop, Package-Subdirectories headers are being collected. This assumes
> that the headers should be in the .el files of lisp-dir (following your
> logic of scanning all '.el' files, rather than relying on a single
> mainfile). Then, with the collected list of subdirectories, I am
> scanning through each of their .el files to collect other
> package-requires.
>
> Since dependencies are installed before the VC package itself, all
> mentions of the package itself as a dependency is removed. I am also
> deleting duplicate dependencies here rather than waiting for the
> delete-dups call here: (setf missing (package-vc-install-dependencies
> (delete-dups deps))). This is because we are setting package-desc-reqs
> to deps, where duplicates and self-references don't make any sense, nor
> does calling version-to-list on the duplicates.
>
> Lastly, in package-vc--unpack, I have changed the when null check to
> unless equal. I have raised the point in bug#80015, but in case you have
> missed that patch, here is a summary. In commit
> 573acd97e54ceead6d11b330909ffb8e744247cc this change was made:
>
> - (unless (seq-some (lambda (alist) (equal (alist-get name (cdr
> alist)) pkg-spec))
> - package-vc--archive-spec-alists)
> + (when (null (package-vc--desc->spec pkg-desc name))
>
> I believe that the logic here is faulty - we have to check if the user
> provided pkg-spec can be found, and only then should the block be
> ignored. Otherwise, for most (all?) packages, since they do have a
> pkg-spec in the archives (MELPA, ELPA, etc.), any user-provided pkg-spec
> will effectively be ignored. We should check for equality to ensure the
> user specified pkg-spec is found, rather than checking for the existence
> of any pkg-spec.
>
> However, this brings forth a interesting question: suppose in the
> archives the authors ignores a file, and when installing from VC the
> user ignores some more files - shouldn't these lists be merged? Maybe we
> should use the same logic as the subdirectories, where we "embed" the
> author-specified ignored-files directly in the code itself, possibly
> with another header?
This logic sounds reasonable, I would appreciate it if you could prepare
the patches for the suggestions you made.
> BTW, I have removed the lisp-subdirs keyword from the first diff, but I
> have included a second diff for that feature, since I believe it may be
> necessary for certain packages. For example, treemacs has a number of
> "extra" packages, which are not packed with treemacs itself, but rather
> each of these files are a package unto itself. For example, this is the
> recipe for one such "extra" package:
>
> (treemacs-all-the-icons
> :fetcher github
> :repo "Alexander-Miller/treemacs"
> :files ("src/extra/treemacs-all-the-icons.el"))
>
> In the main treemacs recipe, however, src/extra/* is explicitly
> excluded. Since we don't support treating a single file in a VC package
> as a full-package (and I don't think we should), and for treemacs
> package maintainers, it makes no sense to include the "extras" directory
> in Package-Subdirectories header, the user is stuck with installing
> those extra packages from the archives, unless we give them a way to
> include other subdirectories themselves. They can then prune the
> unneeded "extras" files with ignored-files. Anyways, here is the patch
> that keeps lisp-subdirs keyword too:
>
> diff --git a/doc/emacs/package.texi b/doc/emacs/package.texi
> index 53b339bd606..a66ed9b1e78 100644
> --- a/doc/emacs/package.texi
> +++ b/doc/emacs/package.texi
> @@ -736,6 +736,14 @@ Fetching Package Sources
> use for loading the Lisp sources, which defaults to the root directory
> of the repository.
>
> +@item :lisp-subdirs
> +A string or a list of strings providing the names of subdirectories
> +containing additional Lisp sources. These names are interpreted
> +relative to the directory specified by @code{:lisp-dir} (or the root
> +directory of the repository if @code{:lisp-dir} is unspecified). The
> +specified subdirectories are added to the load path and scanned for
> +library dependencies and autoloads.
> +
> @item :main-file
> A string providing the main file of the project, from which to gather
> package metadata. If not given, the default is the package name with
>
> diff --git a/lisp/package/package-vc.el b/lisp/package/package-vc.el
> index d7d6f4ea586..2b5e984fb40 100644
> --- a/lisp/package/package-vc.el
> +++ b/lisp/package/package-vc.el
> @@ -533,7 +533,7 @@ package-vc--unpack-1
> ;; Directory where package's Lisp code resides. It may be
> ;; equal to `checkout-dir' or be a subdirectory of it.
> (lisp-dir (package-vc--checkout-dir pkg-desc 'lisp-dir))
> - subdirs
> + (subdirs (ensure-list (plist-get pkg-spec :lisp-subdirs)))
> missing)
>
> ;; In case the package was installed directly from source, the
>
> diff --git a/lisp/use-package/use-package-core.el
> b/lisp/use-package/use-package-core.el
> index 0f049a6f86b..a6c92d663cc 100644
> --- a/lisp/use-package/use-package-core.el
> +++ b/lisp/use-package/use-package-core.el
> @@ -1735,7 +1735,7 @@ use-package-handler/:vc
> body))
>
> (defconst use-package-vc-valid-keywords
> - '( :url :branch :lisp-dir :main-file :vc-backend :rev
> + '( :url :branch :lisp-dir :lisp-subdirs :main-file :vc-backend :rev
> :shell-command :make :ignored-files)
> "Valid keywords for the `:vc' keyword.
> See Info node `(emacs)Fetching Package Sources'.")
I remain disinclined towards this point. Generally we don't have the
ambition to support any kind of package layout. On ELPA, we have the
policy that one repository branch corresponds to one package, and
package-vc uses this fact by checking out the entire repository. Am I
understanding correctly that with the approach you suggest package-vc
would check out the repository multiple times over?
> Lastly - and this is slightly off-topic - is there a reason we use regex
> to detect ignored-files? Why is it not something like this:
>
> diff --git a/lisp/package/package-vc.el b/lisp/package/package-vc.el
> index 2b5e984fb40..dad9e1037c9 100644
> --- a/lisp/package/package-vc.el
> +++ b/lisp/package/package-vc.el
> @@ -540,26 +540,19 @@ package-vc--unpack-1
> ;; dependency list wasn't know beforehand, and they might have
> ;; to be installed explicitly.
> (let ((ignored-files
> - (if (plist-get pkg-spec :ignored-files)
> - (mapconcat
> - (lambda (ignore)
> - (wildcard-to-regexp
> - (if (string-match-p "\\`/" ignore)
> - (concat checkout-dir ignore)
> - (concat "*/" ignore))))
> - (plist-get pkg-spec :ignored-files)
> - "\\|")
> - regexp-unmatchable))
> + (mapcar (lambda (file)
> + (expand-file-name file lisp-dir))
> + (plist-get pkg-spec :ignored-files)))
> deps)
> (dolist (file (directory-files lisp-dir t "\\.el\\'" t))
> - (unless (string-match-p ignored-files file)
> + (unless (member file ignored-files)
> (setq deps (nconc deps (lm-package-requires file)))
> (setq subdirs (nconc subdirs (lm-package-subdirectories
> file)))))
> (dolist (file (mapcan (lambda (dir)
> (directory-files
> (expand-file-name dir lisp-dir) t
> "\\.el\\'" t))
> subdirs))
> - (unless (string-match-p ignored-files file)
> + (unless (member file ignored-files)
> (setq deps (nconc deps (lm-package-requires file)))))
> (setq deps (assq-delete-all (package-desc-name pkg-desc)
> (delete-dups deps)))
> (dolist (dep deps)
>
> This seems to be more robust, but I'm unsure whether it'd introduce any
> performance bottlenecks.
Performance is the main reason. But your suggestion doesn't handle
glob-expressions in file names, which the previous implementation does
due to `wildcard-to-regexp'.
bug-gnu-emacs <at> gnu.org:bug#80016; Package emacs.
(Tue, 30 Dec 2025 00:33:02 GMT) Full text and rfc822 format available.Message #32 received at 80016 <at> debbugs.gnu.org (full text, mbox):
From: Aritro Sen <1.sen.aritro <at> gmail.com> To: Philip Kaludercic <philipk <at> posteo.net> Cc: 80016 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org> Subject: Re: bug#80016: [PATCH] Load lisp sub-directories in addition to the lisp-dir Date: Tue, 30 Dec 2025 06:01:52 +0530
On 12/28/25 1:18 AM, Philip Kaludercic wrote:
> Aritro Sen <1.sen.aritro <at> gmail.com> writes:
>
>
> [...]
>
>>>> Btw, I was thinking of a different approach with an lm-header
>>>> 'Package-Subdirectories' or something like that. I also think we do not
>>>> need to scan all the .el files in pkg-dir either. Instead, we can opt
>>>> for the package-get-version solution of only scanning the 'mainfile',
>>>> i.e., (concat pkgname ".el") for the lm-header. This 'mainfile' should
>>>> presumably be in the lisp-dir of pkg-dir, and all the other
>>>> sub-directories mentioned should be relative to this lisp-dir. This way,
>>>> we can extract the header line in package-vc--unpack-1, and then call
>>>> 'package-generate-autoloads' with the directories as an optional
>>>> parameter to avoid double computation. What do you think of this
>>>> approach?
>>>
>>> I was thinking about something like this as well after sending the patch
>>> a few days ago, but I haven't thought too much about it since. The main
>>> issues is that the main file is not always a reliable heuristic and
>>> (much less important) that the header can become long, but package
>>> maintainers should be able to split that if the format of
>>> 'Package-Subdirectories' uses a list syntax like 'Package-Requires'.
>>>
>>> Do you want to take a stab at it and combine the two approaches?
>>> Otherwise it shouldn't be difficult to address the issue you raised
>>> within 'package-vc--unpack-1' based on my proposal.
>>>
>>> Also, I am working on a refactoring of package.el on the
>>> feature/package-refactor, and am adding some features on top of that
>>> branch in feature/package-revamp. If you could prepare the patch on top
>>> of the latter, that would be great, because that would avoid merge
>>> conflicts in the future.
>>
>> This is the revised patch on the feature/package-refactor branch:
>>
>> diff --git a/lisp/emacs-lisp/lisp-mnt.el b/lisp/emacs-lisp/lisp-mnt.el
>> index 111d512ef59..169bbbd7f0a 100644
>> --- a/lisp/emacs-lisp/lisp-mnt.el
>> +++ b/lisp/emacs-lisp/lisp-mnt.el
>> @@ -468,6 +468,13 @@ lm-package-requires
>> (lm--prepare-package-dependencies
>> (package-read-from-string (mapconcat #'identity require-lines "
>> "))))))
>>
>> +(defun lm-package-subdirectories (&optional file)
>> + "Return \"Package-Subdirectories\" header."
>> + (require 'package)
>> + (lm-with-file file
>> + (and-let* ((subdir-lines (lm-header-multiline
>> "package-subdirectories")))
>> + (package-read-from-string (mapconcat #'identity subdir-lines " ")))))
>> +
>> (defun lm-package-version (&optional file)
>> "Return \"Package-Version\" or \"Version\" header.
>> Prefer Package-Version; if defined, the package author
>>
>> diff --git a/lisp/package/package-install.el
>> b/lisp/package/package-install.el
>> index cec08e60ac3..eee39f79899 100644
>> --- a/lisp/package/package-install.el
>> +++ b/lisp/package/package-install.el
>> @@ -976,29 +976,40 @@ package-autoload-ensure-default-file
>> (defvar autoload-timestamps)
>> (defvar version-control)
>>
>> -(defun package-generate-autoloads (name pkg-dir)
>> - "Generate autoloads in PKG-DIR for package named NAME."
>> +(defun package-generate-autoloads (name pkg-dir &optional pkg-subdirs)
>> + "Generate autoloads in PKG-DIR for package named NAME.
>> +If LISP-SUBDIRS is provided, it should be a list of subdirectory names
>> +relative to PKG-DIR."
>> (let* ((auto-name (format "%s-autoloads.el" name))
>> ;;(ignore-name (concat name "-pkg.el"))
>> (output-file (expand-file-name auto-name pkg-dir))
>> + (all-dirs (cons pkg-dir
>> + (mapcar
>> + (lambda (dir) (expand-file-name dir pkg-dir))
>> + pkg-subdirs)))
>> ;; We don't need 'em, and this makes the output reproducible.
>> (autoload-timestamps nil)
>> (backup-inhibited t)
>> (version-control 'never))
>> (loaddefs-generate
>> - pkg-dir output-file nil
>> + all-dirs output-file nil
>> (prin1-to-string
>> - '(add-to-list
>> - 'load-path
>> - ;; Add the directory that will contain the autoload file to
>> - ;; the load path. We don't hard-code `pkg-dir', to avoid
>> - ;; issues if the package directory is moved around.
>> - ;; `loaddefs-generate' has code to do this for us, but it's
>> - ;; not currently exposed. (Bug#63625)
>> - (or (and load-file-name
>> - (directory-file-name
>> - (file-name-directory load-file-name)))
>> - (car load-path)))))
>> + `(let ((dir (or (and load-file-name
>> + (directory-file-name
>> + (file-name-directory load-file-name)))
>> + (car load-path))))
>> + ;; Add the directory that will contain the autoload file to
>> + ;; the load path. We don't hard-code `pkg-dir', to avoid
>> + ;; issues if the package directory is moved around.
>> + ;; `loaddefs-generate' has code to do this for us, but it's
>> + ;; not currently exposed. (Bug#63625)
>> + (add-to-list 'load-path dir)
>> + ,@(mapcar
>> + (lambda (subdir)
>> + `(add-to-list
>> + 'load-path
>> + (expand-file-name ,subdir dir)))
>> + pkg-subdirs))))
>> (let ((buf (find-buffer-visiting output-file)))
>> (when buf (kill-buffer buf)))
>> auto-name))
>>
>> diff --git a/lisp/package/package-vc.el b/lisp/package/package-vc.el
>> index d6b84d896f9..d7d6f4ea586 100644
>> --- a/lisp/package/package-vc.el
>> +++ b/lisp/package/package-vc.el
>> @@ -533,6 +533,7 @@ package-vc--unpack-1
>> ;; Directory where package's Lisp code resides. It may be
>> ;; equal to `checkout-dir' or be a subdirectory of it.
>> (lisp-dir (package-vc--checkout-dir pkg-desc 'lisp-dir))
>> + subdirs
>> missing)
>
> I'd merge these two into a single line.
>
>>
>> ;; In case the package was installed directly from source, the
>> @@ -549,30 +550,29 @@ package-vc--unpack-1
>> (plist-get pkg-spec :ignored-files)
>> "\\|")
>> regexp-unmatchable))
>> - (deps '()))
>> + deps)
>
> And I wouldn't revert this change. Perhaps this is just personal
> preference, but I prefer to be explicit when binding nil or '() when
> initialising a variable. I think about it like in Scheme, where an
> undefined value, a boolean or an empty list are all distinct.
>
>> (dolist (file (directory-files lisp-dir t "\\.el\\'" t))
>> (unless (string-match-p ignored-files file)
>> - (with-temp-buffer
>> - (insert-file-contents file)
>> - (when-let* ((require-lines (lm-header-multiline
>> "package-requires")))
>> - (setq deps
>> - (nconc deps
>> - (lm--prepare-package-dependencies
>> - (package-read-from-string
>> - (mapconcat (function identity)
>> - require-lines " ")))))))))
>> + (setq deps (nconc deps (lm-package-requires file)))
>> + (setq subdirs (nconc subdirs (lm-package-subdirectories file)))))
>> + (dolist (file (mapcan (lambda (dir)
>> + (directory-files
>> + (expand-file-name dir lisp-dir) t
>> "\\.el\\'" t))
>> + subdirs))
>> + (unless (string-match-p ignored-files file)
>> + (setq deps (nconc deps (lm-package-requires file)))))
>> + (setq deps (assq-delete-all (package-desc-name pkg-desc)
>> (delete-dups deps)))
>> (dolist (dep deps)
>> (cl-callf version-to-list (cadr dep)))
>> (setf (package-desc-reqs pkg-desc) deps)
>> - (setf missing (package-vc-install-dependencies (delete-dups deps)))
>> + (setf missing (package-vc-install-dependencies deps))
>> (setf missing (delq (assq (package-desc-name pkg-desc)
>> missing)
>> missing)))
>>
>> ;; Generate autoloads
>> (let* ((name (package-desc-name pkg-desc))
>> - (auto-name (format "%s-autoloads.el" name)))
>> - (package-generate-autoloads name lisp-dir)
>> + (auto-name (package-generate-autoloads name lisp-dir subdirs)))
>> ;; There are two cases when we wish to "indirect" the loading of
>> ;; autoload files:
>> ;;
>> @@ -777,7 +777,7 @@ package-vc--unpack
>> (user-error "Installation aborted")))
>>
>> ;; Ensure we have a copy of the package specification
>> - (when (null (package-vc--desc->spec pkg-desc name))
>> + (unless (equal (package-vc--desc->spec pkg-desc name) pkg-spec)
>> (customize-save-variable
>> 'package-vc-selected-packages
>> (cons (cons name pkg-spec)
>>
>> Some explanations and design choices are in order. I have introduced a
>> new function to extract the 'Package-Subdirectories' header, named
>> 'lm-package-subdirectories', in line with the other "lm-package-..."
>> functions. It uses the same logic as lm-package-requires, extracting the
>> header with lm-header-multiline. The package-generate-autoloads logic
>> remains unchanged from the previous patch.
>>
>> There are multiple changes in package-vc--unpack-1. First, the previous
>> (with-temp-buffer ...) logic for collecting package-requires was
>> essentially a duplication of the lm-package-requires function, so I have
>> removed that code and used lm-package-requires instead. In the same
>> loop, Package-Subdirectories headers are being collected. This assumes
>> that the headers should be in the .el files of lisp-dir (following your
>> logic of scanning all '.el' files, rather than relying on a single
>> mainfile). Then, with the collected list of subdirectories, I am
>> scanning through each of their .el files to collect other
>> package-requires.
>>
>> Since dependencies are installed before the VC package itself, all
>> mentions of the package itself as a dependency is removed. I am also
>> deleting duplicate dependencies here rather than waiting for the
>> delete-dups call here: (setf missing (package-vc-install-dependencies
>> (delete-dups deps))). This is because we are setting package-desc-reqs
>> to deps, where duplicates and self-references don't make any sense, nor
>> does calling version-to-list on the duplicates.
>>
>> Lastly, in package-vc--unpack, I have changed the when null check to
>> unless equal. I have raised the point in bug#80015, but in case you have
>> missed that patch, here is a summary. In commit
>> 573acd97e54ceead6d11b330909ffb8e744247cc this change was made:
>>
>> - (unless (seq-some (lambda (alist) (equal (alist-get name (cdr
>> alist)) pkg-spec))
>> - package-vc--archive-spec-alists)
>> + (when (null (package-vc--desc->spec pkg-desc name))
>>
>> I believe that the logic here is faulty - we have to check if the user
>> provided pkg-spec can be found, and only then should the block be
>> ignored. Otherwise, for most (all?) packages, since they do have a
>> pkg-spec in the archives (MELPA, ELPA, etc.), any user-provided pkg-spec
>> will effectively be ignored. We should check for equality to ensure the
>> user specified pkg-spec is found, rather than checking for the existence
>> of any pkg-spec.
>>
>> However, this brings forth a interesting question: suppose in the
>> archives the authors ignores a file, and when installing from VC the
>> user ignores some more files - shouldn't these lists be merged? Maybe we
>> should use the same logic as the subdirectories, where we "embed" the
>> author-specified ignored-files directly in the code itself, possibly
>> with another header?
>
> This logic sounds reasonable, I would appreciate it if you could prepare
> the patches for the suggestions you made.
For the ignored-files, which solution sounds better though? I am a bit
reluctant towards the merging solution, because then there is always an
inherent dependency on the archives. However, I am also not sure whether
introducing another header for ignored-files is a good idea. Let me know
which idea sounds better to you. Or should we just leave this as-is?
>> BTW, I have removed the lisp-subdirs keyword from the first diff, but I
>> have included a second diff for that feature, since I believe it may be
>> necessary for certain packages. For example, treemacs has a number of
>> "extra" packages, which are not packed with treemacs itself, but rather
>> each of these files are a package unto itself. For example, this is the
>> recipe for one such "extra" package:
>>
>> (treemacs-all-the-icons
>> :fetcher github
>> :repo "Alexander-Miller/treemacs"
>> :files ("src/extra/treemacs-all-the-icons.el"))
>>
>> In the main treemacs recipe, however, src/extra/* is explicitly
>> excluded. Since we don't support treating a single file in a VC package
>> as a full-package (and I don't think we should), and for treemacs
>> package maintainers, it makes no sense to include the "extras" directory
>> in Package-Subdirectories header, the user is stuck with installing
>> those extra packages from the archives, unless we give them a way to
>> include other subdirectories themselves. They can then prune the
>> unneeded "extras" files with ignored-files. Anyways, here is the patch
>> that keeps lisp-subdirs keyword too:
>>
>> diff --git a/doc/emacs/package.texi b/doc/emacs/package.texi
>> index 53b339bd606..a66ed9b1e78 100644
>> --- a/doc/emacs/package.texi
>> +++ b/doc/emacs/package.texi
>> @@ -736,6 +736,14 @@ Fetching Package Sources
>> use for loading the Lisp sources, which defaults to the root directory
>> of the repository.
>>
>> +@item :lisp-subdirs
>> +A string or a list of strings providing the names of subdirectories
>> +containing additional Lisp sources. These names are interpreted
>> +relative to the directory specified by @code{:lisp-dir} (or the root
>> +directory of the repository if @code{:lisp-dir} is unspecified). The
>> +specified subdirectories are added to the load path and scanned for
>> +library dependencies and autoloads.
>> +
>> @item :main-file
>> A string providing the main file of the project, from which to gather
>> package metadata. If not given, the default is the package name with
>>
>> diff --git a/lisp/package/package-vc.el b/lisp/package/package-vc.el
>> index d7d6f4ea586..2b5e984fb40 100644
>> --- a/lisp/package/package-vc.el
>> +++ b/lisp/package/package-vc.el
>> @@ -533,7 +533,7 @@ package-vc--unpack-1
>> ;; Directory where package's Lisp code resides. It may be
>> ;; equal to `checkout-dir' or be a subdirectory of it.
>> (lisp-dir (package-vc--checkout-dir pkg-desc 'lisp-dir))
>> - subdirs
>> + (subdirs (ensure-list (plist-get pkg-spec :lisp-subdirs)))
>> missing)
>>
>> ;; In case the package was installed directly from source, the
>>
>> diff --git a/lisp/use-package/use-package-core.el
>> b/lisp/use-package/use-package-core.el
>> index 0f049a6f86b..a6c92d663cc 100644
>> --- a/lisp/use-package/use-package-core.el
>> +++ b/lisp/use-package/use-package-core.el
>> @@ -1735,7 +1735,7 @@ use-package-handler/:vc
>> body))
>>
>> (defconst use-package-vc-valid-keywords
>> - '( :url :branch :lisp-dir :main-file :vc-backend :rev
>> + '( :url :branch :lisp-dir :lisp-subdirs :main-file :vc-backend :rev
>> :shell-command :make :ignored-files)
>> "Valid keywords for the `:vc' keyword.
>> See Info node `(emacs)Fetching Package Sources'.")
>
> I remain disinclined towards this point. Generally we don't have the
> ambition to support any kind of package layout. On ELPA, we have the
> policy that one repository branch corresponds to one package, and
> package-vc uses this fact by checking out the entire repository. Am I
> understanding correctly that with the approach you suggest package-vc
> would check out the repository multiple times over?
No, this will not check out the repository multiple times. I think I
should have clarified the purpose of this lisp-subdirs keyword in a
better way. Let's consider treemacs as an example. The author has a
number of extra related packages in the aptly named extras directory of
the root git directory. Now, the author doesn't package this extras
directory with the treemacs package. Instead, they opt to package the
files in extras separately. So, the author would not want to put the
extras directory in the Package-Subdirectories header. But a user might.
When the user uses use-package with vc, the whole git directory for
treemacs is cloned, along with the extras directory. My opinion is: in
cases like this, the user might want to load the extras directory too,
but without the lisp-subdirs keyword, the only option the user has is to
patch treemacs itself and include the "extras" directory in
Package-Subdirectories. Instead, this keyword just provides the user the
option to include some directories as subdirs too, even if the author
decides that they are not. I used treemacs as an example, but there may
be many other packages like that, where the author decides some
directory to not be part of the package, but as auxiliary ones, so they
don't package them together as a single tarball. But since the user is
cloning the whole git directory, the user may want to include the
subdirectory(ies). For this sole purpose, I was thinking of keeping
lisp-subdirs. However, if ELPA doesn't have the capability to do what I
mentioned, and if this is just MELPA specific, I guess we may not
include this lisp-subdirs keyword.
>> Lastly - and this is slightly off-topic - is there a reason we use regex
>> to detect ignored-files? Why is it not something like this:
>>
>> diff --git a/lisp/package/package-vc.el b/lisp/package/package-vc.el
>> index 2b5e984fb40..dad9e1037c9 100644
>> --- a/lisp/package/package-vc.el
>> +++ b/lisp/package/package-vc.el
>> @@ -540,26 +540,19 @@ package-vc--unpack-1
>> ;; dependency list wasn't know beforehand, and they might have
>> ;; to be installed explicitly.
>> (let ((ignored-files
>> - (if (plist-get pkg-spec :ignored-files)
>> - (mapconcat
>> - (lambda (ignore)
>> - (wildcard-to-regexp
>> - (if (string-match-p "\\`/" ignore)
>> - (concat checkout-dir ignore)
>> - (concat "*/" ignore))))
>> - (plist-get pkg-spec :ignored-files)
>> - "\\|")
>> - regexp-unmatchable))
>> + (mapcar (lambda (file)
>> + (expand-file-name file lisp-dir))
>> + (plist-get pkg-spec :ignored-files)))
>> deps)
>> (dolist (file (directory-files lisp-dir t "\\.el\\'" t))
>> - (unless (string-match-p ignored-files file)
>> + (unless (member file ignored-files)
>> (setq deps (nconc deps (lm-package-requires file)))
>> (setq subdirs (nconc subdirs (lm-package-subdirectories
>> file)))))
>> (dolist (file (mapcan (lambda (dir)
>> (directory-files
>> (expand-file-name dir lisp-dir) t
>> "\\.el\\'" t))
>> subdirs))
>> - (unless (string-match-p ignored-files file)
>> + (unless (member file ignored-files)
>> (setq deps (nconc deps (lm-package-requires file)))))
>> (setq deps (assq-delete-all (package-desc-name pkg-desc)
>> (delete-dups deps)))
>> (dolist (dep deps)
>>
>> This seems to be more robust, but I'm unsure whether it'd introduce any
>> performance bottlenecks.
>
> Performance is the main reason. But your suggestion doesn't handle
> glob-expressions in file names, which the previous implementation does
> due to `wildcard-to-regexp'.
Ah OK, I missed the glob-expression part. And yeah, I had a fleeting
suspicion that there might be a performance issue. Anyways, yeah, this
patch was unnecessary, a bit of curiosity from my part, nothing else.
bug-gnu-emacs <at> gnu.org:bug#80016; Package emacs.
(Tue, 30 Dec 2025 09:52:01 GMT) Full text and rfc822 format available.Message #35 received at 80016 <at> debbugs.gnu.org (full text, mbox):
From: Philip Kaludercic <philipk <at> posteo.net> To: Aritro Sen <1.sen.aritro <at> gmail.com> Cc: 80016 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org> Subject: Re: bug#80016: [PATCH] Load lisp sub-directories in addition to the lisp-dir Date: Tue, 30 Dec 2025 09:51:33 +0000
Aritro Sen <1.sen.aritro <at> gmail.com> writes:
> On 12/28/25 1:18 AM, Philip Kaludercic wrote:
>> Aritro Sen <1.sen.aritro <at> gmail.com> writes:
>> [...]
>>
>>>>> Btw, I was thinking of a different approach with an lm-header
>>>>> 'Package-Subdirectories' or something like that. I also think we do not
>>>>> need to scan all the .el files in pkg-dir either. Instead, we can opt
>>>>> for the package-get-version solution of only scanning the 'mainfile',
>>>>> i.e., (concat pkgname ".el") for the lm-header. This 'mainfile' should
>>>>> presumably be in the lisp-dir of pkg-dir, and all the other
>>>>> sub-directories mentioned should be relative to this lisp-dir. This way,
>>>>> we can extract the header line in package-vc--unpack-1, and then call
>>>>> 'package-generate-autoloads' with the directories as an optional
>>>>> parameter to avoid double computation. What do you think of this
>>>>> approach?
>>>>
>>>> I was thinking about something like this as well after sending the patch
>>>> a few days ago, but I haven't thought too much about it since. The main
>>>> issues is that the main file is not always a reliable heuristic and
>>>> (much less important) that the header can become long, but package
>>>> maintainers should be able to split that if the format of
>>>> 'Package-Subdirectories' uses a list syntax like 'Package-Requires'.
>>>>
>>>> Do you want to take a stab at it and combine the two approaches?
>>>> Otherwise it shouldn't be difficult to address the issue you raised
>>>> within 'package-vc--unpack-1' based on my proposal.
>>>>
>>>> Also, I am working on a refactoring of package.el on the
>>>> feature/package-refactor, and am adding some features on top of that
>>>> branch in feature/package-revamp. If you could prepare the patch on top
>>>> of the latter, that would be great, because that would avoid merge
>>>> conflicts in the future.
>>>
>>> This is the revised patch on the feature/package-refactor branch:
>>>
>>> diff --git a/lisp/emacs-lisp/lisp-mnt.el b/lisp/emacs-lisp/lisp-mnt.el
>>> index 111d512ef59..169bbbd7f0a 100644
>>> --- a/lisp/emacs-lisp/lisp-mnt.el
>>> +++ b/lisp/emacs-lisp/lisp-mnt.el
>>> @@ -468,6 +468,13 @@ lm-package-requires
>>> (lm--prepare-package-dependencies
>>> (package-read-from-string (mapconcat #'identity require-lines "
>>> "))))))
>>>
>>> +(defun lm-package-subdirectories (&optional file)
>>> + "Return \"Package-Subdirectories\" header."
>>> + (require 'package)
>>> + (lm-with-file file
>>> + (and-let* ((subdir-lines (lm-header-multiline
>>> "package-subdirectories")))
>>> + (package-read-from-string (mapconcat #'identity subdir-lines " ")))))
>>> +
>>> (defun lm-package-version (&optional file)
>>> "Return \"Package-Version\" or \"Version\" header.
>>> Prefer Package-Version; if defined, the package author
>>>
>>> diff --git a/lisp/package/package-install.el
>>> b/lisp/package/package-install.el
>>> index cec08e60ac3..eee39f79899 100644
>>> --- a/lisp/package/package-install.el
>>> +++ b/lisp/package/package-install.el
>>> @@ -976,29 +976,40 @@ package-autoload-ensure-default-file
>>> (defvar autoload-timestamps)
>>> (defvar version-control)
>>>
>>> -(defun package-generate-autoloads (name pkg-dir)
>>> - "Generate autoloads in PKG-DIR for package named NAME."
>>> +(defun package-generate-autoloads (name pkg-dir &optional pkg-subdirs)
>>> + "Generate autoloads in PKG-DIR for package named NAME.
>>> +If LISP-SUBDIRS is provided, it should be a list of subdirectory names
>>> +relative to PKG-DIR."
>>> (let* ((auto-name (format "%s-autoloads.el" name))
>>> ;;(ignore-name (concat name "-pkg.el"))
>>> (output-file (expand-file-name auto-name pkg-dir))
>>> + (all-dirs (cons pkg-dir
>>> + (mapcar
>>> + (lambda (dir) (expand-file-name dir pkg-dir))
>>> + pkg-subdirs)))
>>> ;; We don't need 'em, and this makes the output reproducible.
>>> (autoload-timestamps nil)
>>> (backup-inhibited t)
>>> (version-control 'never))
>>> (loaddefs-generate
>>> - pkg-dir output-file nil
>>> + all-dirs output-file nil
>>> (prin1-to-string
>>> - '(add-to-list
>>> - 'load-path
>>> - ;; Add the directory that will contain the autoload file to
>>> - ;; the load path. We don't hard-code `pkg-dir', to avoid
>>> - ;; issues if the package directory is moved around.
>>> - ;; `loaddefs-generate' has code to do this for us, but it's
>>> - ;; not currently exposed. (Bug#63625)
>>> - (or (and load-file-name
>>> - (directory-file-name
>>> - (file-name-directory load-file-name)))
>>> - (car load-path)))))
>>> + `(let ((dir (or (and load-file-name
>>> + (directory-file-name
>>> + (file-name-directory load-file-name)))
>>> + (car load-path))))
>>> + ;; Add the directory that will contain the autoload file to
>>> + ;; the load path. We don't hard-code `pkg-dir', to avoid
>>> + ;; issues if the package directory is moved around.
>>> + ;; `loaddefs-generate' has code to do this for us, but it's
>>> + ;; not currently exposed. (Bug#63625)
>>> + (add-to-list 'load-path dir)
>>> + ,@(mapcar
>>> + (lambda (subdir)
>>> + `(add-to-list
>>> + 'load-path
>>> + (expand-file-name ,subdir dir)))
>>> + pkg-subdirs))))
>>> (let ((buf (find-buffer-visiting output-file)))
>>> (when buf (kill-buffer buf)))
>>> auto-name))
>>>
>>> diff --git a/lisp/package/package-vc.el b/lisp/package/package-vc.el
>>> index d6b84d896f9..d7d6f4ea586 100644
>>> --- a/lisp/package/package-vc.el
>>> +++ b/lisp/package/package-vc.el
>>> @@ -533,6 +533,7 @@ package-vc--unpack-1
>>> ;; Directory where package's Lisp code resides. It may be
>>> ;; equal to `checkout-dir' or be a subdirectory of it.
>>> (lisp-dir (package-vc--checkout-dir pkg-desc 'lisp-dir))
>>> + subdirs
>>> missing)
>> I'd merge these two into a single line.
>>
>>>
>>> ;; In case the package was installed directly from source, the
>>> @@ -549,30 +550,29 @@ package-vc--unpack-1
>>> (plist-get pkg-spec :ignored-files)
>>> "\\|")
>>> regexp-unmatchable))
>>> - (deps '()))
>>> + deps)
>> And I wouldn't revert this change. Perhaps this is just personal
>> preference, but I prefer to be explicit when binding nil or '() when
>> initialising a variable. I think about it like in Scheme, where an
>> undefined value, a boolean or an empty list are all distinct.
>>
>>> (dolist (file (directory-files lisp-dir t "\\.el\\'" t))
>>> (unless (string-match-p ignored-files file)
>>> - (with-temp-buffer
>>> - (insert-file-contents file)
>>> - (when-let* ((require-lines (lm-header-multiline
>>> "package-requires")))
>>> - (setq deps
>>> - (nconc deps
>>> - (lm--prepare-package-dependencies
>>> - (package-read-from-string
>>> - (mapconcat (function identity)
>>> - require-lines " ")))))))))
>>> + (setq deps (nconc deps (lm-package-requires file)))
>>> + (setq subdirs (nconc subdirs (lm-package-subdirectories file)))))
>>> + (dolist (file (mapcan (lambda (dir)
>>> + (directory-files
>>> + (expand-file-name dir lisp-dir) t
>>> "\\.el\\'" t))
>>> + subdirs))
>>> + (unless (string-match-p ignored-files file)
>>> + (setq deps (nconc deps (lm-package-requires file)))))
>>> + (setq deps (assq-delete-all (package-desc-name pkg-desc)
>>> (delete-dups deps)))
>>> (dolist (dep deps)
>>> (cl-callf version-to-list (cadr dep)))
>>> (setf (package-desc-reqs pkg-desc) deps)
>>> - (setf missing (package-vc-install-dependencies (delete-dups deps)))
>>> + (setf missing (package-vc-install-dependencies deps))
>>> (setf missing (delq (assq (package-desc-name pkg-desc)
>>> missing)
>>> missing)))
>>>
>>> ;; Generate autoloads
>>> (let* ((name (package-desc-name pkg-desc))
>>> - (auto-name (format "%s-autoloads.el" name)))
>>> - (package-generate-autoloads name lisp-dir)
>>> + (auto-name (package-generate-autoloads name lisp-dir subdirs)))
>>> ;; There are two cases when we wish to "indirect" the loading of
>>> ;; autoload files:
>>> ;;
>>> @@ -777,7 +777,7 @@ package-vc--unpack
>>> (user-error "Installation aborted")))
>>>
>>> ;; Ensure we have a copy of the package specification
>>> - (when (null (package-vc--desc->spec pkg-desc name))
>>> + (unless (equal (package-vc--desc->spec pkg-desc name) pkg-spec)
>>> (customize-save-variable
>>> 'package-vc-selected-packages
>>> (cons (cons name pkg-spec)
>>>
>>> Some explanations and design choices are in order. I have introduced a
>>> new function to extract the 'Package-Subdirectories' header, named
>>> 'lm-package-subdirectories', in line with the other "lm-package-..."
>>> functions. It uses the same logic as lm-package-requires, extracting the
>>> header with lm-header-multiline. The package-generate-autoloads logic
>>> remains unchanged from the previous patch.
>>>
>>> There are multiple changes in package-vc--unpack-1. First, the previous
>>> (with-temp-buffer ...) logic for collecting package-requires was
>>> essentially a duplication of the lm-package-requires function, so I have
>>> removed that code and used lm-package-requires instead. In the same
>>> loop, Package-Subdirectories headers are being collected. This assumes
>>> that the headers should be in the .el files of lisp-dir (following your
>>> logic of scanning all '.el' files, rather than relying on a single
>>> mainfile). Then, with the collected list of subdirectories, I am
>>> scanning through each of their .el files to collect other
>>> package-requires.
>>>
>>> Since dependencies are installed before the VC package itself, all
>>> mentions of the package itself as a dependency is removed. I am also
>>> deleting duplicate dependencies here rather than waiting for the
>>> delete-dups call here: (setf missing (package-vc-install-dependencies
>>> (delete-dups deps))). This is because we are setting package-desc-reqs
>>> to deps, where duplicates and self-references don't make any sense, nor
>>> does calling version-to-list on the duplicates.
>>>
>>> Lastly, in package-vc--unpack, I have changed the when null check to
>>> unless equal. I have raised the point in bug#80015, but in case you have
>>> missed that patch, here is a summary. In commit
>>> 573acd97e54ceead6d11b330909ffb8e744247cc this change was made:
>>>
>>> - (unless (seq-some (lambda (alist) (equal (alist-get name (cdr
>>> alist)) pkg-spec))
>>> - package-vc--archive-spec-alists)
>>> + (when (null (package-vc--desc->spec pkg-desc name))
>>>
>>> I believe that the logic here is faulty - we have to check if the user
>>> provided pkg-spec can be found, and only then should the block be
>>> ignored. Otherwise, for most (all?) packages, since they do have a
>>> pkg-spec in the archives (MELPA, ELPA, etc.), any user-provided pkg-spec
>>> will effectively be ignored. We should check for equality to ensure the
>>> user specified pkg-spec is found, rather than checking for the existence
>>> of any pkg-spec.
>>>
>>> However, this brings forth a interesting question: suppose in the
>>> archives the authors ignores a file, and when installing from VC the
>>> user ignores some more files - shouldn't these lists be merged? Maybe we
>>> should use the same logic as the subdirectories, where we "embed" the
>>> author-specified ignored-files directly in the code itself, possibly
>>> with another header?
>> This logic sounds reasonable, I would appreciate it if you could
>> prepare
>> the patches for the suggestions you made.
>
> For the ignored-files, which solution sounds better though? I am a bit
> reluctant towards the merging solution, because then there is always an
> inherent dependency on the archives. However, I am also not sure whether
> introducing another header for ignored-files is a good idea. Let me know
> which idea sounds better to you. Or should we just leave this as-is?
Why would we have another header for ignored files? That is what the
.elpaignore file is for? What I meant was that the merging solution
sounds reasonable.
>>> BTW, I have removed the lisp-subdirs keyword from the first diff, but I
>>> have included a second diff for that feature, since I believe it may be
>>> necessary for certain packages. For example, treemacs has a number of
>>> "extra" packages, which are not packed with treemacs itself, but rather
>>> each of these files are a package unto itself. For example, this is the
>>> recipe for one such "extra" package:
>>>
>>> (treemacs-all-the-icons
>>> :fetcher github
>>> :repo "Alexander-Miller/treemacs"
>>> :files ("src/extra/treemacs-all-the-icons.el"))
>>>
>>> In the main treemacs recipe, however, src/extra/* is explicitly
>>> excluded. Since we don't support treating a single file in a VC package
>>> as a full-package (and I don't think we should), and for treemacs
>>> package maintainers, it makes no sense to include the "extras" directory
>>> in Package-Subdirectories header, the user is stuck with installing
>>> those extra packages from the archives, unless we give them a way to
>>> include other subdirectories themselves. They can then prune the
>>> unneeded "extras" files with ignored-files. Anyways, here is the patch
>>> that keeps lisp-subdirs keyword too:
>>>
>>> diff --git a/doc/emacs/package.texi b/doc/emacs/package.texi
>>> index 53b339bd606..a66ed9b1e78 100644
>>> --- a/doc/emacs/package.texi
>>> +++ b/doc/emacs/package.texi
>>> @@ -736,6 +736,14 @@ Fetching Package Sources
>>> use for loading the Lisp sources, which defaults to the root directory
>>> of the repository.
>>>
>>> +@item :lisp-subdirs
>>> +A string or a list of strings providing the names of subdirectories
>>> +containing additional Lisp sources. These names are interpreted
>>> +relative to the directory specified by @code{:lisp-dir} (or the root
>>> +directory of the repository if @code{:lisp-dir} is unspecified). The
>>> +specified subdirectories are added to the load path and scanned for
>>> +library dependencies and autoloads.
>>> +
>>> @item :main-file
>>> A string providing the main file of the project, from which to gather
>>> package metadata. If not given, the default is the package name with
>>>
>>> diff --git a/lisp/package/package-vc.el b/lisp/package/package-vc.el
>>> index d7d6f4ea586..2b5e984fb40 100644
>>> --- a/lisp/package/package-vc.el
>>> +++ b/lisp/package/package-vc.el
>>> @@ -533,7 +533,7 @@ package-vc--unpack-1
>>> ;; Directory where package's Lisp code resides. It may be
>>> ;; equal to `checkout-dir' or be a subdirectory of it.
>>> (lisp-dir (package-vc--checkout-dir pkg-desc 'lisp-dir))
>>> - subdirs
>>> + (subdirs (ensure-list (plist-get pkg-spec :lisp-subdirs)))
>>> missing)
>>>
>>> ;; In case the package was installed directly from source, the
>>>
>>> diff --git a/lisp/use-package/use-package-core.el
>>> b/lisp/use-package/use-package-core.el
>>> index 0f049a6f86b..a6c92d663cc 100644
>>> --- a/lisp/use-package/use-package-core.el
>>> +++ b/lisp/use-package/use-package-core.el
>>> @@ -1735,7 +1735,7 @@ use-package-handler/:vc
>>> body))
>>>
>>> (defconst use-package-vc-valid-keywords
>>> - '( :url :branch :lisp-dir :main-file :vc-backend :rev
>>> + '( :url :branch :lisp-dir :lisp-subdirs :main-file :vc-backend :rev
>>> :shell-command :make :ignored-files)
>>> "Valid keywords for the `:vc' keyword.
>>> See Info node `(emacs)Fetching Package Sources'.")
>> I remain disinclined towards this point. Generally we don't have
>> the
>> ambition to support any kind of package layout. On ELPA, we have the
>> policy that one repository branch corresponds to one package, and
>> package-vc uses this fact by checking out the entire repository. Am I
>> understanding correctly that with the approach you suggest package-vc
>> would check out the repository multiple times over?
>
> No, this will not check out the repository multiple times. I think I
> should have clarified the purpose of this lisp-subdirs keyword in a
> better way. Let's consider treemacs as an example. The author has a
> number of extra related packages in the aptly named extras directory of
> the root git directory. Now, the author doesn't package this extras
> directory with the treemacs package. Instead, they opt to package the
> files in extras separately. So, the author would not want to put the
> extras directory in the Package-Subdirectories header. But a user might.
> When the user uses use-package with vc, the whole git directory for
> treemacs is cloned, along with the extras directory. My opinion is: in
> cases like this, the user might want to load the extras directory too,
> but without the lisp-subdirs keyword, the only option the user has is to
> patch treemacs itself and include the "extras" directory in
> Package-Subdirectories. Instead, this keyword just provides the user the
> option to include some directories as subdirs too, even if the author
> decides that they are not. I used treemacs as an example, but there may
> be many other packages like that, where the author decides some
> directory to not be part of the package, but as auxiliary ones, so they
> don't package them together as a single tarball. But since the user is
> cloning the whole git directory, the user may want to include the
> subdirectory(ies). For this sole purpose, I was thinking of keeping
> lisp-subdirs. However, if ELPA doesn't have the capability to do what I
> mentioned, and if this is just MELPA specific, I guess we may not
> include this lisp-subdirs keyword.
OK, thank you for the clarification but as mentioned before, I don't
want to add keys to package specifications that don't also make sense
for elpa-admin. I would prefer it if we could first figure out what it
would the proper way to add support for this during the creation of
tarballs would be, before we consider it here.
I am also not interested in complications for a package that is not even
on GNU or NonGNU ELPA. My go-to recommendation for related sub-packages
has been to use Git branches.
>>> Lastly - and this is slightly off-topic - is there a reason we use regex
>>> to detect ignored-files? Why is it not something like this:
>>>
>>> diff --git a/lisp/package/package-vc.el b/lisp/package/package-vc.el
>>> index 2b5e984fb40..dad9e1037c9 100644
>>> --- a/lisp/package/package-vc.el
>>> +++ b/lisp/package/package-vc.el
>>> @@ -540,26 +540,19 @@ package-vc--unpack-1
>>> ;; dependency list wasn't know beforehand, and they might have
>>> ;; to be installed explicitly.
>>> (let ((ignored-files
>>> - (if (plist-get pkg-spec :ignored-files)
>>> - (mapconcat
>>> - (lambda (ignore)
>>> - (wildcard-to-regexp
>>> - (if (string-match-p "\\`/" ignore)
>>> - (concat checkout-dir ignore)
>>> - (concat "*/" ignore))))
>>> - (plist-get pkg-spec :ignored-files)
>>> - "\\|")
>>> - regexp-unmatchable))
>>> + (mapcar (lambda (file)
>>> + (expand-file-name file lisp-dir))
>>> + (plist-get pkg-spec :ignored-files)))
>>> deps)
>>> (dolist (file (directory-files lisp-dir t "\\.el\\'" t))
>>> - (unless (string-match-p ignored-files file)
>>> + (unless (member file ignored-files)
>>> (setq deps (nconc deps (lm-package-requires file)))
>>> (setq subdirs (nconc subdirs (lm-package-subdirectories
>>> file)))))
>>> (dolist (file (mapcan (lambda (dir)
>>> (directory-files
>>> (expand-file-name dir lisp-dir) t
>>> "\\.el\\'" t))
>>> subdirs))
>>> - (unless (string-match-p ignored-files file)
>>> + (unless (member file ignored-files)
>>> (setq deps (nconc deps (lm-package-requires file)))))
>>> (setq deps (assq-delete-all (package-desc-name pkg-desc)
>>> (delete-dups deps)))
>>> (dolist (dep deps)
>>>
>>> This seems to be more robust, but I'm unsure whether it'd introduce any
>>> performance bottlenecks.
>> Performance is the main reason. But your suggestion doesn't handle
>> glob-expressions in file names, which the previous implementation does
>> due to `wildcard-to-regexp'.
>
> Ah OK, I missed the glob-expression part. And yeah, I had a fleeting
> suspicion that there might be a performance issue. Anyways, yeah, this
> patch was unnecessary, a bit of curiosity from my part, nothing else.
No problem :)
So what is the conclusion? You wanted to prepare a patch that would
make package.el and package-vc.el lookup a special header to indicate
what subdirectories of the checkout also contain Lisp files.
bug-gnu-emacs <at> gnu.org:bug#80016; Package emacs.
(Wed, 31 Dec 2025 00:31:01 GMT) Full text and rfc822 format available.Message #38 received at 80016 <at> debbugs.gnu.org (full text, mbox):
From: Aritro Sen <1.sen.aritro <at> gmail.com> To: Philip Kaludercic <philipk <at> posteo.net> Cc: 80016 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org> Subject: Re: bug#80016: [PATCH] Load lisp sub-directories in addition to the lisp-dir Date: Wed, 31 Dec 2025 06:00:23 +0530
On 12/30/25 3:21 PM, Philip Kaludercic wrote:
> Aritro Sen <1.sen.aritro <at> gmail.com> writes:
>
>> On 12/28/25 1:18 AM, Philip Kaludercic wrote:
>>> Aritro Sen <1.sen.aritro <at> gmail.com> writes:
>>> [...]
>>>
>>>>>> Btw, I was thinking of a different approach with an lm-header
>>>>>> 'Package-Subdirectories' or something like that. I also think we do not
>>>>>> need to scan all the .el files in pkg-dir either. Instead, we can opt
>>>>>> for the package-get-version solution of only scanning the 'mainfile',
>>>>>> i.e., (concat pkgname ".el") for the lm-header. This 'mainfile' should
>>>>>> presumably be in the lisp-dir of pkg-dir, and all the other
>>>>>> sub-directories mentioned should be relative to this lisp-dir. This way,
>>>>>> we can extract the header line in package-vc--unpack-1, and then call
>>>>>> 'package-generate-autoloads' with the directories as an optional
>>>>>> parameter to avoid double computation. What do you think of this
>>>>>> approach?
>>>>>
>>>>> I was thinking about something like this as well after sending the patch
>>>>> a few days ago, but I haven't thought too much about it since. The main
>>>>> issues is that the main file is not always a reliable heuristic and
>>>>> (much less important) that the header can become long, but package
>>>>> maintainers should be able to split that if the format of
>>>>> 'Package-Subdirectories' uses a list syntax like 'Package-Requires'.
>>>>>
>>>>> Do you want to take a stab at it and combine the two approaches?
>>>>> Otherwise it shouldn't be difficult to address the issue you raised
>>>>> within 'package-vc--unpack-1' based on my proposal.
>>>>>
>>>>> Also, I am working on a refactoring of package.el on the
>>>>> feature/package-refactor, and am adding some features on top of that
>>>>> branch in feature/package-revamp. If you could prepare the patch on top
>>>>> of the latter, that would be great, because that would avoid merge
>>>>> conflicts in the future.
>>>>
>>>> This is the revised patch on the feature/package-refactor branch:
>>>>
>>>> diff --git a/lisp/emacs-lisp/lisp-mnt.el b/lisp/emacs-lisp/lisp-mnt.el
>>>> index 111d512ef59..169bbbd7f0a 100644
>>>> --- a/lisp/emacs-lisp/lisp-mnt.el
>>>> +++ b/lisp/emacs-lisp/lisp-mnt.el
>>>> @@ -468,6 +468,13 @@ lm-package-requires
>>>> (lm--prepare-package-dependencies
>>>> (package-read-from-string (mapconcat #'identity require-lines "
>>>> "))))))
>>>>
>>>> +(defun lm-package-subdirectories (&optional file)
>>>> + "Return \"Package-Subdirectories\" header."
>>>> + (require 'package)
>>>> + (lm-with-file file
>>>> + (and-let* ((subdir-lines (lm-header-multiline
>>>> "package-subdirectories")))
>>>> + (package-read-from-string (mapconcat #'identity subdir-lines " ")))))
>>>> +
>>>> (defun lm-package-version (&optional file)
>>>> "Return \"Package-Version\" or \"Version\" header.
>>>> Prefer Package-Version; if defined, the package author
>>>>
>>>> diff --git a/lisp/package/package-install.el
>>>> b/lisp/package/package-install.el
>>>> index cec08e60ac3..eee39f79899 100644
>>>> --- a/lisp/package/package-install.el
>>>> +++ b/lisp/package/package-install.el
>>>> @@ -976,29 +976,40 @@ package-autoload-ensure-default-file
>>>> (defvar autoload-timestamps)
>>>> (defvar version-control)
>>>>
>>>> -(defun package-generate-autoloads (name pkg-dir)
>>>> - "Generate autoloads in PKG-DIR for package named NAME."
>>>> +(defun package-generate-autoloads (name pkg-dir &optional pkg-subdirs)
>>>> + "Generate autoloads in PKG-DIR for package named NAME.
>>>> +If LISP-SUBDIRS is provided, it should be a list of subdirectory names
>>>> +relative to PKG-DIR."
>>>> (let* ((auto-name (format "%s-autoloads.el" name))
>>>> ;;(ignore-name (concat name "-pkg.el"))
>>>> (output-file (expand-file-name auto-name pkg-dir))
>>>> + (all-dirs (cons pkg-dir
>>>> + (mapcar
>>>> + (lambda (dir) (expand-file-name dir pkg-dir))
>>>> + pkg-subdirs)))
>>>> ;; We don't need 'em, and this makes the output reproducible.
>>>> (autoload-timestamps nil)
>>>> (backup-inhibited t)
>>>> (version-control 'never))
>>>> (loaddefs-generate
>>>> - pkg-dir output-file nil
>>>> + all-dirs output-file nil
>>>> (prin1-to-string
>>>> - '(add-to-list
>>>> - 'load-path
>>>> - ;; Add the directory that will contain the autoload file to
>>>> - ;; the load path. We don't hard-code `pkg-dir', to avoid
>>>> - ;; issues if the package directory is moved around.
>>>> - ;; `loaddefs-generate' has code to do this for us, but it's
>>>> - ;; not currently exposed. (Bug#63625)
>>>> - (or (and load-file-name
>>>> - (directory-file-name
>>>> - (file-name-directory load-file-name)))
>>>> - (car load-path)))))
>>>> + `(let ((dir (or (and load-file-name
>>>> + (directory-file-name
>>>> + (file-name-directory load-file-name)))
>>>> + (car load-path))))
>>>> + ;; Add the directory that will contain the autoload file to
>>>> + ;; the load path. We don't hard-code `pkg-dir', to avoid
>>>> + ;; issues if the package directory is moved around.
>>>> + ;; `loaddefs-generate' has code to do this for us, but it's
>>>> + ;; not currently exposed. (Bug#63625)
>>>> + (add-to-list 'load-path dir)
>>>> + ,@(mapcar
>>>> + (lambda (subdir)
>>>> + `(add-to-list
>>>> + 'load-path
>>>> + (expand-file-name ,subdir dir)))
>>>> + pkg-subdirs))))
>>>> (let ((buf (find-buffer-visiting output-file)))
>>>> (when buf (kill-buffer buf)))
>>>> auto-name))
>>>>
>>>> diff --git a/lisp/package/package-vc.el b/lisp/package/package-vc.el
>>>> index d6b84d896f9..d7d6f4ea586 100644
>>>> --- a/lisp/package/package-vc.el
>>>> +++ b/lisp/package/package-vc.el
>>>> @@ -533,6 +533,7 @@ package-vc--unpack-1
>>>> ;; Directory where package's Lisp code resides. It may be
>>>> ;; equal to `checkout-dir' or be a subdirectory of it.
>>>> (lisp-dir (package-vc--checkout-dir pkg-desc 'lisp-dir))
>>>> + subdirs
>>>> missing)
>>> I'd merge these two into a single line.
>>>
>>>>
>>>> ;; In case the package was installed directly from source, the
>>>> @@ -549,30 +550,29 @@ package-vc--unpack-1
>>>> (plist-get pkg-spec :ignored-files)
>>>> "\\|")
>>>> regexp-unmatchable))
>>>> - (deps '()))
>>>> + deps)
>>> And I wouldn't revert this change. Perhaps this is just personal
>>> preference, but I prefer to be explicit when binding nil or '() when
>>> initialising a variable. I think about it like in Scheme, where an
>>> undefined value, a boolean or an empty list are all distinct.
>>>
>>>> (dolist (file (directory-files lisp-dir t "\\.el\\'" t))
>>>> (unless (string-match-p ignored-files file)
>>>> - (with-temp-buffer
>>>> - (insert-file-contents file)
>>>> - (when-let* ((require-lines (lm-header-multiline
>>>> "package-requires")))
>>>> - (setq deps
>>>> - (nconc deps
>>>> - (lm--prepare-package-dependencies
>>>> - (package-read-from-string
>>>> - (mapconcat (function identity)
>>>> - require-lines " ")))))))))
>>>> + (setq deps (nconc deps (lm-package-requires file)))
>>>> + (setq subdirs (nconc subdirs (lm-package-subdirectories file)))))
>>>> + (dolist (file (mapcan (lambda (dir)
>>>> + (directory-files
>>>> + (expand-file-name dir lisp-dir) t
>>>> "\\.el\\'" t))
>>>> + subdirs))
>>>> + (unless (string-match-p ignored-files file)
>>>> + (setq deps (nconc deps (lm-package-requires file)))))
>>>> + (setq deps (assq-delete-all (package-desc-name pkg-desc)
>>>> (delete-dups deps)))
>>>> (dolist (dep deps)
>>>> (cl-callf version-to-list (cadr dep)))
>>>> (setf (package-desc-reqs pkg-desc) deps)
>>>> - (setf missing (package-vc-install-dependencies (delete-dups deps)))
>>>> + (setf missing (package-vc-install-dependencies deps))
>>>> (setf missing (delq (assq (package-desc-name pkg-desc)
>>>> missing)
>>>> missing)))
>>>>
>>>> ;; Generate autoloads
>>>> (let* ((name (package-desc-name pkg-desc))
>>>> - (auto-name (format "%s-autoloads.el" name)))
>>>> - (package-generate-autoloads name lisp-dir)
>>>> + (auto-name (package-generate-autoloads name lisp-dir subdirs)))
>>>> ;; There are two cases when we wish to "indirect" the loading of
>>>> ;; autoload files:
>>>> ;;
>>>> @@ -777,7 +777,7 @@ package-vc--unpack
>>>> (user-error "Installation aborted")))
>>>>
>>>> ;; Ensure we have a copy of the package specification
>>>> - (when (null (package-vc--desc->spec pkg-desc name))
>>>> + (unless (equal (package-vc--desc->spec pkg-desc name) pkg-spec)
>>>> (customize-save-variable
>>>> 'package-vc-selected-packages
>>>> (cons (cons name pkg-spec)
>>>>
>>>> Some explanations and design choices are in order. I have introduced a
>>>> new function to extract the 'Package-Subdirectories' header, named
>>>> 'lm-package-subdirectories', in line with the other "lm-package-..."
>>>> functions. It uses the same logic as lm-package-requires, extracting the
>>>> header with lm-header-multiline. The package-generate-autoloads logic
>>>> remains unchanged from the previous patch.
>>>>
>>>> There are multiple changes in package-vc--unpack-1. First, the previous
>>>> (with-temp-buffer ...) logic for collecting package-requires was
>>>> essentially a duplication of the lm-package-requires function, so I have
>>>> removed that code and used lm-package-requires instead. In the same
>>>> loop, Package-Subdirectories headers are being collected. This assumes
>>>> that the headers should be in the .el files of lisp-dir (following your
>>>> logic of scanning all '.el' files, rather than relying on a single
>>>> mainfile). Then, with the collected list of subdirectories, I am
>>>> scanning through each of their .el files to collect other
>>>> package-requires.
>>>>
>>>> Since dependencies are installed before the VC package itself, all
>>>> mentions of the package itself as a dependency is removed. I am also
>>>> deleting duplicate dependencies here rather than waiting for the
>>>> delete-dups call here: (setf missing (package-vc-install-dependencies
>>>> (delete-dups deps))). This is because we are setting package-desc-reqs
>>>> to deps, where duplicates and self-references don't make any sense, nor
>>>> does calling version-to-list on the duplicates.
>>>>
>>>> Lastly, in package-vc--unpack, I have changed the when null check to
>>>> unless equal. I have raised the point in bug#80015, but in case you have
>>>> missed that patch, here is a summary. In commit
>>>> 573acd97e54ceead6d11b330909ffb8e744247cc this change was made:
>>>>
>>>> - (unless (seq-some (lambda (alist) (equal (alist-get name (cdr
>>>> alist)) pkg-spec))
>>>> - package-vc--archive-spec-alists)
>>>> + (when (null (package-vc--desc->spec pkg-desc name))
>>>>
>>>> I believe that the logic here is faulty - we have to check if the user
>>>> provided pkg-spec can be found, and only then should the block be
>>>> ignored. Otherwise, for most (all?) packages, since they do have a
>>>> pkg-spec in the archives (MELPA, ELPA, etc.), any user-provided pkg-spec
>>>> will effectively be ignored. We should check for equality to ensure the
>>>> user specified pkg-spec is found, rather than checking for the existence
>>>> of any pkg-spec.
>>>>
>>>> However, this brings forth a interesting question: suppose in the
>>>> archives the authors ignores a file, and when installing from VC the
>>>> user ignores some more files - shouldn't these lists be merged? Maybe we
>>>> should use the same logic as the subdirectories, where we "embed" the
>>>> author-specified ignored-files directly in the code itself, possibly
>>>> with another header?
>>> This logic sounds reasonable, I would appreciate it if you could
>>> prepare
>>> the patches for the suggestions you made.
>>
>> For the ignored-files, which solution sounds better though? I am a bit
>> reluctant towards the merging solution, because then there is always an
>> inherent dependency on the archives. However, I am also not sure whether
>> introducing another header for ignored-files is a good idea. Let me know
>> which idea sounds better to you. Or should we just leave this as-is?
>
> Why would we have another header for ignored files? That is what the
> .elpaignore file is for? What I meant was that the merging solution
> sounds reasonable.
I have no idea how I missed the fact that .elpaignore is, in fact, the
list of ignored-files. Thanks for the heads up!
>
>>>> BTW, I have removed the lisp-subdirs keyword from the first diff, but I
>>>> have included a second diff for that feature, since I believe it may be
>>>> necessary for certain packages. For example, treemacs has a number of
>>>> "extra" packages, which are not packed with treemacs itself, but rather
>>>> each of these files are a package unto itself. For example, this is the
>>>> recipe for one such "extra" package:
>>>>
>>>> (treemacs-all-the-icons
>>>> :fetcher github
>>>> :repo "Alexander-Miller/treemacs"
>>>> :files ("src/extra/treemacs-all-the-icons.el"))
>>>>
>>>> In the main treemacs recipe, however, src/extra/* is explicitly
>>>> excluded. Since we don't support treating a single file in a VC package
>>>> as a full-package (and I don't think we should), and for treemacs
>>>> package maintainers, it makes no sense to include the "extras" directory
>>>> in Package-Subdirectories header, the user is stuck with installing
>>>> those extra packages from the archives, unless we give them a way to
>>>> include other subdirectories themselves. They can then prune the
>>>> unneeded "extras" files with ignored-files. Anyways, here is the patch
>>>> that keeps lisp-subdirs keyword too:
>>>>
>>>> diff --git a/doc/emacs/package.texi b/doc/emacs/package.texi
>>>> index 53b339bd606..a66ed9b1e78 100644
>>>> --- a/doc/emacs/package.texi
>>>> +++ b/doc/emacs/package.texi
>>>> @@ -736,6 +736,14 @@ Fetching Package Sources
>>>> use for loading the Lisp sources, which defaults to the root directory
>>>> of the repository.
>>>>
>>>> +@item :lisp-subdirs
>>>> +A string or a list of strings providing the names of subdirectories
>>>> +containing additional Lisp sources. These names are interpreted
>>>> +relative to the directory specified by @code{:lisp-dir} (or the root
>>>> +directory of the repository if @code{:lisp-dir} is unspecified). The
>>>> +specified subdirectories are added to the load path and scanned for
>>>> +library dependencies and autoloads.
>>>> +
>>>> @item :main-file
>>>> A string providing the main file of the project, from which to gather
>>>> package metadata. If not given, the default is the package name with
>>>>
>>>> diff --git a/lisp/package/package-vc.el b/lisp/package/package-vc.el
>>>> index d7d6f4ea586..2b5e984fb40 100644
>>>> --- a/lisp/package/package-vc.el
>>>> +++ b/lisp/package/package-vc.el
>>>> @@ -533,7 +533,7 @@ package-vc--unpack-1
>>>> ;; Directory where package's Lisp code resides. It may be
>>>> ;; equal to `checkout-dir' or be a subdirectory of it.
>>>> (lisp-dir (package-vc--checkout-dir pkg-desc 'lisp-dir))
>>>> - subdirs
>>>> + (subdirs (ensure-list (plist-get pkg-spec :lisp-subdirs)))
>>>> missing)
>>>>
>>>> ;; In case the package was installed directly from source, the
>>>>
>>>> diff --git a/lisp/use-package/use-package-core.el
>>>> b/lisp/use-package/use-package-core.el
>>>> index 0f049a6f86b..a6c92d663cc 100644
>>>> --- a/lisp/use-package/use-package-core.el
>>>> +++ b/lisp/use-package/use-package-core.el
>>>> @@ -1735,7 +1735,7 @@ use-package-handler/:vc
>>>> body))
>>>>
>>>> (defconst use-package-vc-valid-keywords
>>>> - '( :url :branch :lisp-dir :main-file :vc-backend :rev
>>>> + '( :url :branch :lisp-dir :lisp-subdirs :main-file :vc-backend :rev
>>>> :shell-command :make :ignored-files)
>>>> "Valid keywords for the `:vc' keyword.
>>>> See Info node `(emacs)Fetching Package Sources'.")
>>> I remain disinclined towards this point. Generally we don't have
>>> the
>>> ambition to support any kind of package layout. On ELPA, we have the
>>> policy that one repository branch corresponds to one package, and
>>> package-vc uses this fact by checking out the entire repository. Am I
>>> understanding correctly that with the approach you suggest package-vc
>>> would check out the repository multiple times over?
>>
>> No, this will not check out the repository multiple times. I think I
>> should have clarified the purpose of this lisp-subdirs keyword in a
>> better way. Let's consider treemacs as an example. The author has a
>> number of extra related packages in the aptly named extras directory of
>> the root git directory. Now, the author doesn't package this extras
>> directory with the treemacs package. Instead, they opt to package the
>> files in extras separately. So, the author would not want to put the
>> extras directory in the Package-Subdirectories header. But a user might.
>> When the user uses use-package with vc, the whole git directory for
>> treemacs is cloned, along with the extras directory. My opinion is: in
>> cases like this, the user might want to load the extras directory too,
>> but without the lisp-subdirs keyword, the only option the user has is to
>> patch treemacs itself and include the "extras" directory in
>> Package-Subdirectories. Instead, this keyword just provides the user the
>> option to include some directories as subdirs too, even if the author
>> decides that they are not. I used treemacs as an example, but there may
>> be many other packages like that, where the author decides some
>> directory to not be part of the package, but as auxiliary ones, so they
>> don't package them together as a single tarball. But since the user is
>> cloning the whole git directory, the user may want to include the
>> subdirectory(ies). For this sole purpose, I was thinking of keeping
>> lisp-subdirs. However, if ELPA doesn't have the capability to do what I
>> mentioned, and if this is just MELPA specific, I guess we may not
>> include this lisp-subdirs keyword.
>
> OK, thank you for the clarification but as mentioned before, I don't
> want to add keys to package specifications that don't also make sense
> for elpa-admin. I would prefer it if we could first figure out what it
> would the proper way to add support for this during the creation of
> tarballs would be, before we consider it here.
>
> I am also not interested in complications for a package that is not even
> on GNU or NonGNU ELPA. My go-to recommendation for related sub-packages
> has been to use Git branches.
Ah OK. Truth be told, I don't use treemacs either, so this was based on
hypothetical situations, and I agree that we should be more in line with
ELPA than MELPA.
>
>>>> Lastly - and this is slightly off-topic - is there a reason we use regex
>>>> to detect ignored-files? Why is it not something like this:
>>>>
>>>> diff --git a/lisp/package/package-vc.el b/lisp/package/package-vc.el
>>>> index 2b5e984fb40..dad9e1037c9 100644
>>>> --- a/lisp/package/package-vc.el
>>>> +++ b/lisp/package/package-vc.el
>>>> @@ -540,26 +540,19 @@ package-vc--unpack-1
>>>> ;; dependency list wasn't know beforehand, and they might have
>>>> ;; to be installed explicitly.
>>>> (let ((ignored-files
>>>> - (if (plist-get pkg-spec :ignored-files)
>>>> - (mapconcat
>>>> - (lambda (ignore)
>>>> - (wildcard-to-regexp
>>>> - (if (string-match-p "\\`/" ignore)
>>>> - (concat checkout-dir ignore)
>>>> - (concat "*/" ignore))))
>>>> - (plist-get pkg-spec :ignored-files)
>>>> - "\\|")
>>>> - regexp-unmatchable))
>>>> + (mapcar (lambda (file)
>>>> + (expand-file-name file lisp-dir))
>>>> + (plist-get pkg-spec :ignored-files)))
>>>> deps)
>>>> (dolist (file (directory-files lisp-dir t "\\.el\\'" t))
>>>> - (unless (string-match-p ignored-files file)
>>>> + (unless (member file ignored-files)
>>>> (setq deps (nconc deps (lm-package-requires file)))
>>>> (setq subdirs (nconc subdirs (lm-package-subdirectories
>>>> file)))))
>>>> (dolist (file (mapcan (lambda (dir)
>>>> (directory-files
>>>> (expand-file-name dir lisp-dir) t
>>>> "\\.el\\'" t))
>>>> subdirs))
>>>> - (unless (string-match-p ignored-files file)
>>>> + (unless (member file ignored-files)
>>>> (setq deps (nconc deps (lm-package-requires file)))))
>>>> (setq deps (assq-delete-all (package-desc-name pkg-desc)
>>>> (delete-dups deps)))
>>>> (dolist (dep deps)
>>>>
>>>> This seems to be more robust, but I'm unsure whether it'd introduce any
>>>> performance bottlenecks.
>>> Performance is the main reason. But your suggestion doesn't handle
>>> glob-expressions in file names, which the previous implementation does
>>> due to `wildcard-to-regexp'.
>>
>> Ah OK, I missed the glob-expression part. And yeah, I had a fleeting
>> suspicion that there might be a performance issue. Anyways, yeah, this
>> patch was unnecessary, a bit of curiosity from my part, nothing else.
>
> No problem :)
>
> So what is the conclusion? You wanted to prepare a patch that would
> make package.el and package-vc.el lookup a special header to indicate
> what subdirectories of the checkout also contain Lisp files.
Anyways, this is the patch so far, based on our discussion:
diff --git a/lisp/emacs-lisp/lisp-mnt.el b/lisp/emacs-lisp/lisp-mnt.el
index 111d512ef59..169bbbd7f0a 100644
--- a/lisp/emacs-lisp/lisp-mnt.el
+++ b/lisp/emacs-lisp/lisp-mnt.el
@@ -468,6 +468,13 @@ lm-package-requires
(lm--prepare-package-dependencies
(package-read-from-string (mapconcat #'identity require-lines "
"))))))
+(defun lm-package-subdirectories (&optional file)
+ "Return \"Package-Subdirectories\" header."
+ (require 'package)
+ (lm-with-file file
+ (and-let* ((subdir-lines (lm-header-multiline
"package-subdirectories")))
+ (package-read-from-string (mapconcat #'identity subdir-lines " ")))))
+
(defun lm-package-version (&optional file)
"Return \"Package-Version\" or \"Version\" header.
Prefer Package-Version; if defined, the package author
diff --git a/lisp/package/package-install.el
b/lisp/package/package-install.el
index 74cc767cff7..8dc9fd8dc69 100644
--- a/lisp/package/package-install.el
+++ b/lisp/package/package-install.el
@@ -999,29 +999,40 @@ package-autoload-ensure-default-file
(defvar autoload-timestamps)
(defvar version-control)
-(defun package-generate-autoloads (name pkg-dir)
- "Generate autoloads in PKG-DIR for package named NAME."
+(defun package-generate-autoloads (name pkg-dir &optional pkg-subdirs)
+ "Generate autoloads in PKG-DIR for package named NAME.
+If LISP-SUBDIRS is provided, it should be a list of subdirectory names
+relative to PKG-DIR."
(let* ((auto-name (format "%s-autoloads.el" name))
;;(ignore-name (concat name "-pkg.el"))
(output-file (expand-file-name auto-name pkg-dir))
+ (all-dirs (cons pkg-dir
+ (mapcar
+ (lambda (dir) (expand-file-name dir pkg-dir))
+ pkg-subdirs)))
;; We don't need 'em, and this makes the output reproducible.
(autoload-timestamps nil)
(backup-inhibited t)
(version-control 'never))
(loaddefs-generate
- pkg-dir output-file nil
+ all-dirs output-file nil
(prin1-to-string
- '(add-to-list
- 'load-path
- ;; Add the directory that will contain the autoload file to
- ;; the load path. We don't hard-code `pkg-dir', to avoid
- ;; issues if the package directory is moved around.
- ;; `loaddefs-generate' has code to do this for us, but it's
- ;; not currently exposed. (Bug#63625)
- (or (and load-file-name
- (directory-file-name
- (file-name-directory load-file-name)))
- (car load-path)))))
+ `(let ((dir (or (and load-file-name
+ (directory-file-name
+ (file-name-directory load-file-name)))
+ (car load-path))))
+ ;; Add the directory that will contain the autoload file to
+ ;; the load path. We don't hard-code `pkg-dir', to avoid
+ ;; issues if the package directory is moved around.
+ ;; `loaddefs-generate' has code to do this for us, but it's
+ ;; not currently exposed. (Bug#63625)
+ (add-to-list 'load-path dir)
+ ,@(mapcar
+ (lambda (subdir)
+ `(add-to-list
+ 'load-path
+ (expand-file-name ,subdir dir)))
+ pkg-subdirs))))
(let ((buf (find-buffer-visiting output-file)))
(when buf (kill-buffer buf)))
auto-name))
diff --git a/lisp/package/package-vc.el b/lisp/package/package-vc.el
index d6b84d896f9..29de8769bde 100644
--- a/lisp/package/package-vc.el
+++ b/lisp/package/package-vc.el
@@ -533,46 +533,46 @@ package-vc--unpack-1
;; Directory where package's Lisp code resides. It may be
;; equal to `checkout-dir' or be a subdirectory of it.
(lisp-dir (package-vc--checkout-dir pkg-desc 'lisp-dir))
- missing)
+ subdirs missing)
;; In case the package was installed directly from source, the
;; dependency list wasn't know beforehand, and they might have
;; to be installed explicitly.
(let ((ignored-files
- (if (plist-get pkg-spec :ignored-files)
- (mapconcat
- (lambda (ignore)
- (wildcard-to-regexp
- (if (string-match-p "\\`/" ignore)
- (concat checkout-dir ignore)
- (concat "*/" ignore))))
- (plist-get pkg-spec :ignored-files)
- "\\|")
+ (if-let* ((ignored (append
+ (package--parse-elpaignore pkg-desc)
+ (mapcar
+ (lambda (ignore)
+ (wildcard-to-regexp
+ (if (string-match-p "\\`/" ignore)
+ (concat checkout-dir ignore)
+ (concat "*/" ignore))))
+ (plist-get pkg-spec :ignored-files)))))
+ (string-join ignored "\\|")
regexp-unmatchable))
(deps '()))
(dolist (file (directory-files lisp-dir t "\\.el\\'" t))
(unless (string-match-p ignored-files file)
- (with-temp-buffer
- (insert-file-contents file)
- (when-let* ((require-lines (lm-header-multiline
"package-requires")))
- (setq deps
- (nconc deps
- (lm--prepare-package-dependencies
- (package-read-from-string
- (mapconcat (function identity)
- require-lines " ")))))))))
+ (setq deps (nconc deps (lm-package-requires file)))
+ (setq subdirs (nconc subdirs (lm-package-subdirectories file)))))
+ (dolist (file (mapcan (lambda (dir)
+ (directory-files
+ (expand-file-name dir lisp-dir) t
"\\.el\\'" t))
+ subdirs))
+ (unless (string-match-p ignored-files file)
+ (setq deps (nconc deps (lm-package-requires file)))))
+ (setq deps (assq-delete-all (package-desc-name pkg-desc)
(delete-dups deps)))
(dolist (dep deps)
(cl-callf version-to-list (cadr dep)))
(setf (package-desc-reqs pkg-desc) deps)
- (setf missing (package-vc-install-dependencies (delete-dups deps)))
+ (setf missing (package-vc-install-dependencies deps))
(setf missing (delq (assq (package-desc-name pkg-desc)
missing)
missing)))
;; Generate autoloads
(let* ((name (package-desc-name pkg-desc))
- (auto-name (format "%s-autoloads.el" name)))
- (package-generate-autoloads name lisp-dir)
+ (auto-name (package-generate-autoloads name lisp-dir subdirs)))
;; There are two cases when we wish to "indirect" the loading of
;; autoload files:
;;
@@ -777,7 +777,7 @@ package-vc--unpack
(user-error "Installation aborted")))
;; Ensure we have a copy of the package specification
- (when (null (package-vc--desc->spec pkg-desc name))
+ (unless (equal (package-vc--desc->spec pkg-desc name) pkg-spec)
(customize-save-variable
'package-vc-selected-packages
(cons (cons name pkg-spec)
I have tested with vertico and corfu so far, and this works perfectly.
bug-gnu-emacs <at> gnu.org:bug#80016; Package emacs.
(Fri, 02 Jan 2026 15:19:02 GMT) Full text and rfc822 format available.Message #41 received at 80016 <at> debbugs.gnu.org (full text, mbox):
From: Philip Kaludercic <philipk <at> posteo.net> To: Aritro Sen <1.sen.aritro <at> gmail.com> Cc: Daniel Mendler <mail <at> daniel-mendler.de>, 80016 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org> Subject: Re: bug#80016: [PATCH] Load lisp sub-directories in addition to the lisp-dir Date: Fri, 02 Jan 2026 15:18:03 +0000
Aritro Sen <1.sen.aritro <at> gmail.com> writes:
> On 12/30/25 3:21 PM, Philip Kaludercic wrote:
>> Aritro Sen <1.sen.aritro <at> gmail.com> writes:
>>
>>> On 12/28/25 1:18 AM, Philip Kaludercic wrote:
>>>> Aritro Sen <1.sen.aritro <at> gmail.com> writes:
>>>> [...]
>>>>
>>>>>>> Btw, I was thinking of a different approach with an lm-header
>>>>>>> 'Package-Subdirectories' or something like that. I also think we do not
>>>>>>> need to scan all the .el files in pkg-dir either. Instead, we can opt
>>>>>>> for the package-get-version solution of only scanning the 'mainfile',
>>>>>>> i.e., (concat pkgname ".el") for the lm-header. This 'mainfile' should
>>>>>>> presumably be in the lisp-dir of pkg-dir, and all the other
>>>>>>> sub-directories mentioned should be relative to this lisp-dir. This way,
>>>>>>> we can extract the header line in package-vc--unpack-1, and then call
>>>>>>> 'package-generate-autoloads' with the directories as an optional
>>>>>>> parameter to avoid double computation. What do you think of this
>>>>>>> approach?
>>>>>>
>>>>>> I was thinking about something like this as well after sending the patch
>>>>>> a few days ago, but I haven't thought too much about it since. The main
>>>>>> issues is that the main file is not always a reliable heuristic and
>>>>>> (much less important) that the header can become long, but package
>>>>>> maintainers should be able to split that if the format of
>>>>>> 'Package-Subdirectories' uses a list syntax like 'Package-Requires'.
>>>>>>
>>>>>> Do you want to take a stab at it and combine the two approaches?
>>>>>> Otherwise it shouldn't be difficult to address the issue you raised
>>>>>> within 'package-vc--unpack-1' based on my proposal.
>>>>>>
>>>>>> Also, I am working on a refactoring of package.el on the
>>>>>> feature/package-refactor, and am adding some features on top of that
>>>>>> branch in feature/package-revamp. If you could prepare the patch on top
>>>>>> of the latter, that would be great, because that would avoid merge
>>>>>> conflicts in the future.
>>>>>
>>>>> This is the revised patch on the feature/package-refactor branch:
>>>>>
>>>>> diff --git a/lisp/emacs-lisp/lisp-mnt.el b/lisp/emacs-lisp/lisp-mnt.el
>>>>> index 111d512ef59..169bbbd7f0a 100644
>>>>> --- a/lisp/emacs-lisp/lisp-mnt.el
>>>>> +++ b/lisp/emacs-lisp/lisp-mnt.el
>>>>> @@ -468,6 +468,13 @@ lm-package-requires
>>>>> (lm--prepare-package-dependencies
>>>>> (package-read-from-string (mapconcat #'identity require-lines "
>>>>> "))))))
>>>>>
>>>>> +(defun lm-package-subdirectories (&optional file)
>>>>> + "Return \"Package-Subdirectories\" header."
>>>>> + (require 'package)
>>>>> + (lm-with-file file
>>>>> + (and-let* ((subdir-lines (lm-header-multiline
>>>>> "package-subdirectories")))
>>>>> + (package-read-from-string (mapconcat #'identity subdir-lines " ")))))
>>>>> +
>>>>> (defun lm-package-version (&optional file)
>>>>> "Return \"Package-Version\" or \"Version\" header.
>>>>> Prefer Package-Version; if defined, the package author
>>>>>
>>>>> diff --git a/lisp/package/package-install.el
>>>>> b/lisp/package/package-install.el
>>>>> index cec08e60ac3..eee39f79899 100644
>>>>> --- a/lisp/package/package-install.el
>>>>> +++ b/lisp/package/package-install.el
>>>>> @@ -976,29 +976,40 @@ package-autoload-ensure-default-file
>>>>> (defvar autoload-timestamps)
>>>>> (defvar version-control)
>>>>>
>>>>> -(defun package-generate-autoloads (name pkg-dir)
>>>>> - "Generate autoloads in PKG-DIR for package named NAME."
>>>>> +(defun package-generate-autoloads (name pkg-dir &optional pkg-subdirs)
>>>>> + "Generate autoloads in PKG-DIR for package named NAME.
>>>>> +If LISP-SUBDIRS is provided, it should be a list of subdirectory names
>>>>> +relative to PKG-DIR."
>>>>> (let* ((auto-name (format "%s-autoloads.el" name))
>>>>> ;;(ignore-name (concat name "-pkg.el"))
>>>>> (output-file (expand-file-name auto-name pkg-dir))
>>>>> + (all-dirs (cons pkg-dir
>>>>> + (mapcar
>>>>> + (lambda (dir) (expand-file-name dir pkg-dir))
>>>>> + pkg-subdirs)))
>>>>> ;; We don't need 'em, and this makes the output reproducible.
>>>>> (autoload-timestamps nil)
>>>>> (backup-inhibited t)
>>>>> (version-control 'never))
>>>>> (loaddefs-generate
>>>>> - pkg-dir output-file nil
>>>>> + all-dirs output-file nil
>>>>> (prin1-to-string
>>>>> - '(add-to-list
>>>>> - 'load-path
>>>>> - ;; Add the directory that will contain the autoload file to
>>>>> - ;; the load path. We don't hard-code `pkg-dir', to avoid
>>>>> - ;; issues if the package directory is moved around.
>>>>> - ;; `loaddefs-generate' has code to do this for us, but it's
>>>>> - ;; not currently exposed. (Bug#63625)
>>>>> - (or (and load-file-name
>>>>> - (directory-file-name
>>>>> - (file-name-directory load-file-name)))
>>>>> - (car load-path)))))
>>>>> + `(let ((dir (or (and load-file-name
>>>>> + (directory-file-name
>>>>> + (file-name-directory load-file-name)))
>>>>> + (car load-path))))
>>>>> + ;; Add the directory that will contain the autoload file to
>>>>> + ;; the load path. We don't hard-code `pkg-dir', to avoid
>>>>> + ;; issues if the package directory is moved around.
>>>>> + ;; `loaddefs-generate' has code to do this for us, but it's
>>>>> + ;; not currently exposed. (Bug#63625)
>>>>> + (add-to-list 'load-path dir)
>>>>> + ,@(mapcar
>>>>> + (lambda (subdir)
>>>>> + `(add-to-list
>>>>> + 'load-path
>>>>> + (expand-file-name ,subdir dir)))
>>>>> + pkg-subdirs))))
>>>>> (let ((buf (find-buffer-visiting output-file)))
>>>>> (when buf (kill-buffer buf)))
>>>>> auto-name))
>>>>>
>>>>> diff --git a/lisp/package/package-vc.el b/lisp/package/package-vc.el
>>>>> index d6b84d896f9..d7d6f4ea586 100644
>>>>> --- a/lisp/package/package-vc.el
>>>>> +++ b/lisp/package/package-vc.el
>>>>> @@ -533,6 +533,7 @@ package-vc--unpack-1
>>>>> ;; Directory where package's Lisp code resides. It may be
>>>>> ;; equal to `checkout-dir' or be a subdirectory of it.
>>>>> (lisp-dir (package-vc--checkout-dir pkg-desc 'lisp-dir))
>>>>> + subdirs
>>>>> missing)
>>>> I'd merge these two into a single line.
>>>>
>>>>>
>>>>> ;; In case the package was installed directly from source, the
>>>>> @@ -549,30 +550,29 @@ package-vc--unpack-1
>>>>> (plist-get pkg-spec :ignored-files)
>>>>> "\\|")
>>>>> regexp-unmatchable))
>>>>> - (deps '()))
>>>>> + deps)
>>>> And I wouldn't revert this change. Perhaps this is just personal
>>>> preference, but I prefer to be explicit when binding nil or '() when
>>>> initialising a variable. I think about it like in Scheme, where an
>>>> undefined value, a boolean or an empty list are all distinct.
>>>>
>>>>> (dolist (file (directory-files lisp-dir t "\\.el\\'" t))
>>>>> (unless (string-match-p ignored-files file)
>>>>> - (with-temp-buffer
>>>>> - (insert-file-contents file)
>>>>> - (when-let* ((require-lines (lm-header-multiline
>>>>> "package-requires")))
>>>>> - (setq deps
>>>>> - (nconc deps
>>>>> - (lm--prepare-package-dependencies
>>>>> - (package-read-from-string
>>>>> - (mapconcat (function identity)
>>>>> - require-lines " ")))))))))
>>>>> + (setq deps (nconc deps (lm-package-requires file)))
>>>>> + (setq subdirs (nconc subdirs (lm-package-subdirectories file)))))
>>>>> + (dolist (file (mapcan (lambda (dir)
>>>>> + (directory-files
>>>>> + (expand-file-name dir lisp-dir) t
>>>>> "\\.el\\'" t))
>>>>> + subdirs))
>>>>> + (unless (string-match-p ignored-files file)
>>>>> + (setq deps (nconc deps (lm-package-requires file)))))
>>>>> + (setq deps (assq-delete-all (package-desc-name pkg-desc)
>>>>> (delete-dups deps)))
>>>>> (dolist (dep deps)
>>>>> (cl-callf version-to-list (cadr dep)))
>>>>> (setf (package-desc-reqs pkg-desc) deps)
>>>>> - (setf missing (package-vc-install-dependencies (delete-dups deps)))
>>>>> + (setf missing (package-vc-install-dependencies deps))
>>>>> (setf missing (delq (assq (package-desc-name pkg-desc)
>>>>> missing)
>>>>> missing)))
>>>>>
>>>>> ;; Generate autoloads
>>>>> (let* ((name (package-desc-name pkg-desc))
>>>>> - (auto-name (format "%s-autoloads.el" name)))
>>>>> - (package-generate-autoloads name lisp-dir)
>>>>> + (auto-name (package-generate-autoloads name lisp-dir subdirs)))
>>>>> ;; There are two cases when we wish to "indirect" the loading of
>>>>> ;; autoload files:
>>>>> ;;
>>>>> @@ -777,7 +777,7 @@ package-vc--unpack
>>>>> (user-error "Installation aborted")))
>>>>>
>>>>> ;; Ensure we have a copy of the package specification
>>>>> - (when (null (package-vc--desc->spec pkg-desc name))
>>>>> + (unless (equal (package-vc--desc->spec pkg-desc name) pkg-spec)
>>>>> (customize-save-variable
>>>>> 'package-vc-selected-packages
>>>>> (cons (cons name pkg-spec)
>>>>>
>>>>> Some explanations and design choices are in order. I have introduced a
>>>>> new function to extract the 'Package-Subdirectories' header, named
>>>>> 'lm-package-subdirectories', in line with the other "lm-package-..."
>>>>> functions. It uses the same logic as lm-package-requires, extracting the
>>>>> header with lm-header-multiline. The package-generate-autoloads logic
>>>>> remains unchanged from the previous patch.
>>>>>
>>>>> There are multiple changes in package-vc--unpack-1. First, the previous
>>>>> (with-temp-buffer ...) logic for collecting package-requires was
>>>>> essentially a duplication of the lm-package-requires function, so I have
>>>>> removed that code and used lm-package-requires instead. In the same
>>>>> loop, Package-Subdirectories headers are being collected. This assumes
>>>>> that the headers should be in the .el files of lisp-dir (following your
>>>>> logic of scanning all '.el' files, rather than relying on a single
>>>>> mainfile). Then, with the collected list of subdirectories, I am
>>>>> scanning through each of their .el files to collect other
>>>>> package-requires.
>>>>>
>>>>> Since dependencies are installed before the VC package itself, all
>>>>> mentions of the package itself as a dependency is removed. I am also
>>>>> deleting duplicate dependencies here rather than waiting for the
>>>>> delete-dups call here: (setf missing (package-vc-install-dependencies
>>>>> (delete-dups deps))). This is because we are setting package-desc-reqs
>>>>> to deps, where duplicates and self-references don't make any sense, nor
>>>>> does calling version-to-list on the duplicates.
>>>>>
>>>>> Lastly, in package-vc--unpack, I have changed the when null check to
>>>>> unless equal. I have raised the point in bug#80015, but in case you have
>>>>> missed that patch, here is a summary. In commit
>>>>> 573acd97e54ceead6d11b330909ffb8e744247cc this change was made:
>>>>>
>>>>> - (unless (seq-some (lambda (alist) (equal (alist-get name (cdr
>>>>> alist)) pkg-spec))
>>>>> - package-vc--archive-spec-alists)
>>>>> + (when (null (package-vc--desc->spec pkg-desc name))
>>>>>
>>>>> I believe that the logic here is faulty - we have to check if the user
>>>>> provided pkg-spec can be found, and only then should the block be
>>>>> ignored. Otherwise, for most (all?) packages, since they do have a
>>>>> pkg-spec in the archives (MELPA, ELPA, etc.), any user-provided pkg-spec
>>>>> will effectively be ignored. We should check for equality to ensure the
>>>>> user specified pkg-spec is found, rather than checking for the existence
>>>>> of any pkg-spec.
>>>>>
>>>>> However, this brings forth a interesting question: suppose in the
>>>>> archives the authors ignores a file, and when installing from VC the
>>>>> user ignores some more files - shouldn't these lists be merged? Maybe we
>>>>> should use the same logic as the subdirectories, where we "embed" the
>>>>> author-specified ignored-files directly in the code itself, possibly
>>>>> with another header?
>>>> This logic sounds reasonable, I would appreciate it if you could
>>>> prepare
>>>> the patches for the suggestions you made.
>>>
>>> For the ignored-files, which solution sounds better though? I am a bit
>>> reluctant towards the merging solution, because then there is always an
>>> inherent dependency on the archives. However, I am also not sure whether
>>> introducing another header for ignored-files is a good idea. Let me know
>>> which idea sounds better to you. Or should we just leave this as-is?
>>
>> Why would we have another header for ignored files? That is what the
>> .elpaignore file is for? What I meant was that the merging solution
>> sounds reasonable.
>
> I have no idea how I missed the fact that .elpaignore is, in fact, the
> list of ignored-files. Thanks for the heads up!
>
>>
>>>>> BTW, I have removed the lisp-subdirs keyword from the first diff, but I
>>>>> have included a second diff for that feature, since I believe it may be
>>>>> necessary for certain packages. For example, treemacs has a number of
>>>>> "extra" packages, which are not packed with treemacs itself, but rather
>>>>> each of these files are a package unto itself. For example, this is the
>>>>> recipe for one such "extra" package:
>>>>>
>>>>> (treemacs-all-the-icons
>>>>> :fetcher github
>>>>> :repo "Alexander-Miller/treemacs"
>>>>> :files ("src/extra/treemacs-all-the-icons.el"))
>>>>>
>>>>> In the main treemacs recipe, however, src/extra/* is explicitly
>>>>> excluded. Since we don't support treating a single file in a VC package
>>>>> as a full-package (and I don't think we should), and for treemacs
>>>>> package maintainers, it makes no sense to include the "extras" directory
>>>>> in Package-Subdirectories header, the user is stuck with installing
>>>>> those extra packages from the archives, unless we give them a way to
>>>>> include other subdirectories themselves. They can then prune the
>>>>> unneeded "extras" files with ignored-files. Anyways, here is the patch
>>>>> that keeps lisp-subdirs keyword too:
>>>>>
>>>>> diff --git a/doc/emacs/package.texi b/doc/emacs/package.texi
>>>>> index 53b339bd606..a66ed9b1e78 100644
>>>>> --- a/doc/emacs/package.texi
>>>>> +++ b/doc/emacs/package.texi
>>>>> @@ -736,6 +736,14 @@ Fetching Package Sources
>>>>> use for loading the Lisp sources, which defaults to the root directory
>>>>> of the repository.
>>>>>
>>>>> +@item :lisp-subdirs
>>>>> +A string or a list of strings providing the names of subdirectories
>>>>> +containing additional Lisp sources. These names are interpreted
>>>>> +relative to the directory specified by @code{:lisp-dir} (or the root
>>>>> +directory of the repository if @code{:lisp-dir} is unspecified). The
>>>>> +specified subdirectories are added to the load path and scanned for
>>>>> +library dependencies and autoloads.
>>>>> +
>>>>> @item :main-file
>>>>> A string providing the main file of the project, from which to gather
>>>>> package metadata. If not given, the default is the package name with
>>>>>
>>>>> diff --git a/lisp/package/package-vc.el b/lisp/package/package-vc.el
>>>>> index d7d6f4ea586..2b5e984fb40 100644
>>>>> --- a/lisp/package/package-vc.el
>>>>> +++ b/lisp/package/package-vc.el
>>>>> @@ -533,7 +533,7 @@ package-vc--unpack-1
>>>>> ;; Directory where package's Lisp code resides. It may be
>>>>> ;; equal to `checkout-dir' or be a subdirectory of it.
>>>>> (lisp-dir (package-vc--checkout-dir pkg-desc 'lisp-dir))
>>>>> - subdirs
>>>>> + (subdirs (ensure-list (plist-get pkg-spec :lisp-subdirs)))
>>>>> missing)
>>>>>
>>>>> ;; In case the package was installed directly from source, the
>>>>>
>>>>> diff --git a/lisp/use-package/use-package-core.el
>>>>> b/lisp/use-package/use-package-core.el
>>>>> index 0f049a6f86b..a6c92d663cc 100644
>>>>> --- a/lisp/use-package/use-package-core.el
>>>>> +++ b/lisp/use-package/use-package-core.el
>>>>> @@ -1735,7 +1735,7 @@ use-package-handler/:vc
>>>>> body))
>>>>>
>>>>> (defconst use-package-vc-valid-keywords
>>>>> - '( :url :branch :lisp-dir :main-file :vc-backend :rev
>>>>> + '( :url :branch :lisp-dir :lisp-subdirs :main-file :vc-backend :rev
>>>>> :shell-command :make :ignored-files)
>>>>> "Valid keywords for the `:vc' keyword.
>>>>> See Info node `(emacs)Fetching Package Sources'.")
>>>> I remain disinclined towards this point. Generally we don't have
>>>> the
>>>> ambition to support any kind of package layout. On ELPA, we have the
>>>> policy that one repository branch corresponds to one package, and
>>>> package-vc uses this fact by checking out the entire repository. Am I
>>>> understanding correctly that with the approach you suggest package-vc
>>>> would check out the repository multiple times over?
>>>
>>> No, this will not check out the repository multiple times. I think I
>>> should have clarified the purpose of this lisp-subdirs keyword in a
>>> better way. Let's consider treemacs as an example. The author has a
>>> number of extra related packages in the aptly named extras directory of
>>> the root git directory. Now, the author doesn't package this extras
>>> directory with the treemacs package. Instead, they opt to package the
>>> files in extras separately. So, the author would not want to put the
>>> extras directory in the Package-Subdirectories header. But a user might.
>>> When the user uses use-package with vc, the whole git directory for
>>> treemacs is cloned, along with the extras directory. My opinion is: in
>>> cases like this, the user might want to load the extras directory too,
>>> but without the lisp-subdirs keyword, the only option the user has is to
>>> patch treemacs itself and include the "extras" directory in
>>> Package-Subdirectories. Instead, this keyword just provides the user the
>>> option to include some directories as subdirs too, even if the author
>>> decides that they are not. I used treemacs as an example, but there may
>>> be many other packages like that, where the author decides some
>>> directory to not be part of the package, but as auxiliary ones, so they
>>> don't package them together as a single tarball. But since the user is
>>> cloning the whole git directory, the user may want to include the
>>> subdirectory(ies). For this sole purpose, I was thinking of keeping
>>> lisp-subdirs. However, if ELPA doesn't have the capability to do what I
>>> mentioned, and if this is just MELPA specific, I guess we may not
>>> include this lisp-subdirs keyword.
>>
>> OK, thank you for the clarification but as mentioned before, I don't
>> want to add keys to package specifications that don't also make sense
>> for elpa-admin. I would prefer it if we could first figure out what it
>> would the proper way to add support for this during the creation of
>> tarballs would be, before we consider it here.
>>
>> I am also not interested in complications for a package that is not even
>> on GNU or NonGNU ELPA. My go-to recommendation for related sub-packages
>> has been to use Git branches.
>
> Ah OK. Truth be told, I don't use treemacs either, so this was based on
> hypothetical situations, and I agree that we should be more in line with
> ELPA than MELPA.
>
>>
>>>>> Lastly - and this is slightly off-topic - is there a reason we use regex
>>>>> to detect ignored-files? Why is it not something like this:
>>>>>
>>>>> diff --git a/lisp/package/package-vc.el b/lisp/package/package-vc.el
>>>>> index 2b5e984fb40..dad9e1037c9 100644
>>>>> --- a/lisp/package/package-vc.el
>>>>> +++ b/lisp/package/package-vc.el
>>>>> @@ -540,26 +540,19 @@ package-vc--unpack-1
>>>>> ;; dependency list wasn't know beforehand, and they might have
>>>>> ;; to be installed explicitly.
>>>>> (let ((ignored-files
>>>>> - (if (plist-get pkg-spec :ignored-files)
>>>>> - (mapconcat
>>>>> - (lambda (ignore)
>>>>> - (wildcard-to-regexp
>>>>> - (if (string-match-p "\\`/" ignore)
>>>>> - (concat checkout-dir ignore)
>>>>> - (concat "*/" ignore))))
>>>>> - (plist-get pkg-spec :ignored-files)
>>>>> - "\\|")
>>>>> - regexp-unmatchable))
>>>>> + (mapcar (lambda (file)
>>>>> + (expand-file-name file lisp-dir))
>>>>> + (plist-get pkg-spec :ignored-files)))
>>>>> deps)
>>>>> (dolist (file (directory-files lisp-dir t "\\.el\\'" t))
>>>>> - (unless (string-match-p ignored-files file)
>>>>> + (unless (member file ignored-files)
>>>>> (setq deps (nconc deps (lm-package-requires file)))
>>>>> (setq subdirs (nconc subdirs (lm-package-subdirectories
>>>>> file)))))
>>>>> (dolist (file (mapcan (lambda (dir)
>>>>> (directory-files
>>>>> (expand-file-name dir lisp-dir) t
>>>>> "\\.el\\'" t))
>>>>> subdirs))
>>>>> - (unless (string-match-p ignored-files file)
>>>>> + (unless (member file ignored-files)
>>>>> (setq deps (nconc deps (lm-package-requires file)))))
>>>>> (setq deps (assq-delete-all (package-desc-name pkg-desc)
>>>>> (delete-dups deps)))
>>>>> (dolist (dep deps)
>>>>>
>>>>> This seems to be more robust, but I'm unsure whether it'd introduce any
>>>>> performance bottlenecks.
>>>> Performance is the main reason. But your suggestion doesn't handle
>>>> glob-expressions in file names, which the previous implementation does
>>>> due to `wildcard-to-regexp'.
>>>
>>> Ah OK, I missed the glob-expression part. And yeah, I had a fleeting
>>> suspicion that there might be a performance issue. Anyways, yeah, this
>>> patch was unnecessary, a bit of curiosity from my part, nothing else.
>>
>> No problem :)
>>
>> So what is the conclusion? You wanted to prepare a patch that would
>> make package.el and package-vc.el lookup a special header to indicate
>> what subdirectories of the checkout also contain Lisp files.
>
> Anyways, this is the patch so far, based on our discussion:
>
> diff --git a/lisp/emacs-lisp/lisp-mnt.el b/lisp/emacs-lisp/lisp-mnt.el
> index 111d512ef59..169bbbd7f0a 100644
> --- a/lisp/emacs-lisp/lisp-mnt.el
> +++ b/lisp/emacs-lisp/lisp-mnt.el
> @@ -468,6 +468,13 @@ lm-package-requires
> (lm--prepare-package-dependencies
> (package-read-from-string (mapconcat #'identity require-lines "
> "))))))
>
> +(defun lm-package-subdirectories (&optional file)
> + "Return \"Package-Subdirectories\" header."
> + (require 'package)
> + (lm-with-file file
> + (and-let* ((subdir-lines (lm-header-multiline
> "package-subdirectories")))
> + (package-read-from-string (mapconcat #'identity subdir-lines " ")))))
> +
> (defun lm-package-version (&optional file)
> "Return \"Package-Version\" or \"Version\" header.
> Prefer Package-Version; if defined, the package author
>
> diff --git a/lisp/package/package-install.el
> b/lisp/package/package-install.el
> index 74cc767cff7..8dc9fd8dc69 100644
> --- a/lisp/package/package-install.el
> +++ b/lisp/package/package-install.el
> @@ -999,29 +999,40 @@ package-autoload-ensure-default-file
> (defvar autoload-timestamps)
> (defvar version-control)
>
> -(defun package-generate-autoloads (name pkg-dir)
> - "Generate autoloads in PKG-DIR for package named NAME."
> +(defun package-generate-autoloads (name pkg-dir &optional pkg-subdirs)
> + "Generate autoloads in PKG-DIR for package named NAME.
> +If LISP-SUBDIRS is provided, it should be a list of subdirectory names
> +relative to PKG-DIR."
> (let* ((auto-name (format "%s-autoloads.el" name))
> ;;(ignore-name (concat name "-pkg.el"))
> (output-file (expand-file-name auto-name pkg-dir))
> + (all-dirs (cons pkg-dir
> + (mapcar
> + (lambda (dir) (expand-file-name dir pkg-dir))
> + pkg-subdirs)))
> ;; We don't need 'em, and this makes the output reproducible.
> (autoload-timestamps nil)
> (backup-inhibited t)
> (version-control 'never))
> (loaddefs-generate
> - pkg-dir output-file nil
> + all-dirs output-file nil
> (prin1-to-string
> - '(add-to-list
> - 'load-path
> - ;; Add the directory that will contain the autoload file to
> - ;; the load path. We don't hard-code `pkg-dir', to avoid
> - ;; issues if the package directory is moved around.
> - ;; `loaddefs-generate' has code to do this for us, but it's
> - ;; not currently exposed. (Bug#63625)
> - (or (and load-file-name
> - (directory-file-name
> - (file-name-directory load-file-name)))
> - (car load-path)))))
> + `(let ((dir (or (and load-file-name
> + (directory-file-name
> + (file-name-directory load-file-name)))
> + (car load-path))))
> + ;; Add the directory that will contain the autoload file to
> + ;; the load path. We don't hard-code `pkg-dir', to avoid
> + ;; issues if the package directory is moved around.
> + ;; `loaddefs-generate' has code to do this for us, but it's
> + ;; not currently exposed. (Bug#63625)
> + (add-to-list 'load-path dir)
> + ,@(mapcar
> + (lambda (subdir)
> + `(add-to-list
> + 'load-path
> + (expand-file-name ,subdir dir)))
> + pkg-subdirs))))
> (let ((buf (find-buffer-visiting output-file)))
> (when buf (kill-buffer buf)))
> auto-name))
>
> diff --git a/lisp/package/package-vc.el b/lisp/package/package-vc.el
> index d6b84d896f9..29de8769bde 100644
> --- a/lisp/package/package-vc.el
> +++ b/lisp/package/package-vc.el
> @@ -533,46 +533,46 @@ package-vc--unpack-1
> ;; Directory where package's Lisp code resides. It may be
> ;; equal to `checkout-dir' or be a subdirectory of it.
> (lisp-dir (package-vc--checkout-dir pkg-desc 'lisp-dir))
> - missing)
> + subdirs missing)
>
> ;; In case the package was installed directly from source, the
> ;; dependency list wasn't know beforehand, and they might have
> ;; to be installed explicitly.
> (let ((ignored-files
> - (if (plist-get pkg-spec :ignored-files)
> - (mapconcat
> - (lambda (ignore)
> - (wildcard-to-regexp
> - (if (string-match-p "\\`/" ignore)
> - (concat checkout-dir ignore)
> - (concat "*/" ignore))))
> - (plist-get pkg-spec :ignored-files)
> - "\\|")
> + (if-let* ((ignored (append
> + (package--parse-elpaignore pkg-desc)
> + (mapcar
> + (lambda (ignore)
> + (wildcard-to-regexp
> + (if (string-match-p "\\`/" ignore)
> + (concat checkout-dir ignore)
> + (concat "*/" ignore))))
> + (plist-get pkg-spec :ignored-files)))))
> + (string-join ignored "\\|")
> regexp-unmatchable))
> (deps '()))
> (dolist (file (directory-files lisp-dir t "\\.el\\'" t))
> (unless (string-match-p ignored-files file)
> - (with-temp-buffer
> - (insert-file-contents file)
> - (when-let* ((require-lines (lm-header-multiline
> "package-requires")))
> - (setq deps
> - (nconc deps
> - (lm--prepare-package-dependencies
> - (package-read-from-string
> - (mapconcat (function identity)
> - require-lines " ")))))))))
> + (setq deps (nconc deps (lm-package-requires file)))
> + (setq subdirs (nconc subdirs (lm-package-subdirectories file)))))
> + (dolist (file (mapcan (lambda (dir)
> + (directory-files
> + (expand-file-name dir lisp-dir) t
> "\\.el\\'" t))
> + subdirs))
> + (unless (string-match-p ignored-files file)
> + (setq deps (nconc deps (lm-package-requires file)))))
> + (setq deps (assq-delete-all (package-desc-name pkg-desc)
> (delete-dups deps)))
> (dolist (dep deps)
> (cl-callf version-to-list (cadr dep)))
> (setf (package-desc-reqs pkg-desc) deps)
> - (setf missing (package-vc-install-dependencies (delete-dups deps)))
> + (setf missing (package-vc-install-dependencies deps))
> (setf missing (delq (assq (package-desc-name pkg-desc)
> missing)
> missing)))
>
> ;; Generate autoloads
> (let* ((name (package-desc-name pkg-desc))
> - (auto-name (format "%s-autoloads.el" name)))
> - (package-generate-autoloads name lisp-dir)
> + (auto-name (package-generate-autoloads name lisp-dir subdirs)))
> ;; There are two cases when we wish to "indirect" the loading of
> ;; autoload files:
> ;;
> @@ -777,7 +777,7 @@ package-vc--unpack
> (user-error "Installation aborted")))
>
> ;; Ensure we have a copy of the package specification
> - (when (null (package-vc--desc->spec pkg-desc name))
> + (unless (equal (package-vc--desc->spec pkg-desc name) pkg-spec)
> (customize-save-variable
> 'package-vc-selected-packages
> (cons (cons name pkg-spec)
>
>
> I have tested with vertico and corfu so far, and this works perfectly.
I am adding Daniel Mendler to the CC's, as his packages are currently
the only ones on ELPA that would profit from the change we are
discussing.
For Daniel's convenience: We are considering at least two methods of
having a package indicate that package.el should prepare and load Lisp
code in a subdirectory:
1. The patch proposed above that uses a "Package-Subdirectories" header
in the main file to list additional directories.
2. The patch I proposed a few messages back in
<875x9yvz8t.fsf <at> posteo.net> that adds a `package-add-subdir' macro
that you would invoke to indicate what subdirectories to load.
The advantage of the second implementation is that it is simpler (but I
am biased ;)), while the latter avoids the need to search all files for
top-level `package-add-subdir' invocations -- which also means that it
avoid the confusion of wrapping `package-add-subdir' in some other
top-level construct that could confuse lead to confusing behaviour.
I'd be interested to hear your opinions on these proposals, of if you
have any ideas of your own.
bug-gnu-emacs <at> gnu.org:bug#80016; Package emacs.
(Fri, 02 Jan 2026 16:37:02 GMT) Full text and rfc822 format available.Message #44 received at 80016 <at> debbugs.gnu.org (full text, mbox):
From: Daniel Mendler <mail <at> daniel-mendler.de> To: Philip Kaludercic <philipk <at> posteo.net> Cc: 80016 <at> debbugs.gnu.org, Aritro Sen <1.sen.aritro <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org> Subject: Re: bug#80016: [PATCH] Load lisp sub-directories in addition to the lisp-dir Date: Fri, 02 Jan 2026 17:36:46 +0100
Philip Kaludercic <philipk <at> posteo.net> writes: > 1. The patch proposed above that uses a "Package-Subdirectories" header > in the main file to list additional directories. Maybe such a header would be good indeed if it would replace the additional package configuration in ELPA (:renames) and MELPA (:defaults). As an alternative you could use an empty marker file like `.elpadir'? I think I would prefer that. The problem with all such additions is that every package manager or archive comes up with their own solution, and that there is little coordination. I would like to see .elpaignore supported by MELPA for example. Furthermore, info manual building is handled differently, versioning is handled differently and so on. Daniel
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.