bug#72453: 29.4; "use-package" macro does not load all expected related files

[Stefan Kangas <stefankangas@gmail.com>]

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

Derek Upham via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs@gnu.org> writes:

>   emacs --init-directory use-package-ensure-test
>
> The init file sets use-package-always-ensure to true, and we expect
> trace warnings to show up for each use-package invocation.  In fact, we
> only see them for the third and beyond.
>
>  Warning (emacs): use-package-ensure-trace files (t) nil
>  Warning (emacs): use-package-ensure-trace rx (t) nil
>  Warning (emacs): use-package-ensure-trace avoid (t) nil

Thanks for the bug report and the clear recipe to reproduce it.

I can reproduce this bug here also.

> Moving the "use-package" macro into the top-level "use-package" file
> should fix the problem.  The "use-package" file explicitly loads the
> other files, which sets up all of the defaults.

The attached less intrusive patch fixes it here.  Could you please test
that it fixes the problem for you?

The patch just moves the relevant definitions to use-package-core.el.
If Eli and Andrea see no problem with this approach, I think we could
install it on emacs-30, after the first pre-release, and otherwise on
master.

Unrelatedly, I also noticed that the defcustoms in use-package-core.el
are all missing :version tags.  I'll fix that on emacs-30 after the
first pre-release is out.

[-- Attachment #2: 0001-Fix-early-use-of-use-package-always-ensure.patch --]
[-- Type: text/x-patch, Size: 8471 bytes --]

From 00dc146e9b8e41c534c19f78986f8e944681aa07 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Sun, 18 Aug 2024 13:10:19 +0200
Subject: [PATCH] Fix early use of use-package-always-ensure

* lisp/use-package/use-package-core.el:
* lisp/use-package/use-package-delight.el:
* lisp/use-package/use-package-diminish.el:
* lisp/use-package/use-package-ensure.el: Move add-to-list for
use-package-keywords and use-package-defaults from here...
* lisp/use-package/use-package-core.el (use-package-ensure)
(use-package-keywords): ...to the default definition here.

* lisp/use-package/use-package-ensure.el
(use-package-ensure): Move this defgroup...
(use-package-always-ensure)
(use-package-always-pin, use-package-ensure-function): ...and
these defcustoms from here...
* lisp/use-package/use-package-core.el (use-package-ensure)
(use-package-always-ensure, use-package-always-pin)
(use-package-ensure-function): ...to here.  (Bug#72453)
---
 lisp/use-package/use-package-core.el     | 54 ++++++++++++++++++++++--
 lisp/use-package/use-package-delight.el  |  2 -
 lisp/use-package/use-package-diminish.el |  2 -
 lisp/use-package/use-package-ensure.el   | 48 ---------------------
 4 files changed, 51 insertions(+), 55 deletions(-)

diff --git a/lisp/use-package/use-package-core.el b/lisp/use-package/use-package-core.el
index 8c3241d5ee0..c42007a2d10 100644
--- a/lisp/use-package/use-package-core.el
+++ b/lisp/use-package/use-package-core.el
@@ -65,6 +65,13 @@ use-package
   :link '(custom-manual "(use-package) Top")
   :version "29.1")
 
+;; TODO: I don't think this package group is required.  Delete it.
+(defgroup use-package-ensure nil
+  "Support for :ensure and :pin keywords in `use-package' declarations."
+  :group 'use-package
+  :link '(custom-manual "(use-package) Installing packages")
+  :version "29.1")
+
 (defconst use-package-version "2.4.5"
   "This version of `use-package'.")
 
@@ -76,6 +83,10 @@ use-package-keywords
     :functions
     :preface
     :if :when :unless
+    :ensure
+    :pin
+    :delight
+    :diminish
     :vc
     :no-require
     :catch
@@ -114,7 +125,8 @@ use-package-keywords
 nothing at all to happen, even if the rest of the `use-package'
 declaration is incorrect."
   :type '(repeat symbol)
-  :group 'use-package)
+  :group 'use-package
+  :version "30.1")
 
 (defcustom use-package-deferring-keywords
   '(:bind-keymap
@@ -189,7 +201,12 @@ use-package-defaults
              (lambda (name args)
                (and use-package-always-demand
                     (not (plist-member args :defer))
-                    (not (plist-member args :demand))))))
+                    (not (plist-member args :demand)))))
+    (:ensure (list use-package-always-ensure)
+             (lambda (name args)
+               (and use-package-always-ensure
+                    (not (plist-member args :load-path)))))
+    (:pin use-package-always-pin use-package-always-pin))
   "Default values for specified `use-package' keywords.
 Each entry in the alist is a list of three elements:
 The first element is the `use-package' keyword.
@@ -214,7 +231,8 @@ use-package-defaults
           (list (symbol :tag "Keyword")
                 (choice :tag "Default value" sexp function)
                 (choice :tag "Enable if non-nil" sexp function)))
-  :group 'use-package)
+  :group 'use-package
+  :version "30.1")
 
 (defcustom use-package-merge-key-alist
   '((:if    . (lambda (new old) `(and ,new ,old)))
