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
View this message in rfc822 format
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: 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)]
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.