From: Philip Kaludercic <philipk@posteo.net>
To: Thierry Volpiatto <thievol@posteo.net>
Cc: Eli Zaretskii <eliz@gnu.org>, 72141@debbugs.gnu.org
Subject: bug#72141: 29.4; package-upgrade vs package-load-list
Date: Sun, 04 Aug 2024 14:57:23 +0000 [thread overview]
Message-ID: <8734nkz67g.fsf@posteo.net> (raw)
In-Reply-To: <8734nmguiq.fsf@posteo.net> (Thierry Volpiatto's message of "Sat, 03 Aug 2024 03:22:37 +0000")
Thierry Volpiatto <thievol@posteo.net> writes:
> Thierry Volpiatto <thievol@posteo.net> writes:
>
>> Thierry Volpiatto <thievol@posteo.net> writes:
>>
>>> Eli Zaretskii <eliz@gnu.org> writes:
>>>
>>>>> From: Philip Kaludercic <philipk@posteo.net>
>>>>> Cc: Thierry Volpiatto <thievol@posteo.net>, 72141@debbugs.gnu.org
>>>>> Date: Sun, 28 Jul 2024 11:47:44 +0000
>>>>>
>>>>> 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.
>>>>
>>>> The latter, I'd say. It makes little sense to upgrade disabled
>>>> packages.
>>>
>>> When I posted initially this bugreport I wrote this (fully not tested):
>>>
>>> (defun package--upgradeable-packages (&optional include-builtins filter-load-list)
>>> ;; Initialize the package system to get the list of package
>>> ;; symbols for completion.
>>> (package--archives-initialize)
>>> (let ((pkgs (if include-builtins
>>> (append package-alist
>>> (mapcan
>>> (lambda (elt)
>>> (when (not (assq (car elt) package-alist))
>>> (list (list (car elt) (package--from-builtin elt)))))
>>> package--builtins))
>>> package-alist)))
>>> (cl-loop for (sym desc) in pkgs
>>> for available = (assq sym package-archive-contents)
>>> when (or (and available
>>> (or (and
>>> include-builtins
>>> (not (package-desc-version desc)))
>>> (version-list-<
>>> (package-desc-version desc)
>>> (package-desc-version (cadr available)))
>>> (and filter-load-list
>>> (pcase (assq p package-load-list)
>>> (`(,sym ,val) (or (not (eq val nil))
>>> (not (stringp val))))))))
>>> (package-vc-p desc))
>>> collect sym)))
>>>
>>> Perhaps package-disabled-p can be used instead of the pcase (I didn't
>>> know its existence).
>>
>> Here a version fixing typo and using package-disabled-p (same, still
>> fully untested)
>> Note the extra optional arg filter-load-list that allow preserving the initial behavior
>> if needed (better name?).
>>
>> (defun package--upgradeable-packages (&optional include-builtins filter-load-list)
>> ;; Initialize the package system to get the list of package
>> ;; symbols for completion.
>> (package--archives-initialize)
>> (let ((pkgs (if include-builtins
>> (append package-alist
>> (mapcan
>> (lambda (elt)
>> (when (not (assq (car elt) package-alist))
>> (list (list (car elt) (package--from-builtin elt)))))
>> package--builtins))
>> package-alist)))
>> (cl-loop for (sym desc) in pkgs
>> for available = (assq sym package-archive-contents)
>> for cversion = (and available (package-desc-version desc))
>> when (or (and available
>> (or (and
>> include-builtins
>> (not (package-desc-version desc)))
>> (version-list-<
>> cversion
>> (package-desc-version (cadr available)))
>> (and filter-load-list
>> (package-disabled-p sym cversion))))
>> (package-vc-p desc))
>> collect sym)))
>>
>> Also there is IMO another inconsistency in package-upgrade where the
>> completion is done inconditionally on packages+builtins and later
>> package-install-upgrade-built-in is let bounded to prevent
>> package-install to upgrade built-in in case user chose a built-in!
>
> I finally rewrited a `package--upgradeable-packages` for helm and could test
> it (versions I sent previously haven't been tested and are wrong), it is
> working fine. I can send a patch if you want let me know.
Gladly, then I'd like to try it out it and perhaps write a ERT test.
> Thanks.
>
> https://github.com/emacs-helm/helm/blob/master/helm-packages.el#L266
--
Philip Kaludercic on peregrine
next prev parent reply other threads:[~2024-08-04 14:57 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-16 14:46 bug#72141: 29.4; package-upgrade vs package-load-list Thierry Volpiatto
2024-07-27 7:14 ` Eli Zaretskii
2024-07-28 11:47 ` Philip Kaludercic
2024-07-28 12:27 ` Thierry Volpiatto
2024-07-28 12:27 ` Eli Zaretskii
2024-07-28 12:39 ` Thierry Volpiatto
2024-08-01 6:48 ` Thierry Volpiatto
2024-08-03 3:22 ` Thierry Volpiatto
2024-08-04 14:57 ` Philip Kaludercic [this message]
2024-08-04 17:15 ` Thierry Volpiatto
2024-08-10 17:16 ` Thierry Volpiatto
2024-08-12 16:36 ` Philip Kaludercic
2024-08-12 17:36 ` Thierry Volpiatto
2024-08-12 18:16 ` Thierry Volpiatto
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8734nkz67g.fsf@posteo.net \
--to=philipk@posteo.net \
--cc=72141@debbugs.gnu.org \
--cc=eliz@gnu.org \
--cc=thievol@posteo.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).