bug#65283: 29.1; package-vc-install fails to install package "ement" because some dependencies installed by it are omitted from load-path

[Philip Kaludercic <philipk@posteo.net>]

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

Adam Porter <adam@alphapapa.net> writes:

> Hi Philip,
>
> On 8/15/23 15:11, Philip Kaludercic wrote:
>> Adam Porter <adam@alphapapa.net> writes:
>>
>>> On 8/14/23 03:13, Philip Kaludercic wrote:
>>>> Adam Porter <adam@alphapapa.net> writes:
>>>>
>>>>> Hi,
>>>>>
>>>>> Wanting to test whether package-vc-install in Emacs 29.1 is now a
>>>>> suitable replacement for Quelpa for installing my Ement.el
>      package from
>>>>> git, I started an instance of Emacs 29.1 in a clean configuration and
>>>>> evaluated:
>>
>> When you say clean configuration, do you mean that you invoked emacs
>> with -Q, or also --init-directory=[some fresh directory]?  I noticed
>> that you appear to be using Guix, so did you perhaps use a Guix
>> container shell?
> I've tried three different ways now:
>
> 1. Using my with-emacs.sh script [0] (which was written for Emacs
> before the "--init-directory" option was added, but remains compatible
> with Emacs 29.1):
>
>   with-emacs.sh -e emacs-29.1 -- --eval '(package-vc-install
>   "https://github.com/alphapapa/ement.el")'
>
> 2. Using "-q --init-directory", i.e.
>
>   emacs-29.1 -q --init-directory=$(mktemp -d) --eval
>   '(package-vc-install "https://github.com/alphapapa/ement.el")'
>
> 3. Using "-Q --init-directory", i.e.
>
>   emacs-29.1 -Q --init-directory=$(mktemp -d) --eval
>   '(package-vc-install "https://github.com/alphapapa/ement.el")'
>
> All three ways seem to produce the same problem with the same error
> messages and the same load-path missing taxy-magit-section's
> directory.

