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

* Re: [PATCH] Only signal package.el warnings when needed
  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
                     ` (2 more replies)
  2019-01-14 19:55 ` Stefan Monnier
  1 sibling, 3 replies; 24+ messages in thread
From: Eli Zaretskii @ 2019-01-14 15:48 UTC (permalink / raw)
  To: Radon Rosborough; +Cc: emacs-devel

> From: Radon Rosborough <radon.neon@gmail.com>
> Date: Sun, 13 Jan 2019 21:17:58 -0700
> 
> 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.

But we didn't actually hear complaints about that, did we?  I wonder
why should we complicate this stuff even more before collecting enough
real-life experience, which will allow us to have some basis for code
changes -- provided that changes are indeed in order.

Thanks.



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Only signal package.el warnings when needed
  2019-01-14 15:48 ` Eli Zaretskii
@ 2019-01-14 16:10   ` Robert Pluim
  2019-01-14 16:39     ` Eli Zaretskii
                       ` (2 more replies)
  2019-01-14 21:46   ` Radon Rosborough
  2019-01-14 22:13   ` Óscar Fuentes
  2 siblings, 3 replies; 24+ messages in thread
From: Robert Pluim @ 2019-01-14 16:10 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Radon Rosborough, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Radon Rosborough <radon.neon@gmail.com>
>> Date: Sun, 13 Jan 2019 21:17:58 -0700
>> 
>> 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.
>
> But we didn't actually hear complaints about that, did we?  I wonder
> why should we complicate this stuff even more before collecting enough
> real-life experience, which will allow us to have some basis for code
> changes -- provided that changes are indeed in order.

Itʼs not caused me any problems, but Iʼm a simple user of
'package-initialize', so calling it once or twice would make no
difference.

What is annoying is the startup message I get on stdout because I have
package-quickstart set to t:

    Loading ~/.emacs.d/package-quickstart.el (source)...
    Loading ~/.emacs.d/package-quickstart.el (source)...done

It would be great if that disappeared.

Robert



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Only signal package.el warnings when needed
  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
  2 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2019-01-14 16:39 UTC (permalink / raw)
  To: Robert Pluim; +Cc: emacs-devel

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Radon Rosborough <radon.neon@gmail.com>,  emacs-devel@gnu.org
> Date: Mon, 14 Jan 2019 17:10:21 +0100
> 
> What is annoying is the startup message I get on stdout because I have
> package-quickstart set to t:
> 
>     Loading ~/.emacs.d/package-quickstart.el (source)...
>     Loading ~/.emacs.d/package-quickstart.el (source)...done
> 
> It would be great if that disappeared.

'load' has an optional argument to suppress that.



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Only signal package.el warnings when needed
  2019-01-14 16:39     ` Eli Zaretskii
@ 2019-01-14 16:48       ` Robert Pluim
  0 siblings, 0 replies; 24+ messages in thread
From: Robert Pluim @ 2019-01-14 16:48 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Robert Pluim <rpluim@gmail.com>
>> Cc: Radon Rosborough <radon.neon@gmail.com>,  emacs-devel@gnu.org
>> Date: Mon, 14 Jan 2019 17:10:21 +0100
>> 
>> What is annoying is the startup message I get on stdout because I have
>> package-quickstart set to t:
>> 
>>     Loading ~/.emacs.d/package-quickstart.el (source)...
>>     Loading ~/.emacs.d/package-quickstart.el (source)...done
>> 
>> It would be great if that disappeared.
>
> 'load' has an optional argument to suppress that.

It does, but package.el doesnʼt use it.

Robert



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Only signal package.el warnings when needed
  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 19:55 ` Stefan Monnier
  2019-01-14 20:19   ` Stefan Monnier
  1 sibling, 1 reply; 24+ messages in thread
From: Stefan Monnier @ 2019-01-14 19:55 UTC (permalink / raw)
  To: emacs-devel

> 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).

Actually, the startup code calls package-activate-all.

Currently, this ends up calling package-initialize but I think it's
a mistake: package-initialize does two things (activate all packages,
and load the information about the various packages available in ELPA
archives) and only one of the two (the activation of all packages)
should be done at that point.

