bug#73812: 30.0.91; ERC 5.6.0.30.1: Customizing erc-modules loads ERC when starting Emacs

bug#73812: 30.0.91; ERC 5.6.0.30.1: Customizing erc-modules loads ERC when starting Emacs

[J.P.]

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

Tags: patch

From emacs -Q

1. $ mkdir /tmp/fake-home
2. $ echo "(custom-set-variables '(erc-modules ()))" > /tmp/fake-home/.emacs
3. $ HOME=/tmp/fake-home ./src/emacs
4. M-: (featurep 'erc) RET

Eli, can we add this patch to the release branch? It removes a single
line containing an autoload cookie that's new in Emacs 30. (I'm running
the check-expensive suite with the patch now.) Thanks.


In GNU Emacs 30.0.91 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.43, cairo version 1.18.0) of 2024-10-14 built on localhost
Repository revision: b87fda63dd4a29c3c28e235904405f2d6709239e
Repository branch: emacs-30
Windowing system distributor 'The X.Org Foundation', version 11.0.12401002
System Description: Fedora Linux 40 (Workstation Edition)

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NATIVE_COMP
NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  minibuffer-regexp-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec epa epg rfc6068 epg-config gnus-util
time-date mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils compile text-property-search comint ansi-osc
ansi-color ring comp-run comp-common erc derived auth-source eieio
eieio-core icons password-cache json map format-spec erc-backend
erc-networks easy-mmode byte-opt bytecomp byte-compile erc-common inline
cl-extra help-mode erc-compat cl-seq cl-macs gv pcase rx compat subr-x
cl-loaddefs cl-lib erc-loaddefs rmc iso-transl tooltip cconv eldoc paren
electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/x-win x-win term/common-win x-dnd touch-screen tool-bar dnd fontset
image regexp-opt fringe tabulated-list replace newcomment text-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
easymenu timer select scroll-bar mouse jit-lock font-lock syntax
font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify lcms2
dynamic-setting system-font-setting font-render-setting cairo gtk
x-toolkit xinput2 x multi-tty move-toolbar make-network-process
native-compile emacs)

Memory information:
((conses 16 166001 9858) (symbols 48 11695 0) (strings 32 30570 5080)
 (string-bytes 1 1067417) (vectors 16 19000)
 (vector-slots 8 192763 8903) (floats 8 35 1) (intervals 56 339 0)
 (buffers 984 11))


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Don-t-autoload-erc-modules.patch --]
[-- Type: text/x-patch, Size: 1295 bytes --]

From 560390753c087a8e0dd8e6e1d2803ff8fff4cf24 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Mon, 14 Oct 2024 19:32:16 -0700
Subject: [PATCH] Don't autoload erc-modules

