Package: emacs;
Reported by: Thierry Volpiatto <thievol <at> posteo.net>
Date: Tue, 16 Jul 2024 14:44:02 UTC
Severity: normal
Found in version 29.4
To reply to this bug, email your comments to 72141 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#72141; Package emacs.
(Tue, 16 Jul 2024 14:44:02 GMT) Full text and rfc822 format available.Thierry Volpiatto <thievol <at> posteo.net>:bug-gnu-emacs <at> gnu.org.
(Tue, 16 Jul 2024 14:44:02 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Thierry Volpiatto <thievol <at> posteo.net> To: bug-gnu-emacs <at> gnu.org Subject: 29.4; package-upgrade vs package-load-list 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. In GNU Emacs 29.4 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo version 1.16.0, Xaw3d scroll bars) of 2024-06-23 built on IPad-S340 Windowing system distributor 'The X.Org Foundation', version 11.0.12101004 System Description: Linux Mint 21.3 Configured using: 'configure CFLAGS=-O8 --bindir=/usr/local/sbin/emacs-29.3 --with-cairo --with-x-toolkit=lucid --with-modules --without-tree-sitter --without-native-compilation' Configured features: ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS X11 XAW3D XDBE XIM XINPUT2 XPM LUCID ZLIB Important settings: value of $LANG: fr_FR.UTF-8 locale-coding-system: utf-8-unix Major mode: Minor modes in effect: emms-mode-line-mode: t emms-playing-time-display-mode: t emms-playing-time-mode: t server-mode: t psession-mode: t psession-savehist-mode: t register-preview-mode: t global-git-gutter-mode: t display-time-mode: t winner-mode: t tv-save-place-mode: t helm-epa-mode: t helm-descbinds-mode: t helm-top-poll-mode: t helm-adaptive-mode: t helm-mode: t helm-minibuffer-history-mode: t helm-ff-icon-mode: t shell-dirtrack-mode: t helm-popup-tip-mode: t async-bytecomp-package-mode: t dired-async-mode: t minibuffer-depth-indicate-mode: t gcmh-mode: t tooltip-mode: t global-eldoc-mode: t eldoc-mode: t show-paren-mode: t mouse-wheel-mode: t file-name-shadow-mode: t global-font-lock-mode: t font-lock-mode: t column-number-mode: t line-number-mode: t transient-mark-mode: t auto-composition-mode: t auto-encryption-mode: t auto-compression-mode: t Features: (shadow epa-mail face-remap helm-ring emacsbug ediff ediff-merg ediff-mult ediff-wind ediff-diff ediff-help ediff-init ediff-util gnus-cite smiley w3m-form w3m-symbol qp config-w3m w3m timezone w3m-hist bookmark-w3m w3m-ems w3m-favicon w3m-image w3m-fb tab-line w3m-proc w3m-util mail-extr textsec uni-scripts idna-mapping ucs-normalize uni-confusable textsec-check addressbook-bookmark tv-mu4e-config gnus-and-mu4e mu4e-patch mu4e-contrib eshell esh-cmd esh-ext esh-opt esh-proc esh-io esh-arg esh-module esh-groups esh-util mu4e mu4e-org mu4e-notification notifications mu4e-main smtpmail mu4e-view mu4e-mime-parts mu4e-headers mu4e-thread mu4e-actions mu4e-compose mu4e-draft gnus-msg mu4e-search mu4e-lists mu4e-bookmarks mu4e-mark mu4e-message flow-fill hl-line mu4e-contacts mu4e-update mu4e-folders mu4e-context mu4e-query-items mu4e-server mu4e-modeline mu4e-vars mu4e-helpers mu4e-config mu4e-window ido mu4e-obsolete svg-lib color epa-file dired-x image-file image-converter char-fold tramp-adb tramp-sh ffap cl-indent cl-print helm-command helm-elisp helm-eval edebug debug backtrace mm-archive network-stream url-cache url-http url-auth url-gw nsm helm-packages async-package finder lisp-mnt tramp-archive tramp-gvfs tramp-cache time-stamp zeroconf helm-x-files helm-for-files helm-bookmark helm-info bookmark emms-config emms-idapi-browser emms-idapi emms-idapi-musicbrainz emms-mpris emms-librefm-stream emms-librefm-scrobbler emms-playlist-limit emms-i18n emms-history emms-score emms-stream-info emms-metaplaylist-mode emms-bookmarks emms-cue emms-mode-line-icon emms-browser sort emms-volume emms-volume-sndioctl emms-volume-mixerctl emms-volume-pulse emms-volume-amixer emms-playlist-sort emms-last-played emms-player-xine emms-player-mpd tq emms-lyrics emms-url emms-streams emms-show-all emms-tag-editor emms-tag-tracktag emms-mark emms-mode-line emms-cache emms-info-native emms-info-native-spc emms-info-native-mp3 emms-info-native-ogg emms-info-native-opus emms-info-native-flac emms-info-native-vorbis bindat emms-info-exiftool emms-info-tinytag emms-info-metaflac emms-info-opusinfo emms-info-ogginfo emms-info-mp3info emms-playlist-mode emms-player-vlc emms-player-mpv emms-playing-time emms-info emms-later-do emms-player-mplayer emms-player-simple emms-source-playlist emms-source-file locate emms-setup emms emms-compat emms-auto helm-external helm-net helm-ls-git vc-git diff-mode vc vc-dispatcher conf-mode flymake-shellcheck cus-start flymake-proc flymake project warnings sh-script smie treesit executable org-element org-persist org-id org-refile avl-tree generator oc-basic cl-extra ol-eww eww url-queue thingatpt mm-url ol-rmail ol-mhe ol-irc ol-info ol-gnus nnselect gnus-art mm-uu mml2015 mm-view mml-smime smime gnutls dig gnus-sum shr pixel-fill kinsoku url-file svg dom gnus-group gnus-undo gnus-start gnus-dbus dbus xml gnus-cloud nnimap nnmail mail-source utf7 nnoo gnus-spec gnus-int gnus-range message sendmail yank-media puny rfc822 mml mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 ietf-drums mailabbrev gmm-utils mailheader gnus-win gnus nnheader gnus-util mail-utils range mm-util mail-prsvr ol-docview doc-view jka-compr ol-bibtex bibtex ol-bbdb ol-w3m ol-doi org-link-doi org-config ob-gnuplot org-crypt org-protocol org ob ob-tangle ob-ref ob-lob ob-table ob-exp org-macro org-src ob-comint org-pcomplete org-list org-footnote org-faces org-entities noutline outline ob-emacs-lisp ob-core ob-eval org-cycle org-table ol org-fold org-fold-core org-keys oc org-loaddefs find-func org-version org-compat org-macs make-mode bug-reference naquadah-theme view solar cal-dst holidays holiday-loaddefs appt diary-lib diary-loaddefs cal-menu calendar cal-loaddefs server imenu psession frameset w3m-load register-preview git-gutter mule-util dired-extension time winner describe-variable help-fns radix-tree help-mode tv-utils tv-save-place.el advice init-helm epa derived epg rfc6068 epg-config helm-epa helm-descbinds cus-edit pp icons wid-edit helm-sys helm-adaptive helm-mode helm-misc helm-files image-dired image-dired-tags image-dired-external image-dired-util xdg image-mode exif filenotify tramp tramp-loaddefs trampver tramp-integration files-x tramp-compat rx shell pcomplete parse-time iso8601 time-date helm-buffers all-the-icons all-the-icons-faces data-material data-weathericons data-octicons data-fileicons data-faicons data-alltheicons helm-occur helm-tags helm-locate helm-grep wgrep-helm wgrep grep compile text-property-search comint ansi-osc ring helm-regexp format-spec ansi-color helm-utils helm-help helm-types helm-extensions-autoloads helm-autoloads helm helm-global-bindings helm-easymenu edmacro kmacro helm-core async-bytecomp helm-source helm-multi-match helm-lib dired-async async dired-aux dired dired-loaddefs isl-autoloads mb-depth avoid cus-load gcmh easy-mmode corfu-autoloads filechooser-autoloads ledger-mode-autoloads magit-autoloads pcase git-commit-autoloads magit-section-autoloads dash-autoloads markdown-mode-autoloads transient-autoloads finder-inf with-editor-autoloads info compat-autoloads package browse-url url url-proxy url-privacy url-expand url-methods url-history url-cookie generate-lisp-file url-domsuf url-util mailcap url-handlers url-parse auth-source cl-seq eieio eieio-core cl-macs password-cache json subr-x map byte-opt gv bytecomp byte-compile url-vars cl-loaddefs cl-lib rmc iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe tabulated-list replace newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic indonesian philippine cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european ethiopic indian cyrillic chinese composite emoji-zwj charscript charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp files window text-properties overlay sha1 md5 base64 format env code-pages mule custom widget keymap hashtable-print-readable backquote threads dbusbind inotify lcms2 dynamic-setting system-font-setting font-render-setting cairo x-toolkit xinput2 x multi-tty make-network-process emacs) Memory information: ((conses 16 1126986 282642) (symbols 48 47791 7) (strings 32 329278 39126) (string-bytes 1 9084964) (vectors 16 119581) (vector-slots 8 2518384 196657) (floats 8 4123 3733) (intervals 56 15474 3437) (buffers 976 165)) <#secure method=pgpmime mode=sign> -- Thierry
bug-gnu-emacs <at> gnu.org:bug#72141; Package emacs.
(Sat, 27 Jul 2024 07:16:01 GMT) Full text and rfc822 format available.Message #8 received at 72141 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: Thierry Volpiatto <thievol <at> posteo.net>, Philip Kaludercic <philipk <at> posteo.net> Cc: 72141 <at> debbugs.gnu.org Subject: Re: bug#72141: 29.4; package-upgrade vs package-load-list Date: Sat, 27 Jul 2024 10:14:59 +0300
> From: Thierry Volpiatto <thievol <at> 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?
bug-gnu-emacs <at> gnu.org:bug#72141; Package emacs.
(Sun, 28 Jul 2024 11:49:02 GMT) Full text and rfc822 format available.Message #11 received at 72141 <at> debbugs.gnu.org (full text, mbox):
From: Philip Kaludercic <philipk <at> posteo.net> To: Eli Zaretskii <eliz <at> gnu.org> Cc: Thierry Volpiatto <thievol <at> posteo.net>, 72141 <at> debbugs.gnu.org Subject: Re: bug#72141: 29.4; package-upgrade vs package-load-list Date: Sun, 28 Jul 2024 11:47:44 +0000
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes: >> From: Thierry Volpiatto <thievol <at> 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:
[Message part 2 (text/plain, inline)]
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))))
[Message part 3 (text/plain, inline)]
-- Philip Kaludercic on peregrine
bug-gnu-emacs <at> gnu.org:bug#72141; Package emacs.
(Sun, 28 Jul 2024 12:25:02 GMT) Full text and rfc822 format available.Message #14 received at 72141 <at> debbugs.gnu.org (full text, mbox):
From: Thierry Volpiatto <thievol <at> posteo.net> To: Philip Kaludercic <philipk <at> posteo.net> Cc: Thierry Volpiatto <thievol <at> posteo.net>, Eli Zaretskii <eliz <at> gnu.org>, 72141 <at> debbugs.gnu.org Subject: Re: bug#72141: 29.4; package-upgrade vs package-load-list Date: Sun, 28 Jul 2024 12:27:10 +0000
[Message part 1 (text/plain, inline)]
Philip Kaludercic <philipk <at> posteo.net> writes: > Eli Zaretskii <eliz <at> gnu.org> writes: > >>> From: Thierry Volpiatto <thievol <at> 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
[signature.asc (application/pgp-signature, inline)]
bug-gnu-emacs <at> gnu.org:bug#72141; Package emacs.
(Sun, 28 Jul 2024 12:31:01 GMT) Full text and rfc822 format available.Message #17 received at 72141 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: Philip Kaludercic <philipk <at> posteo.net> Cc: thievol <at> posteo.net, 72141 <at> debbugs.gnu.org Subject: Re: bug#72141: 29.4; package-upgrade vs package-load-list Date: Sun, 28 Jul 2024 15:27:31 +0300
> From: Philip Kaludercic <philipk <at> posteo.net> > Cc: Thierry Volpiatto <thievol <at> posteo.net>, 72141 <at> debbugs.gnu.org > Date: Sun, 28 Jul 2024 11:47:44 +0000 > > Eli Zaretskii <eliz <at> gnu.org> writes: > > >> From: Thierry Volpiatto <thievol <at> 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.
bug-gnu-emacs <at> gnu.org:bug#72141; Package emacs.
(Sun, 28 Jul 2024 12:37:01 GMT) Full text and rfc822 format available.Message #20 received at 72141 <at> debbugs.gnu.org (full text, mbox):
From: Thierry Volpiatto <thievol <at> posteo.net> To: Eli Zaretskii <eliz <at> gnu.org> Cc: thievol <at> posteo.net, Philip Kaludercic <philipk <at> posteo.net>, 72141 <at> debbugs.gnu.org Subject: Re: bug#72141: 29.4; package-upgrade vs package-load-list Date: Sun, 28 Jul 2024 12:39:51 +0000
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: Thierry Volpiatto <thievol <at> posteo.net>, 72141 <at> debbugs.gnu.org
>> Date: Sun, 28 Jul 2024 11:47:44 +0000
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>> >> From: Thierry Volpiatto <thievol <at> 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).
--
Thierry
[signature.asc (application/pgp-signature, inline)]
bug-gnu-emacs <at> gnu.org:bug#72141; Package emacs.
(Thu, 01 Aug 2024 06:46:01 GMT) Full text and rfc822 format available.Message #23 received at 72141 <at> debbugs.gnu.org (full text, mbox):
From: Thierry Volpiatto <thievol <at> posteo.net> To: Thierry Volpiatto <thievol <at> posteo.net> Cc: Eli Zaretskii <eliz <at> gnu.org>, Philip Kaludercic <philipk <at> posteo.net>, 72141 <at> debbugs.gnu.org Subject: Re: bug#72141: 29.4; package-upgrade vs package-load-list Date: Thu, 01 Aug 2024 06:48:13 +0000
[Message part 1 (text/plain, inline)]
Thierry Volpiatto <thievol <at> posteo.net> writes:
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> From: Philip Kaludercic <philipk <at> posteo.net>
>>> Cc: Thierry Volpiatto <thievol <at> posteo.net>, 72141 <at> debbugs.gnu.org
>>> Date: Sun, 28 Jul 2024 11:47:44 +0000
>>>
>>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>>
>>> >> From: Thierry Volpiatto <thievol <at> 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!
--
Thierry
[signature.asc (application/pgp-signature, inline)]
bug-gnu-emacs <at> gnu.org:bug#72141; Package emacs.
(Sat, 03 Aug 2024 03:20:01 GMT) Full text and rfc822 format available.Message #26 received at 72141 <at> debbugs.gnu.org (full text, mbox):
From: Thierry Volpiatto <thievol <at> posteo.net> To: Thierry Volpiatto <thievol <at> posteo.net> Cc: Eli Zaretskii <eliz <at> gnu.org>, Philip Kaludercic <philipk <at> posteo.net>, 72141 <at> debbugs.gnu.org Subject: Re: bug#72141: 29.4; package-upgrade vs package-load-list Date: Sat, 03 Aug 2024 03:22:37 +0000
[Message part 1 (text/plain, inline)]
Thierry Volpiatto <thievol <at> posteo.net> writes: > Thierry Volpiatto <thievol <at> posteo.net> writes: > >> Eli Zaretskii <eliz <at> gnu.org> writes: >> >>>> From: Philip Kaludercic <philipk <at> posteo.net> >>>> Cc: Thierry Volpiatto <thievol <at> posteo.net>, 72141 <at> debbugs.gnu.org >>>> Date: Sun, 28 Jul 2024 11:47:44 +0000 >>>> >>>> Eli Zaretskii <eliz <at> gnu.org> writes: >>>> >>>> >> From: Thierry Volpiatto <thievol <at> 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. Thanks. https://github.com/emacs-helm/helm/blob/master/helm-packages.el#L266 -- Thierry
[signature.asc (application/pgp-signature, inline)]
bug-gnu-emacs <at> gnu.org:bug#72141; Package emacs.
(Sun, 04 Aug 2024 14:58:02 GMT) Full text and rfc822 format available.Message #29 received at 72141 <at> debbugs.gnu.org (full text, mbox):
From: Philip Kaludercic <philipk <at> posteo.net> To: Thierry Volpiatto <thievol <at> posteo.net> Cc: Eli Zaretskii <eliz <at> gnu.org>, 72141 <at> debbugs.gnu.org Subject: Re: bug#72141: 29.4; package-upgrade vs package-load-list Date: Sun, 04 Aug 2024 14:57:23 +0000
Thierry Volpiatto <thievol <at> posteo.net> writes: > Thierry Volpiatto <thievol <at> posteo.net> writes: > >> Thierry Volpiatto <thievol <at> posteo.net> writes: >> >>> Eli Zaretskii <eliz <at> gnu.org> writes: >>> >>>>> From: Philip Kaludercic <philipk <at> posteo.net> >>>>> Cc: Thierry Volpiatto <thievol <at> posteo.net>, 72141 <at> debbugs.gnu.org >>>>> Date: Sun, 28 Jul 2024 11:47:44 +0000 >>>>> >>>>> Eli Zaretskii <eliz <at> gnu.org> writes: >>>>> >>>>> >> From: Thierry Volpiatto <thievol <at> 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
bug-gnu-emacs <at> gnu.org:bug#72141; Package emacs.
(Sun, 04 Aug 2024 17:13:02 GMT) Full text and rfc822 format available.Message #32 received at 72141 <at> debbugs.gnu.org (full text, mbox):
From: Thierry Volpiatto <thievol <at> posteo.net> To: Philip Kaludercic <philipk <at> posteo.net> Cc: Thierry Volpiatto <thievol <at> posteo.net>, Eli Zaretskii <eliz <at> gnu.org>, 72141 <at> debbugs.gnu.org Subject: Re: bug#72141: 29.4; package-upgrade vs package-load-list Date: Sun, 04 Aug 2024 17:15:04 +0000
[Message part 1 (text/plain, inline)]
Hello Philip, Philip Kaludercic <philipk <at> posteo.net> writes: > Gladly, then I'd like to try it out it and perhaps write a ERT test. Patch attached, please review and test it before merging ;-) Thanks. -- Thierry
[0001-Fix-bug-72141-package-upgrade-should-not-include-dis.patch (text/x-diff, attachment)]
[signature.asc (application/pgp-signature, inline)]
bug-gnu-emacs <at> gnu.org:bug#72141; Package emacs.
(Sat, 10 Aug 2024 17:14:01 GMT) Full text and rfc822 format available.Message #35 received at 72141 <at> debbugs.gnu.org (full text, mbox):
From: Thierry Volpiatto <thievol <at> posteo.net> To: Thierry Volpiatto <thievol <at> posteo.net> Cc: Philip Kaludercic <philipk <at> posteo.net>, Eli Zaretskii <eliz <at> gnu.org>, 72141 <at> debbugs.gnu.org Subject: Re: bug#72141: 29.4; package-upgrade vs package-load-list Date: Sat, 10 Aug 2024 17:16:54 +0000
[Message part 1 (text/plain, inline)]
Thierry Volpiatto <thievol <at> posteo.net> writes:
> Hello Philip,
>
> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> Gladly, then I'd like to try it out it and perhaps write a ERT test.
>
> Patch attached, please review and test it before merging ;-)
>
> Thanks.
Also, I recently had to read package.el code and I found many loops are
too complex and/or too difficult to read, here are some:
--8<---------------cut here---------------start------------->8---
(equal
;; old
(mapcar #'symbol-name (mapcar #'car package-alist))
;; new
(mapcar (lambda (pkg) (symbol-name (car pkg))) package-alist))
(equal
;; old
(mapcar
(lambda (p) (cons (package-desc-full-name p) p))
(delq nil
(mapcar (lambda (p) (unless (package-built-in-p p) p))
(apply #'append (mapcar #'cdr (package--alist))))))
;; new
(cl-loop for (p desc) in package-alist
unless (package-built-in-p p)
collect (cons (package-desc-full-name desc) desc)))
;; Change w3m to an installed package in your Emacs.
(let ((alist (package-desc-extras (cadr (assq 'w3m package-alist)))))
(equal
;; old
(mapcar #'macroexp-quote
(apply #'nconc
(mapcar (lambda (pair) (list (car pair) (cdr pair))) alist)))
;; new
(cl-loop for lst in alist
nconc `(,(car lst) ,(macroexp-quote (cdr lst))))))
(equal
;; old
(mapcar #'file-truename
(cl-remove-if-not #'stringp
(mapcar #'car load-history)))
;; new
(cl-loop for (name _rest) in load-history
when (stringp name)
collect (file-truename name)))
--8<---------------cut here---------------end--------------->8---
I have a patch for this, let me know if interested, or perhaps I should
open a new bug report ?
--
Thierry
[signature.asc (application/pgp-signature, inline)]
bug-gnu-emacs <at> gnu.org:bug#72141; Package emacs.
(Mon, 12 Aug 2024 16:38:02 GMT) Full text and rfc822 format available.Message #38 received at 72141 <at> debbugs.gnu.org (full text, mbox):
From: Philip Kaludercic <philipk <at> posteo.net> To: Thierry Volpiatto <thievol <at> posteo.net> Cc: Eli Zaretskii <eliz <at> gnu.org>, 72141 <at> debbugs.gnu.org Subject: Re: bug#72141: 29.4; package-upgrade vs package-load-list Date: Mon, 12 Aug 2024 16:36:43 +0000
Thierry Volpiatto <thievol <at> posteo.net> writes:
> Hello Philip,
>
> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> Gladly, then I'd like to try it out it and perhaps write a ERT test.
>
> Patch attached, please review and test it before merging ;-)
Sorry for the delay.
> Thanks.
>
> --
> Thierry
>
> From 292e251a383c1fb53cc377cd32f71705e6742f85 Mon Sep 17 00:00:00 2001
> From: Thierry Volpiatto <thievol <at> posteo.net>
> Date: Sat, 3 Aug 2024 06:07:28 +0200
> Subject: [PATCH] Fix bug#72141, package-upgrade should not include disabled
> packages
>
> * lisp/emacs-lisp/package.el (package--upgradeable-packages):
> Rewrite with a new optional arg to filter out disabled packages from
> output.
> (package-upgrade, package-upgrade-all): Use it and filter out built-in
> packages from completion according package-install-upgrade-built-in
> value.
> ---
> lisp/emacs-lisp/package.el | 60 ++++++++++++++++++++++----------------
> 1 file changed, 35 insertions(+), 25 deletions(-)
>
> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index 7cae8d68bc0..83996c9d6de 100644
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -2259,11 +2259,15 @@ had been enabled."
> "Upgrade package NAME if a newer version exists."
> (interactive
> (list (completing-read
> - "Upgrade package: " (package--upgradeable-packages t) nil t)))
> + "Upgrade package: " (package--upgradeable-packages
> + package-install-upgrade-built-in
> + 'ignore-disabled)
> + nil t)))
> (let* ((package (if (symbolp name)
> name
> (intern name)))
> (pkg-desc (cadr (assq package package-alist)))
> + ;; Keep this binding for non-interactive use.
> (package-install-upgrade-built-in (not pkg-desc)))
> ;; `pkg-desc' will be nil when the package is an "active built-in".
> (if (and pkg-desc (package-vc-p pkg-desc))
> @@ -2275,32 +2279,37 @@ had been enabled."
> ;; before. Mark it as installed explicitly.
> (and pkg-desc 'dont-select)))))
>
> -(defun package--upgradeable-packages (&optional include-builtins)
> +(defun package--upgradeable-packages (&optional
> + include-builtins ignore-disabled)
> ;; Initialize the package system to get the list of package
> ;; symbols for completion.
> (package--archives-initialize)
> - (mapcar
> - #'car
> - (seq-filter
> - (lambda (elt)
> - (or (let ((available
> - (assq (car elt) package-archive-contents)))
> - (and available
> - (or (and
> - include-builtins
> - (not (package-desc-version (cadr elt))))
> - (version-list-<
> - (package-desc-version (cadr elt))
> - (package-desc-version (cadr available))))))
> - (package-vc-p (cadr elt))))
> - (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))))
> + (let ((pkgs (if include-builtins
> + (append package-alist
> + (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 =
> + (if-let ((av (assq sym package-archive-contents)))
> + (if ignore-disabled
> + (and (not (package-disabled-p sym cversion)) av) av))
^
This loop really confused me. Specifically here
If I am not mistaken, cversion might be set by the last iteration, no?
In that case, we are passing some version string unrelated to sym?
I am leaning towards keeping the existing loop, i.e. not rewriting
anything while fixing a bug. It is probably easier for me to do that.
Do you have any comments on tricks or traps that I should keep in mind?
> + for cversion = (and available (package-desc-version desc))
> + when (or (and available
> + (or (and include-builtins (not cversion))
> + (and cversion
> + (version-list-<
> + cversion
> + (package-desc-version (cadr available))))))
> + (package-vc-p desc))
> + collect sym)))
>
> ;;;###autoload
> (defun package-upgrade-all (&optional query)
> @@ -2315,7 +2324,8 @@ from ELPA by either using `\\[package-upgrade]' or
> `\\<package-menu-mode-map>\\[package-menu-mark-install]' after `\\[list-packages]'."
> (interactive (list (not noninteractive)))
> (package-refresh-contents)
> - (let ((upgradeable (package--upgradeable-packages)))
> + (let ((upgradeable (package--upgradeable-packages
> + package-install-upgrade-built-in 'ignore-disabled)))
> (if (not upgradeable)
> (message "No packages to upgrade")
> (when (and query
Thierry Volpiatto <thievol <at> posteo.net> writes:
> Thierry Volpiatto <thievol <at> posteo.net> writes:
>
>> Hello Philip,
>>
>> Philip Kaludercic <philipk <at> posteo.net> writes:
>>
>>> Gladly, then I'd like to try it out it and perhaps write a ERT test.
>>
>> Patch attached, please review and test it before merging ;-)
>>
>> Thanks.
>
> Also, I recently had to read package.el code and I found many loops are
> too complex and/or too difficult to read, here are some:
>
[...]
>
> I have a patch for this, let me know if interested, or perhaps I should
> open a new bug report ?
I have some spare time now, and if I also find some spare energy, I plan
to clean up a number of things in package.el, mainly removing duplicate
logic and making the code more flexible. I'll start work on a separate
branch, and that's why I am not too enthusiastic about these kinds of
changes on master (for now).
--
Philip Kaludercic on peregrine
bug-gnu-emacs <at> gnu.org:bug#72141; Package emacs.
(Mon, 12 Aug 2024 17:34:02 GMT) Full text and rfc822 format available.Message #41 received at 72141 <at> debbugs.gnu.org (full text, mbox):
From: Thierry Volpiatto <thievol <at> posteo.net> To: Philip Kaludercic <philipk <at> posteo.net> Cc: Thierry Volpiatto <thievol <at> posteo.net>, Eli Zaretskii <eliz <at> gnu.org>, 72141 <at> debbugs.gnu.org Subject: Re: bug#72141: 29.4; package-upgrade vs package-load-list Date: Mon, 12 Aug 2024 17:36:21 +0000
[Message part 1 (text/plain, inline)]
Philip Kaludercic <philipk <at> posteo.net> writes: > Thierry Volpiatto <thievol <at> posteo.net> writes: > >> Hello Philip, >> >> Philip Kaludercic <philipk <at> posteo.net> writes: >> >>> Gladly, then I'd like to try it out it and perhaps write a ERT test. >> >> Patch attached, please review and test it before merging ;-) > > Sorry for the delay. > >> Thanks. >> >> -- >> Thierry >> >> From 292e251a383c1fb53cc377cd32f71705e6742f85 Mon Sep 17 00:00:00 2001 >> From: Thierry Volpiatto <thievol <at> posteo.net> >> Date: Sat, 3 Aug 2024 06:07:28 +0200 >> Subject: [PATCH] Fix bug#72141, package-upgrade should not include disabled >> packages >> >> * lisp/emacs-lisp/package.el (package--upgradeable-packages): >> Rewrite with a new optional arg to filter out disabled packages from >> output. >> (package-upgrade, package-upgrade-all): Use it and filter out built-in >> packages from completion according package-install-upgrade-built-in >> value. >> --- >> lisp/emacs-lisp/package.el | 60 ++++++++++++++++++++++---------------- >> 1 file changed, 35 insertions(+), 25 deletions(-) >> >> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el >> index 7cae8d68bc0..83996c9d6de 100644 >> --- a/lisp/emacs-lisp/package.el >> +++ b/lisp/emacs-lisp/package.el >> @@ -2259,11 +2259,15 @@ had been enabled." >> "Upgrade package NAME if a newer version exists." >> (interactive >> (list (completing-read >> - "Upgrade package: " (package--upgradeable-packages t) nil t))) >> + "Upgrade package: " (package--upgradeable-packages >> + package-install-upgrade-built-in >> + 'ignore-disabled) >> + nil t))) >> (let* ((package (if (symbolp name) >> name >> (intern name))) >> (pkg-desc (cadr (assq package package-alist))) >> + ;; Keep this binding for non-interactive use. >> (package-install-upgrade-built-in (not pkg-desc))) >> ;; `pkg-desc' will be nil when the package is an "active built-in". >> (if (and pkg-desc (package-vc-p pkg-desc)) >> @@ -2275,32 +2279,37 @@ had been enabled." >> ;; before. Mark it as installed explicitly. >> (and pkg-desc 'dont-select))))) >> >> -(defun package--upgradeable-packages (&optional include-builtins) >> +(defun package--upgradeable-packages (&optional >> + include-builtins ignore-disabled) >> ;; Initialize the package system to get the list of package >> ;; symbols for completion. >> (package--archives-initialize) >> - (mapcar >> - #'car >> - (seq-filter >> - (lambda (elt) >> - (or (let ((available >> - (assq (car elt) package-archive-contents))) >> - (and available >> - (or (and >> - include-builtins >> - (not (package-desc-version (cadr elt)))) >> - (version-list-< >> - (package-desc-version (cadr elt)) >> - (package-desc-version (cadr available)))))) >> - (package-vc-p (cadr elt)))) >> - (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)))) >> + (let ((pkgs (if include-builtins >> + (append package-alist >> + (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 = >> + (if-let ((av (assq sym package-archive-contents))) >> + (if ignore-disabled >> + (and (not (package-disabled-p sym cversion)) av) av)) > ^ > This loop really confused me. Specifically here > If I am not mistaken, cversion might be set by the last iteration, no? > In that case, we are passing some version string unrelated to sym? Yes, indeed, this is an error, thanks to catch it. > I am leaning towards keeping the existing loop, i.e. not rewriting > anything while fixing a bug. It is probably easier for me to do that. No problem, it is probably better to not introduce a new bug for now. > Do you have any comments on tricks or traps that I should keep in > mind? Yes the other changes are about built-in packages that are shown in completion, and refused later by package-install. >> (defun package-upgrade-all (&optional query) >> @@ -2315,7 +2324,8 @@ from ELPA by either using `\\[package-upgrade]' or >> `\\<package-menu-mode-map>\\[package-menu-mark-install]' after `\\[list-packages]'." >> (interactive (list (not noninteractive))) >> (package-refresh-contents) >> - (let ((upgradeable (package--upgradeable-packages))) >> + (let ((upgradeable (package--upgradeable-packages >> + package-install-upgrade-built-in Here. >> I have a patch for this, let me know if interested, or perhaps I should >> open a new bug report ? > > I have some spare time now, and if I also find some spare energy, I plan > to clean up a number of things in package.el, mainly removing duplicate > logic and making the code more flexible. I'll start work on a separate > branch, and that's why I am not too enthusiastic about these kinds of > changes on master (for now). Great, no problem, as long as it is cleaned up that's good ;-) Thanks. -- Thierry
[signature.asc (application/pgp-signature, inline)]
bug-gnu-emacs <at> gnu.org:bug#72141; Package emacs.
(Mon, 12 Aug 2024 18:14:02 GMT) Full text and rfc822 format available.Message #44 received at 72141 <at> debbugs.gnu.org (full text, mbox):
From: Thierry Volpiatto <thievol <at> posteo.net> To: Philip Kaludercic <philipk <at> posteo.net> Cc: Thierry Volpiatto <thievol <at> posteo.net>, Eli Zaretskii <eliz <at> gnu.org>, 72141 <at> debbugs.gnu.org Subject: Re: bug#72141: 29.4; package-upgrade vs package-load-list Date: Mon, 12 Aug 2024 18:16:29 +0000
[Message part 1 (text/plain, inline)]
Philip Kaludercic <philipk <at> posteo.net> writes:
> This loop really confused me. Specifically here
> If I am not mistaken, cversion might be set by the last iteration, no?
> In that case, we are passing some version string unrelated to sym?
For the record:
[...]
(cl-loop for (sym desc) in pkgs
for pkg = (assq sym package-archive-contents)
for cversion = (and pkg (package-desc-version desc))
for available = (when pkg
(if ignore-disabled
(and (not (package-disabled-p
sym cversion))
pkg)
pkg))
[...]
--
Thierry
[signature.asc (application/pgp-signature, inline)]
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.