I think we should install a patch like the one below to deal with this.
It will reduce the number of variables that need to be set in
early-init.el, and will avoid problems where some packages end up
appearing incorrectly as new (because package-initialize at startup
didn't know about some ELPA archive, so when we "refresh" the contents,
all the packages in that archive are suddenly considered to be new).

> 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).

Agreed (tho I wouldn't even emit a warning in that case).


        Stefan


diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index cf39fa2896..b8242e58f6 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -1492,8 +1492,8 @@ package-activate-all
       ;; any decoding).
       (let ((load-source-file-function nil))
         (load package-quickstart-file))
-    (unless package--initialized
-      (package-initialize t))
+    (unless package-alist
+      (package-load-all-descriptors))
     (dolist (elt package-alist)
       (condition-case err
           (package-activate (car elt))




^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH] Only signal package.el warnings when needed
  2019-01-14 16:10   ` Robert Pluim
  2019-01-14 16:39     ` Eli Zaretskii
@ 2019-01-14 20:14     ` Stefan Monnier
  2019-01-14 21:46     ` Radon Rosborough
  2 siblings, 0 replies; 24+ messages in thread
From: Stefan Monnier @ 2019-01-14 20:14 UTC (permalink / raw)
  To: emacs-devel

> What is annoying is the startup message I get on stdout because I have
> package-quickstart set to t:
>
>     Loading ~/.emacs.d/package-quickstart.el (source)...
>     Loading ~/.emacs.d/package-quickstart.el (source)...done
>
> It would be great if that disappeared.

I just installed a patch below which should do just that,


        Stefan




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Only signal package.el warnings when needed
  2019-01-14 19:55 ` Stefan Monnier
@ 2019-01-14 20:19   ` Stefan Monnier
  0 siblings, 0 replies; 24+ messages in thread
From: Stefan Monnier @ 2019-01-14 20:19 UTC (permalink / raw)
  To: emacs-devel

> I think we should install a patch like the one below to deal with this.

Actually, there are a few other places where we'd benefit from loading
the package-alist without initializing the whole package.el.
So maybe we should apply a patch like the one below instead (see
bug#31397).


        Stefan


diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 9a7b54fa01..503fc7e4cd 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -609,6 +609,12 @@ package-load-all-descriptors
             (when (file-directory-p pkg-dir)
               (package-load-descriptor pkg-dir))))))))
 
