Subject: bug#79188: 31.0.50; Cannot build packages installed from VC

From: Przemysław Kryger <[email-masked]>
Date: Fri, 08 Aug 2025 12:07:14 +0100

tags 79188 + patch
quit

Przemysław Kryger <[email-masked]> writes:
>> That seems reasonable, but in that case we should be able to reproduce
>> the issue with a more simple example (especially one that doesn't involve
>> use-package, MELPA and missing dependencies). [...]

I managed to reproduce the issue with
https://github.com/pkryger/basic-stats.el, which has no extra
dependencies.  I could reproduce all the issues I described in previous
message (https://debbugs.gnu.org/cgi/bugreport.cgi?bug=79188#11).

In addition I noticed, that `package-vc-upgrade' doesn't work as
well. This one was trying to pull inside the package install directory,
i.e, under `packgage-user-dir'.

>> This should fix the first issue, but it probably won't change anything
>> about the latter:
>>
>> diff --git a/lisp/package/package-vc.el b/lisp/package/package-vc.el
>> index db12c76133e..f2c7c460f6d 100644
>> --- a/lisp/package/package-vc.el
>> +++ b/lisp/package/package-vc.el
>> @@ -549,7 +549,14 @@ package-vc--unpack-1
>>          ;; FIXME: Compilation should be done as a separate, optional, st=
> ep.
>>          ;; E.g. for multi-package installs, we should first install all =
> packages
>>          ;; and then compile them.
>> -        (package--compile new-desc)
>> +        (package--compile
>> +         (if lisp-dir
>> +             ;; In case we are installing a package from a local
>> +             ;; checkout, we want to compile the checkout, not the
>> +             ;; redirection!
>> +             (package-desc-create :dir lisp-dir)
>> +          new-desc))
>> +
>>          (when package-native-compile
>>            (package--native-compile-async new-desc))
>>          ;; After compilation, load again any files loaded by
>>
>
> I don't have lisp/pakcage/package-vc.el, but I changed the file name to
> lisp/emacs-lisp/packgage-vc.el and the patch applied cleanly.
>
> [...]
>
> After I repeated the experiment the package installed with just
> `package-vc-install-from-checkout' (as described above) and
> helm-projectile.elc file has been produced into package source
> directory.

Based on that work I developed a new patch.  With both of the attached
patches applied I was able to run `package-vc-rebuild' and
`package-vc-reinstall' on the aforementioned package basic-stats and
observed that *.elc files were created in the source directory (i.e.,
the local checkout) and not in package install directory.

> A question: shouldn't the newly generated *.elc files be put in
> package install directory, just like the (non compiled) autoloads file
> is?  In my - very na=C3=AFve - view this would not only remove burden
> of doubling `load-path' entries (will that happen) but would also
> allow for a complete separation of compiled files between multiple
> Emacs versions coexisting on the same machine and reusing the same
> package source directories.

While the patch attached fixes basic workflows, I wonder idea of having
*.elc in a package install directory is worth exploring.  Would that
affect other functionalities?  For example `load' and `require' should
just work (provided the package install directory is in front of package
source directory in `load-pat'), but what will happen with
`find-library'/`locate-library'?  Are there any others?

>From 56fa1014b1f8f2eb7f6d87304c3f31604fed48ba Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <[email-masked]>
Date: Fri, 8 Aug 2025 11:43:24 +0100
Subject: [PATCH 1/2] Compile file in local checkout directory

This partially fixes bug#79188.

* lisp/emacs-lisp/package-vc.el (package-vc--unpack-1): Compile package
  in a local checkout directory when it is installed from such a
  location, for example with `package-vc-install-from-checkout'.
---
 lisp/emacs-lisp/package-vc.el | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
index 7433fce2d89..9e118c7af02 100644
--- a/lisp/emacs-lisp/package-vc.el
+++ b/lisp/emacs-lisp/package-vc.el
@@ -546,7 +546,14 @@ package-vc--unpack-1
         ;; FIXME: Compilation should be done as a separate, optional, step.
         ;; E.g. for multi-package installs, we should first install all packages
         ;; and then compile them.
-        (package--compile new-desc)
+        (package--compile
+         (if lisp-dir
+             ;; In case we are installing a package from a local
+             ;; checkout, we want to compile the checkout, not the
+             ;; redirection!
+             (package-desc-create :dir lisp-dir)
+          new-desc))
+
         (when package-native-compile
           (package--native-compile-async new-desc))
         ;; After compilation, load again any files loaded by
-- 
2.50.1

>From 07596327df8bee5831003b62c811dd3fc53f2a88 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Przemys=C5=82aw=20Kryger?= <[email-masked]>
Date: Fri, 8 Aug 2025 11:43:52 +0100
Subject: [PATCH 2/2] Store local checkout directory in autoloads indirection

* lisp/emacs-lisp/package-vc.el (package-vc--unpack-1): When installing
  from a local checkout, store the value of `:lisp-dir' property of
  `pkg-spec' in a "Package-spec" header in a generated autoloads
  indirection file.
* lisp/emacs-lisp/package-vc.el (package-vc--pkg-spec-from-autoloads):
  New function to retrieve package spec from a "Package-spec" header
  from autoloads indirection file (if any).
* lisp/emacs-lisp/package-vc.el (package-vc-rebuild): Retrieve package
  spec from autoloads indirection file (if any) and use it while calling
  `package-vc--unpack-1'.
* lisp/emacs-lisp/package-vc.el (package-vc-upgrade): Retrieve package
  spec from autoloads indirection file (if any) and use it while calling
  `package-vc--unpack-1' and for `vc-pull'.

fixes: bug#79188
---
 lisp/emacs-lisp/package-vc.el | 46 +++++++++++++++++++++++++++++------
 1 file changed, 39 insertions(+), 7 deletions(-)

diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
index 9e118c7af02..400e648b8f5 100644
--- a/lisp/emacs-lisp/package-vc.el
+++ b/lisp/emacs-lisp/package-vc.el
@@ -508,7 +508,14 @@ package-vc--unpack-1
         (when lisp-dir
           (write-region
            (with-temp-buffer
-             (insert ";; Autoload indirection for package-vc\n\n")
+             (insert ";; Autoload indirection for package-vc\n")
+             (insert ";; Package-spec: ")
+             ;; Store the pkg-spec such that it can be reused by
+             ;; `package-rebuild' and `package-vc-upgrade' to restore
+             ;; the same conditions as were when the indirection has
+             ;; been created for the first time.
+             (prin1 pkg-spec (current-buffer))
+             (insert "\n\n")
              (prin1 `(load (expand-file-name
                             ,(expand-file-name auto-name lisp-dir)
                             (or (and load-file-name
@@ -589,6 +596,19 @@ package-vc--unpack-1
 
 (declare-function project-remember-projects-under "project" (dir &optional recursive))
 
+(defun package-vc--pkg-spec-from-autoloads (pkg-desc)
+  "Read a \"Packcage-spec\" header from autoloads file for PKG-DESC."
+  (when-let* ((name (package-desc-name pkg-desc))
+              (autoloads (expand-file-name (format "%s-autoloads.el" name)
+                                           (package-desc-dir pkg-desc)))
+              ((file-exists-p autoloads))
+              (spec (with-temp-buffer
+                      (insert-file-contents autoloads)
+                      (ignore-errors
+                        (read (lm-header "package-spec"))))))
+    (cons (symbol-name name)
+          spec)))
+
 (defun package-vc--clone (pkg-desc pkg-spec dir rev)
   "Clone the package PKG-DESC whose spec is PKG-SPEC into the directory DIR.
 REV specifies a specific revision to checkout.  This overrides the `:branch'
@@ -756,7 +776,10 @@ package-vc-upgrade
   ;;
   ;; If there is a better way to do this, it should be done.
   (cl-assert (package-vc-p pkg-desc))
-  (letrec ((pkg-dir (package-desc-dir pkg-desc))
+  (letrec ((pkg-spec (package-vc--pkg-spec-from-autoloads pkg-desc))
+           (pkg-dir (package-desc-dir pkg-desc))
+           (source-dir (or (plist-get (cdr pkg-spec) :lisp-dir)
+                           pkg-dir))
            (vc-flags)
            (vc-filter-command-function
             (lambda (command file-or-list flags)
@@ -764,18 +787,22 @@ package-vc-upgrade
               (list command file-or-list flags)))
            (post-upgrade
             (lambda (_command _file-or-list flags)
-              (when (and (file-equal-p pkg-dir default-directory)
+              (when (and (file-equal-p source-dir default-directory)
                          (eq flags vc-flags))
                 (unwind-protect
                     (with-demoted-errors "Failed to activate: %S"
-                      (package-vc--unpack-1 pkg-desc pkg-dir))
+                      (let ((package-vc-selected-packages
+                             (if pkg-spec
+                                 (cons pkg-spec package-vc-selected-packages)
+                               package-vc-selected-packages)))
+                        (package-vc--unpack-1 pkg-desc pkg-dir)))
                   (remove-hook 'vc-post-command-functions post-upgrade))))))
     (add-hook 'vc-post-command-functions post-upgrade)
     (with-demoted-errors "Failed to fetch: %S"
       (require 'vc-dir)
       (with-current-buffer (vc-dir-prepare-status-buffer
-                            (format " *package-vc-dir: %s*" pkg-dir)
-                            pkg-dir (vc-responsible-backend pkg-dir))
+                            (format " *package-vc-dir: %s*" source-dir)
+                            source-dir (vc-responsible-backend source-dir))
         (vc-pull)))))
 
 (defun package-vc--archives-initialize ()
@@ -966,7 +993,12 @@ package-vc-rebuild
 is the responsibility of `package-vc-upgrade'.  Interactively,
 prompt for the name of the package to rebuild."
   (interactive (list (package-vc--read-package-desc "Rebuild package: " t)))
-  (package-vc--unpack-1 pkg-desc (package-desc-dir pkg-desc)))
+  (let* ((pkg-spec (package-vc--pkg-spec-from-autoloads pkg-desc))
+         (package-vc-selected-packages
+          (if pkg-spec
+              (cons pkg-spec package-vc-selected-packages)
+            package-vc-selected-packages)))
+    (package-vc--unpack-1 pkg-desc (package-desc-dir pkg-desc))))
 
 ;;;###autoload
 (defun package-vc-prepare-patch (pkg-desc subject revisions)
-- 
2.50.1