Package: emacs;
Reported by: Ship Mints <shipmints <at> gmail.com>
Date: Tue, 25 Feb 2025 20:53:01 UTC
Severity: normal
Tags: patch
To reply to this bug, email your comments to 76568 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#76568
; Package emacs
.
(Tue, 25 Feb 2025 20:53:02 GMT) Full text and rfc822 format available.Ship Mints <shipmints <at> gmail.com>
:bug-gnu-emacs <at> gnu.org
.
(Tue, 25 Feb 2025 20:53:02 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Ship Mints <shipmints <at> gmail.com> To: bug-gnu-emacs <at> gnu.org Subject: 'package-install' should not install duplicate packages Date: Tue, 25 Feb 2025 15:52:02 -0500
[Message part 1 (text/plain, inline)]
As part of my production upgrade to 30.1, and before I wrote a program to install my local ELPA tree from scratch, I tried to first curate my packages and change from MELPA to generally equivalent GNU ELPA or non-GNU ELPA archives. The result was that I had two of each package installed. I think there's a bug in 'package-install' which, when invoked from 'package-install-button-action', processes the new package spec, and incorrectly checks to see if the package is already installed. Interactive invocation of 'package-install' yields the package name from the prompt, not its archive description. If the below is correct, I can submit a patch to make 'package-install' behave like 'package-reinstall' for the non-interactive case. (defun package-install (pkg &optional dont-select) ... (if-let* ((transaction (if (package-desc-p pkg) ;; Problem seems to be here. If the new pkg desc is for a ;; different archive directory name style, package-installed-p ;; fails as it checks to see if the new directory exists (which ;; does not), ignoring the old archive directory. (unless (package-installed-p pkg) (package-compute-transaction (list pkg) (package-desc-reqs pkg))) In contrast, 'package-reinstall' does the right thing by first deleting the existing package before installing the new one. (defun package-reinstall (pkg) ... (package-delete (if (package-desc-p pkg) pkg (cadr (assq pkg package-alist))) 'force 'nosave) (package-install pkg 'dont-select)) -Stephane
[Message part 2 (text/html, inline)]
bug-gnu-emacs <at> gnu.org
:bug#76568
; Package emacs
.
(Wed, 26 Feb 2025 13:33:02 GMT) Full text and rfc822 format available.Message #8 received at 76568 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: Ship Mints <shipmints <at> gmail.com>, Philip Kaludercic <philipk <at> posteo.net> Cc: 76568 <at> debbugs.gnu.org Subject: Re: bug#76568: 'package-install' should not install duplicate packages Date: Wed, 26 Feb 2025 15:31:52 +0200
> From: Ship Mints <shipmints <at> gmail.com> > Date: Tue, 25 Feb 2025 15:52:02 -0500 > > As part of my production upgrade to 30.1, and before I wrote a program to install my local ELPA tree from > scratch, I tried to first curate my packages and change from MELPA to generally equivalent GNU ELPA or > non-GNU ELPA archives. The result was that I had two of each package installed. > > I think there's a bug in 'package-install' which, when invoked from 'package-install-button-action', processes > the new package spec, and incorrectly checks to see if the package is already installed. Interactive > invocation of 'package-install' yields the package name from the prompt, not its archive description. > > If the below is correct, I can submit a patch to make 'package-install' behave like 'package-reinstall' for the > non-interactive case. > > (defun package-install (pkg &optional dont-select) > ... > (if-let* ((transaction > (if (package-desc-p pkg) > ;; Problem seems to be here. If the new pkg desc is for a > ;; different archive directory name style, package-installed-p > ;; fails as it checks to see if the new directory exists (which > ;; does not), ignoring the old archive directory. > (unless (package-installed-p pkg) > (package-compute-transaction (list pkg) > (package-desc-reqs pkg))) > > In contrast, 'package-reinstall' does the right thing by first deleting the existing package before installing the > new one. > > (defun package-reinstall (pkg) > ... > (package-delete > (if (package-desc-p pkg) pkg (cadr (assq pkg package-alist))) > 'force 'nosave) > (package-install pkg 'dont-select)) Philip, any comments or suggestions?
bug-gnu-emacs <at> gnu.org
:bug#76568
; Package emacs
.
(Tue, 04 Mar 2025 02:36:02 GMT) Full text and rfc822 format available.Message #11 received at 76568 <at> debbugs.gnu.org (full text, mbox):
From: Stefan Kangas <stefankangas <at> gmail.com> To: Ship Mints <shipmints <at> gmail.com> Cc: 76568 <at> debbugs.gnu.org Subject: Re: bug#76568: 'package-install' should not install duplicate packages Date: Tue, 4 Mar 2025 02:34:54 +0000
Ship Mints <shipmints <at> gmail.com> writes: > As part of my production upgrade to 30.1, and before I wrote a program to install my local > ELPA tree from scratch, I tried to first curate my packages and change from MELPA to > generally equivalent GNU ELPA or non-GNU ELPA archives. The result was that I had two of > each package installed. > > I think there's a bug in 'package-install' which, when invoked from > 'package-install-button-action', processes the new package spec, and incorrectly checks to > see if the package is already installed. Interactive invocation of 'package-install' yields the > package name from the prompt, not its archive description. > > If the below is correct, I can submit a patch to make 'package-install' behave like > 'package-reinstall' for the non-interactive case. Please submit a patch, but could we also have tests for this please? Thanks in advance. > > (defun package-install (pkg &optional dont-select) > ... > (if-let* ((transaction > (if (package-desc-p pkg) > ;; Problem seems to be here. If the new pkg desc is for a > ;; different archive directory name style, package-installed-p > ;; fails as it checks to see if the new directory exists (which > ;; does not), ignoring the old archive directory. > (unless (package-installed-p pkg) > (package-compute-transaction (list pkg) > (package-desc-reqs pkg))) > > In contrast, 'package-reinstall' does the right thing by first deleting the existing package before > installing the new one. > > (defun package-reinstall (pkg) > ... > (package-delete > (if (package-desc-p pkg) pkg (cadr (assq pkg package-alist))) > 'force 'nosave) > (package-install pkg 'dont-select)) > > -Stephane
bug-gnu-emacs <at> gnu.org
:bug#76568
; Package emacs
.
(Wed, 05 Mar 2025 17:02:02 GMT) Full text and rfc822 format available.Message #14 received at 76568 <at> debbugs.gnu.org (full text, mbox):
From: Ship Mints <shipmints <at> gmail.com> To: Stefan Kangas <stefankangas <at> gmail.com> Cc: 76568 <at> debbugs.gnu.org Subject: Re: bug#76568: 'package-install' should not install duplicate packages Date: Wed, 5 Mar 2025 12:00:55 -0500
[Message part 1 (text/plain, inline)]
On Mon, Mar 3, 2025 at 9:34 PM Stefan Kangas <stefankangas <at> gmail.com> wrote: > Ship Mints <shipmints <at> gmail.com> writes: > > > As part of my production upgrade to 30.1, and before I wrote a program > to install my local > > ELPA tree from scratch, I tried to first curate my packages and change > from MELPA to > > generally equivalent GNU ELPA or non-GNU ELPA archives. The result was > that I had two of > > each package installed. > > > > I think there's a bug in 'package-install' which, when invoked from > > 'package-install-button-action', processes the new package spec, and > incorrectly checks to > > see if the package is already installed. Interactive invocation of > 'package-install' yields the > > package name from the prompt, not its archive description. > > > > If the below is correct, I can submit a patch to make 'package-install' > behave like > > 'package-reinstall' for the non-interactive case. > > Please submit a patch, but could we also have tests for this please? > > Thanks in advance. > Patch attached. It prevents the menu-driven case from erasing the already installed message. It could suggest to the user to remove and then install or we could offer to use package upgrade to the chosen package-desc. At the very least, the patch prevents duplicates. -Stephane
[Message part 2 (text/html, inline)]
[0001-Correct-package-install-to-detect-installed-packages.patch (application/octet-stream, attachment)]
Stefan Kangas <stefankangas <at> gmail.com>
to control <at> debbugs.gnu.org
.
(Wed, 05 Mar 2025 17:55:01 GMT) Full text and rfc822 format available.bug-gnu-emacs <at> gnu.org
:bug#76568
; Package emacs
.
(Wed, 05 Mar 2025 19:56:01 GMT) Full text and rfc822 format available.Message #19 received at 76568 <at> debbugs.gnu.org (full text, mbox):
From: Ship Mints <shipmints <at> gmail.com> To: Stefan Kangas <stefankangas <at> gmail.com> Cc: 76568 <at> debbugs.gnu.org Subject: Re: bug#76568: 'package-install' should not install duplicate packages Date: Wed, 5 Mar 2025 14:55:38 -0500
[Message part 1 (text/plain, inline)]
On Wed, Mar 5, 2025 at 12:00 PM Ship Mints <shipmints <at> gmail.com> wrote: > On Mon, Mar 3, 2025 at 9:34 PM Stefan Kangas <stefankangas <at> gmail.com> > wrote: > >> Ship Mints <shipmints <at> gmail.com> writes: >> >> > As part of my production upgrade to 30.1, and before I wrote a program >> to install my local >> > ELPA tree from scratch, I tried to first curate my packages and change >> from MELPA to >> > generally equivalent GNU ELPA or non-GNU ELPA archives. The result was >> that I had two of >> > each package installed. >> > >> > I think there's a bug in 'package-install' which, when invoked from >> > 'package-install-button-action', processes the new package spec, and >> incorrectly checks to >> > see if the package is already installed. Interactive invocation of >> 'package-install' yields the >> > package name from the prompt, not its archive description. >> > >> > If the below is correct, I can submit a patch to make 'package-install' >> behave like >> > 'package-reinstall' for the non-interactive case. >> >> Please submit a patch, but could we also have tests for this please? >> >> Thanks in advance. >> > > Patch attached. It prevents the menu-driven case from erasing the already > installed message. It could suggest to the user to remove and then install > or we could offer to use package upgrade to the chosen package-desc. At > the very least, the patch prevents duplicates. > Thinking about it a bit more, this patch needs a little more work. The menu-driven package "upgrade" in describe-package-1 workflow will be interrupted by this patch. It does not differentiate install vs. upgrade and calls package-install which will now complain the package already exists. This should be changed to run package-upgrade. I'll take a look. Any feedback on the minimal patch is still welcome in the meantime. >
[Message part 2 (text/html, inline)]
bug-gnu-emacs <at> gnu.org
:bug#76568
; Package emacs
.
(Wed, 05 Mar 2025 21:04:02 GMT) Full text and rfc822 format available.Message #22 received at 76568 <at> debbugs.gnu.org (full text, mbox):
From: Ship Mints <shipmints <at> gmail.com> To: Stefan Kangas <stefankangas <at> gmail.com> Cc: 76568 <at> debbugs.gnu.org Subject: Re: bug#76568: 'package-install' should not install duplicate packages Date: Wed, 5 Mar 2025 16:02:57 -0500
[Message part 1 (text/plain, inline)]
On Wed, Mar 5, 2025 at 2:55 PM Ship Mints <shipmints <at> gmail.com> wrote: > On Wed, Mar 5, 2025 at 12:00 PM Ship Mints <shipmints <at> gmail.com> wrote: > >> On Mon, Mar 3, 2025 at 9:34 PM Stefan Kangas <stefankangas <at> gmail.com> >> wrote: >> >>> Ship Mints <shipmints <at> gmail.com> writes: >>> >>> > As part of my production upgrade to 30.1, and before I wrote a program >>> to install my local >>> > ELPA tree from scratch, I tried to first curate my packages and change >>> from MELPA to >>> > generally equivalent GNU ELPA or non-GNU ELPA archives. The result >>> was that I had two of >>> > each package installed. >>> > >>> > I think there's a bug in 'package-install' which, when invoked from >>> > 'package-install-button-action', processes the new package spec, and >>> incorrectly checks to >>> > see if the package is already installed. Interactive invocation of >>> 'package-install' yields the >>> > package name from the prompt, not its archive description. >>> > >>> > If the below is correct, I can submit a patch to make >>> 'package-install' behave like >>> > 'package-reinstall' for the non-interactive case. >>> >>> Please submit a patch, but could we also have tests for this please? >>> >>> Thanks in advance. >>> >> >> Patch attached. It prevents the menu-driven case from erasing the >> already installed message. It could suggest to the user to remove and then >> install or we could offer to use package upgrade to the chosen >> package-desc. At the very least, the patch prevents duplicates. >> > > Thinking about it a bit more, this patch needs a little more work. The > menu-driven package "upgrade" in describe-package-1 workflow will be interrupted by > this patch. It does not differentiate install vs. upgrade and calls > package-install which will now complain the package already exists. This > should be changed to run package-upgrade. I'll take a look. > > Any feedback on the minimal patch is still welcome in the meantime. > This patch now accommodates both scenarios.
[Message part 2 (text/html, inline)]
[0001-Correct-package-install-to-detect-installed-packages.patch (application/octet-stream, attachment)]
bug-gnu-emacs <at> gnu.org
:bug#76568
; Package emacs
.
(Wed, 05 Mar 2025 21:06:02 GMT) Full text and rfc822 format available.Message #25 received at 76568 <at> debbugs.gnu.org (full text, mbox):
From: Ship Mints <shipmints <at> gmail.com> To: Stefan Kangas <stefankangas <at> gmail.com> Cc: 76568 <at> debbugs.gnu.org Subject: Re: bug#76568: 'package-install' should not install duplicate packages Date: Wed, 5 Mar 2025 16:05:25 -0500
[Message part 1 (text/plain, inline)]
On Wed, Mar 5, 2025 at 4:02 PM Ship Mints <shipmints <at> gmail.com> wrote: > On Wed, Mar 5, 2025 at 2:55 PM Ship Mints <shipmints <at> gmail.com> wrote: > >> On Wed, Mar 5, 2025 at 12:00 PM Ship Mints <shipmints <at> gmail.com> wrote: >> >>> On Mon, Mar 3, 2025 at 9:34 PM Stefan Kangas <stefankangas <at> gmail.com> >>> wrote: >>> >>>> Ship Mints <shipmints <at> gmail.com> writes: >>>> >>>> > As part of my production upgrade to 30.1, and before I wrote a >>>> program to install my local >>>> > ELPA tree from scratch, I tried to first curate my packages and >>>> change from MELPA to >>>> > generally equivalent GNU ELPA or non-GNU ELPA archives. The result >>>> was that I had two of >>>> > each package installed. >>>> > >>>> > I think there's a bug in 'package-install' which, when invoked from >>>> > 'package-install-button-action', processes the new package spec, and >>>> incorrectly checks to >>>> > see if the package is already installed. Interactive invocation of >>>> 'package-install' yields the >>>> > package name from the prompt, not its archive description. >>>> > >>>> > If the below is correct, I can submit a patch to make >>>> 'package-install' behave like >>>> > 'package-reinstall' for the non-interactive case. >>>> >>>> Please submit a patch, but could we also have tests for this please? >>>> >>>> Thanks in advance. >>>> >>> >>> Patch attached. It prevents the menu-driven case from erasing the >>> already installed message. It could suggest to the user to remove and then >>> install or we could offer to use package upgrade to the chosen >>> package-desc. At the very least, the patch prevents duplicates. >>> >> >> Thinking about it a bit more, this patch needs a little more work. The >> menu-driven package "upgrade" in describe-package-1 workflow will be interrupted by >> this patch. It does not differentiate install vs. upgrade and calls >> package-install which will now complain the package already exists. This >> should be changed to run package-upgrade. I'll take a look. >> >> Any feedback on the minimal patch is still welcome in the meantime. >> > > This patch now accommodates both scenarios. > Now with an amended commit log.
[Message part 2 (text/html, inline)]
[0001-Correct-package-install-to-detect-installed-packages.patch (application/octet-stream, attachment)]
bug-gnu-emacs <at> gnu.org
:bug#76568
; Package emacs
.
(Fri, 07 Mar 2025 11:34:02 GMT) Full text and rfc822 format available.Message #28 received at 76568 <at> debbugs.gnu.org (full text, mbox):
From: Ship Mints <shipmints <at> gmail.com> To: Stefan Kangas <stefankangas <at> gmail.com> Cc: 76568 <at> debbugs.gnu.org Subject: Re: bug#76568: 'package-install' should not install duplicate packages Date: Fri, 7 Mar 2025 06:33:21 -0500
[Message part 1 (text/plain, inline)]
On Wed, Mar 5, 2025 at 4:05 PM Ship Mints <shipmints <at> gmail.com> wrote: > On Wed, Mar 5, 2025 at 4:02 PM Ship Mints <shipmints <at> gmail.com> wrote: > >> On Wed, Mar 5, 2025 at 2:55 PM Ship Mints <shipmints <at> gmail.com> wrote: >> >>> On Wed, Mar 5, 2025 at 12:00 PM Ship Mints <shipmints <at> gmail.com> wrote: >>> >>>> On Mon, Mar 3, 2025 at 9:34 PM Stefan Kangas <stefankangas <at> gmail.com> >>>> wrote: >>>> >>>>> Ship Mints <shipmints <at> gmail.com> writes: >>>>> >>>>> > As part of my production upgrade to 30.1, and before I wrote a >>>>> program to install my local >>>>> > ELPA tree from scratch, I tried to first curate my packages and >>>>> change from MELPA to >>>>> > generally equivalent GNU ELPA or non-GNU ELPA archives. The result >>>>> was that I had two of >>>>> > each package installed. >>>>> > >>>>> > I think there's a bug in 'package-install' which, when invoked from >>>>> > 'package-install-button-action', processes the new package spec, and >>>>> incorrectly checks to >>>>> > see if the package is already installed. Interactive invocation of >>>>> 'package-install' yields the >>>>> > package name from the prompt, not its archive description. >>>>> > >>>>> > If the below is correct, I can submit a patch to make >>>>> 'package-install' behave like >>>>> > 'package-reinstall' for the non-interactive case. >>>>> >>>>> Please submit a patch, but could we also have tests for this please? >>>>> >>>>> Thanks in advance. >>>>> >>>> >>>> Patch attached. It prevents the menu-driven case from erasing the >>>> already installed message. It could suggest to the user to remove and then >>>> install or we could offer to use package upgrade to the chosen >>>> package-desc. At the very least, the patch prevents duplicates. >>>> >>> >>> Thinking about it a bit more, this patch needs a little more work. The >>> menu-driven package "upgrade" in describe-package-1 workflow will be interrupted by >>> this patch. It does not differentiate install vs. upgrade and calls >>> package-install which will now complain the package already exists. This >>> should be changed to run package-upgrade. I'll take a look. >>> >>> Any feedback on the minimal patch is still welcome in the meantime. >>> >> >> This patch now accommodates both scenarios. >> > > Now with an amended commit log. > Attached patch with improved package replacement prompt.
[Message part 2 (text/html, inline)]
[0001-Correct-package-install-to-detect-installed-packages.patch (application/octet-stream, attachment)]
bug-gnu-emacs <at> gnu.org
:bug#76568
; Package emacs
.
(Sun, 16 Mar 2025 18:53:02 GMT) Full text and rfc822 format available.Message #31 received at 76568 <at> debbugs.gnu.org (full text, mbox):
From: Ship Mints <shipmints <at> gmail.com> To: Stefan Kangas <stefankangas <at> gmail.com> Cc: 76568 <at> debbugs.gnu.org Subject: Re: bug#76568: 'package-install' should not install duplicate packages Date: Sun, 16 Mar 2025 14:52:33 -0400
[Message part 1 (text/plain, inline)]
Stefan, I see Philip and David working on package.el. It would be fruitful for them to do that on top of this patch. On Fri, Mar 7, 2025 at 6:33 AM Ship Mints <shipmints <at> gmail.com> wrote: > On Wed, Mar 5, 2025 at 4:05 PM Ship Mints <shipmints <at> gmail.com> wrote: > >> On Wed, Mar 5, 2025 at 4:02 PM Ship Mints <shipmints <at> gmail.com> wrote: >> >>> On Wed, Mar 5, 2025 at 2:55 PM Ship Mints <shipmints <at> gmail.com> wrote: >>> >>>> On Wed, Mar 5, 2025 at 12:00 PM Ship Mints <shipmints <at> gmail.com> wrote: >>>> >>>>> On Mon, Mar 3, 2025 at 9:34 PM Stefan Kangas <stefankangas <at> gmail.com> >>>>> wrote: >>>>> >>>>>> Ship Mints <shipmints <at> gmail.com> writes: >>>>>> >>>>>> > As part of my production upgrade to 30.1, and before I wrote a >>>>>> program to install my local >>>>>> > ELPA tree from scratch, I tried to first curate my packages and >>>>>> change from MELPA to >>>>>> > generally equivalent GNU ELPA or non-GNU ELPA archives. The result >>>>>> was that I had two of >>>>>> > each package installed. >>>>>> > >>>>>> > I think there's a bug in 'package-install' which, when invoked from >>>>>> > 'package-install-button-action', processes the new package spec, >>>>>> and incorrectly checks to >>>>>> > see if the package is already installed. Interactive invocation of >>>>>> 'package-install' yields the >>>>>> > package name from the prompt, not its archive description. >>>>>> > >>>>>> > If the below is correct, I can submit a patch to make >>>>>> 'package-install' behave like >>>>>> > 'package-reinstall' for the non-interactive case. >>>>>> >>>>>> Please submit a patch, but could we also have tests for this please? >>>>>> >>>>>> Thanks in advance. >>>>>> >>>>> >>>>> Patch attached. It prevents the menu-driven case from erasing the >>>>> already installed message. It could suggest to the user to remove and then >>>>> install or we could offer to use package upgrade to the chosen >>>>> package-desc. At the very least, the patch prevents duplicates. >>>>> >>>> >>>> Thinking about it a bit more, this patch needs a little more work. The >>>> menu-driven package "upgrade" in describe-package-1 workflow will be interrupted by >>>> this patch. It does not differentiate install vs. upgrade and calls >>>> package-install which will now complain the package already exists. This >>>> should be changed to run package-upgrade. I'll take a look. >>>> >>>> Any feedback on the minimal patch is still welcome in the meantime. >>>> >>> >>> This patch now accommodates both scenarios. >>> >> >> Now with an amended commit log. >> > > Attached patch with improved package replacement prompt. >
[Message part 2 (text/html, inline)]
bug-gnu-emacs <at> gnu.org
:bug#76568
; Package emacs
.
(Thu, 20 Mar 2025 14:42:02 GMT) Full text and rfc822 format available.Message #34 received at 76568 <at> debbugs.gnu.org (full text, mbox):
From: Ship Mints <shipmints <at> gmail.com> To: Stefan Kangas <stefankangas <at> gmail.com> Cc: 76568 <at> debbugs.gnu.org Subject: Re: bug#76568: 'package-install' should not install duplicate packages Date: Thu, 20 Mar 2025 10:41:22 -0400
[Message part 1 (text/plain, inline)]
On Fri, Mar 7, 2025 at 6:33 AM Ship Mints <shipmints <at> gmail.com> wrote: > On Wed, Mar 5, 2025 at 4:05 PM Ship Mints <shipmints <at> gmail.com> wrote: > >> On Wed, Mar 5, 2025 at 4:02 PM Ship Mints <shipmints <at> gmail.com> wrote: >> >>> On Wed, Mar 5, 2025 at 2:55 PM Ship Mints <shipmints <at> gmail.com> wrote: >>> >>>> On Wed, Mar 5, 2025 at 12:00 PM Ship Mints <shipmints <at> gmail.com> wrote: >>>> >>>>> On Mon, Mar 3, 2025 at 9:34 PM Stefan Kangas <stefankangas <at> gmail.com> >>>>> wrote: >>>>> >>>>>> Ship Mints <shipmints <at> gmail.com> writes: >>>>>> >>>>>> > As part of my production upgrade to 30.1, and before I wrote a >>>>>> program to install my local >>>>>> > ELPA tree from scratch, I tried to first curate my packages and >>>>>> change from MELPA to >>>>>> > generally equivalent GNU ELPA or non-GNU ELPA archives. The result >>>>>> was that I had two of >>>>>> > each package installed. >>>>>> > >>>>>> > I think there's a bug in 'package-install' which, when invoked from >>>>>> > 'package-install-button-action', processes the new package spec, >>>>>> and incorrectly checks to >>>>>> > see if the package is already installed. Interactive invocation of >>>>>> 'package-install' yields the >>>>>> > package name from the prompt, not its archive description. >>>>>> > >>>>>> > If the below is correct, I can submit a patch to make >>>>>> 'package-install' behave like >>>>>> > 'package-reinstall' for the non-interactive case. >>>>>> >>>>>> Please submit a patch, but could we also have tests for this please? >>>>>> >>>>>> Thanks in advance. >>>>>> >>>>> >>>>> Patch attached. It prevents the menu-driven case from erasing the >>>>> already installed message. It could suggest to the user to remove and then >>>>> install or we could offer to use package upgrade to the chosen >>>>> package-desc. At the very least, the patch prevents duplicates. >>>>> >>>> >>>> Thinking about it a bit more, this patch needs a little more work. The >>>> menu-driven package "upgrade" in describe-package-1 workflow will be interrupted by >>>> this patch. It does not differentiate install vs. upgrade and calls >>>> package-install which will now complain the package already exists. This >>>> should be changed to run package-upgrade. I'll take a look. >>>> >>>> Any feedback on the minimal patch is still welcome in the meantime. >>>> >>> >>> This patch now accommodates both scenarios. >>> >> >> Now with an amended commit log. >> > > Attached patch with improved package replacement prompt. > Here's a refined version of the patch which allows users to optionally replace already-installed packages, or install/keep both. Testing this revealed some interesting other issues which may already be known. I'll see if I can find bug reports and if not I'll submit new ones. One example is that packages from different archives have incompatible version numbering schemes which causes false positives with the obsolete package detection mechanism. Another is that package-reinstall does not respect the originating archive, instead preferring package-archive-priorities which may install a different version than is expected by a user. Now might be a good time to consider that packages should be installed in archive-specific subdirectories rather than commingling them? I'd appreciate it if some others would look at this and test it. It might be adequate as a stop-gap for its intended purpose to avoid casual duplicate installs and some feedback would help. -Stephane
[Message part 2 (text/html, inline)]
[0001-Correct-package-install-to-detect-installed-packages.patch (application/octet-stream, attachment)]
bug-gnu-emacs <at> gnu.org
:bug#76568
; Package emacs
.
(Sat, 10 May 2025 11:36:01 GMT) Full text and rfc822 format available.Message #37 received at 76568 <at> debbugs.gnu.org (full text, mbox):
From: Ship Mints <shipmints <at> gmail.com> To: Stefan Kangas <stefankangas <at> gmail.com> Cc: 76568 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, Philip Kaludercic <philipk <at> posteo.net> Subject: Re: bug#76568: 'package-install' should not install duplicate packages Date: Sat, 10 May 2025 07:35:27 -0400
[Message part 1 (text/plain, inline)]
On Thu, Mar 20, 2025 at 10:41 AM Ship Mints <shipmints <at> gmail.com> wrote: > On Fri, Mar 7, 2025 at 6:33 AM Ship Mints <shipmints <at> gmail.com> wrote: > >> On Wed, Mar 5, 2025 at 4:05 PM Ship Mints <shipmints <at> gmail.com> wrote: >> >>> On Wed, Mar 5, 2025 at 4:02 PM Ship Mints <shipmints <at> gmail.com> wrote: >>> >>>> On Wed, Mar 5, 2025 at 2:55 PM Ship Mints <shipmints <at> gmail.com> wrote: >>>> >>>>> On Wed, Mar 5, 2025 at 12:00 PM Ship Mints <shipmints <at> gmail.com> >>>>> wrote: >>>>> >>>>>> On Mon, Mar 3, 2025 at 9:34 PM Stefan Kangas <stefankangas <at> gmail.com> >>>>>> wrote: >>>>>> >>>>>>> Ship Mints <shipmints <at> gmail.com> writes: >>>>>>> >>>>>>> > As part of my production upgrade to 30.1, and before I wrote a >>>>>>> program to install my local >>>>>>> > ELPA tree from scratch, I tried to first curate my packages and >>>>>>> change from MELPA to >>>>>>> > generally equivalent GNU ELPA or non-GNU ELPA archives. The >>>>>>> result was that I had two of >>>>>>> > each package installed. >>>>>>> > >>>>>>> > I think there's a bug in 'package-install' which, when invoked from >>>>>>> > 'package-install-button-action', processes the new package spec, >>>>>>> and incorrectly checks to >>>>>>> > see if the package is already installed. Interactive invocation >>>>>>> of 'package-install' yields the >>>>>>> > package name from the prompt, not its archive description. >>>>>>> > >>>>>>> > If the below is correct, I can submit a patch to make >>>>>>> 'package-install' behave like >>>>>>> > 'package-reinstall' for the non-interactive case. >>>>>>> >>>>>>> Please submit a patch, but could we also have tests for this please? >>>>>>> >>>>>>> Thanks in advance. >>>>>>> >>>>>> >>>>>> Patch attached. It prevents the menu-driven case from erasing the >>>>>> already installed message. It could suggest to the user to remove and then >>>>>> install or we could offer to use package upgrade to the chosen >>>>>> package-desc. At the very least, the patch prevents duplicates. >>>>>> >>>>> >>>>> Thinking about it a bit more, this patch needs a little more work. The >>>>> menu-driven package "upgrade" in describe-package-1 workflow will be interrupted by >>>>> this patch. It does not differentiate install vs. upgrade and calls >>>>> package-install which will now complain the package already exists. This >>>>> should be changed to run package-upgrade. I'll take a look. >>>>> >>>>> Any feedback on the minimal patch is still welcome in the meantime. >>>>> >>>> >>>> This patch now accommodates both scenarios. >>>> >>> >>> Now with an amended commit log. >>> >> >> Attached patch with improved package replacement prompt. >> > > Here's a refined version of the patch which allows users to optionally > replace already-installed packages, or install/keep both. > > Testing this revealed some interesting other issues which may already be > known. I'll see if I can find bug reports and if not I'll submit new > ones. One example is that packages from different archives have > incompatible version numbering schemes which causes false positives with > the obsolete package detection mechanism. Another is that > package-reinstall does not respect the originating archive, instead > preferring package-archive-priorities which may install a different version > than is expected by a user. Now might be a good time to consider that > packages should be installed in archive-specific subdirectories rather than > commingling them? > > I'd appreciate it if some others would look at this and test it. It might > be adequate as a stop-gap for its intended purpose to avoid casual > duplicate installs and some feedback would help. > This bug/patch seems to have languished. It would be good to get it done. Duplicate installed packages are annoying to say the least.
[Message part 2 (text/html, inline)]
bug-gnu-emacs <at> gnu.org
:bug#76568
; Package emacs
.
(Fri, 16 May 2025 15:37:05 GMT) Full text and rfc822 format available.Message #40 received at 76568 <at> debbugs.gnu.org (full text, mbox):
From: Philip Kaludercic <philipk <at> posteo.net> To: Ship Mints <shipmints <at> gmail.com> Cc: 76568 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com> Subject: Re: bug#76568: 'package-install' should not install duplicate packages Date: Fri, 16 May 2025 15:36:44 +0000
Ship Mints <shipmints <at> gmail.com> writes: > On Thu, Mar 20, 2025 at 10:41 AM Ship Mints <shipmints <at> gmail.com> wrote: > >> On Fri, Mar 7, 2025 at 6:33 AM Ship Mints <shipmints <at> gmail.com> wrote: >> >>> On Wed, Mar 5, 2025 at 4:05 PM Ship Mints <shipmints <at> gmail.com> wrote: >>> >>>> On Wed, Mar 5, 2025 at 4:02 PM Ship Mints <shipmints <at> gmail.com> wrote: >>>> >>>>> On Wed, Mar 5, 2025 at 2:55 PM Ship Mints <shipmints <at> gmail.com> wrote: >>>>> >>>>>> On Wed, Mar 5, 2025 at 12:00 PM Ship Mints <shipmints <at> gmail.com> >>>>>> wrote: >>>>>> >>>>>>> On Mon, Mar 3, 2025 at 9:34 PM Stefan Kangas <stefankangas <at> gmail.com> >>>>>>> wrote: >>>>>>> >>>>>>>> Ship Mints <shipmints <at> gmail.com> writes: >>>>>>>> >>>>>>>> > As part of my production upgrade to 30.1, and before I wrote a >>>>>>>> program to install my local >>>>>>>> > ELPA tree from scratch, I tried to first curate my packages and >>>>>>>> change from MELPA to >>>>>>>> > generally equivalent GNU ELPA or non-GNU ELPA archives. The >>>>>>>> result was that I had two of >>>>>>>> > each package installed. >>>>>>>> > >>>>>>>> > I think there's a bug in 'package-install' which, when invoked from >>>>>>>> > 'package-install-button-action', processes the new package spec, >>>>>>>> and incorrectly checks to >>>>>>>> > see if the package is already installed. Interactive invocation >>>>>>>> of 'package-install' yields the >>>>>>>> > package name from the prompt, not its archive description. >>>>>>>> > >>>>>>>> > If the below is correct, I can submit a patch to make >>>>>>>> 'package-install' behave like >>>>>>>> > 'package-reinstall' for the non-interactive case. >>>>>>>> >>>>>>>> Please submit a patch, but could we also have tests for this please? >>>>>>>> >>>>>>>> Thanks in advance. >>>>>>>> >>>>>>> >>>>>>> Patch attached. It prevents the menu-driven case from erasing the >>>>>>> already installed message. It could suggest to the user to remove and then >>>>>>> install or we could offer to use package upgrade to the chosen >>>>>>> package-desc. At the very least, the patch prevents duplicates. >>>>>>> >>>>>> >>>>>> Thinking about it a bit more, this patch needs a little more work. The >>>>>> menu-driven package "upgrade" in describe-package-1 workflow will be interrupted by >>>>>> this patch. It does not differentiate install vs. upgrade and calls >>>>>> package-install which will now complain the package already exists. This >>>>>> should be changed to run package-upgrade. I'll take a look. >>>>>> >>>>>> Any feedback on the minimal patch is still welcome in the meantime. >>>>>> >>>>> >>>>> This patch now accommodates both scenarios. >>>>> >>>> >>>> Now with an amended commit log. >>>> >>> >>> Attached patch with improved package replacement prompt. >>> >> >> Here's a refined version of the patch which allows users to optionally >> replace already-installed packages, or install/keep both. >> >> Testing this revealed some interesting other issues which may already be >> known. I'll see if I can find bug reports and if not I'll submit new >> ones. One example is that packages from different archives have >> incompatible version numbering schemes which causes false positives with >> the obsolete package detection mechanism. Another is that >> package-reinstall does not respect the originating archive, instead >> preferring package-archive-priorities which may install a different version >> than is expected by a user. Now might be a good time to consider that >> packages should be installed in archive-specific subdirectories rather than >> commingling them? >> >> I'd appreciate it if some others would look at this and test it. It might >> be adequate as a stop-gap for its intended purpose to avoid casual >> duplicate installs and some feedback would help. >> > > This bug/patch seems to have languished. It would be good to get it done. > Duplicate installed packages are annoying to say the least. Can you please summarise the relevant parts of this discussion? I see that a patch is being mentioned above, should I review it?
bug-gnu-emacs <at> gnu.org
:bug#76568
; Package emacs
.
(Fri, 16 May 2025 15:43:03 GMT) Full text and rfc822 format available.Message #43 received at 76568 <at> debbugs.gnu.org (full text, mbox):
From: Ship Mints <shipmints <at> gmail.com> To: Philip Kaludercic <philipk <at> posteo.net> Cc: 76568 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com> Subject: Re: bug#76568: 'package-install' should not install duplicate packages Date: Fri, 16 May 2025 11:42:26 -0400
[Message part 1 (text/plain, inline)]
On Fri, May 16, 2025 at 11:36 AM Philip Kaludercic <philipk <at> posteo.net> wrote: > Ship Mints <shipmints <at> gmail.com> writes: > > > On Thu, Mar 20, 2025 at 10:41 AM Ship Mints <shipmints <at> gmail.com> wrote: > > > >> On Fri, Mar 7, 2025 at 6:33 AM Ship Mints <shipmints <at> gmail.com> wrote: > >> > >>> On Wed, Mar 5, 2025 at 4:05 PM Ship Mints <shipmints <at> gmail.com> wrote: > >>> > >>>> On Wed, Mar 5, 2025 at 4:02 PM Ship Mints <shipmints <at> gmail.com> > wrote: > >>>> > >>>>> On Wed, Mar 5, 2025 at 2:55 PM Ship Mints <shipmints <at> gmail.com> > wrote: > >>>>> > >>>>>> On Wed, Mar 5, 2025 at 12:00 PM Ship Mints <shipmints <at> gmail.com> > >>>>>> wrote: > >>>>>> > >>>>>>> On Mon, Mar 3, 2025 at 9:34 PM Stefan Kangas < > stefankangas <at> gmail.com> > >>>>>>> wrote: > >>>>>>> > >>>>>>>> Ship Mints <shipmints <at> gmail.com> writes: > >>>>>>>> > >>>>>>>> > As part of my production upgrade to 30.1, and before I wrote a > >>>>>>>> program to install my local > >>>>>>>> > ELPA tree from scratch, I tried to first curate my packages and > >>>>>>>> change from MELPA to > >>>>>>>> > generally equivalent GNU ELPA or non-GNU ELPA archives. The > >>>>>>>> result was that I had two of > >>>>>>>> > each package installed. > >>>>>>>> > > >>>>>>>> > I think there's a bug in 'package-install' which, when invoked > from > >>>>>>>> > 'package-install-button-action', processes the new package spec, > >>>>>>>> and incorrectly checks to > >>>>>>>> > see if the package is already installed. Interactive invocation > >>>>>>>> of 'package-install' yields the > >>>>>>>> > package name from the prompt, not its archive description. > >>>>>>>> > > >>>>>>>> > If the below is correct, I can submit a patch to make > >>>>>>>> 'package-install' behave like > >>>>>>>> > 'package-reinstall' for the non-interactive case. > >>>>>>>> > >>>>>>>> Please submit a patch, but could we also have tests for this > please? > >>>>>>>> > >>>>>>>> Thanks in advance. > >>>>>>>> > >>>>>>> > >>>>>>> Patch attached. It prevents the menu-driven case from erasing the > >>>>>>> already installed message. It could suggest to the user to remove > and then > >>>>>>> install or we could offer to use package upgrade to the chosen > >>>>>>> package-desc. At the very least, the patch prevents duplicates. > >>>>>>> > >>>>>> > >>>>>> Thinking about it a bit more, this patch needs a little more work. > The > >>>>>> menu-driven package "upgrade" in describe-package-1 workflow will > be interrupted by > >>>>>> this patch. It does not differentiate install vs. upgrade and calls > >>>>>> package-install which will now complain the package already > exists. This > >>>>>> should be changed to run package-upgrade. I'll take a look. > >>>>>> > >>>>>> Any feedback on the minimal patch is still welcome in the meantime. > >>>>>> > >>>>> > >>>>> This patch now accommodates both scenarios. > >>>>> > >>>> > >>>> Now with an amended commit log. > >>>> > >>> > >>> Attached patch with improved package replacement prompt. > >>> > >> > >> Here's a refined version of the patch which allows users to optionally > >> replace already-installed packages, or install/keep both. > >> > >> Testing this revealed some interesting other issues which may already be > >> known. I'll see if I can find bug reports and if not I'll submit new > >> ones. One example is that packages from different archives have > >> incompatible version numbering schemes which causes false positives with > >> the obsolete package detection mechanism. Another is that > >> package-reinstall does not respect the originating archive, instead > >> preferring package-archive-priorities which may install a different > version > >> than is expected by a user. Now might be a good time to consider that > >> packages should be installed in archive-specific subdirectories rather > than > >> commingling them? > >> > >> I'd appreciate it if some others would look at this and test it. It > might > >> be adequate as a stop-gap for its intended purpose to avoid casual > >> duplicate installs and some feedback would help. > >> > > > > This bug/patch seems to have languished. It would be good to get it > done. > > Duplicate installed packages are annoying to say the least. > > Can you please summarise the relevant parts of this discussion? I see > that a patch is being mentioned above, should I review it? > The latest version of this patch from this discussion is attached. It amends both the menu-driven package upgrade and the package-upgrade command to assist the user with avoiding (or permitting) installing a package more than once. I introduced "tangents" in the discussion along the way as I learned more about the package infrastructure: "Testing this revealed some interesting other issues which may already be known. I'll see if I can find bug reports and if not I'll submit new ones. One example is that packages from different archives have incompatible version numbering schemes which causes false positives with the obsolete package detection mechanism. Another is that package-reinstall does not respect the originating archive, instead preferring package-archive-priorities which may install a different version than is expected by a user. Now might be a good time to consider that packages should be installed in archive-specific subdirectories rather than commingling them?" -Stephane
[Message part 2 (text/html, inline)]
[0001-Correct-package-install-to-detect-installed-packages.patch (application/octet-stream, attachment)]
bug-gnu-emacs <at> gnu.org
:bug#76568
; Package emacs
.
(Sat, 24 May 2025 09:14:01 GMT) Full text and rfc822 format available.Message #46 received at 76568 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: philipk <at> posteo.net, Ship Mints <shipmints <at> gmail.com> Cc: 76568 <at> debbugs.gnu.org, stefankangas <at> gmail.com Subject: Re: bug#76568: 'package-install' should not install duplicate packages Date: Sat, 24 May 2025 12:13:19 +0300
Ping! Can we make some further progress in this matter? > From: Ship Mints <shipmints <at> gmail.com> > Date: Fri, 16 May 2025 11:42:26 -0400 > Cc: Stefan Kangas <stefankangas <at> gmail.com>, 76568 <at> debbugs.gnu.org, > Eli Zaretskii <eliz <at> gnu.org> > > On Fri, May 16, 2025 at 11:36 AM Philip Kaludercic <philipk <at> posteo.net> wrote: > > Ship Mints <shipmints <at> gmail.com> writes: > > > On Thu, Mar 20, 2025 at 10:41 AM Ship Mints <shipmints <at> gmail.com> wrote: > > > >> On Fri, Mar 7, 2025 at 6:33 AM Ship Mints <shipmints <at> gmail.com> wrote: > >> > >>> On Wed, Mar 5, 2025 at 4:05 PM Ship Mints <shipmints <at> gmail.com> wrote: > >>> > >>>> On Wed, Mar 5, 2025 at 4:02 PM Ship Mints <shipmints <at> gmail.com> wrote: > >>>> > >>>>> On Wed, Mar 5, 2025 at 2:55 PM Ship Mints <shipmints <at> gmail.com> wrote: > >>>>> > >>>>>> On Wed, Mar 5, 2025 at 12:00 PM Ship Mints <shipmints <at> gmail.com> > >>>>>> wrote: > >>>>>> > >>>>>>> On Mon, Mar 3, 2025 at 9:34 PM Stefan Kangas <stefankangas <at> gmail.com> > >>>>>>> wrote: > >>>>>>> > >>>>>>>> Ship Mints <shipmints <at> gmail.com> writes: > >>>>>>>> > >>>>>>>> > As part of my production upgrade to 30.1, and before I wrote a > >>>>>>>> program to install my local > >>>>>>>> > ELPA tree from scratch, I tried to first curate my packages and > >>>>>>>> change from MELPA to > >>>>>>>> > generally equivalent GNU ELPA or non-GNU ELPA archives. The > >>>>>>>> result was that I had two of > >>>>>>>> > each package installed. > >>>>>>>> > > >>>>>>>> > I think there's a bug in 'package-install' which, when invoked from > >>>>>>>> > 'package-install-button-action', processes the new package spec, > >>>>>>>> and incorrectly checks to > >>>>>>>> > see if the package is already installed. Interactive invocation > >>>>>>>> of 'package-install' yields the > >>>>>>>> > package name from the prompt, not its archive description. > >>>>>>>> > > >>>>>>>> > If the below is correct, I can submit a patch to make > >>>>>>>> 'package-install' behave like > >>>>>>>> > 'package-reinstall' for the non-interactive case. > >>>>>>>> > >>>>>>>> Please submit a patch, but could we also have tests for this please? > >>>>>>>> > >>>>>>>> Thanks in advance. > >>>>>>>> > >>>>>>> > >>>>>>> Patch attached. It prevents the menu-driven case from erasing the > >>>>>>> already installed message. It could suggest to the user to remove and then > >>>>>>> install or we could offer to use package upgrade to the chosen > >>>>>>> package-desc. At the very least, the patch prevents duplicates. > >>>>>>> > >>>>>> > >>>>>> Thinking about it a bit more, this patch needs a little more work. The > >>>>>> menu-driven package "upgrade" in describe-package-1 workflow will be interrupted by > >>>>>> this patch. It does not differentiate install vs. upgrade and calls > >>>>>> package-install which will now complain the package already exists. This > >>>>>> should be changed to run package-upgrade. I'll take a look. > >>>>>> > >>>>>> Any feedback on the minimal patch is still welcome in the meantime. > >>>>>> > >>>>> > >>>>> This patch now accommodates both scenarios. > >>>>> > >>>> > >>>> Now with an amended commit log. > >>>> > >>> > >>> Attached patch with improved package replacement prompt. > >>> > >> > >> Here's a refined version of the patch which allows users to optionally > >> replace already-installed packages, or install/keep both. > >> > >> Testing this revealed some interesting other issues which may already be > >> known. I'll see if I can find bug reports and if not I'll submit new > >> ones. One example is that packages from different archives have > >> incompatible version numbering schemes which causes false positives with > >> the obsolete package detection mechanism. Another is that > >> package-reinstall does not respect the originating archive, instead > >> preferring package-archive-priorities which may install a different version > >> than is expected by a user. Now might be a good time to consider that > >> packages should be installed in archive-specific subdirectories rather than > >> commingling them? > >> > >> I'd appreciate it if some others would look at this and test it. It might > >> be adequate as a stop-gap for its intended purpose to avoid casual > >> duplicate installs and some feedback would help. > >> > > > > This bug/patch seems to have languished. It would be good to get it done. > > Duplicate installed packages are annoying to say the least. > > Can you please summarise the relevant parts of this discussion? I see > that a patch is being mentioned above, should I review it? > > The latest version of this patch from this discussion is attached. It amends both the menu-driven package > upgrade and the package-upgrade command to assist the user with avoiding (or permitting) installing a > package more than once. > > I introduced "tangents" in the discussion along the way as I learned more about the package infrastructure: > > "Testing this revealed some interesting other issues which may already be known. I'll see if I can find bug > reports and if not I'll submit new ones. One example is that packages from different archives have > incompatible version numbering schemes which causes false positives with the obsolete package detection > mechanism. Another is that package-reinstall does not respect the originating archive, instead preferring > package-archive-priorities which may install a different version than is expected by a user. Now might be a > good time to consider that packages should be installed in archive-specific subdirectories rather than > commingling them?" > > -Stephane
bug-gnu-emacs <at> gnu.org
:bug#76568
; Package emacs
.
(Sun, 25 May 2025 15:03:01 GMT) Full text and rfc822 format available.Message #49 received at 76568 <at> debbugs.gnu.org (full text, mbox):
From: Philip Kaludercic <philipk <at> posteo.net> To: Ship Mints <shipmints <at> gmail.com> Cc: 76568 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com> Subject: Re: bug#76568: 'package-install' should not install duplicate packages Date: Sun, 25 May 2025 15:01:53 +0000
Ship Mints <shipmints <at> gmail.com> writes: [...] >> Can you please summarise the relevant parts of this discussion? I see >> that a patch is being mentioned above, should I review it? >> > > The latest version of this patch from this discussion is attached. It > amends both the menu-driven package upgrade and the package-upgrade command > to assist the user with avoiding (or permitting) installing a package more > than once. > [...] > > -Stephane > From 9deee448e27f94c21469e59677fde2cbce8f9bcd Mon Sep 17 00:00:00 2001 > From: shipmints <shipmints <at> gmail.com> > Date: Wed, 5 Mar 2025 11:33:07 -0500 > Subject: [PATCH] Correct 'package-install' to detect installed packages > (bug#76568) > > * lisp/emacs-lisp/package.el > (package-install): Check for already installed package using its symbol > rather than rely on differing archive directory structures. Return t if > installed, nil otherwise. > (package-install-button-action): 'describe-package' only when > 'package-install' actually installed the package. Prompt the user to > install or upgrade an already installed package. > * test/lisp/emacs-lisp/package-tests.el (package-test-install-single): > Add already-installed test using 'package-install' with a > 'package-desc'. > --- > lisp/emacs-lisp/package.el | 54 +++++++++++++++++++++------ > test/lisp/emacs-lisp/package-tests.el | 4 ++ > 2 files changed, 47 insertions(+), 11 deletions(-) > > diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el > index 8d498c216dc..7c7e99168aa 100644 > --- a/lisp/emacs-lisp/package.el > +++ b/lisp/emacs-lisp/package.el > @@ -2194,7 +2194,7 @@ package-install-upgrade-built-in > :version "29.1") > > ;;;###autoload > -(defun package-install (pkg &optional dont-select) > +(defun package-install (pkg &optional dont-select allow-mult-or-reinstall) > "Install the package PKG. > > PKG can be a `package-desc', or a symbol naming one of the available > @@ -2207,8 +2207,13 @@ package-install > non-nil, install the package but do not add it to > `package-selected-packages'. > > -If PKG is a `package-desc' and it is already installed, don't try > -to install it but still mark it as selected. > +When called from Lisp and optional argument ALLOW-MULT-OR-REINSTALL is > +non-nil, install the package even if it is already installed from > +another source, allowing more than one simultaneous version. > + > +If PKG is a `package-desc' and it is already installed, and > +ALLOW-MULT-OR-REINSTALL is nil, don't try to install it but still mark > +it as selected. Why is this yet another optional argument, instead of a user option? > > If the command is invoked with a prefix argument, it will allow > upgrading of built-in packages, as if `package-install-upgrade-built-in' > @@ -2243,16 +2248,22 @@ package-install > (package--active-built-in-p pkg)) > (setq pkg (or (cadr (assq name package-archive-contents)) pkg))) > (if-let* ((transaction > + ;; Test for already installed using the pkg symbol, not > + ;; the archive-specific directory structure. > (if (package-desc-p pkg) > - (unless (package-installed-p pkg) > + (unless (and (not allow-mult-or-reinstall) > + (package-installed-p (package-desc-name pkg))) > (package-compute-transaction (list pkg) > (package-desc-reqs pkg))) > - (package-compute-transaction () (list (list pkg)))))) > + (unless (and (not allow-mult-or-reinstall) > + (package-installed-p pkg)) > + (package-compute-transaction () (list (list pkg))))))) > (progn > (package-download-transaction transaction) > (package--quickstart-maybe-refresh) > (message "Package `%s' installed." name)) > - (message "`%s' is already installed" name)))) > + (message "`%s' is already installed" name) > + nil))) > > (declare-function package-vc-upgrade "package-vc" (pkg)) > > @@ -2587,7 +2598,7 @@ package-reinstall > (package-delete > (if (package-desc-p pkg) pkg (cadr (assq pkg package-alist))) > 'force 'nosave) > - (package-install pkg 'dont-select)) > + (package-install pkg 'dont-select 'allow-mult-or-reinstall)) > > ;;;###autoload > (defun package-recompile (pkg) > @@ -3051,10 +3062,31 @@ package-install-button-action > Used for the `action' property of buttons in the buffer created by > `describe-package'." > (let ((pkg-desc (button-get button 'package-desc))) > - (when (y-or-n-p (format-message "Install package `%s'? " > - (package-desc-full-name pkg-desc))) > - (package-install pkg-desc nil) > - (describe-package (package-desc-name pkg-desc))))) > + (if (package-installed-p (package-desc-name pkg-desc)) > + (let ((installed-pkg-desc (cadr (assq > + (package-desc-name pkg-desc) > + package-alist)))) > + (pcase (let ((read-answer-short t)) > + (read-answer > + (format-message > + "Replace `%s' with `%s', or Install both (advanced users) " > + (package-desc-full-name installed-pkg-desc) > + (package-desc-full-name pkg-desc)) > + '(("replace" ?r "Replace existing") > + ("install" ?i "Install both") > + ("help" ?h "Help") > + ("quit" ?q "Quit to abort")))) > + ("replace" > + (package-delete installed-pkg-desc 'force 'dont-unselect) > + (when (package-install pkg-desc nil) > + (describe-package (package-desc-name pkg-desc)))) > + ("install" > + (when (package-install pkg-desc nil 'allow-mult-or-reinstall) > + (describe-package (package-desc-name pkg-desc)))))) > + (when (y-or-n-p (format-message "Install package `%s'? " > + (package-desc-full-name pkg-desc))) > + (when (package-install pkg-desc nil) > + (describe-package (package-desc-name pkg-desc))))))) > > (defun package-delete-button-action (button) > "Run `package-delete' on the package BUTTON points to. > diff --git a/test/lisp/emacs-lisp/package-tests.el b/test/lisp/emacs-lisp/package-tests.el > index d8e260319bd..36617ed6f6e 100644 > --- a/test/lisp/emacs-lisp/package-tests.el > +++ b/test/lisp/emacs-lisp/package-tests.el > @@ -248,6 +248,10 @@ package-test-install-single > (should (string-match "^[`‘']simple-single[’'] is already installed\n?\\'" > (buffer-string)))) > (should (package-installed-p 'simple-single)) > + ;; Test for `package-install' already installed using a > + ;; package-desc. `package-install' returns nil if already > + ;; installed. > + (should-not (package-install simple-single-desc)) > (let* ((simple-pkg-dir (file-name-as-directory > (expand-file-name > "simple-single-1.3" For some reason I find this patch very difficult to follow... Perhaps I don't understand the issue you are trying to fix, or how it is related to your patch.
bug-gnu-emacs <at> gnu.org
:bug#76568
; Package emacs
.
(Sun, 25 May 2025 15:09:02 GMT) Full text and rfc822 format available.Message #52 received at 76568 <at> debbugs.gnu.org (full text, mbox):
From: Ship Mints <shipmints <at> gmail.com> To: Philip Kaludercic <philipk <at> posteo.net> Cc: 76568 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com> Subject: Re: bug#76568: 'package-install' should not install duplicate packages Date: Sun, 25 May 2025 11:08:20 -0400
[Message part 1 (text/plain, inline)]
On Sun, May 25, 2025 at 11:01 AM Philip Kaludercic <philipk <at> posteo.net> wrote: > Ship Mints <shipmints <at> gmail.com> writes: > > > [...] > > >> Can you please summarise the relevant parts of this discussion? I see > >> that a patch is being mentioned above, should I review it? > >> > > > > The latest version of this patch from this discussion is attached. It > > amends both the menu-driven package upgrade and the package-upgrade > command > > to assist the user with avoiding (or permitting) installing a package > more > > than once. > > > > [...] > > > > > -Stephane > > From 9deee448e27f94c21469e59677fde2cbce8f9bcd Mon Sep 17 00:00:00 2001 > > From: shipmints <shipmints <at> gmail.com> > > Date: Wed, 5 Mar 2025 11:33:07 -0500 > > Subject: [PATCH] Correct 'package-install' to detect installed packages > > (bug#76568) > > > > * lisp/emacs-lisp/package.el > > (package-install): Check for already installed package using its symbol > > rather than rely on differing archive directory structures. Return t if > > installed, nil otherwise. > > (package-install-button-action): 'describe-package' only when > > 'package-install' actually installed the package. Prompt the user to > > install or upgrade an already installed package. > > * test/lisp/emacs-lisp/package-tests.el (package-test-install-single): > > Add already-installed test using 'package-install' with a > > 'package-desc'. > > --- > > lisp/emacs-lisp/package.el | 54 +++++++++++++++++++++------ > > test/lisp/emacs-lisp/package-tests.el | 4 ++ > > 2 files changed, 47 insertions(+), 11 deletions(-) > > > > diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el > > index 8d498c216dc..7c7e99168aa 100644 > > --- a/lisp/emacs-lisp/package.el > > +++ b/lisp/emacs-lisp/package.el > > @@ -2194,7 +2194,7 @@ package-install-upgrade-built-in > > :version "29.1") > > > > ;;;###autoload > > -(defun package-install (pkg &optional dont-select) > > +(defun package-install (pkg &optional dont-select > allow-mult-or-reinstall) > > "Install the package PKG. > > > > PKG can be a `package-desc', or a symbol naming one of the available > > @@ -2207,8 +2207,13 @@ package-install > > non-nil, install the package but do not add it to > > `package-selected-packages'. > > > > -If PKG is a `package-desc' and it is already installed, don't try > > -to install it but still mark it as selected. > > +When called from Lisp and optional argument ALLOW-MULT-OR-REINSTALL is > > +non-nil, install the package even if it is already installed from > > +another source, allowing more than one simultaneous version. > > + > > +If PKG is a `package-desc' and it is already installed, and > > +ALLOW-MULT-OR-REINSTALL is nil, don't try to install it but still mark > > +it as selected. > > Why is this yet another optional argument, instead of a user option? > As the maintainer, you pick what suits your taste. > > > If the command is invoked with a prefix argument, it will allow > > upgrading of built-in packages, as if `package-install-upgrade-built-in' > > @@ -2243,16 +2248,22 @@ package-install > > (package--active-built-in-p pkg)) > > (setq pkg (or (cadr (assq name package-archive-contents)) pkg))) > > (if-let* ((transaction > > + ;; Test for already installed using the pkg symbol, not > > + ;; the archive-specific directory structure. > > (if (package-desc-p pkg) > > - (unless (package-installed-p pkg) > > + (unless (and (not allow-mult-or-reinstall) > > + (package-installed-p (package-desc-name > pkg))) > > (package-compute-transaction (list pkg) > > (package-desc-reqs > pkg))) > > - (package-compute-transaction () (list (list pkg)))))) > > + (unless (and (not allow-mult-or-reinstall) > > + (package-installed-p pkg)) > > + (package-compute-transaction () (list (list > pkg))))))) > > (progn > > (package-download-transaction transaction) > > (package--quickstart-maybe-refresh) > > (message "Package `%s' installed." name)) > > - (message "`%s' is already installed" name)))) > > + (message "`%s' is already installed" name) > > + nil))) > > > > (declare-function package-vc-upgrade "package-vc" (pkg)) > > > > @@ -2587,7 +2598,7 @@ package-reinstall > > (package-delete > > (if (package-desc-p pkg) pkg (cadr (assq pkg package-alist))) > > 'force 'nosave) > > - (package-install pkg 'dont-select)) > > + (package-install pkg 'dont-select 'allow-mult-or-reinstall)) > > > > ;;;###autoload > > (defun package-recompile (pkg) > > @@ -3051,10 +3062,31 @@ package-install-button-action > > Used for the `action' property of buttons in the buffer created by > > `describe-package'." > > (let ((pkg-desc (button-get button 'package-desc))) > > - (when (y-or-n-p (format-message "Install package `%s'? " > > - (package-desc-full-name pkg-desc))) > > - (package-install pkg-desc nil) > > - (describe-package (package-desc-name pkg-desc))))) > > + (if (package-installed-p (package-desc-name pkg-desc)) > > + (let ((installed-pkg-desc (cadr (assq > > + (package-desc-name pkg-desc) > > + package-alist)))) > > + (pcase (let ((read-answer-short t)) > > + (read-answer > > + (format-message > > + "Replace `%s' with `%s', or Install both (advanced > users) " > > + (package-desc-full-name installed-pkg-desc) > > + (package-desc-full-name pkg-desc)) > > + '(("replace" ?r "Replace existing") > > + ("install" ?i "Install both") > > + ("help" ?h "Help") > > + ("quit" ?q "Quit to abort")))) > > + ("replace" > > + (package-delete installed-pkg-desc 'force 'dont-unselect) > > + (when (package-install pkg-desc nil) > > + (describe-package (package-desc-name pkg-desc)))) > > + ("install" > > + (when (package-install pkg-desc nil > 'allow-mult-or-reinstall) > > + (describe-package (package-desc-name pkg-desc)))))) > > + (when (y-or-n-p (format-message "Install package `%s'? " > > + (package-desc-full-name > pkg-desc))) > > + (when (package-install pkg-desc nil) > > + (describe-package (package-desc-name pkg-desc))))))) > > > > (defun package-delete-button-action (button) > > "Run `package-delete' on the package BUTTON points to. > > diff --git a/test/lisp/emacs-lisp/package-tests.el > b/test/lisp/emacs-lisp/package-tests.el > > index d8e260319bd..36617ed6f6e 100644 > > --- a/test/lisp/emacs-lisp/package-tests.el > > +++ b/test/lisp/emacs-lisp/package-tests.el > > @@ -248,6 +248,10 @@ package-test-install-single > > (should (string-match "^[`‘']simple-single[’'] is already > installed\n?\\'" > > (buffer-string)))) > > (should (package-installed-p 'simple-single)) > > + ;; Test for `package-install' already installed using a > > + ;; package-desc. `package-install' returns nil if already > > + ;; installed. > > + (should-not (package-install simple-single-desc)) > > (let* ((simple-pkg-dir (file-name-as-directory > > (expand-file-name > > "simple-single-1.3" > > For some reason I find this patch very difficult to follow... Perhaps I > don't understand the issue you are trying to fix, or how it is related > to your patch. > The issue is that package install will install multiple versions of the same package (from the same archive) without warning. This will and does confuse users especially using the interactive package menu. The patch works for me. The related issue is that multiple versions of the same packages that come from different archives are also installed but this patch does not attempt to deal with that. I proposed that we segregate archives under the package tree to ease the package infrastructure's ability to detect that.
[Message part 2 (text/html, inline)]
bug-gnu-emacs <at> gnu.org
:bug#76568
; Package emacs
.
(Sat, 07 Jun 2025 08:37:01 GMT) Full text and rfc822 format available.Message #55 received at 76568 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: philipk <at> posteo.net, Ship Mints <shipmints <at> gmail.com> Cc: 76568 <at> debbugs.gnu.org, stefankangas <at> gmail.com Subject: Re: bug#76568: 'package-install' should not install duplicate packages Date: Sat, 07 Jun 2025 11:35:52 +0300
Ping! Can we make further progress with this issue? > From: Ship Mints <shipmints <at> gmail.com> > Date: Sun, 25 May 2025 11:08:20 -0400 > Cc: 76568 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, > Stefan Kangas <stefankangas <at> gmail.com> > > On Sun, May 25, 2025 at 11:01 AM Philip Kaludercic <philipk <at> posteo.net> wrote: > > Ship Mints <shipmints <at> gmail.com> writes: > > [...] > > >> Can you please summarise the relevant parts of this discussion? I see > >> that a patch is being mentioned above, should I review it? > >> > > > > The latest version of this patch from this discussion is attached. It > > amends both the menu-driven package upgrade and the package-upgrade command > > to assist the user with avoiding (or permitting) installing a package more > > than once. > > > > [...] > > > > > -Stephane > > From 9deee448e27f94c21469e59677fde2cbce8f9bcd Mon Sep 17 00:00:00 2001 > > From: shipmints <shipmints <at> gmail.com> > > Date: Wed, 5 Mar 2025 11:33:07 -0500 > > Subject: [PATCH] Correct 'package-install' to detect installed packages > > (bug#76568) > > > > * lisp/emacs-lisp/package.el > > (package-install): Check for already installed package using its symbol > > rather than rely on differing archive directory structures. Return t if > > installed, nil otherwise. > > (package-install-button-action): 'describe-package' only when > > 'package-install' actually installed the package. Prompt the user to > > install or upgrade an already installed package. > > * test/lisp/emacs-lisp/package-tests.el (package-test-install-single): > > Add already-installed test using 'package-install' with a > > 'package-desc'. > > --- > > lisp/emacs-lisp/package.el | 54 +++++++++++++++++++++------ > > test/lisp/emacs-lisp/package-tests.el | 4 ++ > > 2 files changed, 47 insertions(+), 11 deletions(-) > > > > diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el > > index 8d498c216dc..7c7e99168aa 100644 > > --- a/lisp/emacs-lisp/package.el > > +++ b/lisp/emacs-lisp/package.el > > @@ -2194,7 +2194,7 @@ package-install-upgrade-built-in > > :version "29.1") > > > > ;;;###autoload > > -(defun package-install (pkg &optional dont-select) > > +(defun package-install (pkg &optional dont-select allow-mult-or-reinstall) > > "Install the package PKG. > > > > PKG can be a `package-desc', or a symbol naming one of the available > > @@ -2207,8 +2207,13 @@ package-install > > non-nil, install the package but do not add it to > > `package-selected-packages'. > > > > -If PKG is a `package-desc' and it is already installed, don't try > > -to install it but still mark it as selected. > > +When called from Lisp and optional argument ALLOW-MULT-OR-REINSTALL is > > +non-nil, install the package even if it is already installed from > > +another source, allowing more than one simultaneous version. > > + > > +If PKG is a `package-desc' and it is already installed, and > > +ALLOW-MULT-OR-REINSTALL is nil, don't try to install it but still mark > > +it as selected. > > Why is this yet another optional argument, instead of a user option? > > As the maintainer, you pick what suits your taste. > > > > > If the command is invoked with a prefix argument, it will allow > > upgrading of built-in packages, as if `package-install-upgrade-built-in' > > @@ -2243,16 +2248,22 @@ package-install > > (package--active-built-in-p pkg)) > > (setq pkg (or (cadr (assq name package-archive-contents)) pkg))) > > (if-let* ((transaction > > + ;; Test for already installed using the pkg symbol, not > > + ;; the archive-specific directory structure. > > (if (package-desc-p pkg) > > - (unless (package-installed-p pkg) > > + (unless (and (not allow-mult-or-reinstall) > > + (package-installed-p (package-desc-name pkg))) > > (package-compute-transaction (list pkg) > > (package-desc-reqs pkg))) > > - (package-compute-transaction () (list (list pkg)))))) > > + (unless (and (not allow-mult-or-reinstall) > > + (package-installed-p pkg)) > > + (package-compute-transaction () (list (list pkg))))))) > > (progn > > (package-download-transaction transaction) > > (package--quickstart-maybe-refresh) > > (message "Package `%s' installed." name)) > > - (message "`%s' is already installed" name)))) > > + (message "`%s' is already installed" name) > > + nil))) > > > > (declare-function package-vc-upgrade "package-vc" (pkg)) > > > > @@ -2587,7 +2598,7 @@ package-reinstall > > (package-delete > > (if (package-desc-p pkg) pkg (cadr (assq pkg package-alist))) > > 'force 'nosave) > > - (package-install pkg 'dont-select)) > > + (package-install pkg 'dont-select 'allow-mult-or-reinstall)) > > > > ;;;###autoload > > (defun package-recompile (pkg) > > @@ -3051,10 +3062,31 @@ package-install-button-action > > Used for the `action' property of buttons in the buffer created by > > `describe-package'." > > (let ((pkg-desc (button-get button 'package-desc))) > > - (when (y-or-n-p (format-message "Install package `%s'? " > > - (package-desc-full-name pkg-desc))) > > - (package-install pkg-desc nil) > > - (describe-package (package-desc-name pkg-desc))))) > > + (if (package-installed-p (package-desc-name pkg-desc)) > > + (let ((installed-pkg-desc (cadr (assq > > + (package-desc-name pkg-desc) > > + package-alist)))) > > + (pcase (let ((read-answer-short t)) > > + (read-answer > > + (format-message > > + "Replace `%s' with `%s', or Install both (advanced users) " > > + (package-desc-full-name installed-pkg-desc) > > + (package-desc-full-name pkg-desc)) > > + '(("replace" ?r "Replace existing") > > + ("install" ?i "Install both") > > + ("help" ?h "Help") > > + ("quit" ?q "Quit to abort")))) > > + ("replace" > > + (package-delete installed-pkg-desc 'force 'dont-unselect) > > + (when (package-install pkg-desc nil) > > + (describe-package (package-desc-name pkg-desc)))) > > + ("install" > > + (when (package-install pkg-desc nil 'allow-mult-or-reinstall) > > + (describe-package (package-desc-name pkg-desc)))))) > > + (when (y-or-n-p (format-message "Install package `%s'? " > > + (package-desc-full-name pkg-desc))) > > + (when (package-install pkg-desc nil) > > + (describe-package (package-desc-name pkg-desc))))))) > > > > (defun package-delete-button-action (button) > > "Run `package-delete' on the package BUTTON points to. > > diff --git a/test/lisp/emacs-lisp/package-tests.el b/test/lisp/emacs-lisp/package-tests.el > > index d8e260319bd..36617ed6f6e 100644 > > --- a/test/lisp/emacs-lisp/package-tests.el > > +++ b/test/lisp/emacs-lisp/package-tests.el > > @@ -248,6 +248,10 @@ package-test-install-single > > (should (string-match "^[`‘']simple-single[’'] is already installed\n?\\'" > > (buffer-string)))) > > (should (package-installed-p 'simple-single)) > > + ;; Test for `package-install' already installed using a > > + ;; package-desc. `package-install' returns nil if already > > + ;; installed. > > + (should-not (package-install simple-single-desc)) > > (let* ((simple-pkg-dir (file-name-as-directory > > (expand-file-name > > "simple-single-1.3" > > For some reason I find this patch very difficult to follow... Perhaps I > don't understand the issue you are trying to fix, or how it is related > to your patch. > > The issue is that package install will install multiple versions of the same package (from the same archive) > without warning. This will and does confuse users especially using the interactive package menu. The > patch works for me. > > The related issue is that multiple versions of the same packages that come from different archives are also > installed but this patch does not attempt to deal with that. I proposed that we segregate archives under the > package tree to ease the package infrastructure's ability to detect that.
bug-gnu-emacs <at> gnu.org
:bug#76568
; Package emacs
.
(Sat, 07 Jun 2025 21:24:02 GMT) Full text and rfc822 format available.Message #58 received at 76568 <at> debbugs.gnu.org (full text, mbox):
From: Philip Kaludercic <philipk <at> posteo.net> To: Eli Zaretskii <eliz <at> gnu.org> Cc: 76568 <at> debbugs.gnu.org, Ship Mints <shipmints <at> gmail.com>, Stefan Monnier <monnier <at> iro.umontreal.ca>, stefankangas <at> gmail.com Subject: Re: bug#76568: 'package-install' should not install duplicate packages Date: Sat, 07 Jun 2025 21:23:11 +0000
Eli Zaretskii <eliz <at> gnu.org> writes: > Ping! Can we make further progress with this issue? Sorry, understanding this bug report still takes me forever... Just composing this message took me an hour. I have added Stefan to the CCs to get some more input. >> From: Ship Mints <shipmints <at> gmail.com> >> Date: Sun, 25 May 2025 11:08:20 -0400 >> Cc: 76568 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, >> Stefan Kangas <stefankangas <at> gmail.com> >> >> On Sun, May 25, 2025 at 11:01 AM Philip Kaludercic <philipk <at> posteo.net> wrote: >> >> Ship Mints <shipmints <at> gmail.com> writes: >> >> [...] >> >> >> Can you please summarise the relevant parts of this discussion? I see >> >> that a patch is being mentioned above, should I review it? >> >> >> > >> > The latest version of this patch from this discussion is attached. It >> > amends both the menu-driven package upgrade and the package-upgrade command >> > to assist the user with avoiding (or permitting) installing a package more >> > than once. >> > >> >> [...] >> >> > >> > -Stephane >> > From 9deee448e27f94c21469e59677fde2cbce8f9bcd Mon Sep 17 00:00:00 2001 >> > From: shipmints <shipmints <at> gmail.com> >> > Date: Wed, 5 Mar 2025 11:33:07 -0500 >> > Subject: [PATCH] Correct 'package-install' to detect installed packages >> > (bug#76568) >> > >> > * lisp/emacs-lisp/package.el >> > (package-install): Check for already installed package using its symbol >> > rather than rely on differing archive directory structures. Return t if ^ Why are you mentioning this here? >> > installed, nil otherwise. >> > (package-install-button-action): 'describe-package' only when >> > 'package-install' actually installed the package. Prompt the user to >> > install or upgrade an already installed package. >> > * test/lisp/emacs-lisp/package-tests.el (package-test-install-single): >> > Add already-installed test using 'package-install' with a >> > 'package-desc'. >> > --- >> > lisp/emacs-lisp/package.el | 54 +++++++++++++++++++++------ >> > test/lisp/emacs-lisp/package-tests.el | 4 ++ >> > 2 files changed, 47 insertions(+), 11 deletions(-) >> > >> > diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el >> > index 8d498c216dc..7c7e99168aa 100644 >> > --- a/lisp/emacs-lisp/package.el >> > +++ b/lisp/emacs-lisp/package.el >> > @@ -2194,7 +2194,7 @@ package-install-upgrade-built-in >> > :version "29.1") >> > >> > ;;;###autoload >> > -(defun package-install (pkg &optional dont-select) >> > +(defun package-install (pkg &optional dont-select allow-mult-or-reinstall) >> > "Install the package PKG. >> > >> > PKG can be a `package-desc', or a symbol naming one of the available >> > @@ -2207,8 +2207,13 @@ package-install >> > non-nil, install the package but do not add it to >> > `package-selected-packages'. >> > >> > -If PKG is a `package-desc' and it is already installed, don't try >> > -to install it but still mark it as selected. >> > +When called from Lisp and optional argument ALLOW-MULT-OR-REINSTALL is >> > +non-nil, install the package even if it is already installed from >> > +another source, allowing more than one simultaneous version. >> > + >> > +If PKG is a `package-desc' and it is already installed, and >> > +ALLOW-MULT-OR-REINSTALL is nil, don't try to install it but still mark >> > +it as selected. >> >> Why is this yet another optional argument, instead of a user option? >> >> As the maintainer, you pick what suits your taste. No, what I meant is if there is a reason that we should sometimes allow installing multiple versions of a package in some cases and not in others? Is that a genuine use-case, or do you (as the person experiencing the issue you are trying to solve) assume that most people with the same problem would be OK with setting this once via a user-option. I am under the impression that that would simplify the change. (Also, if we go with the optional argument, please rename it to something cleaner like "allow-multiple". Perhaps it is just me, but an "-or-" in a argument name bugs me.) >> > >> > If the command is invoked with a prefix argument, it will allow >> > upgrading of built-in packages, as if `package-install-upgrade-built-in' >> > @@ -2243,16 +2248,22 @@ package-install >> > (package--active-built-in-p pkg)) >> > (setq pkg (or (cadr (assq name package-archive-contents)) pkg))) >> > (if-let* ((transaction >> > + ;; Test for already installed using the pkg symbol, not >> > + ;; the archive-specific directory structure. >> > (if (package-desc-p pkg) >> > - (unless (package-installed-p pkg) >> > + (unless (and (not allow-mult-or-reinstall) >> > + (package-installed-p (package-desc-name pkg))) >> > (package-compute-transaction (list pkg) >> > (package-desc-reqs pkg))) >> > - (package-compute-transaction () (list (list pkg)))))) >> > + (unless (and (not allow-mult-or-reinstall) >> > + (package-installed-p pkg)) >> > + (package-compute-transaction () (list (list pkg))))))) >> > (progn >> > (package-download-transaction transaction) >> > (package--quickstart-maybe-refresh) >> > (message "Package `%s' installed." name)) >> > - (message "`%s' is already installed" name)))) >> > + (message "`%s' is already installed" name) >> > + nil))) This last change appears to be unrelated to the remaining patch? >> > >> > (declare-function package-vc-upgrade "package-vc" (pkg)) >> > >> > @@ -2587,7 +2598,7 @@ package-reinstall >> > (package-delete >> > (if (package-desc-p pkg) pkg (cadr (assq pkg package-alist))) >> > 'force 'nosave) >> > - (package-install pkg 'dont-select)) >> > + (package-install pkg 'dont-select 'allow-mult-or-reinstall)) >> > >> > ;;;###autoload >> > (defun package-recompile (pkg) >> > @@ -3051,10 +3062,31 @@ package-install-button-action (OK, one of the things that had me confused is that I misread the diff and assumed you were patching `package-recompile' ^^) >> > Used for the `action' property of buttons in the buffer created by >> > `describe-package'." >> > (let ((pkg-desc (button-get button 'package-desc))) >> > - (when (y-or-n-p (format-message "Install package `%s'? " >> > - (package-desc-full-name pkg-desc))) >> > - (package-install pkg-desc nil) >> > - (describe-package (package-desc-name pkg-desc))))) >> > + (if (package-installed-p (package-desc-name pkg-desc)) >> > + (let ((installed-pkg-desc (cadr (assq >> > + (package-desc-name pkg-desc) >> > + package-alist)))) >> > + (pcase (let ((read-answer-short t)) I don't think you should shadow the user's preference here. >> > + (read-answer >> > + (format-message >> > + "Replace `%s' with `%s', or Install both (advanced users) " Without some additional explanation here, I would assume that this prompt could confuse users. If we go with this approach, can you look into describing why the user would be interested in either replacing or keeping both versions of a package somewhere? Also, why not move this prompt into `package-install', that the user would get presented with if they invoke the command interactively? >> > + (package-desc-full-name installed-pkg-desc) >> > + (package-desc-full-name pkg-desc)) >> > + '(("replace" ?r "Replace existing") >> > + ("install" ?i "Install both") >> > + ("help" ?h "Help") >> > + ("quit" ?q "Quit to abort")))) >> > + ("replace" >> > + (package-delete installed-pkg-desc 'force 'dont-unselect) >> > + (when (package-install pkg-desc nil) >> > + (describe-package (package-desc-name pkg-desc)))) >> > + ("install" >> > + (when (package-install pkg-desc nil 'allow-mult-or-reinstall) >> > + (describe-package (package-desc-name pkg-desc)))))) I think it would be good to add a message confirming that the "quit" operation. >> > + (when (y-or-n-p (format-message "Install package `%s'? " >> > + (package-desc-full-name pkg-desc))) >> > + (when (package-install pkg-desc nil) >> > + (describe-package (package-desc-name pkg-desc))))))) >> > >> > (defun package-delete-button-action (button) >> > "Run `package-delete' on the package BUTTON points to. >> > diff --git a/test/lisp/emacs-lisp/package-tests.el b/test/lisp/emacs-lisp/package-tests.el >> > index d8e260319bd..36617ed6f6e 100644 >> > --- a/test/lisp/emacs-lisp/package-tests.el >> > +++ b/test/lisp/emacs-lisp/package-tests.el >> > @@ -248,6 +248,10 @@ package-test-install-single >> > (should (string-match "^[`‘']simple-single[’'] is already installed\n?\\'" >> > (buffer-string)))) >> > (should (package-installed-p 'simple-single)) >> > + ;; Test for `package-install' already installed using a >> > + ;; package-desc. `package-install' returns nil if already >> > + ;; installed. >> > + (should-not (package-install simple-single-desc)) >> > (let* ((simple-pkg-dir (file-name-as-directory >> > (expand-file-name >> > "simple-single-1.3" >> >> For some reason I find this patch very difficult to follow... Perhaps I >> don't understand the issue you are trying to fix, or how it is related >> to your patch. >> >> The issue is that package install will install multiple versions of the same package (from the same archive) >> without warning. This will and does confuse users especially using the interactive package menu. The >> patch works for me. Thanks for the explanation! I would rephrase the commit message to express the "Warn user when installing multiple versions of the same package" part more prominently. >> The related issue is that multiple versions of the same packages that come from different archives are also >> installed but this patch does not attempt to deal with that. I proposed that we segregate archives under the >> package tree to ease the package infrastructure's ability to detect that. That is a different issue, one that I would like to solve ideally by convincing package maintainers not to have their packages on multiple archives, but that is not an issue we can solve here.
bug-gnu-emacs <at> gnu.org
:bug#76568
; Package emacs
.
(Sun, 08 Jun 2025 04:39:01 GMT) Full text and rfc822 format available.Message #61 received at 76568 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: Philip Kaludercic <philipk <at> posteo.net> Cc: 76568 <at> debbugs.gnu.org, shipmints <at> gmail.com, monnier <at> iro.umontreal.ca, stefankangas <at> gmail.com Subject: Re: bug#76568: 'package-install' should not install duplicate packages Date: Sun, 08 Jun 2025 07:38:05 +0300
> From: Philip Kaludercic <philipk <at> posteo.net> > Cc: Ship Mints <shipmints <at> gmail.com>, 76568 <at> debbugs.gnu.org, > stefankangas <at> gmail.com, "Stefan Monnier" <monnier <at> iro.umontreal.ca> > Date: Sat, 07 Jun 2025 21:23:11 +0000 > > Eli Zaretskii <eliz <at> gnu.org> writes: > > > Ping! Can we make further progress with this issue? > > Sorry, understanding this bug report still takes me forever... Just > composing this message took me an hour. I have added Stefan to the CCs > to get some more input. Thank you for your efforts.
bug-gnu-emacs <at> gnu.org
:bug#76568
; Package emacs
.
(Fri, 13 Jun 2025 06:40:02 GMT) Full text and rfc822 format available.Message #64 received at 76568 <at> debbugs.gnu.org (full text, mbox):
From: Stéphane Marks <shipmints <at> gmail.com> To: Philip Kaludercic <philipk <at> posteo.net> Cc: 76568 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, stefankangas <at> gmail.com, Stefan Monnier <monnier <at> iro.umontreal.ca> Subject: Re: bug#76568: 'package-install' should not install duplicate packages Date: Fri, 13 Jun 2025 08:39:11 +0200
[Message part 1 (text/plain, inline)]
On Sat, Jun 7, 2025 at 11:23 PM Philip Kaludercic <philipk <at> posteo.net> wrote: > Eli Zaretskii <eliz <at> gnu.org> writes: > > > Ping! Can we make further progress with this issue? > > Sorry, understanding this bug report still takes me forever... Just > composing this message took me an hour. I have added Stefan to the CCs > to get some more input. > > >> From: Ship Mints <shipmints <at> gmail.com> > >> Date: Sun, 25 May 2025 11:08:20 -0400 > >> Cc: 76568 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, > >> Stefan Kangas <stefankangas <at> gmail.com> > >> > >> On Sun, May 25, 2025 at 11:01 AM Philip Kaludercic <philipk <at> posteo.net> > wrote: > >> > >> Ship Mints <shipmints <at> gmail.com> writes: > >> > >> [...] > >> > >> >> Can you please summarise the relevant parts of this discussion? I > see > >> >> that a patch is being mentioned above, should I review it? > >> >> > >> > > >> > The latest version of this patch from this discussion is attached. > It > >> > amends both the menu-driven package upgrade and the package-upgrade > command > >> > to assist the user with avoiding (or permitting) installing a > package more > >> > than once. > >> > > >> > >> [...] > >> > >> > > >> > -Stephane > >> > From 9deee448e27f94c21469e59677fde2cbce8f9bcd Mon Sep 17 00:00:00 > 2001 > >> > From: shipmints <shipmints <at> gmail.com> > >> > Date: Wed, 5 Mar 2025 11:33:07 -0500 > >> > Subject: [PATCH] Correct 'package-install' to detect installed > packages > >> > (bug#76568) > >> > > >> > * lisp/emacs-lisp/package.el > >> > (package-install): Check for already installed package using its > symbol > >> > rather than rely on differing archive directory structures. Return > t if > ^ > Why are you mentioning this here? > That's the root source of duplicate package installations--packages that exist in multiple archives. Expanding the language to be more expansive is useful and as we move ahead with this, I'll update the text. >> > installed, nil otherwise. > >> > (package-install-button-action): 'describe-package' only when > >> > 'package-install' actually installed the package. Prompt the user to > >> > install or upgrade an already installed package. > >> > * test/lisp/emacs-lisp/package-tests.el > (package-test-install-single): > >> > Add already-installed test using 'package-install' with a > >> > 'package-desc'. > >> > --- > >> > lisp/emacs-lisp/package.el | 54 > +++++++++++++++++++++------ > >> > test/lisp/emacs-lisp/package-tests.el | 4 ++ > >> > 2 files changed, 47 insertions(+), 11 deletions(-) > >> > > >> > diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el > >> > index 8d498c216dc..7c7e99168aa 100644 > >> > --- a/lisp/emacs-lisp/package.el > >> > +++ b/lisp/emacs-lisp/package.el > >> > @@ -2194,7 +2194,7 @@ package-install-upgrade-built-in > >> > :version "29.1") > >> > > >> > ;;;###autoload > >> > -(defun package-install (pkg &optional dont-select) > >> > +(defun package-install (pkg &optional dont-select > allow-mult-or-reinstall) > >> > "Install the package PKG. > >> > > >> > PKG can be a `package-desc', or a symbol naming one of the available > >> > @@ -2207,8 +2207,13 @@ package-install > >> > non-nil, install the package but do not add it to > >> > `package-selected-packages'. > >> > > >> > -If PKG is a `package-desc' and it is already installed, don't try > >> > -to install it but still mark it as selected. > >> > +When called from Lisp and optional argument ALLOW-MULT-OR-REINSTALL > is > >> > +non-nil, install the package even if it is already installed from > >> > +another source, allowing more than one simultaneous version. > >> > + > >> > +If PKG is a `package-desc' and it is already installed, and > >> > +ALLOW-MULT-OR-REINSTALL is nil, don't try to install it but still > mark > >> > +it as selected. > >> > >> Why is this yet another optional argument, instead of a user option? > >> > >> As the maintainer, you pick what suits your taste. > > No, what I meant is if there is a reason that we should sometimes allow > installing multiple versions of a package in some cases and not in > others? Is that a genuine use-case, or do you (as the person > experiencing the issue you are trying to solve) assume that most people > with the same problem would be OK with setting this once via a > user-option. I am under the impression that that would simplify the > change. > I figured there's someone out there that depends on this for some reason and Emacs tries to provide backward compatibility. I prefer the simpler route, if agreed. (Also, if we go with the optional argument, please rename it to > something cleaner like "allow-multiple". Perhaps it is just me, but an > "-or-" in a argument name bugs me.) > Sure. >> > If the command is invoked with a prefix argument, it will allow > >> > upgrading of built-in packages, as if > `package-install-upgrade-built-in' > >> > @@ -2243,16 +2248,22 @@ package-install > >> > (package--active-built-in-p pkg)) > >> > (setq pkg (or (cadr (assq name package-archive-contents)) > pkg))) > >> > (if-let* ((transaction > >> > + ;; Test for already installed using the pkg symbol, > not > >> > + ;; the archive-specific directory structure. > >> > (if (package-desc-p pkg) > >> > - (unless (package-installed-p pkg) > >> > + (unless (and (not allow-mult-or-reinstall) > >> > + (package-installed-p > (package-desc-name pkg))) > >> > (package-compute-transaction (list pkg) > >> > > (package-desc-reqs pkg))) > >> > - (package-compute-transaction () (list (list > pkg)))))) > >> > + (unless (and (not allow-mult-or-reinstall) > >> > + (package-installed-p pkg)) > >> > + (package-compute-transaction () (list (list > pkg))))))) > >> > (progn > >> > (package-download-transaction transaction) > >> > (package--quickstart-maybe-refresh) > >> > (message "Package `%s' installed." name)) > >> > - (message "`%s' is already installed" name)))) > >> > + (message "`%s' is already installed" name) > >> > + nil))) > > This last change appears to be unrelated to the remaining patch? > Last meaning the message, or last meaning the whole of the above? >> > (declare-function package-vc-upgrade "package-vc" (pkg)) > >> > > >> > @@ -2587,7 +2598,7 @@ package-reinstall > >> > (package-delete > >> > (if (package-desc-p pkg) pkg (cadr (assq pkg package-alist))) > >> > 'force 'nosave) > >> > - (package-install pkg 'dont-select)) > >> > + (package-install pkg 'dont-select 'allow-mult-or-reinstall)) > >> > > >> > ;;;###autoload > >> > (defun package-recompile (pkg) > >> > @@ -3051,10 +3062,31 @@ package-install-button-action > > (OK, one of the things that had me confused is that I misread the diff > and assumed you were patching `package-recompile' ^^) > > >> > Used for the `action' property of buttons in the buffer created by > >> > `describe-package'." > >> > (let ((pkg-desc (button-get button 'package-desc))) > >> > - (when (y-or-n-p (format-message "Install package `%s'? " > >> > - (package-desc-full-name > pkg-desc))) > >> > - (package-install pkg-desc nil) > >> > - (describe-package (package-desc-name pkg-desc))))) > >> > + (if (package-installed-p (package-desc-name pkg-desc)) > >> > + (let ((installed-pkg-desc (cadr (assq > >> > + (package-desc-name > pkg-desc) > >> > + package-alist)))) > >> > + (pcase (let ((read-answer-short t)) > > I don't think you should shadow the user's preference here. > I need more coffee. Not following. >> > + (read-answer > >> > + (format-message > >> > + "Replace `%s' with `%s', or Install both > (advanced users) " > > Without some additional explanation here, I would assume that this > prompt could confuse users. If we go with this approach, can you look > into describing why the user would be interested in either replacing or > keeping both versions of a package somewhere? > If we decide to disallow multiple side-by-side installs, this becomes moot. Also, why not move this prompt into `package-install', that the user > would get presented with if they invoke the command interactively? > I'd have to reread this from scratch to see as I can't recall. >> > + (package-desc-full-name installed-pkg-desc) > >> > + (package-desc-full-name pkg-desc)) > >> > + '(("replace" ?r "Replace existing") > >> > + ("install" ?i "Install both") > >> > + ("help" ?h "Help") > >> > + ("quit" ?q "Quit to abort")))) > >> > + ("replace" > >> > + (package-delete installed-pkg-desc 'force > 'dont-unselect) > >> > + (when (package-install pkg-desc nil) > >> > + (describe-package (package-desc-name pkg-desc)))) > >> > + ("install" > >> > + (when (package-install pkg-desc nil > 'allow-mult-or-reinstall) > >> > + (describe-package (package-desc-name pkg-desc)))))) > > I think it would be good to add a message confirming that the "quit" > operation. > Alright. > >> > + (when (y-or-n-p (format-message "Install package `%s'? " > >> > + (package-desc-full-name > pkg-desc))) > >> > + (when (package-install pkg-desc nil) > >> > + (describe-package (package-desc-name pkg-desc))))))) > >> > > >> > (defun package-delete-button-action (button) > >> > "Run `package-delete' on the package BUTTON points to. > >> > diff --git a/test/lisp/emacs-lisp/package-tests.el > b/test/lisp/emacs-lisp/package-tests.el > >> > index d8e260319bd..36617ed6f6e 100644 > >> > --- a/test/lisp/emacs-lisp/package-tests.el > >> > +++ b/test/lisp/emacs-lisp/package-tests.el > >> > @@ -248,6 +248,10 @@ package-test-install-single > >> > (should (string-match "^[`‘']simple-single[’'] is already > installed\n?\\'" > >> > (buffer-string)))) > >> > (should (package-installed-p 'simple-single)) > >> > + ;; Test for `package-install' already installed using a > >> > + ;; package-desc. `package-install' returns nil if already > >> > + ;; installed. > >> > + (should-not (package-install simple-single-desc)) > >> > (let* ((simple-pkg-dir (file-name-as-directory > >> > (expand-file-name > >> > "simple-single-1.3" > >> > >> For some reason I find this patch very difficult to follow... Perhaps > I > >> don't understand the issue you are trying to fix, or how it is related > >> to your patch. > >> > >> The issue is that package install will install multiple versions of the > same package (from the same archive) > >> without warning. This will and does confuse users especially using the > interactive package menu. The > >> patch works for me. > > Thanks for the explanation! I would rephrase the commit message to > express the > > "Warn user when installing multiple versions of the same > package" > > part more prominently. > Okay. >> The related issue is that multiple versions of the same packages that > come from different archives are also > >> installed but this patch does not attempt to deal with that. I > proposed that we segregate archives under the > >> package tree to ease the package infrastructure's ability to detect > that. > > That is a different issue, one that I would like to solve ideally by > convincing package maintainers not to have their packages on multiple > archives, but that is not an issue we can solve here. > I suppose the "convenience" of MELPA's publish on most recent commit is attractive to some. Doesn't this still bite us if people use both ELPA devel and ELPA production since they're treated as different archives? Perhaps we can unify duplicate package detection at least in the core Emacs archives. -Stephane
[Message part 2 (text/html, inline)]
bug-gnu-emacs <at> gnu.org
:bug#76568
; Package emacs
.
(Sat, 14 Jun 2025 22:41:02 GMT) Full text and rfc822 format available.Message #67 received at 76568 <at> debbugs.gnu.org (full text, mbox):
From: Philip Kaludercic <philipk <at> posteo.net> To: Stéphane Marks <shipmints <at> gmail.com> Cc: 76568 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, stefankangas <at> gmail.com, Stefan Monnier <monnier <at> iro.umontreal.ca> Subject: Re: bug#76568: 'package-install' should not install duplicate packages Date: Sat, 14 Jun 2025 22:39:53 +0000
Stéphane Marks <shipmints <at> gmail.com> writes: > On Sat, Jun 7, 2025 at 11:23 PM Philip Kaludercic <philipk <at> posteo.net> > wrote: > >> Eli Zaretskii <eliz <at> gnu.org> writes: >> >> > Ping! Can we make further progress with this issue? >> >> Sorry, understanding this bug report still takes me forever... Just >> composing this message took me an hour. I have added Stefan to the CCs >> to get some more input. >> >> >> From: Ship Mints <shipmints <at> gmail.com> >> >> Date: Sun, 25 May 2025 11:08:20 -0400 >> >> Cc: 76568 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, >> >> Stefan Kangas <stefankangas <at> gmail.com> >> >> >> >> On Sun, May 25, 2025 at 11:01 AM Philip Kaludercic <philipk <at> posteo.net> >> wrote: >> >> >> >> Ship Mints <shipmints <at> gmail.com> writes: >> >> >> >> [...] >> >> >> >> >> Can you please summarise the relevant parts of this discussion? I >> see >> >> >> that a patch is being mentioned above, should I review it? >> >> >> >> >> > >> >> > The latest version of this patch from this discussion is attached. >> It >> >> > amends both the menu-driven package upgrade and the package-upgrade >> command >> >> > to assist the user with avoiding (or permitting) installing a >> package more >> >> > than once. >> >> > >> >> >> >> [...] >> >> >> >> > >> >> > -Stephane >> >> > From 9deee448e27f94c21469e59677fde2cbce8f9bcd Mon Sep 17 00:00:00 >> 2001 >> >> > From: shipmints <shipmints <at> gmail.com> >> >> > Date: Wed, 5 Mar 2025 11:33:07 -0500 >> >> > Subject: [PATCH] Correct 'package-install' to detect installed >> packages >> >> > (bug#76568) >> >> > >> >> > * lisp/emacs-lisp/package.el >> >> > (package-install): Check for already installed package using its >> symbol >> >> > rather than rely on differing archive directory structures. Return >> t if >> ^ >> Why are you mentioning this here? >> > > That's the root source of duplicate package installations--packages that > exist in multiple archives. Expanding the language to be more expansive is > useful and as we move ahead with this, I'll update the text. My point why are you documenting a fix that you are not implementing in this patch? >>> > installed, nil otherwise. >> >> > (package-install-button-action): 'describe-package' only when >> >> > 'package-install' actually installed the package. Prompt the user to >> >> > install or upgrade an already installed package. >> >> > * test/lisp/emacs-lisp/package-tests.el >> (package-test-install-single): >> >> > Add already-installed test using 'package-install' with a >> >> > 'package-desc'. >> >> > --- >> >> > lisp/emacs-lisp/package.el | 54 >> +++++++++++++++++++++------ >> >> > test/lisp/emacs-lisp/package-tests.el | 4 ++ >> >> > 2 files changed, 47 insertions(+), 11 deletions(-) >> >> > >> >> > diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el >> >> > index 8d498c216dc..7c7e99168aa 100644 >> >> > --- a/lisp/emacs-lisp/package.el >> >> > +++ b/lisp/emacs-lisp/package.el >> >> > @@ -2194,7 +2194,7 @@ package-install-upgrade-built-in >> >> > :version "29.1") >> >> > >> >> > ;;;###autoload >> >> > -(defun package-install (pkg &optional dont-select) >> >> > +(defun package-install (pkg &optional dont-select >> allow-mult-or-reinstall) >> >> > "Install the package PKG. >> >> > >> >> > PKG can be a `package-desc', or a symbol naming one of the available >> >> > @@ -2207,8 +2207,13 @@ package-install >> >> > non-nil, install the package but do not add it to >> >> > `package-selected-packages'. >> >> > >> >> > -If PKG is a `package-desc' and it is already installed, don't try >> >> > -to install it but still mark it as selected. >> >> > +When called from Lisp and optional argument ALLOW-MULT-OR-REINSTALL >> is >> >> > +non-nil, install the package even if it is already installed from >> >> > +another source, allowing more than one simultaneous version. >> >> > + >> >> > +If PKG is a `package-desc' and it is already installed, and >> >> > +ALLOW-MULT-OR-REINSTALL is nil, don't try to install it but still >> mark >> >> > +it as selected. >> >> >> >> Why is this yet another optional argument, instead of a user option? >> >> >> >> As the maintainer, you pick what suits your taste. >> >> No, what I meant is if there is a reason that we should sometimes allow >> installing multiple versions of a package in some cases and not in >> others? Is that a genuine use-case, or do you (as the person >> experiencing the issue you are trying to solve) assume that most people >> with the same problem would be OK with setting this once via a >> user-option. I am under the impression that that would simplify the >> change. >> > > I figured there's someone out there that depends on this for some reason > and Emacs tries to provide backward compatibility. I prefer the simpler > route, if agreed. > > (Also, if we go with the optional argument, please rename it to >> something cleaner like "allow-multiple". Perhaps it is just me, but an >> "-or-" in a argument name bugs me.) >> > > Sure. Thanks. >>> > If the command is invoked with a prefix argument, it will allow >> >> > upgrading of built-in packages, as if >> `package-install-upgrade-built-in' >> >> > @@ -2243,16 +2248,22 @@ package-install >> >> > (package--active-built-in-p pkg)) >> >> > (setq pkg (or (cadr (assq name package-archive-contents)) >> pkg))) >> >> > (if-let* ((transaction >> >> > + ;; Test for already installed using the pkg symbol, >> not >> >> > + ;; the archive-specific directory structure. >> >> > (if (package-desc-p pkg) >> >> > - (unless (package-installed-p pkg) >> >> > + (unless (and (not allow-mult-or-reinstall) >> >> > + (package-installed-p >> (package-desc-name pkg))) >> >> > (package-compute-transaction (list pkg) >> >> > >> (package-desc-reqs pkg))) >> >> > - (package-compute-transaction () (list (list >> pkg)))))) >> >> > + (unless (and (not allow-mult-or-reinstall) >> >> > + (package-installed-p pkg)) >> >> > + (package-compute-transaction () (list (list >> pkg))))))) >> >> > (progn >> >> > (package-download-transaction transaction) >> >> > (package--quickstart-maybe-refresh) >> >> > (message "Package `%s' installed." name)) >> >> > - (message "`%s' is already installed" name)))) >> >> > + (message "`%s' is already installed" name) >> >> > + nil))) >> >> This last change appears to be unrelated to the remaining patch? >> > > Last meaning the message, or last meaning the whole of the above? I meant the addition of returning `nil'. But I have since understood that you needed to add this for the test to work. >>> > (declare-function package-vc-upgrade "package-vc" (pkg)) >> >> > >> >> > @@ -2587,7 +2598,7 @@ package-reinstall >> >> > (package-delete >> >> > (if (package-desc-p pkg) pkg (cadr (assq pkg package-alist))) >> >> > 'force 'nosave) >> >> > - (package-install pkg 'dont-select)) >> >> > + (package-install pkg 'dont-select 'allow-mult-or-reinstall)) >> >> > >> >> > ;;;###autoload >> >> > (defun package-recompile (pkg) >> >> > @@ -3051,10 +3062,31 @@ package-install-button-action >> >> (OK, one of the things that had me confused is that I misread the diff >> and assumed you were patching `package-recompile' ^^) >> >> >> > Used for the `action' property of buttons in the buffer created by >> >> > `describe-package'." >> >> > (let ((pkg-desc (button-get button 'package-desc))) >> >> > - (when (y-or-n-p (format-message "Install package `%s'? " >> >> > - (package-desc-full-name >> pkg-desc))) >> >> > - (package-install pkg-desc nil) >> >> > - (describe-package (package-desc-name pkg-desc))))) >> >> > + (if (package-installed-p (package-desc-name pkg-desc)) >> >> > + (let ((installed-pkg-desc (cadr (assq >> >> > + (package-desc-name >> pkg-desc) >> >> > + package-alist)))) >> >> > + (pcase (let ((read-answer-short t)) >> >> I don't think you should shadow the user's preference here. >> > > I need more coffee. Not following. I don't think you should bind `read-answer-short'. If the user prefers to input short answers, they would adjust the user option. Otherwise, I think it is more consistent to stick to the defaults. >>> > + (read-answer >> >> > + (format-message >> >> > + "Replace `%s' with `%s', or Install both >> (advanced users) " >> >> Without some additional explanation here, I would assume that this >> prompt could confuse users. If we go with this approach, can you look >> into describing why the user would be interested in either replacing or >> keeping both versions of a package somewhere? >> > > If we decide to disallow multiple side-by-side installs, this becomes moot. No, I don't think we should do that for backwards compatibility reasons, but it might be worthwhile considering it as something to deprecate at some point... > Also, why not move this prompt into `package-install', that the user >> would get presented with if they invoke the command interactively? >> > > I'd have to reread this from scratch to see as I can't recall. OK, I can take a closer look at the code as well and see if things can be adjusted before pushing to master. >>> > + (package-desc-full-name installed-pkg-desc) >> >> > + (package-desc-full-name pkg-desc)) >> >> > + '(("replace" ?r "Replace existing") >> >> > + ("install" ?i "Install both") >> >> > + ("help" ?h "Help") >> >> > + ("quit" ?q "Quit to abort")))) >> >> > + ("replace" >> >> > + (package-delete installed-pkg-desc 'force >> 'dont-unselect) >> >> > + (when (package-install pkg-desc nil) >> >> > + (describe-package (package-desc-name pkg-desc)))) >> >> > + ("install" >> >> > + (when (package-install pkg-desc nil >> 'allow-mult-or-reinstall) >> >> > + (describe-package (package-desc-name pkg-desc)))))) >> >> I think it would be good to add a message confirming that the "quit" >> operation. >> > > Alright. > > >> >> > + (when (y-or-n-p (format-message "Install package `%s'? " >> >> > + (package-desc-full-name >> pkg-desc))) >> >> > + (when (package-install pkg-desc nil) >> >> > + (describe-package (package-desc-name pkg-desc))))))) >> >> > >> >> > (defun package-delete-button-action (button) >> >> > "Run `package-delete' on the package BUTTON points to. >> >> > diff --git a/test/lisp/emacs-lisp/package-tests.el >> b/test/lisp/emacs-lisp/package-tests.el >> >> > index d8e260319bd..36617ed6f6e 100644 >> >> > --- a/test/lisp/emacs-lisp/package-tests.el >> >> > +++ b/test/lisp/emacs-lisp/package-tests.el >> >> > @@ -248,6 +248,10 @@ package-test-install-single >> >> > (should (string-match "^[`‘']simple-single[’'] is already >> installed\n?\\'" >> >> > (buffer-string)))) >> >> > (should (package-installed-p 'simple-single)) >> >> > + ;; Test for `package-install' already installed using a >> >> > + ;; package-desc. `package-install' returns nil if already >> >> > + ;; installed. >> >> > + (should-not (package-install simple-single-desc)) >> >> > (let* ((simple-pkg-dir (file-name-as-directory >> >> > (expand-file-name >> >> > "simple-single-1.3" >> >> >> >> For some reason I find this patch very difficult to follow... Perhaps >> I >> >> don't understand the issue you are trying to fix, or how it is related >> >> to your patch. >> >> >> >> The issue is that package install will install multiple versions of the >> same package (from the same archive) >> >> without warning. This will and does confuse users especially using the >> interactive package menu. The >> >> patch works for me. >> >> Thanks for the explanation! I would rephrase the commit message to >> express the >> >> "Warn user when installing multiple versions of the same >> package" >> >> part more prominently. >> > > Okay. > >>> The related issue is that multiple versions of the same packages that >> come from different archives are also >> >> installed but this patch does not attempt to deal with that. I >> proposed that we segregate archives under the >> >> package tree to ease the package infrastructure's ability to detect >> that. >> >> That is a different issue, one that I would like to solve ideally by >> convincing package maintainers not to have their packages on multiple >> archives, but that is not an issue we can solve here. >> > > I suppose the "convenience" of MELPA's publish on most recent commit is > attractive to some. > > Doesn't this still bite us if people use both ELPA devel and ELPA > production since they're treated as different archives? Perhaps we can > unify duplicate package detection at least in the core Emacs archives. That is true, though I am also of the opinion that end-users ought not to use ELPA-devel directly. In that case, we can at least rely on sensible version number handling. > -Stephane
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.