bug#72141: 29.4; package-upgrade vs package-load-list

[Thierry Volpiatto <thievol@posteo.net>]

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

Philip Kaludercic <philipk@posteo.net> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Thierry Volpiatto <thievol@posteo.net>
>>> Date: Tue, 16 Jul 2024 14:46:37 +0000
>>> 
>>> 
>>> I think there is a bug here, but please verify with following recipe as
>>> I don't use widely package installation, at least for myself.  When reading
>>> the code I believe it is reproductible as well on emacs-30+.
>>> 
>>> 1) Install package foo and bar.
>>> 2) Disable them in package-load-list ((foo nil) (bar nil) all).
>>> 3) Wait some time until foo and/or bar have new versions available.
>>> 4) Call package-upgrade-all.  It will call package-upgrade on foo
>>> and bar (and possibly others).  When package-upgrade find foo
>>> package it will (1) delete it and (2) call package-install which
>>> will refuse to install (error) because foo is disabled.
>>> 
>>> As a result we have lost foo package, it is now uninstalled.
>>> Same problem with M-x package-upgrade, foo and bar are listed in
>>> completion and made available whereas they are going to fail to
>>> upgrade.
>>
>> Philip, any comments or suggestions?
>
> The issue is that we don't install a package if it is disabled.  So
> either we allow installing (but don't activate) disabled packages, or we
> ignore disabled packages during upgrades.  That might just need this
> change:
>
> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index 7cae8d68bc0..eb77d99fad2 100644
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -2286,6 +2286,9 @@ package--upgradeable-packages
>        (or (let ((available
>                   (assq (car elt) package-archive-contents)))
>              (and available
> +                 (package-disabled-p
> +                  (cadr elt)
> +                  (package-desc-version (cadr elt)))
>                   (or (and
>                        include-builtins
>                        (not (package-desc-version (cadr elt))))

If nothing in package.el or elsewhere relay on the fact that
package--upgradeable-packages returns the disabled packages
that looks good.
Also why in this function you are using (mapcar 'car (seq-filter ...))?
Perhaps one loop could be avoided here? (just asking, I am not familiar
with seq, I don't use it).

Thanks.

-- 
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 686 bytes --]