emacs-29 2ddc480f44 2/2: Warn of absent networks module in ERC

[F. Jason Park]

branch: emacs-29
commit 2ddc480f4417775d6bf8ebcfc27b8cd7fa761a7d
Author: F. Jason Park <jp@neverwas.me>
Commit: F. Jason Park <jp@neverwas.me>

    Warn of absent networks module in ERC
    
    * doc/misc/erc.texi: Add linkable note in Modules chapter about some
    modules being required.  Also tweak markup in auth-source section.
    * etc/ERC-NEWS: Mention the special role of `networks'.
    * lisp/erc/erc-backend.el (erc--server-post-connect-hook): Add
    internal hook for core modules to perform post-network-process,
    pre-protocol config validation even when they haven't been loaded.
    (erc--register-connection): Run `erc--server-post-connect-hook'.
    * lisp/erc/erc-networks.el (erc-networks--bouncer-targets,
    erc-networks-on-MOTD-end): Fix comments and doc strings.  Also change
    former from constant to internal variable in case adjustment needed
    between releases.
    (erc-networks--warn-on-connect): New function to warn about the
    `networks' module being absent from `erc-modules'.  This could
    probably run at any time up to and including when the logical IRC
    connection is established, but doing so at the process/protocol
    boundary seems ideal.
    * lisp/erc/erc-sasl.el (erc--register-connection): Defer to base
    method instead of calling `erc-login' explicitly.
    * lisp/erc/erc.el (erc-generate-new-buffer-name): Don't reconcile
    buffer names when networks module not in play.
    (erc-format-target-and/or-network): Don't assume networks module
    loaded.
    * test/lisp/erc/erc-scenarios-base-unstable.el:
    (erc-scenarios-networks-no-module): New test.
    * test/lisp/erc/resources/networks/no-module/basic.eld: New test data
    file.  (Bug#60331.)
---
 doc/misc/erc.texi                                  | 12 ++++-
 etc/ERC-NEWS                                       | 11 ++++-
 lisp/erc/erc-backend.el                            | 10 ++++
 lisp/erc/erc-networks.el                           | 26 ++++++++---
 lisp/erc/erc-sasl.el                               |  2 +-
 lisp/erc/erc.el                                    |  6 ++-
 test/lisp/erc/erc-scenarios-base-unstable.el       | 54 +++++++++++++++++++++-
 .../erc/resources/networks/no-module/basic.eld     | 44 ++++++++++++++++++
 8 files changed, 153 insertions(+), 12 deletions(-)

diff --git a/doc/misc/erc.texi b/doc/misc/erc.texi
index 2ab2e90894..249b58c73d 100644
--- a/doc/misc/erc.texi
+++ b/doc/misc/erc.texi
@@ -529,6 +529,16 @@ Translate morse code in messages
 
 @end table
 
+@anchor{Required Modules}
+@subheading Required Modules
+@cindex required modules
+
+Note that some modules are essential to core IRC operations and thus
+not listed above.  You can nevertheless still remove these, but doing
+so demands special precautions to avoid degrading the user experience.
+At present, the only such module is @code{networks}, whose library ERC
+always loads anyway.
+
 @subheading Local Modules
 @cindex local modules
 
@@ -1290,7 +1300,7 @@ When preparing entries for your backend, it may help to 
get a feel for
 how ERC and its modules conduct searches, especially when exploring a
 new context, such as channel keys.  (Hint: in such situations, try
 temporarily setting the variable @code{auth-source-debug} to @code{t}
-and checking @samp{*Messages*} periodically for insights into how
+and checking @file{*Messages*} periodically for insights into how
 auth-source is operating.)  Overall, though, ERC tries to be
 consistent in performing queries across various authentication
 contexts.  Here's what to expect with respect to the @samp{host}
diff --git a/etc/ERC-NEWS b/etc/ERC-NEWS
index 76439f1d06..b577047ebc 100644
--- a/etc/ERC-NEWS
+++ b/etc/ERC-NEWS
@@ -39,6 +39,14 @@ anew.  The pre-5.4 "disabled" behavior has been restored and 
will
 remain accessible for the foreseeable future, warts and all (e.g.,
 with its often superfluous "/DIALED-HOST" suffixing always present).
 
+** The 'networks' module is now quasi-required.
+The 'networks' module is now all but required for everyday interactive
+use.  A default member of 'erc-modules' since ERC 5.3, 'networks' has
+grown increasingly integral to core client operations over the years.
+From now on, only the most essential operations will be officially
+supported in its absence, and users will see a warning upon
+entry-point invocation when it's not present.
+
 ** Tighter auth-source integration with bigger changes on the horizon.
 The days of hit-and-miss auth-source queries are hopefully behind us.
 With the overhaul of the services module temporarily shelved and the
@@ -111,7 +119,8 @@ and 'erc-backend'.
 
 The function 'erc-network' always returns non-nil in server and target
 buffers belonging to a successfully established IRC connection, even
-after that connection has been closed.
+after that connection has been closed.  (Also see the note in the
+section above about the 'networks' module basically being mandatory.)
 
 In 5.4, support for network symbols as keys was added for
 'erc-autojoin-channels-alist'.  This has been extended to include
diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el
index 43c5faad63..6820bf0d1a 100644
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -320,6 +320,15 @@ session when reconnecting.  Once `erc-reuse-buffers' is 
retired
 and fully removed, modules can switch to leveraging the
 `permanent-local' property instead.")
 
+(defvar erc--server-post-connect-hook '(erc-networks--warn-on-connect)
+  "Functions to run when a network connection is successfully opened.
+Though internal, this complements `erc-connect-pre-hook' in that
+it bookends the process rather than the logical connection, which
+is the domain of `erc-before-connect' and `erc-after-connect'.
+Note that unlike `erc-connect-pre-hook', this only runs in server
+buffers, and it does so immediately before the first protocol
+exchange.")
+
 (defvar-local erc-server-timed-out nil
   "Non-nil if the IRC server failed to respond to a ping.")
 
@@ -646,6 +655,7 @@ The current buffer is given by BUFFER."
 
 (cl-defmethod erc--register-connection ()
   "Perform opening IRC protocol exchange with server."
+  (run-hooks 'erc--server-post-connect-hook)
   (erc-login))
 
 (defvar erc--server-connect-dumb-ipv6-regexp
diff --git a/lisp/erc/erc-networks.el b/lisp/erc/erc-networks.el
index 2e2d093011..f05a98be16 100644
--- a/lisp/erc/erc-networks.el
+++ b/lisp/erc/erc-networks.el
@@ -1472,14 +1472,16 @@ to be a false alarm.  If `erc-reuse-buffers' is nil, let
           (t (rename-buffer (generate-new-buffer-name name)))))
   nil)
 