* lisp/erc/erc.el (erc-modules): Remove autoload because it caused
customizations for this option to load the main library.  This reverts
the thrust of bb894845 "Teach customize-option about erc-modules", which
was added in ERC 5.6 and Emacs 30.  The motivation for the original
offending change was to allow new users to run M-x customize-option RET
erc-modules RET immediately after start up instead of M-x
customize-group RET, followed by an I-search.
---
 lisp/erc/erc.el | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 30641c2bd88..688d2f4b1ae 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -2263,8 +2263,6 @@ erc--sort-modules
       (cl-pushnew mod (if (get mod 'erc--module) built-in third-party)))
     `(,@(sort built-in #'string-lessp) ,@(nreverse third-party))))
 
-;;;###autoload(custom-autoload 'erc-modules "erc")
-
 (defcustom erc-modules '( autojoin button completion fill imenu irccontrols
                           list match menu move-to-prompt netsplit
                           networks readonly ring stamp track)
-- 
2.46.2

bug#73812: 30.0.91; ERC 5.6.0.30.1: Customizing erc-modules loads ERC when starting Emacs

[Eli Zaretskii]

> Cc: emacs-erc@gnu.org, Eli Zaretskii <eliz@gnu.org>
> From: "J.P." <jp@neverwas.me>
> Date: Mon, 14 Oct 2024 19:57:00 -0700
> 
> >From emacs -Q
> 
> 1. $ mkdir /tmp/fake-home
> 2. $ echo "(custom-set-variables '(erc-modules ()))" > /tmp/fake-home/.emacs
> 3. $ HOME=/tmp/fake-home ./src/emacs
> 4. M-: (featurep 'erc) RET
> 
> Eli, can we add this patch to the release branch? It removes a single
> line containing an autoload cookie that's new in Emacs 30. (I'm running
> the check-expensive suite with the patch now.) Thanks.

Are we sure this doesn't break any customization scenarios?  Why was
this line added in the first place?  And why is it urgent to remove it
before Emacs 30 is released?

bug#73812: 30.0.91; ERC 5.6.0.30.1: Customizing erc-modules loads ERC when starting Emacs

[J.P.]

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: emacs-erc@gnu.org, Eli Zaretskii <eliz@gnu.org>
>> From: "J.P." <jp@neverwas.me>
>> Date: Mon, 14 Oct 2024 19:57:00 -0700
>> 
>> >From emacs -Q
>> 
>> 1. $ mkdir /tmp/fake-home
>> 2. $ echo "(custom-set-variables '(erc-modules ()))" > /tmp/fake-home/.emacs
>> 3. $ HOME=/tmp/fake-home ./src/emacs
>> 4. M-: (featurep 'erc) RET
>> 
>> Eli, can we add this patch to the release branch? It removes a single
>> line containing an autoload cookie that's new in Emacs 30. (I'm running
>> the check-expensive suite with the patch now.) Thanks.
>
> Are we sure this doesn't break any customization scenarios?

The worst case is probably someone unaware of the implicit loading who
recently added code referring to symbols defined in erc.el after an
Emacs-managed `custom-set-variables' form in their `custom-file' (or a
`:custom' section in a `use-package' declaration). These customizations
would necessarily have to contain an entry for `erc-modules', but this
is perhaps the most common ERC customization. However, the user would
have to be running the pre-release or master.

>  Why was this line added in the first place?

The line was initially added in a misguided attempt to allow new users
unfamiliar with Emacs to run M-x customize-option RET erc-modules RET
without first having to run M-: (require 'erc) RET.

>  And why is it urgent to remove it before Emacs 30 is released?

ERC has a great many symbols, which people won't want to see in
completion tables, etc. Longtime ERC users trying Emacs 30 for the first
time may find ERC loading whenever they start Emacs, which may not be
desirable in all Emacs sessions. And since, as mentioned, `erc-modules'
is likely among the most commonly customized of ERC's options, this may
also affect non-ERC users who perhaps only tried it once many years ago
or even folks using a shared config containing such a customization. For
these reasons, I suspect we'll start noticing ERC-related pollution in
the automated evidence collection for bug reports filed with M-x
report-emacs-bug RET once Emacs 30 goes mainstream.

bug#73812: 30.0.91; ERC 5.6.0.30.1: Customizing erc-modules loads ERC when starting Emacs

[J.P.]

"J.P." <jp@neverwas.me> writes:

>>> Eli, can we add this patch to the release branch? It removes a single
>>> line containing an autoload cookie that's new in Emacs 30. (I'm running
>>> the check-expensive suite with the patch now.) Thanks.
>>
>> Are we sure this doesn't break any customization scenarios?
>
> The worst case is probably someone unaware of the implicit loading who
> recently added code referring to symbols defined in erc.el after an
> Emacs-managed `custom-set-variables' form in their `custom-file' (or a
> `:custom' section in a `use-package' declaration). These customizations
> would necessarily have to contain an entry for `erc-modules', but this
> is perhaps the most common ERC customization. However, the user would
> have to be running the pre-release or master.
>
>>  Why was this line added in the first place?
>
> The line was initially added in a misguided attempt to allow new users
> unfamiliar with Emacs to run M-x customize-option RET erc-modules RET
> without first having to run M-: (require 'erc) RET.
>
>>  And why is it urgent to remove it before Emacs 30 is released?
>
> ERC has a great many symbols, which people won't want to see in
> completion tables, etc. Longtime ERC users trying Emacs 30 for the first
> time may find ERC loading whenever they start Emacs, which may not be
> desirable in all Emacs sessions. And since, as mentioned, `erc-modules'
> is likely among the most commonly customized of ERC's options, this may
> also affect non-ERC users who perhaps only tried it once many years ago
> or even folks using a shared config containing such a customization. For
> these reasons, I suspect we'll start noticing ERC-related pollution in
> the automated evidence collection for bug reports filed with M-x
> report-emacs-bug RET once Emacs 30 goes mainstream.

While we await a ruling on this...

Here's an example of how `erc-modules' customizations inadvertently
creep into a person's `custom-file' (for any curious future victims of
this bug).

From emacs -Q (using Emacs 27 as an example):

0. Connect to a server
1. M-x customize-group RET erc RET
2. Twirl open the "Erc Modules" section
3. In an ERC buffer: M-x erc-scrolltobottom-mode RET
4. In the Custom buffer, edit an option, like "Erc Email Userid"
5. C-x C-s
6. Notice that your `custom-file' now contains an entry for `erc-modules'

Among those who've ever done the above or similar, only the lucky few to
have since cleaned up their `custom-file' will be spared ERC invasively
loading when next they fire up Emacs 30.

As may be obvious, ERC is guilty of the familiar anti-pattern of
mutating user options silently behind the scenes. For reasons of
compatibility, we've put off addressing this until a major release.
FWIW, new modules following the recommended practice of declaring
themselves `local' do not exhibit this malignant behavior.

bug#73812: 30.0.91; ERC 5.6.0.30.1: Customizing erc-modules loads ERC when starting Emacs

[Eli Zaretskii]

> From: "J.P." <jp@neverwas.me>
> Cc: 73812@debbugs.gnu.org,  emacs-erc@gnu.org
> Date: Tue, 15 Oct 2024 11:00:38 -0700
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Are we sure this doesn't break any customization scenarios?
> 
> The worst case is probably someone unaware of the implicit loading who
> recently added code referring to symbols defined in erc.el after an
> Emacs-managed `custom-set-variables' form in their `custom-file' (or a
> `:custom' section in a `use-package' declaration). These customizations
> would necessarily have to contain an entry for `erc-modules', but this
> is perhaps the most common ERC customization. However, the user would
> have to be running the pre-release or master.
> 
> >  Why was this line added in the first place?
> 
> The line was initially added in a misguided attempt to allow new users
> unfamiliar with Emacs to run M-x customize-option RET erc-modules RET
> without first having to run M-: (require 'erc) RET.

The commit log message says:

    * lisp/erc/erc.el (erc-modules): Make good on decades old language in
    info node "(erc) Modules" by ensuring `customize-option' can find this
    option before its containing library is loaded.  Like
    `gnus-select-method', this option serves as an entry point for
    configuring the application and is presented that way in tutorials and
    library front matter.  Moreover, it can't be reasonably autoloaded in
    the traditional way because of its many dependencies and large textual
    footprint.

So there was some reasonable rationale to this change.

> >  And why is it urgent to remove it before Emacs 30 is released?
> 
> ERC has a great many symbols, which people won't want to see in
> completion tables, etc. Longtime ERC users trying Emacs 30 for the first
> time may find ERC loading whenever they start Emacs, which may not be
> desirable in all Emacs sessions. And since, as mentioned, `erc-modules'
> is likely among the most commonly customized of ERC's options, this may
> also affect non-ERC users who perhaps only tried it once many years ago
> or even folks using a shared config containing such a customization. For
> these reasons, I suspect we'll start noticing ERC-related pollution in
> the automated evidence collection for bug reports filed with M-x
> report-emacs-bug RET once Emacs 30 goes mainstream.

All in all, I'd prefer to leave this alone in Emacs 30.  We have time
to try reverting this on master and seeing whether it's a net win or a
net loss, given the past history of the issue.  (AFAIU, if you remove
this line, some change is pertinent in the manual?)

Thanks.

bug#73812: 30.0.91; ERC 5.6.0.30.1: Customizing erc-modules loads ERC when starting Emacs

[J.P.]

Eli Zaretskii <eliz@gnu.org> writes:

>> >  Why was this line added in the first place?
>> 
>> The line was initially added in a misguided attempt to allow new users
>> unfamiliar with Emacs to run M-x customize-option RET erc-modules RET
>> without first having to run M-: (require 'erc) RET.
>
> The commit log message says:
>
>     * lisp/erc/erc.el (erc-modules): Make good on decades old language in
>     info node "(erc) Modules" by ensuring `customize-option' can find this
>     option before its containing library is loaded.  Like
>     `gnus-select-method', this option serves as an entry point for
>     configuring the application and is presented that way in tutorials and
>     library front matter.  Moreover, it can't be reasonably autoloaded in
>     the traditional way because of its many dependencies and large textual
>     footprint.
>
> So there was some reasonable rationale to this change.

What the commit message doesn't mention is that its author (me) was
ignorant of the fact that customizing the option and saving it would
result in ERC being loaded unconditionally on startup rather than only
when summoned.

>
>> >  And why is it urgent to remove it before Emacs 30 is released?
>> 
>> ERC has a great many symbols, which people won't want to see in
>> completion tables, etc. Longtime ERC users trying Emacs 30 for the first
>> time may find ERC loading whenever they start Emacs, which may not be
>> desirable in all Emacs sessions. And since, as mentioned, `erc-modules'
>> is likely among the most commonly customized of ERC's options, this may
>> also affect non-ERC users who perhaps only tried it once many years ago
>> or even folks using a shared config containing such a customization. For
>> these reasons, I suspect we'll start noticing ERC-related pollution in
>> the automated evidence collection for bug reports filed with M-x
>> report-emacs-bug RET once Emacs 30 goes mainstream.
>
> All in all, I'd prefer to leave this alone in Emacs 30.  We have time
> to try reverting this on master and seeing whether it's a net win or a
> net loss, given the past history of the issue.  (AFAIU, if you remove
> this line, some change is pertinent in the manual?)

The manual currently says:

  4 Modules
  *********
  
  One way to add functionality to ERC is to customize which of its many
  modules are loaded.
  
     There is a spiffy customize interface, which may be reached by typing
  ‘M-x customize-option <RET> erc-modules <RET>’.

We could instead say something like:

  The main way to impact ERC's functionality is by choosing which
  modules it loads.  Do this by typing ‘C-h v erc-modules RET’ to view
  that option's help buffer, then click ‘customize’ near the bottom,
  where it says "You can _customize_ this variable."

(This assumes anyone needing detailed instructions hasn't disabled
`help-enable-completion-autoload'.)

In any case, I don't anticipate much blowback beyond the complaints we
used to get re "it says [no match]", etc. But I will make the change on
master for now, as suggested. Thanks.

bug#73812: 30.0.91; ERC 5.6.0.30.1: Customizing erc-modules loads ERC when starting Emacs

[J.P.]

"J.P." <jp@neverwas.me> writes:

>> All in all, I'd prefer to leave this alone in Emacs 30.  We have time
>> to try reverting this on master and seeing whether it's a net win or a
>> net loss, given the past history of the issue.  (AFAIU, if you remove
>> this line, some change is pertinent in the manual?)

It's been reverted on master for ten days now with no complaints:

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

If that's long enough to qualify as a net win, can we proceed with a
backport?