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

[Philip Kaludercic <philipk@posteo.net>]

[-- Attachment #1: Type: text/plain, Size: 2018 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: joaotavora@gmail.com,  monnier@iro.umontreal.ca,  62720@debbugs.gnu.org,
>>   larsi@gnus.org
>> Date: Thu, 13 Apr 2023 17:49:00 +0000
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Adding an option is fine by me, as long as its default preserves
>> > previous behavior.
>> >
>> > Just to be sure we are on the same page: you suggest _both_ prefix
>> > argument and user option, where user option could be used to avoid the
>> > need for prefix argument?
>> 
>> I was thinking about both, but I supposed that a user option would be
>> enough.
>
> No, I think having both is better.  It is easier to say "C-u" than to
> change the value of an option, so for one-off update of a single
> package, the prefix argument is more convenient.
>
>> >> No, my proposed diff changes what package decides to download (the
>> >> planning phase), but doesn't touch anything after that.  The current
>> >> state is that (package-install 'eglot) just prints
>> >> 
>> >>   ‘eglot’ is already installed
>> >
>> > And does nothing else?  You seem to be saying it still downloads
>> > something, but what is that?
>> 
>> No, it just prints that message but doesn't download anything.
>
> OK, then allowing to install such packages under the proposed changes
> will improve that case as well.

After having added the user option, I am not sure about the prefix
argument.  I see this as a temporary fix due to the time constraints of
releasing Emacs 29.  It is disabled for now, but can be enabled on
master to see if there are any problems.

But for now, this patch supports both the user option and the prefix
argument.  I am still not satisfied with the documentation, but cannot
come up with a better phrase than

  installing potentially newer versions of built-in packages from
  package archives

for explaining the issue without getting too technical.  Do you have any
ideas?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Allow-upgrading-built-in-packages-with-package-insta.patch --]
[-- Type: text/x-diff, Size: 4855 bytes --]

From 99e92d78560ef1aec7217d90ed3a8108bdf2924c Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Thu, 13 Apr 2023 20:13:59 +0200
Subject: [PATCH] Allow upgrading built-in packages with 'package-install'

* etc/NEWS: Mention the change
* lisp/emacs-lisp/package.el (package--upgradable-built-in-p): Add new
predicate.
(package-install-upgrade-built-in): Add new user option to enable
feature.
(package-install): Respect new user option.
---
 etc/NEWS                   |  5 +++++
 lisp/emacs-lisp/package.el | 44 +++++++++++++++++++++++++++++++-------
 2 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 84dbb94a71a..a7834cd0d2b 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1876,6 +1876,11 @@ package maintainers.
 By customizing this user option you can specify specific packages to
 install.
 
+---
+*** New user option 'package-install-upgrade-built-in'.
+When enabled, 'package-install' can be used to install potentially
+newer versions of built-in packages.
+
 ** Emacs Sessions (Desktop)
 
 +++
diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index f92afe56b76..882db8db719 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -797,6 +797,17 @@ package-built-in-p
         (require 'finder-inf nil t) ; For `package--builtins'.
         (assq package package--builtins))))))
 
+(defun package--active-built-in-p (package)
+  "Return non-nil if PACKAGE if the built-in version is used."
+  (and (not (assq (cond
+                   ((package-desc-p package)
+                    (package-desc-name package))
+                   ((stringp package) (intern package))
+                   ((symbolp package) package)
+                   ((error "Unknown package format: %S" 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."
@@ -2182,12 +2193,19 @@ package--archives-initialize
   (unless package-archive-contents
     (package-refresh-contents)))
 
+(defcustom package-install-upgrade-built-in nil
+  "Non-nil means that built-in packages can be upgraded via a package archive.
+If disabled, then `package-install' will not allow installing
+potentially newer versions of built-in packages from package
+archives."
+  :type 'boolean
+  :version "29.1")
+
 ;;;###autoload
 (defun package-install (pkg &optional dont-select)
   "Install the package PKG.
 PKG can be a `package-desc' or a symbol naming one of the
-available packages in an archive in `package-archives'.  When
-called interactively, prompt for the package name.
+available packages in an archive in `package-archives'.
 
 Mark the installed package as selected by adding it to
 `package-selected-packages'.
@@ -2197,7 +2215,11 @@ package-install
 `package-selected-packages'.
 
 If PKG is a `package-desc' and it is already installed, don't try
-to install it but still mark it as selected."
+to install it but still mark it as selected.
+
+If the command is invoked with a prefix argument, the upgrading
+of built-in packages will be possible, as if
+`package-install-upgrade-built-in' had been enabled."
   (interactive
    (progn
      ;; Initialize the package system to get the list of package
@@ -2205,11 +2227,14 @@ 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))
+                    (mapcan
+                     (lambda (elt)
+                       (and (or (and (or current-prefix-arg
+                                         package-install-upgrade-built-in)
+                                     (package--active-built-in-p (car elt)))
+                                (not (package-installed-p (car elt))))
+                            (list (symbol-name (car elt)))))
+                     package-archive-contents)
                     nil t))
            nil)))
   (package--archives-initialize)
@@ -2220,6 +2245,9 @@ package-install
     (unless (or dont-select (package--user-selected-p name))
       (package--save-selected-packages
        (cons name package-selected-packages)))
+    (when (and (or current-prefix-arg package-install-upgrade-built-in)
+               (package--active-built-in-p pkg))
+      (setq pkg (cadr (assq name package-archive-contents))))
     (if-let* ((transaction
                (if (package-desc-p pkg)
                    (unless (package-installed-p pkg)
-- 
2.39.2