-;; Soju v0.4.0 only sends ISUPPORT on upstream reconnect, so this
-;; doesn't apply.  ZNC 1.8.2, however, still sends the entire burst.
-(defconst erc-networks--bouncer-targets '(*status bouncerserv)
-  "Case-mapped symbols matching known bouncer service-bot targets.")
+;; Soju v0.4.0 sends ISUPPORT and nothing else on upstream reconnect,
+;; so this actually doesn't apply.  ZNC 1.8.2, however, still sends
+;; the entire burst.
+(defvar erc-networks--bouncer-targets '(*status bouncerserv)
+  "Symbols matching proxy-bot targets.")
 
 (defun erc-networks-on-MOTD-end (proc parsed)
-  "Call on-connect functions with server PROC and PARSED message.
-This must run before `erc-server-connected' is set."
+  "Call on-connect functions with server PROC and PARSED message."
+  ;; This should normally run before `erc-server-connected' is set.
+  ;; However, bouncers and other proxies may interfere with that.
   (when erc-server-connected
     (unless (erc-buffer-filter (lambda ()
                                  (and erc--target
@@ -1502,6 +1504,18 @@ This must run before `erc-server-connected' is set."
   ((remove-hook 'erc-server-376-functions #'erc-networks-on-MOTD-end)
    (remove-hook 'erc-server-422-functions #'erc-networks-on-MOTD-end)))
 
+(defun erc-networks--warn-on-connect ()
+  "Emit warning when the `networks' module hasn't been loaded.
+Ideally, do so upon opening the network process."
+  (unless (or erc--target erc-networks-mode)
+    (require 'info nil t)
+    (let ((m (concat "Required module `networks' not loaded.  If this "
+                     " was unexpected, please add it to `erc-modules'.")))
+      ;; Assume the server buffer has been marked as active.
+      (erc-display-error-notice
+       nil (concat m "  See Info:\"(erc) Required Modules\" for more."))
+      (lwarn 'erc :warning m))))
+
 (defun erc-ports-list (ports)
   "Return a list of PORTS.
 
diff --git a/lisp/erc/erc-sasl.el b/lisp/erc/erc-sasl.el
index 78d02a4638..23110d74b5 100644
--- a/lisp/erc/erc-sasl.el
+++ b/lisp/erc/erc-sasl.el
@@ -435,7 +435,7 @@ Otherwise, expect it to disappear in subsequent versions.")
                (if (eq :user (alist-get 'user erc-sasl--options))
                    (erc-current-nick)
                  erc-session-username)))
-          (erc-login))
+          (cl-call-next-method))
         (when erc-sasl--send-cap-ls
           (erc-server-send "CAP REQ :sasl"))
         (erc-server-send (format "AUTHENTICATE %s" m)))
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 6a5e001896..16a0aba77b 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1607,7 +1607,8 @@ same manner."
   (when target ; compat
     (setq tgt-info (erc--target-from-string target)))
   (if tgt-info
-      (let* ((esid (erc-networks--id-symbol erc-networks--id))
+      (let* ((esid (and erc-networks--id
+                        (erc-networks--id-symbol erc-networks--id)))
              (name (if esid
                        (erc-networks--reconcile-buffer-names tgt-info
                                                              erc-networks--id)
@@ -6760,7 +6761,8 @@ This should be a string with substitution variables 
recognized by
 If the name of the network is not available, then use the
 shortened server name instead."
   (if-let ((erc--target)
-           (name (if-let ((esid (erc-networks--id-symbol erc-networks--id)))
+           (name (if-let ((erc-networks--id)
+                          (esid (erc-networks--id-symbol erc-networks--id)))
                      (symbol-name esid)
                    (erc-shorten-server-name (or erc-server-announced-name
                                                 erc-session-server)))))
diff --git a/test/lisp/erc/erc-scenarios-base-unstable.el 
b/test/lisp/erc/erc-scenarios-base-unstable.el
index f5b8df6f4a..e6db40c5ef 100644
--- a/test/lisp/erc/erc-scenarios-base-unstable.el
+++ b/test/lisp/erc/erc-scenarios-base-unstable.el
@@ -24,7 +24,7 @@
   (let ((load-path (cons (ert-resource-directory) load-path)))
     (require 'erc-scenarios-common)))
 
-(eval-when-compile (require 'erc-join))
+(eval-when-compile (require 'erc-join) (require 'warnings))
 
 ;; Not unstable, but stashed here for now
 
@@ -132,4 +132,56 @@
               (not (setq failed (zerop (cl-decf tries)))))))
     (should-not failed)))
 
+;; The `erc-networks' library has slowly become a hard dependency of
+;; the interactive client since its incorporation in 2006.  But its
+;; module, which was added in ERC 5.3 (2008) and thereafter loaded by
+;; default, only became quasi-required in ERC 5.5 (2022).  Despite
+;; this, a basic connection should still always succeed, at least long
+;; enough to warn users that their setup is abnormal.  Of course,
+;; third-party code intentionally omitting the module will have to
+;; override various erc-server-*-functions to avoid operating in a
+;; degraded state, which has likely been the case for a while.
+
+(ert-deftest erc-scenarios-networks-no-module ()
+  :tags '(:expensive-test)
+  (erc-scenarios-common-with-cleanup
+      ((erc-scenarios-common-dialog "networks/no-module")
+       (erc-server-flood-penalty 0.1)
+       (erc-networks-mode-orig erc-networks-mode)
+       (dumb-server (erc-d-run "localhost" t 'basic))
+       (port (process-contact dumb-server :service))
+       (erc-modules (remq 'networks erc-modules))
+       (warning-suppress-log-types '((erc)))
+       (expect (erc-d-t-make-expecter)))
+
+    (erc-networks-mode -1)
+    (ert-info ("Connect and retain dialed name")
+      (with-current-buffer (erc :server "127.0.0.1"
+                                :port port
+                                :nick "tester"
+                                :user "tester"
+                                :full-name "tester")
+        (funcall expect 10 "Required module `networks' not loaded")
+        (funcall expect 10 "This server is in debug mode")
+        ;; Buffer not named after network
+        (should (string= (buffer-name) (format "127.0.0.1:%d" port)))
+        (erc-cmd-JOIN "#chan")))
+
+    (ert-info ("Join #chan, change nick, query op")
+      (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#chan"))
+        (funcall expect 20 "Even at thy teat thou")
+        (erc-cmd-NICK "dummy")
+        (funcall expect 10 "Your new nickname is dummy")
+        (erc-scenarios-common-say "/msg alice hi")))
+
+    (ert-info ("Switch to query and quit")
+      (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "alice"))
+        (funcall expect 20 "bye"))
+
+      (with-current-buffer (format "127.0.0.1:%d" port)
+        (erc-cmd-QUIT "")
+        (funcall expect 10 "finished")))
+    (when erc-networks-mode-orig
+      (erc-networks-mode +1))))
+
 ;;; erc-scenarios-base-unstable.el ends here
diff --git a/test/lisp/erc/resources/networks/no-module/basic.eld 
b/test/lisp/erc/resources/networks/no-module/basic.eld
new file mode 100644
index 0000000000..f1bdbd1219
--- /dev/null
+++ b/test/lisp/erc/resources/networks/no-module/basic.eld
@@ -0,0 +1,44 @@
+;; -*- mode: lisp-data; -*-
+((nick 10 "NICK tester"))
+((user 1 "USER tester 0 * :tester")
+ (0.00 ":irc.foonet.org 001 tester :Welcome to the foonet IRC Network tester")
+ (0.00 ":irc.foonet.org 002 tester :Your host is irc.foonet.org, running 
version ergo-v2.8.0")
+ (0.00 ":irc.foonet.org 003 tester :This server was created Mon, 12 Dec 2022 
01:25:38 UTC")
+ (0.00 ":irc.foonet.org 004 tester irc.foonet.org ergo-v2.8.0 BERTZios 
CEIMRUabefhiklmnoqstuv Iabefhkloqv")
+ (0.00 ":irc.foonet.org 005 tester AWAYLEN=390 BOT=B CASEMAPPING=ascii 
CHANLIMIT=#:100 CHANMODES=Ibe,k,fl,CEMRUimnstu CHANNELLEN=64 CHANTYPES=# 
ELIST=U EXCEPTS EXTBAN=,m FORWARD=f INVEX KICKLEN=390 :are supported by this 
server")
+ (0.00 ":irc.foonet.org 005 tester MAXLIST=beI:60 MAXTARGETS=4 MODES 
MONITOR=100 NETWORK=foonet NICKLEN=32 PREFIX=(qaohv)~&@%+ STATUSMSG=~&@%+ 
TARGMAX=NAMES:1,LIST:1,KICK:,WHOIS:1,USERHOST:10,PRIVMSG:4,TAGMSG:4,NOTICE:4,MONITOR:100
 TOPICLEN=390 UTF8MAPPING=rfc8265 UTF8ONLY WHOX :are supported by this server")
+ (0.01 ":irc.foonet.org 005 tester draft/CHATHISTORY=100 :are supported by 
this server")
+ (0.00 ":irc.foonet.org 251 tester :There are 0 users and 4 invisible on 1 
server(s)")
+ (0.00 ":irc.foonet.org 252 tester 0 :IRC Operators online")
+ (0.00 ":irc.foonet.org 253 tester 0 :unregistered connections")
+ (0.00 ":irc.foonet.org 254 tester 1 :channels formed")
+ (0.00 ":irc.foonet.org 255 tester :I have 4 clients and 0 servers")
+ (0.00 ":irc.foonet.org 265 tester 4 4 :Current local users 4, max 4")
+ (0.01 ":irc.foonet.org 266 tester 4 4 :Current global users 4, max 4")
+ (0.00 ":irc.foonet.org 422 tester :MOTD File is missing"))
+
+((mode 10 "MODE tester +i")
+ (0.00 ":irc.foonet.org 221 tester +i")
+ (0.00 ":irc.foonet.org NOTICE tester :This server is in debug mode and is 
logging all user I/O. If you do not wish for everything you send to be readable 
by the server owner(s), please disconnect."))
+
+((join 10 "JOIN #chan")
+ (0.03 ":tester!~u@z5d6jyn8pwxge.irc JOIN #chan"))
+
+((~nick 10 "NICK dummy")
+ (0.01 ":tester!~u@z5d6jyn8pwxge.irc NICK dummy"))
+
+((mode-1 10 "MODE #chan")
+ (0.01 ":irc.foonet.org 353 tester = #chan :@alice bob foonet tester")
+ (0.00 ":irc.foonet.org 366 tester #chan :End of NAMES list")
+ (0.03 ":irc.foonet.org 324 tester #chan +nt")
+ (0.00 ":irc.foonet.org 329 tester #chan 1670808354")
+ (0.00 ":bob!~u@d6ftaiqzk8x2k.irc PRIVMSG #chan :tester, welcome!")
+ (0.00 ":alice!~u@d6ftaiqzk8x2k.irc PRIVMSG #chan :tester, welcome!")
+ (0.03 ":bob!~u@d6ftaiqzk8x2k.irc PRIVMSG #chan :alice: Forbear it therefore; 
give your cause to heaven.")
+ (0.01 ":alice!~u@d6ftaiqzk8x2k.irc PRIVMSG #chan :bob: Even at thy teat thou 
hadst thy tyranny."))
+
+((privmsg 10 "PRIVMSG alice :hi")
+ (0.00 ":alice!~u@d6ftaiqzk8x2k.irc PRIVMSG dummy :bye"))
+
+((quit 10 "QUIT :\2ERC\2")
+ (0.03 ":dummy!~u@z5d6jyn8pwxge.irc QUIT :Quit: \2ERC\2"))