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?

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

[J.P.]

Dear auxiliary maintainers,

It seems Eli has tired of my nagging or is otherwise indisposed when it
comes to this issue. To summarize:

- Due to my own stupidity, the version of ERC in Emacs 30 autoloads the
  option `erc-modules', which is the most important of ERC's options and
  among the first that users typically customize.

- Because of this change, the very presence of `erc-modules' in one's
  `custom-file' now loads all of ERC at startup, including any library
  housing a member module, be it built-in or third-party.

- Users who no longer use ERC but still have a customization entry lying
  around will also be affected. Likewise for users who prefer running
  multiple Emacs instances to segregate concerns.

- This issue won't be solvable by installing ERC 5.6.1 from ELPA, where
  the problematic line will have been removed, because the autoload is
  permanently baked into lisp/ldefs-boot.el.

- The problematic change will be new in Emacs 30.1.

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

> Eli Zaretskii <eliz@gnu.org> 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?

It's been two weeks now since I was tasked with reverting this on master
in order to assess the damage, of which none has since been reported.

Apologies if I'm out of line in pressing the issue, but I'm driven by a
need to advocate for ERC's users, who've suffered greatly in the past
due to my cowardliness in similar situations [1]. As such, I would very
much appreciate a final verdict on this matter.

Thanks,
J.P.


[1] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=62833#14

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

[Stefan Kangas]

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

> - Due to my own stupidity, the version of ERC in Emacs 30 autoloads the
>   option `erc-modules', which is the most important of ERC's options and
>   among the first that users typically customize.
>
> - Because of this change, the very presence of `erc-modules' in one's
>   `custom-file' now loads all of ERC at startup, including any library
>   housing a member module, be it built-in or third-party.
>
> - Users who no longer use ERC but still have a customization entry lying
>   around will also be affected. Likewise for users who prefer running
>   multiple Emacs instances to segregate concerns.
>
> - This issue won't be solvable by installing ERC 5.6.1 from ELPA, where
>   the problematic line will have been removed, because the autoload is
>   permanently baked into lisp/ldefs-boot.el.
>
> - The problematic change will be new in Emacs 30.1.

This does not sound ideal, indeed.

I can only add that some of our users are very concerned with Emacs's
startup time, and spend a lot of time optimizing it.  Unexpectedly
pulling in all of ERC in some cases certainly won't help them.

> "J.P." <jp@neverwas.me> writes:
>
>> Eli Zaretskii <eliz@gnu.org> 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?
>
> It's been two weeks now since I was tasked with reverting this on master
> in order to assess the damage, of which none has since been reported.
>
> Apologies if I'm out of line in pressing the issue, but I'm driven by a
> need to advocate for ERC's users, who've suffered greatly in the past
> due to my cowardliness in similar situations [1]. As such, I would very
> much appreciate a final verdict on this matter.

I assume that we are talking about cherry-picking commit 1854f2751e3f to
the emacs-30 branch.

Can removing the autoload cookie cause an issue outside of ERC, or for
non-users of ERC?  If it cannot, I don't know that I'm in a better
position than you, being the ERC maintainer, to determine what kind of
negative impact removing it might have.  If anything, it sounds like it
is more risky for non-users of ERC to leave things as is?

In summary, my view is that removing it should be low risk, and it fixes
a known bug.  It's arguably minor, but does affect startup performance.
So I think it sounds good to have the patch on emacs-30.

Let's see if Eli or Andrea has anything to add here first, though.

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

[Corwin Brust]

Hi All,

On Fri, Nov 1, 2024 at 9:05 PM Stefan Kangas <stefankangas@gmail.com> wrote:
>
> "J.P." <jp@neverwas.me> writes:
>
> I can only add that some of our users are very concerned with Emacs's
> startup time, and spend a lot of time optimizing it.  Unexpectedly
> pulling in all of ERC in some cases certainly won't help them.
>
> > "J.P." <jp@neverwas.me> writes:
> >
> >> Eli Zaretskii <eliz@gnu.org> 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?
> >
> > It's been two weeks now since I was tasked with reverting this on master
> > in order to assess the damage, of which none has since been reported.
> >
> > Apologies if I'm out of line in pressing the issue, but I'm driven by a
> > need to advocate for ERC's users, who've suffered greatly in the past
> > due to my cowardliness in similar situations [1]. As such, I would very
> > much appreciate a final verdict on this matter.
>
> I assume that we are talking about cherry-picking commit 1854f2751e3f to
> the emacs-30 branch.
>
> Can removing the autoload cookie cause an issue outside of ERC, or for
> non-users of ERC?  If it cannot, I don't know that I'm in a better
> position than you, being the ERC maintainer, to determine what kind of
> negative impact removing it might have.  If anything, it sounds like it
> is more risky for non-users of ERC to leave things as is?
>
> In summary, my view is that removing it should be low risk, and it fixes
> a known bug.  It's arguably minor, but does affect startup performance.
> So I think it sounds good to have the patch on emacs-30.
>
> Let's see if Eli or Andrea has anything to add here first, though.
>

