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

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: João Távora <joaotavora@gmail.com>
>> Date: Wed, 12 Apr 2023 12:15:11 +0100
>> Cc: philipk@posteo.net, monnier@iro.umontreal.ca, 62720@debbugs.gnu.org, 
>>      larsi@gnus.org
>> 
>> > This has the same problem: it modifies a function that is called in
>> > too many places.  package-installed-p has half a dozen callers in
>> > package.el alone.  The change is tiny, but what about its
>> > implications on every use case where it is involved?
>> 
>> What about them?  I volunteer to help test whichever cases you (or
>> anyone else) think may be problematic.
>
> We will never be able to reveal them all, let alone test them, before
> Emacs 29.1 is released.
>
>> Can we put this tiny patch on master then?
>
> Yes, if Philip and Stefan don't object.  But, since there will be a
> command for updating core packages, doesn't this go against your
> desire not to change the UX?

After thinking about this for a bit, I think that the right approach is
to use package-install instead of writing a separate command.  After
all, this will make the behaviour of package-install consistent with
that of the package menu.

It might work but it should be tested somewhat thoroughly before the
patch is applied.  In the meantime, I just finished a similar approach
that does not modify `package-installed-p', but just adds another
utility function:

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index f92afe56b76..5a79c277f0d 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -797,6 +797,14 @@ package-built-in-p
         (require 'finder-inf nil t) ; For `package--builtins'.
         (assq package package--builtins))))))
 
+(defun package-core-p (package)
+  "Return non-nil the built-in version of PACKAGE is loaded."
+  (let ((package (if (package-desc-p package)
+                     (package-desc-name package)
+                   package)))
+    (and (assq package (package--alist))
+         (package-built-in-p package))))
+
 (defun package--autoloads-file-name (pkg-desc)
   "Return the absolute name of the autoloads file, sans extension.
 PKG-DESC is a `package-desc' object."
@@ -1908,7 +1916,7 @@ package-compute-transaction
                  (package-version-join (package-desc-version already)))))
       (cond
        (already nil)
-       ((package-installed-p next-pkg next-version) nil)
+       ((package-core-p (car next-pkg))) ;done
 
        (t
         ;; A package is required, but not installed.  It might also be
@@ -2205,11 +2213,9 @@ package-install
      (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))
+                    (seq-filter
+                     (lambda (elt) (package-core-p (car elt)))
+                     package-archive-contents)
                     nil t))
            nil)))
   (package--archives-initialize)

>> Maybe there's half a chance for a backport until the Emacs 29.1
>> release, or a quarter of a chance that some future Emacs 29.2 will
>> also get it.
>
> I don't think there is such a chance, and I'm not sure we even want
> that, given that we are leaning towards a new special command.