Package: emacs;
Reported by: Ship Mints <shipmints <at> gmail.com>
Date: Tue, 25 Feb 2025 20:53:01 UTC
Severity: normal
Tags: patch
View this message in rfc822 format
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: 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.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.