Thank you JP and Stefan for diligence on this.  I had flagged this for
a reply but did not manage to make one before now.  (Two weeks
already?  Good grief.)

Perhaps importantly: I strongly suspect this bug is responsible for
corruption of my customize file.  (Unfortunately I had a bunch of
stuff in there and wasn't good and pushed my changes to that
particular file to the repo where I version my config, so I've lost a
bit of work.  On the plus side: it's generally of a cosmetic nature,
mostly face tweaks which I found it convenient to "try out" using
customize and then, because they were good, never got around to
encoding into proper elisp.)  I didn't/haven't completed any sort of
minimal reproducer: I could be completely wrong that this bug is what
caused my configuration settings to get blown away.  But the timing is
right.

I will also mention, back to the "known issue" caused by this, that I
maintain separate "desktop shortcuts" for launching Emacs with and
without automatically launching ERC connections.  It would be nice if
this arrangement (once again) had the intended effect of not loading
ERC when I'm not going to need it in the given session.

In any event: +1

I'd be grateful to see this resolved before cutting 30.1

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

[J.P.]

Hi Stefan, Corwin, all,

Corwin Brust <corwin@bru.st> writes:

> Hi All,
>
> On Fri, Nov 1, 2024 at 9:05 PM Stefan Kangas <stefankangas@gmail.com> wrote:
>>
>> "J.P." <jp@neverwas.me> writes:
>>
>> I can only add that some of our users are very concerned with Emacs's
>> startup time, and spend a lot of time optimizing it.  Unexpectedly
>> pulling in all of ERC in some cases certainly won't help them.
>>
>> > "J.P." <jp@neverwas.me> writes:
>> >
>> >> Eli Zaretskii <eliz@gnu.org> 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?
>> >
>> > It's been two weeks now since I was tasked with reverting this on master
>> > in order to assess the damage, of which none has since been reported.
>> >
>> > Apologies if I'm out of line in pressing the issue, but I'm driven by a
>> > need to advocate for ERC's users, who've suffered greatly in the past
>> > due to my cowardliness in similar situations [1]. As such, I would very
>> > much appreciate a final verdict on this matter.
>>
>> I assume that we are talking about cherry-picking commit 1854f2751e3f to
>> the emacs-30 branch.

Yes, exactly.

