GNU bug report logs - #76568
'package-install' should not install duplicate packages

Previous Next

Full log

Message #73 received at 76568 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Ship Mints <shipmints <at> gmail.com>
Cc: 76568 <at> debbugs.gnu.org, Philip Kaludercic <philipk <at> posteo.net>,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#76568: 'package-install' should not install duplicate packages
Date: Wed, 18 Jun 2025 17:03:38 -0400

> @@ -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)))

I think that when `allow-multiple` is non-nil we should still check
(package-installed-p pkg) to preserve the current behavior.
IOW

                 (unless (package-installed-p
                          (if allow-multiple
                              pkg (package-desc-name pkg)))

tho it doesn't seem right to "do nothing" when the package is already
installed but at another version.  Maybe we should do something like

                 (unless (package-installed-p pkg)
                   (when (and (not allow-multiple)
                              (package-installed-p (package-desc-name pkg)))
                     (error "Use package-upgrade"))
                   (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)))))))

Hmm...

              (unless (and (not allow-multiple)
                           (package-installed-p pkg))
=
              (when (not (and (not allow-multiple)
                              (package-installed-p pkg)))
=
              (when (or allow-multiple
                        (not (package-installed-p pkg)))

which seems a bit easier to read.

>          (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)))

If we care about the return value, we should adjust the
docstring accordingly.

> @@ -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)))))))

+1 (but, like Philip pointed out, please don't override the user's
choice of `read-answer-short`).


        Stefan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.