From 2ddc480f4417775d6bf8ebcfc27b8cd7fa761a7d Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Sun, 25 Dec 2022 21:36:53 -0800 Subject: [PATCH] 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 ++++++++++++++++++- .../resources/networks/no-module/basic.eld | 44 +++++++++++++++ 8 files changed, 153 insertions(+), 12 deletions(-) create mode 100644 test/lisp/erc/resources/networks/no-module/basic.eld diff --git a/doc/misc/erc.texi b/doc/misc/erc.texi index 2ab2e908940..249b58c73d8 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 76439f1d068..b577047ebcb 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 43c5faad638..6820bf0d1a3 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 2e2d0930118..f05a98be16d 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 78d02a46381..23110d74b5e 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 6a5e0018964..16a0aba77b1 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 f5b8df6f4a1..e6db40c5efb 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 00000000000..f1bdbd1219f --- /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")) -- 2.39.2