FWIW, something like

  diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
  index 30641c2bd88..807ca81ed6b 100644
  --- a/lisp/erc/erc.el
  +++ b/lisp/erc/erc.el
  @@ -2263,7 +2263,7 @@ 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")
  +;;;###autoload(custom-autoload 'erc-modules "erc" 'noset)

   (defcustom erc-modules '( autojoin button completion fill imenu irccontrols
                             list match menu move-to-prompt netsplit

does appear to suppress autoloading on startup while still fulfilling
our original M-x customize-option RET mission (IOW, exactly what we
want). However, I'd sooner just revert to the way things have been for
20+ years. ("Once bitten," as they say.)

>>
>> Can removing the autoload cookie cause an issue outside of ERC, or for
>> non-users of ERC?  If it cannot, I don't know that I'm in a better
>> position than you, being the ERC maintainer, to determine what kind of
>> negative impact removing it might have.  If anything, it sounds like it
>> is more risky for non-users of ERC to leave things as is?
>>
>> In summary, my view is that removing it should be low risk, and it fixes
>> a known bug.  It's arguably minor, but does affect startup performance.
>> So I think it sounds good to have the patch on emacs-30.
>>
>> Let's see if Eli or Andrea has anything to add here first, though.
>>
>
> Thank you JP and Stefan for diligence on this.  I had flagged this for
> a reply but did not manage to make one before now.  (Two weeks
> already?  Good grief.)
>
> Perhaps importantly: I strongly suspect this bug is responsible for
> corruption of my customize file.  (Unfortunately I had a bunch of
> stuff in there and wasn't good and pushed my changes to that
> particular file to the repo where I version my config, so I've lost a
> bit of work.  On the plus side: it's generally of a cosmetic nature,
> mostly face tweaks which I found it convenient to "try out" using
> customize and then, because they were good, never got around to
> encoding into proper elisp.)  I didn't/haven't completed any sort of
> minimal reproducer: I could be completely wrong that this bug is what
> caused my configuration settings to get blown away.  But the timing is
> right.

So sorry about your data loss and for whatever hand my sloppiness might
have played in such a tragedy. It does indeed sound like a serious issue
that bears investigating. Unfortunately, I'm still a certified dunce
when it comes to all things Customize (but I do pledge to rectify this
situation eventually).

>
> I will also mention, back to the "known issue" caused by this, that I
> maintain separate "desktop shortcuts" for launching Emacs with and
> without automatically launching ERC connections.  It would be nice if
> this arrangement (once again) had the intended effect of not loading
> ERC when I'm not going to need it in the given session.
>
> In any event: +1
>
> I'd be grateful to see this resolved before cutting 30.1

Appreciate the input as always.

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, Eli Zaretskii <eliz@gnu.org>
> Date: Fri, 01 Nov 2024 17:49:43 -0700
> 
> Dear auxiliary maintainers,
> 
> It seems Eli has tired of my nagging or is otherwise indisposed when it
> comes to this issue.

I have not.  But you made the mistake of quoting only a little bit
from the previous message, from which I couldn't even understand those
were my words.  And I generally avoid commenting on changes related to
ERC because I don't use it and am not familiar with its code.

In the future, if it's my/our decision that you are seeking, please
quote more of the context and ask for our response explicitly, to
avoid these misunderstandings.

> > Eli Zaretskii <eliz@gnu.org> 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?
> 
> It's been two weeks now since I was tasked with reverting this on master
> in order to assess the damage, of which none has since been reported.
> 
> Apologies if I'm out of line in pressing the issue, but I'm driven by a
> need to advocate for ERC's users, who've suffered greatly in the past
> due to my cowardliness in similar situations [1]. As such, I would very
> much appreciate a final verdict on this matter.

I'd prefer to wait a little longer for any fallout of reverting this
on master.  IME, two weeks is not long enough, as many people take
several weeks to update their builds.  Emacs 30 is not going to be
released tomorrow, so we still have time.

Let's talk about this in two more weeks, ok?

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:

>> From: "J.P." <jp@neverwas.me>
>> Cc: 73812@debbugs.gnu.org, emacs-erc@gnu.org, Eli Zaretskii <eliz@gnu.org>
>> Date: Fri, 01 Nov 2024 17:49:43 -0700
>> 
>> Dear auxiliary maintainers,
>> 
>> It seems Eli has tired of my nagging or is otherwise indisposed when it
>> comes to this issue.
>
> I have not.  But you made the mistake of quoting only a little bit
> from the previous message, from which I couldn't even understand those
> were my words.  And I generally avoid commenting on changes related to
> ERC because I don't use it and am not familiar with its code.
>
> In the future, if it's my/our decision that you are seeking, please
> quote more of the context and ask for our response explicitly, to
> avoid these misunderstandings.

Makes sense. My apologies.

>
>> > Eli Zaretskii <eliz@gnu.org> 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?
>> 
>> It's been two weeks now since I was tasked with reverting this on master
>> in order to assess the damage, of which none has since been reported.
>> 
>> Apologies if I'm out of line in pressing the issue, but I'm driven by a
>> need to advocate for ERC's users, who've suffered greatly in the past
>> due to my cowardliness in similar situations [1]. As such, I would very
>> much appreciate a final verdict on this matter.
>
> I'd prefer to wait a little longer for any fallout of reverting this
> on master.  IME, two weeks is not long enough, as many people take
> several weeks to update their builds.  Emacs 30 is not going to be
> released tomorrow, so we still have time.
>
> Let's talk about this in two more weeks, ok?

Sounds good. Thanks.

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: stefankangas@gmail.com,  acorallo@gnu.org,  73812@debbugs.gnu.org,
>   emacs-erc@gnu.org
> Date: Sat, 02 Nov 2024 08:40:54 -0700
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Apologies if I'm out of line in pressing the issue, but I'm driven by a
> >> need to advocate for ERC's users, who've suffered greatly in the past
> >> due to my cowardliness in similar situations [1]. As such, I would very
> >> much appreciate a final verdict on this matter.
> >
> > I'd prefer to wait a little longer for any fallout of reverting this
> > on master.  IME, two weeks is not long enough, as many people take
> > several weeks to update their builds.  Emacs 30 is not going to be
> > released tomorrow, so we still have time.
> >
> > Let's talk about this in two more weeks, ok?
> 
> Sounds good. Thanks.

If no issues were reported on master due to this change, feel free to
backport to the emacs-30 branch.

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:

>> From: "J.P." <jp@neverwas.me>
>> Cc: stefankangas@gmail.com,  acorallo@gnu.org,  73812@debbugs.gnu.org,
>>   emacs-erc@gnu.org
>> Date: Sat, 02 Nov 2024 08:40:54 -0700
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> Apologies if I'm out of line in pressing the issue, but I'm driven by a
>> >> need to advocate for ERC's users, who've suffered greatly in the past
>> >> due to my cowardliness in similar situations [1]. As such, I would very
>> >> much appreciate a final verdict on this matter.
>> >
>> > I'd prefer to wait a little longer for any fallout of reverting this
>> > on master.  IME, two weeks is not long enough, as many people take
>> > several weeks to update their builds.  Emacs 30 is not going to be
>> > released tomorrow, so we still have time.
>> >
>> > Let's talk about this in two more weeks, ok?
>> 
>> Sounds good. Thanks.
>
> If no issues were reported on master due to this change, feel free to
> backport to the emacs-30 branch.

Done. Thanks and closing.

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

[J.P.]

Some additional notes for future people investigating similar problems.

To recap, when trying to autoload the option `erc-modules' for the
purpose of getting M-x customize-option RET to work OOTB (a common
request), I found that

  ;;;###autoload
  (defcustom erc-modules ...)

meant we needed to migrate symbols used in the option's complicated :set
function to the main library or else suffer an error during loaddefs
generation. In ERC's case, such a migration would have defied the
purpose of keeping a common base library in erc-common.el.

I also noticed that adding the cookie placed the entire, massive
`defcustom' definition in lisp/loaddefs.el, which seemed rather
unsightly. And it also worried me, perhaps irrationally so, that such an
inclusion might one day potentially interfere with newer versions
installed from ELPA, though this is likely dubious.

So I instead opted for a manual

  ;;;###autoload(custom-autoload 'erc-modules "erc")

which "worked" but caused ERC to load on startup for anyone with a

  (custom-set-variables '(erc-modules ()))

in their `custom-file'.

As mentioned in this ticket's main thread, one possible interpretation
is that I merely forgot to include a non-nil `noset' argument to
`custom-autoload', which does indeed produce the desired behavior: ERC
only loads when a user actually customizes the option. (This has been
observed both on master as well as on upgrades installed atop 27+.)

Unfortunately (for ERC), it seems `loaddefs-generate--make-autoload'
omits `noset' for options that have a `:set' function, which may seem
rather obvious. Despite this, it's still unclear to me whether our use
case aligns fully with those semantics, at least as laid out in the
initial discussions that led to the parameter's introduction [1] (as
well as various mentions that have followed over the years [2]).

Rather than press the issue, I figured we might as well abide by the
guidelines set forth in the info node `(elisp) When to Autoload'

  • Don't autoload a user option just so that a user can set it.

which makes it sound like autoloading options for our use case is
basically deprecated nowadays. Anyway, if new users continue to
complain, we could perhaps add a new autoloaded entry-point command,
something like a `erc-customize-modules', that would basically do

  (require 'erc)
  (customize-option 'erc-modules)

However, the current workaround of recommending C-h v erc-modules RET
doesn't seem too strenuous a detour IMO.

[1] https://lists.gnu.org/archive/html/emacs-devel/2006-07/msg00270.html
[2] https://lists.gnu.org/archive/html/emacs-devel/2009-01/msg00128.html