Subject: bug#71356: use-package doesn't load org from elpa

From: Philip Kaludercic <[email-masked]>
Date: Mon, 10 Jun 2024 15:40:58 +0000

Eli Zaretskii <[email-masked]> writes:

>> From: Andrea Corallo <[email-masked]>
>> Cc: Eli Zaretskii <[email-masked]>,  [email-masked],  [email-masked]
>> Date: Mon, 10 Jun 2024 04:17:21 -0400
>> 
>> Philip Kaludercic <[email-masked]> writes:
>> 
>> > Eli Zaretskii <[email-masked]> writes:
>> >
>> >>> From: Philip Kaludercic <[email-masked]>
>> >>> Cc: Pedro Andres Aranda Gutierrez <[email-masked]>,  [email-masked],
>> >>>   [email-masked]
>> >>> Date: Thu, 06 Jun 2024 06:15:44 +0000
>> >>> 
>> >>> Sorry for the delayed response;  I don't think that has to be expected.
>> >>> While use-package can utilise package.el for package management, my
>> >>> impression is that it is at liberty to be more flexible/declarative.  
>> >>
>> >> Doesn't use-package utilize package.el already?
>> >>
>> >> If not, how does it handle installation and upgrades? by its own code?
>> >
>> > By default it uses package.el, but there is an option to change it.
>> >
>> >>> > Do you have package-install-upgrade-built-in set non-nil?  If not, can
>> >>> > you set it non-nil and try the recipe again?
>> >>> 
>> >>> I have tried it out myself, and it doesn't appear to do anything.  The
>> >>> issue looks like that `package-installed-p' doesn't respect
>> >>> package-install-upgrade-built-in or :pin.
>> >>
>> >> We should fix that, I think.  If package-install-upgrade-built-in is
>> >> non-nil, use-package should upgrade built-in packages.
>> >>
>> >>> > As for a feature request: what exactly is the feature requested here?
>> >>> > Are you saying that use-package should automatically upgrade built-in
>> >>> > packages?  If so, I don't think this will fly, since it would mean
>> >>> > inconsistencies with package-install.
>> >>> 
>> >>> IIUC the feature would be that if a use-package form has a
>> >>> 
>> >>>      :pin gnu
>> >>> 
>> >>> argument, then this is an indication that we want to install the package
>> >>> from GNU ELPA, disregarding the fact that Emacs already has a built-in
>> >>> version of the same package.  Sort of a package-local version of
>> >>> `package-install-upgrade-built-in'.
>> >>
>> >> I'm not sure.  People tend to copy/paste recipes from the Internet
>> >> without really understanding what they do.  I think a simple :pin
>> >> should not be sufficient, we need some specialized keyword (in
>> >> addition to supporting package-install-upgrade-built-in).
>> >
>> > To me :pin would make perfect sense, as it explicitly expresses what
>> > archive we want to follow for package upgrades.
>> 
>> +1, also use-package interface is very declarative and I'm not sure
>> having it influenced by a dynamic var would match user expected
>> behavior.
>
> If you prefer, we could add a new :foo keyword to mean this.  But
> unconditionally changing what :pin means in these cases is out of the
> question.

We wouldn't change what :pin means directly, but just have
package-install respect `package-pinned-packages'.  It seems that all we
have to change is this:

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index fda855d2143..562dc5dbca3 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -2173,7 +2173,8 @@ package-installed-p
             (version-list-<= min-version
                              (package-desc-version (car pkg-descs)))))
      ;; Also check built-in packages.
-     (package-built-in-p package min-version)))))
+     (and (not (package-install-upgrade-built-in-p package))
+          (package-built-in-p package min-version))))))
 
 (defun package-download-transaction (packages)
   "Download and install all the packages in PACKAGES.
@@ -2197,6 +2198,11 @@ package-install-upgrade-built-in
   :type 'boolean
   :version "29.1")
 
+(defun package-install-upgrade-built-in-p (pkg)
+  "Return non-nil if PKG should be upgraded."
+  (or (assq pkg package-pinned-packages)
+      package-install-upgrade-built-in))
+
 ;;;###autoload
 (defun package-install (pkg &optional dont-select)
   "Install the package PKG.
@@ -2226,7 +2232,7 @@ package-install
                     (mapcan
                      (lambda (elt)
                        (and (or (and (or current-prefix-arg
-                                         package-install-upgrade-built-in)
+                                         (package-install-upgrade-built-in-p elt))
                                      (package--active-built-in-p (car elt)))
                                 (not (package-installed-p (car elt))))
                             (list (symbol-name (car elt)))))
@@ -2241,7 +2247,7 @@ 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)
+    (when (and (or current-prefix-arg (package-install-upgrade-built-in-p name))
                (package--active-built-in-p pkg))
       (setq pkg (or (cadr (assq name package-archive-contents)) pkg)))
     (if-let* ((transaction

(not thoroughly tested, just a sketch that makes (use-package org
:ensure t :pin gnu) work)

-- 
	Philip Kaludercic on peregrine