+(defun package--alist ()
+  "Return `package-alist', initializing it if needed."
+  (or package-alist
+      (progn (package-load-all-descriptors)
+             package-alist)))
+
 (defun define-package (_name-string _version-string
                                     &optional _docstring _requirements
                                     &rest _extra-properties)
@@ -785,7 +791,7 @@ package-activate
   "Activate the package named PACKAGE.
 If FORCE is true, (re-)activate it if it's already activated.
 Newer versions are always activated, regardless of FORCE."
-  (let ((pkg-descs (cdr (assq package package-alist))))
+  (let ((pkg-descs (cdr (assq package (package--alist)))))
     ;; Check if PACKAGE is available in `package-alist'.
     (while
         (when pkg-descs
@@ -1492,9 +1498,7 @@ package-activate-all
       ;; any decoding).
       (let ((load-source-file-function nil))
         (load package-quickstart-file nil 'nomessage))
-    (unless package--initialized
-      (package-initialize t))
-    (dolist (elt package-alist)
+    (dolist (elt (package--alist))
       (condition-case err
           (package-activate (car elt))
         ;; Don't let failure of activation of a package arbitrarily stop
@@ -1903,10 +1907,9 @@ package-installed-p
     ;; We used the quickstart: make it possible to use package-installed-p
     ;; even before package is fully initialized.
     (memq package package-activated-list))
-   ((not package--initialized) (error "package.el is not yet initialized!"))
    (t
     (or
-     (let ((pkg-descs (cdr (assq package package-alist))))
+     (let ((pkg-descs (cdr (assq package (package--alist)))))
        (and pkg-descs
             (version-list-<= min-version
                              (package-desc-version (car pkg-descs)))))
@@ -2078,22 +2081,17 @@ package-delete
 If NOSAVE is non-nil, the package is not removed from
 `package-selected-packages'."
   (interactive
-   (progn
-     ;; Initialize the package system to get the list of package
-     ;; symbols for completion.
-     (unless package--initialized
-       (package-initialize t))
-     (let* ((package-table
-             (mapcar
-              (lambda (p) (cons (package-desc-full-name p) p))
-              (delq nil
-                    (mapcar (lambda (p) (unless (package-built-in-p p) p))
-                            (apply #'append (mapcar #'cdr package-alist))))))
-            (package-name (completing-read "Delete package: "
-                                           (mapcar #'car package-table)
-                                           nil t)))
-       (list (cdr (assoc package-name package-table))
-             current-prefix-arg nil))))
+   (let* ((package-table
+           (mapcar
+            (lambda (p) (cons (package-desc-full-name p) p))
+            (delq nil
+                  (mapcar (lambda (p) (unless (package-built-in-p p) p))
+                          (apply #'append (mapcar #'cdr (package--alist)))))))
+          (package-name (completing-read "Delete package: "
+                                         (mapcar #'car package-table)
+                                         nil t)))
+     (list (cdr (assoc package-name package-table))
+           current-prefix-arg nil)))
   (let ((dir (package-desc-dir pkg-desc))
         (name (package-desc-name pkg-desc))
         pkg-used-elsewhere-by)




^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH] Only signal package.el warnings when needed
  2019-01-14 15:48 ` Eli Zaretskii
  2019-01-14 16:10   ` Robert Pluim
@ 2019-01-14 21:46   ` Radon Rosborough
  2019-01-15 17:18     ` Eli Zaretskii
  2019-01-14 22:13   ` Óscar Fuentes
  2 siblings, 1 reply; 24+ messages in thread
From: Radon Rosborough @ 2019-01-14 21:46 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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

> From: Eli Zaretskii <eliz@gnu.org>
> Date: Jan 14, 2019, 8:49 AM
>
>> From: Radon Rosborough <radon.neon@gmail.com>
>> Date: Sun, 13 Jan 2019 21:17:58 -0700
>>
>> Currently, a warning is emitted when this occurs.
>
> But we didn't actually hear complaints about that, did we?

There seems to be some unnecessary confusion in this thread:
<https://github.com/jkitchin/scimax/issues/194>

Also, there's some annoyance here:
<https://debbugs.gnu.org/cgi/bugreport.cgi?bug=31397>

You're right that we have not heard too much about it. However, I
expect that we will hear a lot more about it if we release Emacs 27
with the current code. This is because people who run Emacs from HEAD
are more likely to be advanced users who know how to proceed when
confronted with a warning message like this.

Consider that essentially every modern Emacs init-file contains a call
to `package-initialize', since such a call was inserted whenever the
package manager was used. It seems like a huge amount of unnecessary
work to tell every Emacs user with an init-file to manually edit it --
especially since many users are not even aware that they have an
init-file.

I don't see any downside to installing the patch, but I do see a clear
benefit to many users. Let me know your thoughts.

Best,
Radon

[-- Attachment #2: Type: text/html, Size: 1896 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Only signal package.el warnings when needed
  2019-01-14 16:10   ` Robert Pluim
  2019-01-14 16:39     ` Eli Zaretskii
  2019-01-14 20:14     ` Stefan Monnier
@ 2019-01-14 21:46     ` Radon Rosborough
  2 siblings, 0 replies; 24+ messages in thread
From: Radon Rosborough @ 2019-01-14 21:46 UTC (permalink / raw)
  To: emacs-devel; +Cc: Eli Zaretskii

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

> From: Robert Pluim <rpluim@gmail.com>
> Date: Jan 14, 2019, 9:10 AM
>
> What is annoying is the startup message I get on stdout because I
> have package-quickstart set to t

That seems like a separate problem which deserves its own bug report
so it does not get lost in this discussion.

Best,
Radon

[-- Attachment #2: Type: text/html, Size: 500 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Only signal package.el warnings when needed
  2019-01-14 15:48 ` Eli Zaretskii
  2019-01-14 16:10   ` Robert Pluim
  2019-01-14 21:46   ` Radon Rosborough
@ 2019-01-14 22:13   ` Óscar Fuentes
  2019-01-14 22:59     ` Clément Pit-Claudel
  2 siblings, 1 reply; 24+ messages in thread
From: Óscar Fuentes @ 2019-01-14 22:13 UTC (permalink / raw)
  To: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> But we didn't actually hear complaints about that, did we?  I wonder
> why should we complicate this stuff even more before collecting enough
> real-life experience, which will allow us to have some basis for code
> changes -- provided that changes are indeed in order.

FWIW, I was surprised and annoyed by that warning. Not to the level of
complaining on public, but thinking that it is... less than elegant to
automatically modify the init files and some time later to pester the
user with warnings about that same modification he was forced to accept.

Not to mention the noise it creates for those of us that occasionally
work with multiple Emacs versions and the same init files.




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Only signal package.el warnings when needed
  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
  0 siblings, 1 reply; 24+ messages in thread
From: Clément Pit-Claudel @ 2019-01-14 22:59 UTC (permalink / raw)
  To: emacs-devel

On 14/01/2019 17.13, Óscar Fuentes wrote:
> Not to mention the noise it creates for those of us that occasionally
> work with multiple Emacs versions and the same init files.

This is my main source of annoyance with the warning, thought that annoyance did not raise to the level of complaining about it in public until now :)



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Only signal package.el warnings when needed
  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
  0 siblings, 1 reply; 24+ messages in thread
From: João Távora @ 2019-01-15 10:39 UTC (permalink / raw)
  To: Clément Pit-Claudel; +Cc: emacs-devel

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

On Mon, Jan 14, 2019, 23:37 Clément Pit-Claudel <cpitclaudel@gmail.com
wrote:

> On 14/01/2019 17.13, Óscar Fuentes wrote:
> > Not to mention the noise it creates for those of us that occasionally
> > work with multiple Emacs versions and the same init files.
>
> This is my main source of annoyance with the warning, thought that
> annoyance did not raise to the level of complaining about it in public
> until now :)
>

Ditto :)

João

[-- Attachment #2: Type: text/html, Size: 796 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Only signal package.el warnings when needed
  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
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2019-01-15 17:15 UTC (permalink / raw)
  To: João Távora; +Cc: cpitclaudel, emacs-devel

> From: João Távora <joaotavora@gmail.com>
> Date: Tue, 15 Jan 2019 10:39:16 +0000
> Cc: emacs-devel <emacs-devel@gnu.org>
> 
> On Mon, Jan 14, 2019, 23:37 Clément Pit-Claudel <cpitclaudel@gmail.com wrote:
> 
>  On 14/01/2019 17.13, Óscar Fuentes wrote:
>  > Not to mention the noise it creates for those of us that occasionally
>  > work with multiple Emacs versions and the same init files.
> 
>  This is my main source of annoyance with the warning, thought that annoyance did not raise to the level
>  of complaining about it in public until now :)
> 
> Ditto :)

I'm questioning the wisdom of adding non-trivial complexity on account
of annoyances that "did not raise to the level of complaining about it
in public".  Shouldn't we collect more experience regarding the
current solution before we complicate it more?



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Only signal package.el warnings when needed
  2019-01-14 21:46   ` Radon Rosborough
@ 2019-01-15 17:18     ` Eli Zaretskii
  2019-01-15 18:43       ` Radon Rosborough
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2019-01-15 17:18 UTC (permalink / raw)
  To: Radon Rosborough; +Cc: emacs-devel

> From: Radon Rosborough <radon.neon@gmail.com>
> Date: Mon, 14 Jan 2019 14:46:16 -0700
> Cc: emacs-devel <emacs-devel@gnu.org>
> 
> You're right that we have not heard too much about it. However, I
> expect that we will hear a lot more about it if we release Emacs 27
> with the current code. This is because people who run Emacs from HEAD
> are more likely to be advanced users who know how to proceed when
> confronted with a warning message like this.

I'm saying that we should hear the complaints first and devise the
solution only after that.  It is quite likely that the actual
complaints will tell us something which will affect the solution we
devise.

> I don't see any downside to installing the patch, but I do see a clear
> benefit to many users. Let me know your thoughts.

Increasing complexity is definitely a downside, at least in my book.
Any complex solution runs the risk of having unintended consequences,
so IMO we should understand the problem better before we decide on a
solution.



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Only signal package.el warnings when needed
  2019-01-15 17:15         ` Eli Zaretskii
@ 2019-01-15 17:24           ` João Távora
  0 siblings, 0 replies; 24+ messages in thread
From: João Távora @ 2019-01-15 17:24 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Clément Pit-Claudel, emacs-devel

On Tue, Jan 15, 2019 at 5:15 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: João Távora <joaotavora@gmail.com>
> > Date: Tue, 15 Jan 2019 10:39:16 +0000
> > Cc: emacs-devel <emacs-devel@gnu.org>
> >
> > On Mon, Jan 14, 2019, 23:37 Clément Pit-Claudel <cpitclaudel@gmail.com wrote:
> >
> >  On 14/01/2019 17.13, Óscar Fuentes wrote:
> >  > Not to mention the noise it creates for those of us that occasionally
> >  > work with multiple Emacs versions and the same init files.
> >
> >  This is my main source of annoyance with the warning, thought that annoyance did not raise to the level
> >  of complaining about it in public until now :)
> >
> > Ditto :)
>
> I'm questioning the wisdom of adding non-trivial complexity on account
> of annoyances that "did not raise to the level of complaining about it
> in public".  Shouldn't we collect more experience regarding the
> current solution before we complicate it more?

That makes sense too.  To be honest, I have a commented out
definition of (package-initialize) in my init which works for me.
When I switch to Emacs 26.1 I remember to M-x package-initialize
and that solves it for me (because I don't use package.el much).

João Távora



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Only signal package.el warnings when needed
  2019-01-15 17:18     ` Eli Zaretskii
@ 2019-01-15 18:43       ` Radon Rosborough
  2019-01-15 19:26         ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Radon Rosborough @ 2019-01-15 18:43 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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

> From: Eli Zaretskii <eliz@gnu.org>
> Date: Jan 15, 2019, 10:19 AM
>
> I'm saying that we should hear the complaints first and devise the
> solution only after that.

If we wait until Emacs 27.2 to fix the complaints, then it will
already be too late to do anything useful, since everyone will have
already suffered and changed their init-files. The inconvenience is a
one-time occurrence, so fixing it after the fact will do nothing. It
is like seeing a building about to catch fire, and deciding to wait
until after it's burned down to bring in the fire trucks.

I view the current situation as quite unacceptable: the most prominent
result on Google when you search for the warning message suggests the
following:

(unless package--initialized (package-initialize t))

That means when Emacs 27 is released we're going to see this code show
up in everybody's init-files (upgrade -> "what's this weird warning?"
-> Google -> copy/paste -> "oh good, it's gone now"). I don't think
that's acceptable, because `package--initialized' is an internal
variable which should not be referenced in user code.

Even if we fix the problem in Emacs 27.2, that won't help to get these
references to `package--initialize' out of people's init-files.
Deciding not to merge this patch or something similar will create a
massive amount of technical debt in places we can't touch (everyone's
Emacs configurations).

Best,
Radon

[-- Attachment #2: Type: text/html, Size: 1869 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Only signal package.el warnings when needed
  2019-01-15 18:43       ` Radon Rosborough
@ 2019-01-15 19:26         ` Eli Zaretskii
  2019-01-21 22:45           ` Radon Rosborough
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2019-01-15 19:26 UTC (permalink / raw)
  To: Radon Rosborough; +Cc: emacs-devel

> From: Radon Rosborough <radon.neon@gmail.com>
> Date: Tue, 15 Jan 2019 11:43:39 -0700
> Cc: emacs-devel <emacs-devel@gnu.org>
> 
> > From: Eli Zaretskii <eliz@gnu.org>
> > Date: Jan 15, 2019, 10:19 AM
> >
> > I'm saying that we should hear the complaints first and devise the
> > solution only after that.
> 
> If we wait until Emacs 27.2 to fix the complaints, then it will
> already be too late to do anything useful, since everyone will have
> already suffered and changed their init-files.

That was the situation before the recent changes, and we still made
those changes.  So I think your fears are unjustified.

> I view the current situation as quite unacceptable: the most prominent
> result on Google when you search for the warning message suggests the
> following:
> 
> (unless package--initialized (package-initialize t))
> 
> That means when Emacs 27 is released we're going to see this code show
> up in everybody's init-files (upgrade -> "what's this weird warning?"
> -> Google -> copy/paste -> "oh good, it's gone now"). I don't think
> that's acceptable, because `package--initialized' is an internal
> variable which should not be referenced in user code.

So maybe the right solution is to make that variable public instead.

> Even if we fix the problem in Emacs 27.2, that won't help to get these
> references to `package--initialize' out of people's init-files.
> Deciding not to merge this patch or something similar will create a
> massive amount of technical debt in places we can't touch (everyone's
> Emacs configurations).

I think "massive amount" is a significant exaggeration.

Anyway, there's no way around some of this when we are trying to solve
such problems.  That there's some fallout cannot be used as an
ultimate argument in favor or against some change.



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Only signal package.el warnings when needed
  2019-01-15 19:26         ` Eli Zaretskii
@ 2019-01-21 22:45           ` Radon Rosborough
  2019-01-22 17:29             ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Radon Rosborough @ 2019-01-21 22:45 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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

> That there's some fallout cannot be used as an ultimate argument in
> favor or against some change.

In that case, could you explain what /would/ be a good argument in
favor or against the change I am proposing? As far as I can tell, it
saves people time without any known disadvantages (and with very
little additional complexity -- the patch being about 70 lines of
code), but you don't seem to consider this a good enough argument.
Have I misunderstood something?

> From: Eli Zaretskii <eliz@gnu.org>
> Date: Jan 15, 2019, 11:26 AM
>
> > From: Radon Rosborough <radon.neon@gmail.com>
> > Date: Tue, 15 Jan 2019 11:43:39 -0700
> >
> > > From: Eli Zaretskii <eliz@gnu.org>
> > > Date: Jan 15, 2019, 10:19 AM
> > >
> > > I'm saying that we should hear the complaints first and devise
> > > the solution only after that.
> >
> > If we wait until Emacs 27.2 to fix the complaints, then it will
> > already be too late to do anything useful.
>
> That was the situation before the recent changes, and we still made
> those changes.

I don't see why this would be an argument against the change I am
proposing. The recent changes were useful, which is why we made them,
but they would have been even /more/ useful if we had made them
earlier (before everyone's init-files got changed). The situation is
the same here. Maybe it would still be helpful to make these changes
in Emacs 26.2, but they would be a lot /less/ helpful at that point in
time.

> So maybe the right solution is to make that variable public instead.

Maybe. But this seems like a strictly inferior solution from the
perspective of user experience, since it still results in users being
shown superfluous warnings which waste their time and mental effort.

Best,
Radon

[-- Attachment #2: Type: text/html, Size: 2420 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Only signal package.el warnings when needed
  2019-01-21 22:45           ` Radon Rosborough
@ 2019-01-22 17:29             ` Eli Zaretskii
  2019-01-23  4:06               ` Radon Rosborough
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2019-01-22 17:29 UTC (permalink / raw)
  To: Radon Rosborough; +Cc: emacs-devel

> From: Radon Rosborough <radon.neon@gmail.com>
> Date: Mon, 21 Jan 2019 14:45:36 -0800
> Cc: emacs-devel <emacs-devel@gnu.org>
> 
> > That there's some fallout cannot be used as an ultimate argument in
> > favor or against some change.
> 
> In that case, could you explain what /would/ be a good argument in
> favor or against the change I am proposing? As far as I can tell, it
> saves people time without any known disadvantages (and with very
> little additional complexity -- the patch being about 70 lines of
> code), but you don't seem to consider this a good enough argument.

There's no argument between us about the advantages of the proposed
change.  The argument seems to be about the disadvantages, and
specifically about the risks of unintended consequences of changes in
this area.  The size and the complexity of the patch is therefore not
the important indicator, what's important is the complexity of the
code which it changes, and the associated risk of unintentionally
breaking some important use case out there, of which there are quite a
few.

I realize that you don't necessarily agree with this POV, but we have
different perspectives on this.

> > > If we wait until Emacs 27.2 to fix the complaints, then it will
> > > already be too late to do anything useful.
> >
> > That was the situation before the recent changes, and we still made
> > those changes.
> 
> I don't see why this would be an argument against the change I am
> proposing.

It isn't.  It is an argument against the haste of making changes
because it might be "too late" afterwards.  I'm saying that we are not
afraid of making changes afterwards, and I provided an example of
that, one that you should be familiar with.

> The recent changes were useful, which is why we made them,
> but they would have been even /more/ useful if we had made them
> earlier (before everyone's init-files got changed).

Fixing problems sooner rather than later is always an advantage.  But
it should be weighed against any disadvantages, and in this case it
seems to me that we might not yet have enough experience with the
current code to design a solution whose risks are low enough.

> > So maybe the right solution is to make that variable public instead.
> 
> Maybe. But this seems like a strictly inferior solution from the
> perspective of user experience, since it still results in users being
> shown superfluous warnings which waste their time and mental effort.

It might be a stopgap, but if it improves the situation while running
lower risks, it could be a good interim measure.



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Only signal package.el warnings when needed
  2019-01-22 17:29             ` Eli Zaretskii
@ 2019-01-23  4:06               ` Radon Rosborough
  2019-01-23 15:34                 ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Radon Rosborough @ 2019-01-23  4:06 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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

> From: Eli Zaretskii <eliz@gnu.org>
> Date: Jan 22, 2019, 10:36 AM
>
> I realize that you don't necessarily agree with this POV, but we
> have different perspectives on this.

Okay then. I am a little disappointed that this embarrassing behavior,
which has my name on it, will be released to users. However, it sounds
like there is nothing I can do to convince you, so I guess there's
nothing else for me to do.

Let me know if you think there is an alternative way to fix the
problem that would be acceptable to you.

Best,
Radon

[-- Attachment #2: Type: text/html, Size: 761 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Only signal package.el warnings when needed
  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
  0 siblings, 2 replies; 24+ messages in thread
From: Eli Zaretskii @ 2019-01-23 15:34 UTC (permalink / raw)
  To: Radon Rosborough; +Cc: emacs-devel

> From: Radon Rosborough <radon.neon@gmail.com>
> Date: Tue, 22 Jan 2019 20:06:09 -0800
> Cc: emacs-devel <emacs-devel@gnu.org>
> 
> Okay then. I am a little disappointed that this embarrassing behavior,
> which has my name on it, will be released to users. However, it sounds
> like there is nothing I can do to convince you, so I guess there's
> nothing else for me to do.

I think we should revisit this issue in a few months.

> Let me know if you think there is an alternative way to fix the
> problem that would be acceptable to you.

What about the stopgap solution of making package--initialized a
public variable?



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Only signal package.el warnings when needed
  2019-01-23 15:34                 ` Eli Zaretskii
@ 2019-01-23 16:00                   ` Stefan Monnier
  2019-01-23 17:28                   ` Radon Rosborough
  1 sibling, 0 replies; 24+ messages in thread
From: Stefan Monnier @ 2019-01-23 16:00 UTC (permalink / raw)
  To: emacs-devel

>> Let me know if you think there is an alternative way to fix the
>> problem that would be acceptable to you.
> What about the stopgap solution of making package--initialized a
> public variable?

I'd rather not, and I don't think it's necessary.  I believe with the
change I installed yesterday (needed to fix other problems), this issue
of "spurious" warnings should be largely fixed already.


        Stefan




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Only signal package.el warnings when needed
  2019-01-23 15:34                 ` Eli Zaretskii
  2019-01-23 16:00                   ` Stefan Monnier
@ 2019-01-23 17:28                   ` Radon Rosborough
  1 sibling, 0 replies; 24+ messages in thread
From: Radon Rosborough @ 2019-01-23 17:28 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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

Actually, while we were talking about the issue, Stefan solved it.
See:

https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=dde33727b2ace3ce417d97475d074f0a82b7c2b8

This is a better solution than any of the ones proposed by either of
us, and is completely satisfactory to me.

Best,
Radon

[-- Attachment #2: Type: text/html, Size: 509 bytes --]

^ permalink raw reply	[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).