* bug#72141: 29.4; package-upgrade vs package-load-list @ 2024-07-16 14:46 Thierry Volpiatto 2024-07-27 7:14 ` Eli Zaretskii 0 siblings, 1 reply; 14+ messages in thread From: Thierry Volpiatto @ 2024-07-16 14:46 UTC (permalink / raw) To: 72141 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#72141: 29.4; package-upgrade vs package-load-list 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 0 siblings, 1 reply; 14+ messages in thread From: Eli Zaretskii @ 2024-07-27 7:14 UTC (permalink / raw) To: Thierry Volpiatto, Philip Kaludercic; +Cc: 72141 > 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? ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#72141: 29.4; package-upgrade vs package-load-list 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 0 siblings, 2 replies; 14+ messages in thread From: Philip Kaludercic @ 2024-07-28 11:47 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Thierry Volpiatto, 72141 [-- Attachment #1: Type: text/plain, Size: 1267 bytes --] 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: [-- Attachment #2: Type: text/plain, Size: 596 bytes --] 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)))) [-- Attachment #3: Type: text/plain, Size: 37 bytes --] -- Philip Kaludercic on peregrine ^ permalink raw reply related [flat|nested] 14+ messages in thread
* bug#72141: 29.4; package-upgrade vs package-load-list 2024-07-28 11:47 ` Philip Kaludercic @ 2024-07-28 12:27 ` Thierry Volpiatto 2024-07-28 12:27 ` Eli Zaretskii 1 sibling, 0 replies; 14+ messages in thread From: Thierry Volpiatto @ 2024-07-28 12:27 UTC (permalink / raw) To: Philip Kaludercic; +Cc: Thierry Volpiatto, Eli Zaretskii, 72141 [-- 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 --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#72141: 29.4; package-upgrade vs package-load-list 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 1 sibling, 1 reply; 14+ messages in thread From: Eli Zaretskii @ 2024-07-28 12:27 UTC (permalink / raw) To: Philip Kaludercic; +Cc: thievol, 72141 > 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#72141: 29.4; package-upgrade vs package-load-list 2024-07-28 12:27 ` Eli Zaretskii @ 2024-07-28 12:39 ` Thierry Volpiatto 2024-08-01 6:48 ` Thierry Volpiatto 0 siblings, 1 reply; 14+ messages in thread From: Thierry Volpiatto @ 2024-07-28 12:39 UTC (permalink / raw) To: Eli Zaretskii; +Cc: thievol, Philip Kaludercic, 72141 [-- Attachment #1: Type: text/plain, Size: 3372 bytes --] 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). -- Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 686 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#72141: 29.4; package-upgrade vs package-load-list 2024-07-28 12:39 ` Thierry Volpiatto @ 2024-08-01 6:48 ` Thierry Volpiatto 2024-08-03 3:22 ` Thierry Volpiatto 0 siblings, 1 reply; 14+ messages in thread From: Thierry Volpiatto @ 2024-08-01 6:48 UTC (permalink / raw) To: Thierry Volpiatto; +Cc: Eli Zaretskii, Philip Kaludercic, 72141 [-- Attachment #1: Type: text/plain, Size: 5338 bytes --] 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! -- Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 686 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#72141: 29.4; package-upgrade vs package-load-list 2024-08-01 6:48 ` Thierry Volpiatto @ 2024-08-03 3:22 ` Thierry Volpiatto 2024-08-04 14:57 ` Philip Kaludercic 0 siblings, 1 reply; 14+ messages in thread From: Thierry Volpiatto @ 2024-08-03 3:22 UTC (permalink / raw) To: Thierry Volpiatto; +Cc: Eli Zaretskii, Philip Kaludercic, 72141 [-- Attachment #1: Type: text/plain, Size: 5829 bytes --] 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. Thanks. https://github.com/emacs-helm/helm/blob/master/helm-packages.el#L266 -- Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 686 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#72141: 29.4; package-upgrade vs package-load-list 2024-08-03 3:22 ` Thierry Volpiatto @ 2024-08-04 14:57 ` Philip Kaludercic 2024-08-04 17:15 ` Thierry Volpiatto 0 siblings, 1 reply; 14+ messages in thread From: Philip Kaludercic @ 2024-08-04 14:57 UTC (permalink / raw) To: Thierry Volpiatto; +Cc: Eli Zaretskii, 72141 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#72141: 29.4; package-upgrade vs package-load-list 2024-08-04 14:57 ` Philip Kaludercic @ 2024-08-04 17:15 ` Thierry Volpiatto 2024-08-10 17:16 ` Thierry Volpiatto 2024-08-12 16:36 ` Philip Kaludercic 0 siblings, 2 replies; 14+ messages in thread From: Thierry Volpiatto @ 2024-08-04 17:15 UTC (permalink / raw) To: Philip Kaludercic; +Cc: Thierry Volpiatto, Eli Zaretskii, 72141 [-- Attachment #1.1: Type: text/plain, Size: 232 bytes --] Hello Philip, Philip Kaludercic <philipk@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 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1.2: 0001-Fix-bug-72141-package-upgrade-should-not-include-dis.patch --] [-- Type: text/x-diff, Size: 4983 bytes --] From 292e251a383c1fb53cc377cd32f71705e6742f85 Mon Sep 17 00:00:00 2001 From: Thierry Volpiatto <thievol@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)) + 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 -- 2.34.1 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 686 bytes --] ^ permalink raw reply related [flat|nested] 14+ messages in thread
* bug#72141: 29.4; package-upgrade vs package-load-list 2024-08-04 17:15 ` Thierry Volpiatto @ 2024-08-10 17:16 ` Thierry Volpiatto 2024-08-12 16:36 ` Philip Kaludercic 1 sibling, 0 replies; 14+ messages in thread From: Thierry Volpiatto @ 2024-08-10 17:16 UTC (permalink / raw) To: Thierry Volpiatto; +Cc: Philip Kaludercic, Eli Zaretskii, 72141 [-- Attachment #1: Type: text/plain, Size: 1845 bytes --] Thierry Volpiatto <thievol@posteo.net> writes: > Hello Philip, > > Philip Kaludercic <philipk@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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 686 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#72141: 29.4; package-upgrade vs package-load-list 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 1 sibling, 2 replies; 14+ messages in thread From: Philip Kaludercic @ 2024-08-12 16:36 UTC (permalink / raw) To: Thierry Volpiatto; +Cc: Eli Zaretskii, 72141 Thierry Volpiatto <thievol@posteo.net> writes: > Hello Philip, > > Philip Kaludercic <philipk@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@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@posteo.net> writes: > Thierry Volpiatto <thievol@posteo.net> writes: > >> Hello Philip, >> >> Philip Kaludercic <philipk@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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#72141: 29.4; package-upgrade vs package-load-list 2024-08-12 16:36 ` Philip Kaludercic @ 2024-08-12 17:36 ` Thierry Volpiatto 2024-08-12 18:16 ` Thierry Volpiatto 1 sibling, 0 replies; 14+ messages in thread From: Thierry Volpiatto @ 2024-08-12 17:36 UTC (permalink / raw) To: Philip Kaludercic; +Cc: Thierry Volpiatto, Eli Zaretskii, 72141 [-- Attachment #1: Type: text/plain, Size: 6304 bytes --] Philip Kaludercic <philipk@posteo.net> writes: > Thierry Volpiatto <thievol@posteo.net> writes: > >> Hello Philip, >> >> Philip Kaludercic <philipk@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@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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 686 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#72141: 29.4; package-upgrade vs package-load-list 2024-08-12 16:36 ` Philip Kaludercic 2024-08-12 17:36 ` Thierry Volpiatto @ 2024-08-12 18:16 ` Thierry Volpiatto 1 sibling, 0 replies; 14+ messages in thread From: Thierry Volpiatto @ 2024-08-12 18:16 UTC (permalink / raw) To: Philip Kaludercic; +Cc: Thierry Volpiatto, Eli Zaretskii, 72141 [-- Attachment #1: Type: text/plain, Size: 791 bytes --] Philip Kaludercic <philipk@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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 686 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-08-12 18:16 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 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 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
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).