@@ -360,6 +378,36 @@ use-package-vc-prefer-newest
   :version "30.1"
   :group 'use-package)
 
+(defcustom use-package-always-ensure nil
+  "Treat every package as though it had specified using `:ensure SEXP'.
+See also `use-package-defaults', which uses this value."
+  :type 'sexp
+  :group 'use-package-ensure)
+
+(defcustom use-package-always-pin nil
+  "Treat every package as though it had specified using `:pin SYM'.
+See also `use-package-defaults', which uses this value."
+  :type 'symbol
+  :group 'use-package-ensure)
+
+(defcustom use-package-ensure-function 'use-package-ensure-elpa
+  "Function that ensures a package is installed.
+This function is called with three arguments: the name of the
+package declared in the `use-package' form; the arguments passed
+to all `:ensure' keywords (always a list, even if only one); and
+the current `state' plist created by previous handlers.
+
+Note that this function is called whenever `:ensure' is provided,
+even if it is nil.  It is up to the function to decide on the
+semantics of the various values for `:ensure'.
+
+This function should return non-nil if the package is installed.
+
+The default value uses package.el to install the package."
+  :type '(choice (const :tag "package.el" use-package-ensure-elpa)
+                 (function :tag "Custom"))
+  :group 'use-package-ensure)
+
 (defvar use-package-statistics (make-hash-table))
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
diff --git a/lisp/use-package/use-package-delight.el b/lisp/use-package/use-package-delight.el
index c458d263cf0..c67e2aa6320 100644
--- a/lisp/use-package/use-package-delight.el
+++ b/lisp/use-package/use-package-delight.el
@@ -81,8 +81,6 @@ use-package-handler/:delight
      `((if (fboundp 'delight)
            (delight '(,@args)))))))
 
-(add-to-list 'use-package-keywords :delight t)
-
 (provide 'use-package-delight)
 
 ;;; use-package-delight.el ends here
diff --git a/lisp/use-package/use-package-diminish.el b/lisp/use-package/use-package-diminish.el
index 79421a0e273..0be2ba688a0 100644
--- a/lisp/use-package/use-package-diminish.el
+++ b/lisp/use-package/use-package-diminish.el
@@ -70,8 +70,6 @@ use-package-handler/:diminish
              arg)
      body)))
 
-(add-to-list 'use-package-keywords :diminish t)
-
 (provide 'use-package-diminish)
 
 ;;; use-package-diminish.el ends here
diff --git a/lisp/use-package/use-package-ensure.el b/lisp/use-package/use-package-ensure.el
index 5f75b6b59ea..82ab3256ef0 100644
--- a/lisp/use-package/use-package-ensure.el
+++ b/lisp/use-package/use-package-ensure.el
@@ -32,46 +32,10 @@
 (require 'cl-lib)
 (require 'use-package-core)
 
-(defgroup use-package-ensure nil
-  "Support for :ensure and :pin keywords in `use-package' declarations."
-  :group 'use-package
-  :link '(custom-manual "(use-package) Installing packages")
-  :version "29.1")
-
 (eval-when-compile
   (declare-function package-installed-p "package")
   (declare-function package-read-all-archive-contents "package" ()))
 
-(defcustom use-package-always-ensure nil
-  "Treat every package as though it had specified using `:ensure SEXP'.
-See also `use-package-defaults', which uses this value."
-  :type 'sexp
-  :group 'use-package-ensure)
-
-(defcustom use-package-always-pin nil
-  "Treat every package as though it had specified using `:pin SYM'.
-See also `use-package-defaults', which uses this value."
-  :type 'symbol
-  :group 'use-package-ensure)
-
-(defcustom use-package-ensure-function 'use-package-ensure-elpa
-  "Function that ensures a package is installed.
-This function is called with three arguments: the name of the
-package declared in the `use-package' form; the arguments passed
-to all `:ensure' keywords (always a list, even if only one); and
-the current `state' plist created by previous handlers.
-
-Note that this function is called whenever `:ensure' is provided,
-even if it is nil.  It is up to the function to decide on the
-semantics of the various values for `:ensure'.
-
-This function should return non-nil if the package is installed.
-
-The default value uses package.el to install the package."
-  :type '(choice (const :tag "package.el" use-package-ensure-elpa)
-                 (function :tag "Custom"))
-  :group 'use-package-ensure)
-
 ;;;; :pin
 
 (defun use-package-normalize/:pin (_name keyword args)
@@ -196,18 +160,6 @@ use-package-handler/:ensure
             body))
     body))
 
-(add-to-list 'use-package-defaults
-             '(:ensure (list use-package-always-ensure)
-                       (lambda (name args)
-                         (and use-package-always-ensure
-                              (not (plist-member args :load-path))))) t)
-
-(add-to-list 'use-package-defaults
-             '(:pin use-package-always-pin use-package-always-pin) t)
-
-(add-to-list 'use-package-keywords :ensure)
-(add-to-list 'use-package-keywords :pin)
-
 (provide 'use-package-ensure)
 
 ;;; use-package-ensure.el ends here
-- 
2.45.2