unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH] Only signal package.el warnings when needed
@ 2019-01-14  4:17 Radon Rosborough
  2019-01-14 15:48 ` Eli Zaretskii
  2019-01-14 19:55 ` Stefan Monnier
  0 siblings, 2 replies; 24+ messages in thread
From: Radon Rosborough @ 2019-01-14  4:17 UTC (permalink / raw)
  To: emacs-devel


[-- Attachment #1.1: Type: text/plain, Size: 2019 bytes --]

Dear all,

In January 2018, we added support for an early init-file to Emacs 27,
and adjusted Emacs startup to invoke `package-initialize' before
loading the standard init-file (but after loading the early
init-file). This change was adopted, at my suggestion, in order to
obviate the need for `package-initialize' being called in the user's
init-file, and to remove the feature by which Emacs added the call
automatically (which had various disadvantages).

The problem with this change is that many existing init-files contain
a call to `package-initialize', meaning that the function is called
twice during startup. Currently, a warning is emitted when this
occurs. However, there is a better way to solve the problem: simply do
nothing the second time `package-initialize' is invoked.

The problem with this approach is that sometimes it will change the
behavior of an existing Emacs configuration. If an advanced user
changes the values of `package-load-list', `package-user-dir', and/or
`package-directory-list' in their init-file, and they subsequently
call `package-initialize', then ignoring this call will lead to wrong
behavior.

However, it is easy to address this complication: simply keep track of
the values of these user options. If one or more of them has changed
since the last time `package-initialize' was invoked, then perform the
initialization a second time (and signal a warning).

This strategy should be an improvement in user experience for every
use case of package.el, but especially for inexperienced users who may
not even be aware that the package system requires initialization.

I have attached a patch which implements the behavior described above.
The new warning looks like this:

Warning (package): Package.el reinitialized with changed value for
‘package-user-dir’ ("~/.emacs.d/elpa" -> "~/.emacs.d/elpa-alt")

Feedback is welcome. Please copy me on replies, since I am not
subscribed to emacs-devel.

Best regards,
Radon Rosborough

[-- Attachment #1.2: Type: text/html, Size: 2423 bytes --]

[-- Attachment #2: 0001-Only-signal-package.el-warnings-when-needed.patch --]
[-- Type: application/octet-stream, Size: 5084 bytes --]

From 7362674bb69d0206c9d55e727602a5ba519dde4b Mon Sep 17 00:00:00 2001
From: Radon Rosborough <radon.neon@gmail.com>
Date: Sun, 13 Jan 2019 20:44:15 -0700
Subject: [PATCH] Only signal package.el warnings when needed

Previously, `package-initialize' signaled a warning whenever it was
called more than once during startup.  This was annoying to users.
Now, `package-initialize' simply does nothing on a second call, so it
"just works" without any configuration changes by users.

Advanced users may have customized `package-load-list',
`package-user-dir', or `package-directory-list' in their init-file
before calling `package-initialize', in which case this new behavior
would be incorrect.  To remedy that problem, we still signal a warning
when the value of one of those variables has changed since the last
time `package-initialize' was called, but we dutifully perform the
reinitialization so that their configuration will continue to work.

Such advanced users will probably want to set
`package-enable-at-startup' to nil in their early init-file (see
`early-init-file').  This will suppress the warning as a byproduct.

* lisp/emacs-lisp/package.el (package-initialize): Skip
reinitialization unless the values of relevant user options have
changed, and signal a warning in that case (but not otherwise).
(package--user-option-values): New variable, to check whether the
values of relevant user options have changed.
---
 lisp/emacs-lisp/package.el | 61 ++++++++++++++++++++++++++------------
 1 file changed, 42 insertions(+), 19 deletions(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index cf39fa2896..0bb45addd5 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -1445,6 +1445,11 @@ package-read-all-archive-contents
 ;; available on disk.
 (defvar package--initialized nil)
 
+(defvar package--user-option-values nil
+  "Values of user options last time `package-initialize' was called.
+This is an alist whose keys are symbols naming user options and
+whose values are the previous values of those symbols.")
+
 ;;;###autoload
 (defun package-initialize (&optional no-activate)
   "Load Emacs Lisp packages, and activate them.
@@ -1458,26 +1463,44 @@ package-initialize
 individual packages after calling `package-initialize' -- this is
 taken care of by `package-initialize'.
 
-If `package-initialize' is called twice during Emacs startup,
-signal a warning, since this is a bad idea except in highly
-advanced use cases.  To suppress the warning, remove the
-superfluous call to `package-initialize' from your init-file.  If
-you have code which must run before `package-initialize', put
-that code in the early init-file."
+If `package-initialize' is called more than once with different
+values for the user option `package-load-list',
+`package-user-dir', or `package-directory-list', signal a
+warning.  The most likely cause of this warning is that Emacs
+calls `package-initialize' before loading the early init-file,
+then you adjust one or more of these user options and call
+`package-initialize' again.  To prevent Emacs from calling
+`package-initialize' with the wrong options before loading your
+init-file, set `package-enable-at-startup' to nil in your early
+init-file (see `early-init-file')."
   (interactive)
-  (when (and package--initialized (not after-init-time))
-    (lwarn '(package reinitialization) :warning
-           "Unnecessary call to `package-initialize' in init file"))
-  (setq package-alist nil)
-  (setq package-enable-at-startup nil)
-  (package-load-all-descriptors)
-  (package-read-all-archive-contents)
-  (setq package--initialized t)
-  (unless no-activate
-    (package-activate-all))
-  ;; This uses `package--mapc' so it must be called after
-  ;; `package--initialized' is t.
-  (package--build-compatibility-table))
+  (let ((variable-changed
+         (cl-dolist (link package--user-option-values nil)
+           (unless (equal (symbol-value (car link)) (cdr link))
+             (cl-return (car link))))))
+    (when (and package--initialized variable-changed)
+      (lwarn
+       '(package reinitialization) :warning
+       "Package.el reinitialized with changed value for `%S' (%S -> %S)"
+       variable-changed
+       (alist-get variable-changed package--user-option-values)
+       (symbol-value variable-changed)))
+    (unless (and package--initialized (not variable-changed))
+      (setq package-alist nil)
+      (setq package-enable-at-startup nil)
+      (package-load-all-descriptors)
+      (package-read-all-archive-contents)
+      (setq package--initialized t)
+      (unless no-activate
+        (package-activate-all))
+      ;; This uses `package--mapc' so it must be called after
+      ;; `package--initialized' is t.
+      (package--build-compatibility-table)
+      (dolist (var '(package-load-list
+                     package-user-dir
+                     package-directory-list))
+        (setf (alist-get var package--user-option-values)
+              (symbol-value var))))))
 
 (defvar package-quickstart-file)
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread
end of thread, other threads:[~2019-01-23 17:28 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-14  4:17 [PATCH] Only signal package.el warnings when needed Radon Rosborough
2019-01-14 15:48 ` Eli Zaretskii
2019-01-14 16:10   ` Robert Pluim
2019-01-14 16:39     ` Eli Zaretskii
2019-01-14 16:48       ` Robert Pluim
2019-01-14 20:14     ` Stefan Monnier
2019-01-14 21:46     ` Radon Rosborough
2019-01-14 21:46   ` Radon Rosborough
2019-01-15 17:18     ` Eli Zaretskii
2019-01-15 18:43       ` Radon Rosborough
2019-01-15 19:26         ` Eli Zaretskii
2019-01-21 22:45           ` Radon Rosborough
2019-01-22 17:29             ` Eli Zaretskii
2019-01-23  4:06               ` Radon Rosborough
2019-01-23 15:34                 ` Eli Zaretskii
2019-01-23 16:00                   ` Stefan Monnier
2019-01-23 17:28                   ` Radon Rosborough
2019-01-14 22:13   ` Óscar Fuentes
2019-01-14 22:59     ` Clément Pit-Claudel
2019-01-15 10:39       ` João Távora
2019-01-15 17:15         ` Eli Zaretskii
2019-01-15 17:24           ` João Távora
2019-01-14 19:55 ` Stefan Monnier
2019-01-14 20:19   ` Stefan Monnier

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).