bug#69019: [PATCH] package-vc: scan the correct directory for lisp files

bug#69019: [PATCH] package-vc: scan the correct directory for lisp files

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

[-- Attachment #1: Type: text/plain, Size: 155 bytes --]


Following up on my previous patch (Bug#68761), we need to scan for
package requirements inside the lisp-dir (if set), not in the root
package directory.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-package-vc-scan-the-correct-directory-for-lisp-files.patch --]
[-- Type: text/x-patch, Size: 2168 bytes --]

From 5a4540756e5e30d12010ce58094aafacc1b8fe01 Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@stebalien.com>
Date: Sat, 10 Feb 2024 10:05:11 -0800
Subject: [PATCH] package-vc: scan the correct directory for lisp files

Otherwise, we won't correctly detect package requirements.

* lisp/emacs-lisp/package-vc.el (package-vc--unpack-1):
  Scan 'lisp-dir', if set, for lisp files instead of scanning the root
  package directory.
---
 lisp/emacs-lisp/package-vc.el | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
index fc402716dab..2730100b3d0 100644
--- a/lisp/emacs-lisp/package-vc.el
+++ b/lisp/emacs-lisp/package-vc.el
@@ -501,8 +501,9 @@ package-vc--unpack-1
 autoloads, generating a package description file (used to
 identify a package as a VC package later on), building
 documentation and marking the package as installed."
-  (let ((pkg-spec (package-vc--desc->spec pkg-desc))
-        missing)
+  (let* ((pkg-spec (package-vc--desc->spec pkg-desc))
+         (lisp-dir (plist-get pkg-spec :lisp-dir))
+         missing)
 
     ;; In case the package was installed directly from source, the
     ;; dependency list wasn't know beforehand, and they might have
@@ -519,7 +520,7 @@ package-vc--unpack-1
                 "\\|")
              regexp-unmatchable))
           (deps '()))
-      (dolist (file (directory-files pkg-dir t "\\.el\\'" t))
+      (dolist (file (directory-files (file-name-concat pkg-dir lisp-dir) t "\\.el\\'" t))
         (unless (string-match-p ignored-files file)
           (with-temp-buffer
             (insert-file-contents file)
@@ -542,8 +543,7 @@ package-vc--unpack-1
           (pkg-file (expand-file-name (package--description-file pkg-dir) pkg-dir)))
       ;; Generate autoloads
       (let* ((name (package-desc-name pkg-desc))
-             (auto-name (format "%s-autoloads.el" name))
-             (lisp-dir (plist-get pkg-spec :lisp-dir)))
+             (auto-name (format "%s-autoloads.el" name)))
         (package-generate-autoloads
          name (file-name-concat pkg-dir lisp-dir))
         (when lisp-dir
-- 
2.43.0

bug#69019: [PATCH] package-vc: scan the correct directory for lisp files

[Philip Kaludercic]

Steven Allen <steven@stebalien.com> writes:

> Following up on my previous patch (Bug#68761), we need to scan for
> package requirements inside the lisp-dir (if set), not in the root
> package directory.

Looks good, do you have any specific examples where this caused
problems?

>>From 5a4540756e5e30d12010ce58094aafacc1b8fe01 Mon Sep 17 00:00:00 2001
> From: Steven Allen <steven@stebalien.com>
> Date: Sat, 10 Feb 2024 10:05:11 -0800
> Subject: [PATCH] package-vc: scan the correct directory for lisp files
>
> Otherwise, we won't correctly detect package requirements.
>
> * lisp/emacs-lisp/package-vc.el (package-vc--unpack-1):
>   Scan 'lisp-dir', if set, for lisp files instead of scanning the root
>   package directory.

Just note that the convention of the commit message logs is not to
indent the lines here.  I can take care of that.

> ---
>  lisp/emacs-lisp/package-vc.el | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
> index fc402716dab..2730100b3d0 100644
> --- a/lisp/emacs-lisp/package-vc.el
> +++ b/lisp/emacs-lisp/package-vc.el
> @@ -501,8 +501,9 @@ package-vc--unpack-1
>  autoloads, generating a package description file (used to
>  identify a package as a VC package later on), building
>  documentation and marking the package as installed."
> -  (let ((pkg-spec (package-vc--desc->spec pkg-desc))
> -        missing)
> +  (let* ((pkg-spec (package-vc--desc->spec pkg-desc))
> +         (lisp-dir (plist-get pkg-spec :lisp-dir))
> +         missing)
>  
>      ;; In case the package was installed directly from source, the
>      ;; dependency list wasn't know beforehand, and they might have
> @@ -519,7 +520,7 @@ package-vc--unpack-1
>                  "\\|")
>               regexp-unmatchable))
>            (deps '()))
> -      (dolist (file (directory-files pkg-dir t "\\.el\\'" t))
> +      (dolist (file (directory-files (file-name-concat pkg-dir lisp-dir) t "\\.el\\'" t))
>          (unless (string-match-p ignored-files file)
>            (with-temp-buffer
>              (insert-file-contents file)
> @@ -542,8 +543,7 @@ package-vc--unpack-1
>            (pkg-file (expand-file-name (package--description-file pkg-dir) pkg-dir)))
>        ;; Generate autoloads
>        (let* ((name (package-desc-name pkg-desc))
> -             (auto-name (format "%s-autoloads.el" name))
> -             (lisp-dir (plist-get pkg-spec :lisp-dir)))
> +             (auto-name (format "%s-autoloads.el" name)))
>          (package-generate-autoloads
>           name (file-name-concat pkg-dir lisp-dir))
>          (when lisp-dir

-- 
Philip Kaludercic

bug#69019: [PATCH] package-vc: scan the correct directory for lisp files

[Steven Allen]

>> Following up on my previous patch (Bug#68761), we need to scan for
>> package requirements inside the lisp-dir (if set), not in the root
>> package directory.
>
> Looks good, do you have any specific examples where this caused
> problems?

pdf-tools puts all its files in a "lisp" dir, so they weren't getting
searched when scanning for dependencies. The package definition is:


    (:url "https://github.com/vedang/pdf-tools/pulls"
     :lisp-dir "lisp")

This patch causes package-vc to scan for dependencies in elisp files in
"$pkg-dir/lisp/", instead of looking for elisp files in "$pkg-dir/".

>> * lisp/emacs-lisp/package-vc.el (package-vc--unpack-1):
>>   Scan 'lisp-dir', if set, for lisp files instead of scanning the root
>>   package directory.
>
> Just note that the convention of the commit message logs is not to
> indent the lines here.  I can take care of that.

I did not, thanks!

bug#69019: [PATCH] package-vc: scan the correct directory for lisp files

[Philip Kaludercic]

[-- Attachment #1: Type: text/plain, Size: 874 bytes --]

(Don't forget to keep the bug tracker in the CC's; I have resent your
message so that everything remains in the archives)

Steven Allen <steven@stebalien.com> writes:

>>> Following up on my previous patch (Bug#68761), we need to scan for
>>> package requirements inside the lisp-dir (if set), not in the root
>>> package directory.
>>
>> Looks good, do you have any specific examples where this caused
>> problems?
>
> pdf-tools puts all its files in a "lisp" dir, so they weren't getting
> searched when scanning for dependencies. The package definition is:
>
>     (:url "https://github.com/vedang/pdf-tools/pulls"
>      :lisp-dir "lisp")
>
> This patch causes package-vc to scan for dependencies in elisp files in
> "$pkg-dir/lisp/", instead of looking for elisp files in "$pkg-dir/".

OK, I have slightly modified the commit, can you check if it still works
for you:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Respect-lisp-dir-whilst-scanning-for-VC-package-depe.patch --]
[-- Type: text/x-patch, Size: 2271 bytes --]

From 4758889a51bbc87a330bf21c89745a7b49597119 Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@stebalien.com>
Date: Sat, 10 Feb 2024 10:05:11 -0800
Subject: [PATCH] Respect :lisp-dir whilst scanning for VC package dependencies

* lisp/emacs-lisp/package-vc.el (package-vc--unpack-1):
Scan 'lisp-dir', if set, for lisp files instead of scanning the root
package directory.  (Bug#69019)
---
 lisp/emacs-lisp/package-vc.el | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
index fc402716dab..37980c28b02 100644
--- a/lisp/emacs-lisp/package-vc.el
+++ b/lisp/emacs-lisp/package-vc.el
@@ -501,8 +501,10 @@ package-vc--unpack-1
 autoloads, generating a package description file (used to
 identify a package as a VC package later on), building
 documentation and marking the package as installed."
-  (let ((pkg-spec (package-vc--desc->spec pkg-desc))
-        missing)
+  (let* ((pkg-spec (package-vc--desc->spec pkg-desc))
+         (lisp-dir (plist-get pkg-spec :lisp-dir))
+         (lisp-path (file-name-concat pkg-dir lisp-dir))
+         missing)
 
     ;; In case the package was installed directly from source, the
     ;; dependency list wasn't know beforehand, and they might have
@@ -519,7 +521,7 @@ package-vc--unpack-1
                 "\\|")
              regexp-unmatchable))
           (deps '()))
-      (dolist (file (directory-files pkg-dir t "\\.el\\'" t))
+      (dolist (file (directory-files lisp-path t "\\.el\\'" t))
         (unless (string-match-p ignored-files file)
           (with-temp-buffer
             (insert-file-contents file)
@@ -542,10 +544,8 @@ package-vc--unpack-1
           (pkg-file (expand-file-name (package--description-file pkg-dir) pkg-dir)))
       ;; Generate autoloads
       (let* ((name (package-desc-name pkg-desc))
-             (auto-name (format "%s-autoloads.el" name))
-             (lisp-dir (plist-get pkg-spec :lisp-dir)))
-        (package-generate-autoloads
-         name (file-name-concat pkg-dir lisp-dir))
+             (auto-name (format "%s-autoloads.el" name)))
+        (package-generate-autoloads name lisp-path)
         (when lisp-dir
           (write-region
            (with-temp-buffer
-- 
2.43.0


[-- Attachment #3: Type: text/plain, Size: 309 bytes --]


>>> * lisp/emacs-lisp/package-vc.el (package-vc--unpack-1):
>>>   Scan 'lisp-dir', if set, for lisp files instead of scanning the root
>>>   package directory.
>>
>> Just note that the convention of the commit message logs is not to
>> indent the lines here.  I can take care of that.
>
> I did not, thanks!

bug#69019: [PATCH] package-vc: scan the correct directory for lisp files

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

Philip Kaludercic <philipk@posteo.net> writes:
> OK, I have slightly modified the commit, can you check if it still works
> for you:

Tested and confirmed working. And thanks for CCing the list.

bug#69019: [PATCH] package-vc: scan the correct directory for lisp files

[Philip Kaludercic]

Steven Allen <steven@stebalien.com> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>> OK, I have slightly modified the commit, can you check if it still works
>> for you:
>
> Tested and confirmed working. And thanks for CCing the list.

OK, I'll push that then under your name and close this bug report.
Thank you for your help!