I have been using the third approach (with the difference that I invoked
`package-vc-install' as a command using M-x), and it worked.  When I use
your command, it doesn't -- so that is good from a reproducability
perspective.

> I do have Emacs 29.1 installed with Guix, on top of my Debian
> system. However, I've been using this setup for quite some time now,
> having also used Guix to install Emacs 27 and 28 previously, and I've
> never had this problem before.  So I don't think that Guix is related
> to the problem, although I can't say for certain.

I wasn't implying that it was to blame, rather I just wanted to make
sure whether or not you were/weren't launching Emacs in an isolated
container.

> Scouring the "*Messages*" buffer again, I noticed another series of
> messages:
>
>   Checking /tmp/tmp.m5Cj6y1gaF/elpa/taxy-0.10.1/examples...
>   Done (Total of 1 file compiled, 4 failed, 2 skipped in 2 directories)
>   Contacting host: elpa.gnu.org:443
>   Parsing tar file...done
>   Extracting...done
>     INFO     Scraping files for loaddefs...done
>     GEN      taxy-magit-section-autoloads.el
>   Unable to activate package ‘taxy-magit-section’.
>   Required package ‘magit-section-3.2.1’ is unavailable
>   Contacting host: elpa.gnu.org:443
>   Parsing tar file...done
>   Extracting...done
>     INFO     Scraping files for loaddefs...done
>     GEN      dash-autoloads.el
>   Checking /tmp/tmp.m5Cj6y1gaF/elpa/dash-2.19.1... [2 times]
>
> As you can see, it says that it can't activate taxy-magit-section
> because magit-section-3.2.1 is unavailable.  However, about 20 lines
> later:
>
>   Checking /tmp/tmp.m5Cj6y1gaF/elpa/dash-2.19.1/doc...
>   Done (Total of 2 files compiled, 2 skipped)
>   Contacting host: elpa.nongnu.org:443
>   Parsing tar file...done
>   Extracting...done
>     INFO     Scraping files for loaddefs...done
>     GEN      magit-section-autoloads.el
>   Checking /tmp/tmp.m5Cj6y1gaF/elpa/magit-section-3.3.0... [3 times]
>   Compiling
>   /tmp/tmp.m5Cj6y1gaF/elpa/magit-section-3.3.0/magit-section.el...done
>   Wrote /tmp/tmp.m5Cj6y1gaF/elpa/magit-section-3.3.0/magit-section.elc
>   Checking /tmp/tmp.m5Cj6y1gaF/elpa/magit-section-3.3.0...
>   Done (Total of 1 file compiled, 2 skipped)
>     INFO     Scraping files for loaddefs...done
>     GEN      ement-autoloads.el
>
> So it installs magit-section (at v3.3.0) two packages later.
>
> The taxy-magit-section has these dependencies specified in
> "taxy-magit-section.el":
>
>   ;; Package-Requires: ((emacs "26.3") (magit-section "3.2.1") (taxy
>      "0.10"))
>
> And, as listed in "C-h P taxy-magit-section RET":
>
>   Package taxy-magit-section is dependency.
>
>        Status: Installed in ‘taxy-magit-section-0.12.2/’.
>       Version: 0.12.2
>        Commit: 594531569c03206cbc83b74d679ffd0e93edb4d0
>       Summary: View Taxy structs in a Magit Section buffer
>      Requires: emacs-26.3, magit-section-3.2.1, taxy-0.10
>
> So it seems that magit-section is being installed after
> taxy-magit-section rather than before, which seems to prevent Emacs
> from activating the taxy-magit-section package (and thereby adding it
> to load-path), which later causes the compilation of ement to fail
> since it can't find taxy-magit-section.

That is interesting,  

> And all of this seems to be because the version number of
> magit-section is not being correctly recognized as a later version and
> thereby fulfilling the dependency--this even though it is being
> recognized as a dependency and installed by another part of the
> process.
>
> Meanwhile, if I install using package-install with:
>
>   emacs-29.1 --init-directory=$(mktemp -d) -q --eval "(package-install
>   'ement)"
>
> Then installation of all packages proceeds without error, and
> load-path shows:
>
>     ("/tmp/tmp.19yxzzQiIk/elpa/ement-0.10" "
>     /tmp/tmp.19yxzzQiIk/elpa/persist-0.5" "
>     /tmp/tmp.19yxzzQiIk/elpa/plz-0.7" "
>     /tmp/tmp.19yxzzQiIk/elpa/taxy-magit-section-0.12.2" "
>     /tmp/tmp.19yxzzQiIk/elpa/magit-section-3.3.0" "
>     /tmp/tmp.19yxzzQiIk/elpa/dash-2.19.1" "
>     /tmp/tmp.19yxzzQiIk/elpa/taxy-0.10.1" "
>     /tmp/tmp.19yxzzQiIk/elpa/svg-lib-0.2.7" "
>     /tmp/tmp.19yxzzQiIk/elpa/transient-0.4.1" "
>     /tmp/tmp.19yxzzQiIk/elpa/compat-29.1.4.2" "
>     /home/me/.guix-profile/share/emacs/site-lisp" 

[...]

> So there appears to be a bug somewhere in the package-vc library.

With a slight modification of `package-vc-install-dependencies', we can
see in which order the dependencies are installed:

--8<---------------cut here---------------start------------->8---
(package-vc-install-dependencies (package-desc-reqs (cadr (assoc 'ement package-archive-contents))))
;=> (:to-install (svg-lib persist plz taxy taxy-magit-section) :missing nil)
;                ^ mapc runs over this
--8<---------------cut here---------------end--------------->8---

This made me suspect that there might be a bug in that function, and
with these changes, the above command works (I can't test it because my
matrix account AFAIK isn't supported by Ement):


[-- Attachment #2: Type: text/plain, Size: 1085 bytes --]

diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
index db8b41aee6a..2da788ddeec 100644
--- a/lisp/emacs-lisp/package-vc.el
+++ b/lisp/emacs-lisp/package-vc.el
@@ -451,13 +451,11 @@ package-vc-install-dependencies
                              (desc (cadr (assoc package pac))))
                         (and desc (seq-some
                                    (apply-partially #'depends-on-p target)
-                                   (package-desc-reqs desc))))))
+                                   (mapcar #'car (package-desc-reqs desc)))))))
                 (dependent-order (a b)
                   (let ((desc-a (package-desc-name a))
                         (desc-b (package-desc-name b)))
-                    (or (not desc-a) (not desc-b)
-                        (not (depends-on-p desc-b desc-a))
-                        (depends-on-p desc-a desc-b)))))
+                    (depends-on-p desc-a desc-b))))
       (mapc #'search requirements)
       (cl-callf sort to-install #'version-order)
       (cl-callf seq-uniq to-install #'duplicate-p)

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


It boils down to a lack-of-type-safety kind of bug.  It was comparing
'package-name with (package-name . "version string"), that always ends
up being nil.

Not sure about the change to `dependent-order', I'll have to think about
that in more detail tomorrow.  Perhaps there is an all-together shorter
patch?

> I see that you tested with an Emacs 30.0.50 build.  Could you test
> with 29.1, the version I'm using?
>
> Thanks,
> Adam
>
> 0: https://github.com/alphapapa/with-emacs.sh