Subject: Re: bug#62720: 29.0.60; Not easy at all to upgrade :core packages like Eglot

From: João Távora <[email-masked]>
Date: Wed, 12 Apr 2023 17:13:27 +0100

On Wed, Apr 12, 2023 at 4:17 PM Eli Zaretskii <[email-masked]> wrote:

> The above questions and undocumented subtleties is what scares me in
> installing such changes at this late stage.  I'm not sure everyone
> involved, yourself included, have a clear understanding of what the
> modified code will do in each possible use case.  That is why I very
> much prefer separate code, which will then free us from the need of
> considering all these subtleties, as the last year of user's
> experience with this code can vouch that it does its job correctly, by
> and large.

Alright, I've tried my hand at making this clean separation, so that
no logic of transaction or existing predicates is touched.  I tried to
make it as intelligible as possible perhaps overdoing the commentary
and the naming, but we can always trim it.

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 685f983e285..51d633131d9 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -2178,6 +2178,24 @@ package--archives-initialize
   (unless package-archive-contents
     (package-refresh-contents)))

+(defun package--vestal-builtins-available-for-installation ()
+  "Return built-in packages that can but have never been updated.
+The return value is a subset of `package-archive-contents', i.e.
+a list ((SYM PACKAGE-DESC) ...) where SYM names such a built-in
+package.  For bug#62720.
+
+Because we reject packages that are in `package--alist', this set
+is guaranteed to _not_ intersect with the subset of
+`package-archive-contents', which verifies `package-installed-p'.
+which, for historical reasons (and in suspicious conditions) says
+that built-in packages as installed."
+  (let (res)
+    (dolist (bi package--builtins)
+      (when-let ((probe (and (not (assq (car bi) (package--alist)))
+                             (assq (car bi) package-archive-contents))))
+        (push probe res)))
+    res))
+
 ;;;###autoload
 (defun package-install (pkg &optional dont-select)
   "Install the package PKG.
@@ -2196,19 +2214,27 @@ package-install
 to install it but still mark it as selected."
   (interactive
    (progn
-     ;; Initialize the package system to get the list of package
-     ;; symbols for completion.
      (package--archives-initialize)
      (list (intern (completing-read
                     "Install package: "
-                    (delq nil
-                          (mapcar (lambda (elt)
-                                    (unless (package-installed-p (car elt))
-                                      (symbol-name (car elt))))
-                                  package-archive-contents))
+                    (append
+                     (delq nil
+                           (mapcar (lambda (elt)
+                                     (unless (package-installed-p (car elt))
+                                       (symbol-name (car elt))))
+                                   package-archive-contents))
+                     (mapcar #'car
+
(package--vestal-builtins-available-for-installation)))
                     nil t))
            nil)))
   (package--archives-initialize)
+  ;; See docstring and code of
+  ;; `package--vestal-builtins-available-for-installation' to
+  ;; understand how this code can only kick in if indeed the user
+  ;; selected a such a package from the completion list. bug#62720.
+  (when-let ((name-and-desc
+              (assq pkg
(package--vestal-builtins-available-for-installation))))
+    (setq pkg (cadr name-and-desc)))
   (add-hook 'post-command-hook #'package-menu--post-refresh)
   (let ((name (if (package-desc-p pkg)
                   (package-desc-name pkg)