Package: emacs;
Reported by: Armin Darvish <armindarvish <at> gmail.com>
Date: Sun, 16 Feb 2025 07:31:02 UTC
Severity: normal
Found in version 30.0.93
Message #23 received at 76325 <at> debbugs.gnu.org (full text, mbox):
From: Philip Kaludercic <philipk <at> posteo.net> To: Armin Darvish <armindarvish <at> gmail.com> Cc: 76325 <at> debbugs.gnu.org Subject: Re: bug#76325: 30.0.93; package-vc-install fails with repositories that contain multiple single-file packages Date: Mon, 17 Feb 2025 21:19:00 +0000
[Message part 1 (text/plain, inline)]
I came up with this change, which appears to fix the issue on my end:
[Message part 2 (text/plain, inline)]
diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
index a18841fb64d..1dc427d13ac 100644
--- a/lisp/emacs-lisp/package-vc.el
+++ b/lisp/emacs-lisp/package-vc.el
@@ -485,10 +485,21 @@ package-vc--unpack-1
lm--prepare-package-dependencies
(nconc deps)
(setq 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 (package-desc-reqs pkg-desc)
+ (cl-loop with lessp = (lambda (a b)
+ (if (eq (car a) (car b))
+ (version< (cadr b) (cadr b))
+ (value< (car a) (car b))))
+ ;; sort the dependency list to have the newest
+ ;; versions at the end of the list, so that when
+ ;; iterating through the list we only request to
+ ;; install the newest necessary dependency:
+ with deps = (sort deps :lessp lessp)
+ for rest on (cdr deps)
+ for (name vers) in deps
+ unless (assq name deps)
+ collect (list name (version-to-list vers))))
+ (setf missing (package-vc-install-dependencies deps))
(setf missing (delq (assq (package-desc-name pkg-desc)
missing)
missing)))
[Message part 3 (text/plain, inline)]
Can you try it out as well?
Armin Darvish <armindarvish <at> gmail.com> writes:
> Hi Philip,
>
>>To clarify the terminology, ELPA/MELPA are package archives that take
>>usually take a source code checkout and prepare packages. For each
>>package, they might specify or infer a main file or some other source of
>>information to get the metadata like package dependencies, but they will
>>also throw out all other files that are not part of the package.
> Yes, what I meant was how MELPA builds. Wile it does put the files in
> load-path as well, but it ignores the "Package-Requires:" header for
> inferring package dependencies in all other files, but the main lisp
> file, and it can of course run into the problem you are trying to
> avoid but I would argue the MELPA's approach is better (see below).
But MELPA and package-vc are solving different problems (preparing a
tarball from a VC checkout vs. making a VC checkout loadable by
package.el).
>>Package-vc is kind of like that, in that it is an independent
>>implementation of ELPA's build scripts, but as the goal is to explicitly
>>load the directory with the source code checkout we don't remove the
>>"unrelated" files (that is one of the reasons we also advise against
>>combining multiple packages in the same directory).
>
> I understand the advice against multiple packages in the same repo,
> but a lot of the times a package has its own extensions (that may have
> a different set of dependencies) and it is much easier to have all the
> extensions in one repo for mmanaging the repo becuase it's easier to
> keep consistencies and also it is much easier to track issues and
> etc. when all of the related files are in the same repo.
This is mostly unrelated to the bug at hand, but I just want to mention
that you can still track multiple packages on multiple branches. It is
easy to develop them all at once if you use git-worktree(1) to load all
files at once.
>> I am not sure I follow your argument. The other files are still
>> installed and loadable, but broken since they are missing dependencies.
>> That is the state I wish to avoid.
>>
>> There is a bug in the dependency resolution, but that is a separate
>> issue IMO.
> Yes, but this is exactly what MELPA does. It puts those files in the
> laod path but ignores the "Package-Requires:" header when it comes to
> installing package dependencies.
IIRC most packages that are developed in a shared repository on MELPA
exclude the other files, so there is just a single package with a
dependency list. The ELPA build server does something like that as
well.
> The reason I think this makes more
> sense is because the dependencies are defined at the package level,
> and therefore they should be defined once in the main file, otherwise,
> the dependencies from all different files need to be compared at build
> time and the common required set of dependecies and versions need to
> be inferred. For example, as I showed with the embark example in my
> previous email, different files can require the same package but with
> a different minimum version.
I have already understood this point, don't worry. I hope the above
patch or something based on that should fix the issue. Do you
understand my point with missing dependencies breaking parts of the
installed packages?
>
> Right now `package-vc-installl' is
> pulling different versions (in different folders under "/elpa"
> directory) and I am not even sure in what order the different versions
> of embark are loaded. This can lead to much bigger problems than the
> issue you are trying to avoid (a.k.a. some featuers not working
> because the dependency is not available).
The trick we currently use is that VC packages have the highest priority
in being loaded. It is not elegant, and it would be a nice thing to
have some de-duplication to avoid installing embark if we already have a
VC checkout of embark-consult.
> Of course you can argue that there is a bug in dependency resolution,
> but to me the easiest solution would be to do what MELPA does, inly
> use the main file for inferring dependencies. Alternatively, if we add
> features such as "include-files", "ignore-files", "clone-depth", etc,
> as options for the recipe and do a much smarter job in inferring
> dependencies (e.g. do not create self-dependency, only install the
> highest required version of any package, ...), then there will be
> other ways to avoid this kind of issue.
Package-vc has two hard-constraints, 1. the package specification has to
be compatible with ELPA package specifications 2. has to be VCS
agnostic. A soft-but-important-constraint or goal is that the point of
package-vc is to make package development easier, so excluding files
from a checkout or manipulating the history is not something we are
really interested in supporting.
> Best,
> Armin
>
>
> Philip Kaludercic @ 2025-02-17 17:24 :
>
>> Armin Darvish <armindarvish <at> gmail.com> writes:
>>
>>> Hi Philip,
>>>
>>> I am on emacs versio 30.0.93. and I tried an init file like this:
>>> ,----
>>> | ;;; init.el --- -*- lexical-binding: t; -*-
>>> |
>>> | (require 'package)
>>> | (package-initialize)
>>> | (package-vc-install
>>> | '(consult-omni :url "<https://github.com/armindarvish/consult-omni>"
>>> | :main-file "consult-omni.el"))
>>> `----
>>
>> Yes, I can reproduce the issue now, thanks!
>>
>>>> The issue we have to keep in mind is that since we add the repository
>>>> directly to `load-path', all the files can be used no matter what
>>>> "specific" file you might intend to use. To retain usability and not
>>>> have unexpected function calls fail, we rather install all the
>>>> dependencies.
>>>
>>> My understanding was that packages are either a single-file package,
>>> which won't have this problem, or a multi-file package, in which case,
>>> the "Package-Requires: " header should only be in the main file and
>>> not the extra lisp files. As far as I know, other package managing
>>> systems (like MELPA) don't use hte "Package-Requires: " header in the
>>> files other than the main lisp file either.
>>
>> To clarify the terminology, ELPA/MELPA are package archives that take
>> usually take a source code checkout and prepare packages. For each
>> package, they might specify or infer a main file or some other source of
>> information to get the metadata like package dependencies, but they will
>> also throw out all other files that are not part of the package.
>>
>> Package-vc is kind of like that, in that it is an independent
>> implementation of ELPA's build scripts, but as the goal is to explicitly
>> load the directory with the source code checkout we don't remove the
>> "unrelated" files (that is one of the reasons we also advise against
>> combining multiple packages in the same directory).
>>
>>> Note that, we can still
>>> add all the files in the repo to load-path, but when automatically
>>> making the "define-package" declaration in a "package-pkg.el" file,
>>> the dependencies should be inferred form the main file and not others,
>>> otherwise there will be weird looped dependencies.
>>
>> I am not sure I follow your argument. The other files are still
>> installed and loadable, but broken since they are missing dependencies.
>> That is the state I wish to avoid.
>>
>> There is a bug in the dependency resolution, but that is a separate
>> issue IMO.
>>
>>> For example, in
>>> case of embark with the following init file:
>>>
>>> ,----
>>> | ;;; init.el --- -*- lexical-binding: t; -*-
>>> |
>>> | (require 'package)
>>> | ;; Adds the Melpa archive to the list of available repositories
>>> | (setq package-archives
>>> | '(("elpa" . "<https://elpa.gnu.org/packages/>")
>>> | ("melpa-stable" . "<https://stable.melpa.org/packages/>")
>>> | ("melpa" . "<https://melpa.org/packages/>")))
>>> | ;; Initializes the package infrastructure
>>> | (package-initialize)
>>> | (package-refresh-contents)
>>> |
>>> | (use-package embark
>>> | :vc (:url "<https://github.com/oantolin/embark>"))
>>> |
>>> `----
>>>
>>> I can see multiple versions of embark being pulled from MELPA becuase
>>> of looped depenncies in the embark-pkg.el file automatically created
>>> by package-vc-install. Here is the contents of that file:
>>>
>>> ,----
>>> | (define-package "embark" "1.1" "No description available." '((emacs
>>> | "25.1") (embark "0.9") (avy "0.5") (emacs "27.1") (compat
>>> | "29.1.4.0") (embark "1.0") (consult "1.0")) :kind vc :commit
>>> | "195add1f1ccd1059472c9df7334c97c4d155425e")
>>> `----
>>>
>>> Note that while this installs just fine, it is pulling 3 different
>>> versions of embark from MELPA becuase of this inferred looped
>>> self-dependency from differnt *.el files in embark repo, which is not
>>> the intended behavior by the author of that package.
>>>
>>>
>>> That said, I agree with the point that there can potentially be other
>>> solutions like terminating properly as you said or even better would
>>> be detecting and ignoring any self-dependency. I cannot think of any
>>> scenario where a package should depend on its own or on an older
>>> version of its own.
>>
>> I agree. I'll try to find a solution to the issue by fixing the
>> recursion issue and update this thread.
>>
>>>
>>> Philip Kaludercic @ 2025-02-16 18:53 :
>>>
>>>> Armin Darvish <armindarvish <at> gmail.com> writes:
>>>>
>>>>> Hello,
>>>>>
>>>>> I have noticed that with vc repositories that contain multiple
>>>>> related single-file
>>>>> packages, package-vc-install creates looped self-dependency that
>>>>> can cause errors.
>>>>>
>>>>> For example, trying to install the repository:
>>>>> <https://github.com/armindarvish/consult-omni>
>>>>> will result in "Lisp nesting exceeds ‘max-lisp-eval-depth’: 1601"
>>>>> error. This is
>>>>> because currently package-vc-install tries to read all ".el"
>>>>> files in the root
>>>>> directory to get the dependencies and build the "define-package"
>>>>> declaration in
>>>>> consult-omni-pkg.el. This is not compatible with repositories
>>>>> that have multiple
>>>>> single-file packages in the root directory. Instead, the
>>>>> dependencies should be
>>>>> inferred from the main lisp file only. This will be safe with
>>>>> multi-file packages
>>>>> as well because the convention is to have the "Package-Requires:"
>>>>> header only in
>>>>> the main lisp file and not the additional lisp files.
>>>>
>>>> What version of Emacs are you using? Installing your repository doesn't
>>>> raise any error when I try to do so.
>>>>
>>>>> There are other packages that have multiple single-file packages
>>>>> as well, for
>>>>> example, <https://github.com/oantolin/embark> includes embark and
>>>>> embark-consult in
>>>>> the root directory. Currently, installing embark with
>>>>> package-vc-install causes
>>>>> several different versions of embark being downlaoded because the
>>>>> dependencies are
>>>>> read from all of those files even though they are meant to be
>>>>> separate packages.
>>>>
>>>> The issue we have to keep in mind is that since we add the repository
>>>> directly to `load-path', all the files can be used no matter what
>>>> "specific" file you might intend to use. To retain usability and not
>>>> have unexpected function calls fail, we rather install all the
>>>> dependencies. The recursion error above hints at some programming
>>>> issue, where we don't terminate properly.
>>>>
>>>>>
>>>>> Best Regards,
>>>>> *Armin Darvish*
>>>>>
>>>>> --
>>>>> ------------------------------------------------------------------------
>>>>> [www.armindarvish.com]
>>>>>
>>>>>
>>>>> [www.armindarvish.com] <https://www.armindarvish.com/>
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.