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

From: Dmitry Gutov <[email-masked]>
Date: Tue, 25 Apr 2023 02:45:46 +0300

On 24/04/2023 14:58, Eli Zaretskii wrote:
>> Date: Mon, 24 Apr 2023 00:53:30 +0300
>> Cc: [email-masked], [email-masked], [email-masked],
>>   [email-masked], [email-masked], [email-masked]
>> From: Dmitry Gutov <[email-masked]>
>>
>> On 23/04/2023 17:24, Eli Zaretskii wrote:
>>>> Date: Sun, 23 Apr 2023 16:11:44 +0300
>>>> Cc: [email-masked], [email-masked], [email-masked],
>>>>    [email-masked], [email-masked], [email-masked]
>>>> From: Dmitry Gutov <[email-masked]>
>>>>
>>>>>> It addressed two last points from your previous email, obviously.
>>>>> Which points are those?  Please help me identify them.
>>>>
>>>> These two:
>>>>
>>>>    > There is already such an option, added as part of fixing this bug.
>>>>
>>>> The binding was added, so now we straight away delegate to package-install.
>>>
>>> I meant to give the user the control on whether package-update will
>>> update built-in packages, like we did with package-install: either via
>>> prefix argument or by customizing the user option.
>>
>> That would be a different change, though.
>>
>> So what are we guarding against here? That the user will choose a
>> built-in package to upgrade by accident?
> 
> Yes.  Also, against invocations of this command from other commands
> and from the menu.

It's not in the menu. There are also no known callers aside from 
package-update-all.

>> And we'll show her an error, saying "use the prefix argument"?
> 
> No, I think we'll do whatever the code does today when passed a
> built-in package.

Very well, here's the next version. It adds a new optional argument to 
the function (so that people can evaluate e.g. (package-update 'eglot 
t)). When called interactively, it is determined by current-prefix-argument.

Also please review the docstring change.

Regarding obeying package-install-upgrade-built-in, I think it would 
need to be renamed, and both package-update-all and 
package-menu-mark-upgrades would need to be made obey it too. All that 
could be done in a subsequent change.

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index ffa6272dd1f..b72bf86c773 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -2266,21 +2266,39 @@ package-install
 (declare-function package-vc-update "package-vc" (pkg))
 
 ;;;###autoload
-(defun package-update (name)
-  "Update package NAME if a newer version exists."
+(defun package-update (name &optional update-built-ins)
+  "Update package NAME if a newer version exists.
+
+Only packages installed from ELPA are allowed to be updated this
+way.
+
+But if UPDATE-BUILT-INS is non-nil, or if the command is invoked
+interactively with a prefix argument, it will allow upgrading of
+active built-in packages."
   (interactive
    (list (completing-read
-          "Update package: " (package--updateable-packages) nil t)))
+          "Update package: " (package--updateable-packages
+                              current-prefix-arg)
+          nil t)
+         current-prefix-arg))
   (let* ((package (if (symbolp name)
                       name
                     (intern name)))
-         (pkg-desc (cadr (assq package package-alist))))
-    (if (package-vc-p pkg-desc)
+         (pkg-desc (cadr (assq package package-alist)))
+         (package-install-upgrade-built-in (and
+                                            update-built-ins
+                                            (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))
         (package-vc-update pkg-desc)
-      (package-delete pkg-desc 'force)
-      (package-install package 'dont-select))))
-
-(defun package--updateable-packages ()
+      (when pkg-desc
+        (package-delete pkg-desc 'force 'nosave))
+      (package-install package
+                       ;; An active built-in has never been "selected"
+                       ;; before.  Mark it as installed explicitly.
+                       (and pkg-desc 'dont-select)))))
+
+(defun package--updateable-packages (&optional allow-builtins)
   ;; Initialize the package system to get the list of package
   ;; symbols for completion.
   (package--archives-initialize)
@@ -2291,11 +2309,21 @@ package--updateable-packages
       (or (let ((available
                  (assq (car elt) package-archive-contents)))
             (and available
-                 (version-list-<
-                  (package-desc-version (cadr elt))
-                  (package-desc-version (cadr available)))))
-          (package-vc-p (cadr (assq (car elt) package-alist)))))
-    package-alist)))
+                 (or (and
+                      allow-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 allow-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))))
 
 ;;;###autoload
 (defun package-update-all (&optional query)