Package: guix-patches;
Reported by: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Date: Mon, 31 Mar 2025 02:29:07 UTC
Severity: normal
Tags: patch
Done: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Bug is archived. No further changes may be made.
Message #11 received at 77396 <at> debbugs.gnu.org (full text, mbox):
From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com> To: Ludovic Courtès <ludo <at> gnu.org> Cc: 77396 <at> debbugs.gnu.org Subject: Re: [bug#77396] [PATCH] services: Add ngircd-service-type. Date: Thu, 03 Apr 2025 15:36:40 +0900
Hi!
Thanks for the prompt review!
Ludovic Courtès <ludo <at> gnu.org> writes:
[...]
>> +@cindex IRC (Internet Relay Chat)
>> +
>> +@url{https://ngircd.barton.de/, ngIRCd}, is a lightweight @acronym{IRCd,
>> +Internet Relay Chat daemon}, which can be used to host your own IRC
>> +server.
>
> Could you add an example configuration, as is usually done for services?
> It’s always nice to have something to copy/paste to get started.
I've added a not-too-serious one:
--8<---------------cut here---------------start------------->8---
modified doc/guix.texi
@@ -30365,6 +30365,21 @@ Messaging Services
@defvar ngircd-service-type
The service type for ngIRCd. Its value is a @code{ngircd-configuration}
object, documented below.
+
+A simple example configuration could look like:
+
+@lisp
+(service ngircd-service-type
+ (ngircd-configuration
+ (channels
+ (list (ngircd-channel
+ (name "#fruits")
+ (topic "All things fruits -- veggies are off-topic"))))
+ (operators
+ (list (ngircd-operator
+ (name "mikan")
+ (password "tomatoes-are-fruits/carrots-are-not"))))))
+@end lisp
@end defvar
@c To regenerate the rest of this section documentation, use the
--8<---------------cut here---------------end--------------->8---
>> + <ngircd-configuration>
>> + ngircd-configuration
>> + ngircd-configuration?
>> + <ngircd-global>
>> + ngircd-global
>> + ngircd-global?
>> + <ngircd-limits>
>> + ngircd-limits
>> + ngircd-limits?
>> + <ngircd-options>
>> + ngircd-options
>> + ngircd-options?
>> + <ngircd-ssl>
>> + ngircd-ssl
>> + ngircd-ssl?
>> + <ngircd-operator>
>> + ngircd-operator
>> + ngircd-operator?
>> + <ngircd-server>
>> + ngircd-server
>> + ngircd-server?
>> + <ngircd-channel>
>
> Please don’t export record type descriptors like <ngircd-configuration>
> since that makes it impossible to provide any guarantee (ABI, validity
> of fields, etc.).
Since there would be so many fields to export, I was hoping to punt on
exporting all individual accessors, and at least let users be able to
use 'match-record', which requires the record type. Isn't match-record
intended to be used by users as well as service authors?
>> +(define (ngircd-shepherd-service config)
>> + (match-record config <ngircd-configuration>
>> + (ngircd debug? global)
>> + (let ((ngircd.conf (serialize-ngircd-configuration config))
>> + (ngircd (file-append ngircd "/sbin/ngircd"))
>> + (pid-file (ngircd-global-pid-file global))
>> + (user group (ngircd-user+group config)))
>> + (list (shepherd-service
>> + (provision '(ngircd))
>> + (requirement '(user-processes networking syslogd))
>
> I would drop ‘networking’: see <https://issues.guix.gnu.org/66306>.
I've read the link above, and I think it's probably safer to keep it,
since the interfaces that should be listened can be configured by the
user. Also, the 'contrib' systemd service uses 'After=network.target'
[0].
[0] https://github.com/ngircd/ngircd/blob/512af135d06e7dad93f51eae51b3979e1d4005cc/contrib/ngircd.service#L7
>> + (actions (list (shepherd-configuration-action ngircd.conf)))
>> + (start #~(make-forkexec-constructor
>> + (append (list #$(ngircd-wrapper config)
>> + "--nodaemon" "--syslog"
>
> I’d use #:log-file and drop ‘--syslog’; I find it more convenient.
Do you find it more convenient because of the new 'herd log service'
functionality when #:log-file is used? Otherwise I usually prefer
searching messages in a single place as opposed to various log files.
>> + "--config" #$ngircd.conf)
>> + (if #$debug?
>> + '("--debug")
>> + '()))
>> + #:pid-file #$pid-file))
>
> If ngircd supports socket activation, I’d suggest using
> ‘make-systemd-constructor’ instead of #:pid-file: it equally achieves
> startup synchronization, but it allows for shorter startup times and can
> start the daemon lazily on-demand.
It's a bit weird to me that something as permanent as an IRC server
should be lazily started by socket activation, but it does support that,
and it simplifies things a bit, so I've made the switch, like so:
--8<---------------cut here---------------start------------->8---
1 file changed, 44 insertions(+), 43 deletions(-)
gnu/services/messaging.scm | 87 ++++++++++++++++++++++++++++++++++++++++++++-------------------------------------------
modified gnu/services/messaging.scm
@@ -1040,9 +1040,9 @@ (define-configuration ngircd-global ;[Global]
"Info text of the server. This will be shown by WHOIS and LINKS requests
for example.")
(listen
- (maybe-list-of-strings (list "::" "0.0.0.0"))
+ (list-of-strings (list "::" "0.0.0.0"))
"A list of IP address on which the server should listen. By default it
-listens on all interfaces.")
+listens on all configured IP addresses and interfaces.")
(motd-file
;; Provide an empty default file to avoid a warning when running --conftest
;; in the activation script.
@@ -1064,10 +1064,11 @@ (define-configuration ngircd-global ;[Global]
no password is required. PAM must be disabled for this option to have an
effect.")
(pid-file
- (string "/run/ngircd/ngircd.pid")
- "The file name where the PID of ngIRCd is written after it starts.")
+ maybe-string
+ "The file name where the PID of ngIRCd should be written after it starts.
+By default, no PID file is created.")
(ports
- (maybe-list-of-ports (list 6667))
+ (list-of-ports (list 6667))
"Port number(s) on which the server should listen for @emph{unencrypted}
connections.")
(server-uid
@@ -1207,7 +1208,7 @@ (define-configuration ngircd-ssl ;[SSL]
"File name of the SSL Server Key to be used for SSL connections, which is
required for SSL/TLS support.")
(ca-file
- (string "/etc/ssl/certs/ca-certificates.crt")
+ (maybe-string "/etc/ssl/certs/ca-certificates.crt")
"A file listing all the certificates of the trusted Certificate
Authorities.")
(ports
@@ -1439,7 +1440,6 @@ (define (ngircd-wrapper config)
(let* ((ngircd.conf (serialize-ngircd-configuration config))
(user group (ngircd-user+group config))
(global (ngircd-configuration-global config))
- (pid-file (ngircd-global-pid-file global))
(help-file (ngircd-global-help-file global))
(motd-file (ngircd-global-motd-file global))
(ssl (ngircd-configuration-ssl config))
@@ -1460,11 +1460,7 @@ (define (ngircd-wrapper config)
(target source))
(file-system-mapping
(source ngircd.conf)
- (target source))
- (file-system-mapping
- (source (string-append (dirname pid-file)))
- (target source)
- (writable? #t)))
+ (target source)))
(if (maybe-value-set? help-file)
(list (file-system-mapping
(source help-file)
@@ -1509,53 +1505,58 @@ (define (ngircd-wrapper config)
#:user user
#:group group
;; ngircd wants to look up users in /etc/passwd so run in the global user
- ;; namespace. Also preserve the PID namespaces otherwise the PID file
- ;; would contain an unrelated PID number and confuse Shepherd.
- #:namespaces (fold delq %namespaces '(net pid user)))))
+ ;; namespace.
+ #:namespaces (fold delq %namespaces '(net user)))))
(define (ngircd-shepherd-service config)
(match-record config <ngircd-configuration>
- (ngircd debug? global)
- (let ((ngircd.conf (serialize-ngircd-configuration config))
- (ngircd (file-append ngircd "/sbin/ngircd"))
- (pid-file (ngircd-global-pid-file global))
- (user group (ngircd-user+group config)))
+ (ngircd debug? global ssl)
+ (let* ((ngircd.conf (serialize-ngircd-configuration config))
+ (ngircd (file-append ngircd "/sbin/ngircd"))
+ (addresses (ngircd-global-listen global))
+ (ports* (ngircd-global-ports global))
+ (ports (if (and (maybe-value-set? ssl)
+ (maybe-value-set? (ngircd-ssl-ports ssl)))
+ (append ports* (ngircd-ssl-ports ssl))
+ ports*)))
(list (shepherd-service
(provision '(ngircd))
(requirement '(user-processes networking syslogd))
+ (modules (cons '(srfi srfi-1) %default-modules))
(actions (list (shepherd-configuration-action ngircd.conf)))
- (start #~(make-forkexec-constructor
+ (start #~(make-systemd-constructor
(append (list #$(ngircd-wrapper config)
"--nodaemon" "--syslog"
"--config" #$ngircd.conf)
(if #$debug?
'("--debug")
'()))
- #:pid-file #$pid-file))
-
- (stop #~(make-kill-destructor)))))))
+ ;; Compute endpoints for each listen addresses/ports
+ ;; combinations.
+ (append-map
+ (lambda (port)
+ (map (lambda (addr)
+ (endpoint
+ (addrinfo:addr
+ (car (getaddrinfo
+ addr
+ (number->string port)
+ (logior AI_NUMERICHOST
+ AI_NUMERICSERV))))))
+ (list #$@addresses)))
+ (list #$@ports))))
+ (stop #~(make-systemd-destructor)))))))
(define (ngircd-activation config)
(let* ((ngircd (file-append (ngircd-configuration-ngircd config)))
- (pid-file (ngircd-global-pid-file
- (ngircd-configuration-global config)))
- (ngircd.conf (serialize-ngircd-configuration config))
- (user _ (ngircd-user+group config)))
- (with-imported-modules (source-module-closure
- '((gnu build activation)))
- #~(begin
- (use-modules (gnu build activation)
- (ice-9 match))
- (define pw (match #$user
- ((? number?) (getpwuid #$user))
- ((? string?) (getpwnam #$user))))
- (mkdir-p/perms #$(dirname pid-file) pw #o755)
- (system (string-join
- (list #$(file-append ngircd "/sbin/ngircd")
- "--configtest" "--config" #$ngircd.conf
- ;; Ensure stdin is not a TTY to avoid pausing for a key
- ;; during boot when a problem is detected.
- "<" "/dev/null")))))))
+ (ngircd.conf (serialize-ngircd-configuration config)))
+ #~(begin
+ (system (string-join
+ (list #$(file-append ngircd "/sbin/ngircd")
+ "--configtest" "--config" #$ngircd.conf
+ ;; Ensure stdin is not a TTY to avoid pausing for a key
+ ;; during boot when a problem is detected.
+ "<" "/dev/null"))))))
(define ngircd-service-type
(service-type
--8<---------------cut here---------------end--------------->8---
>> + (mkdir-p/perms #$(dirname pid-file) pw #o755)
>> + (system (string-join
>> + (list #$(file-append ngircd "/sbin/ngircd")
>> + "--configtest" "--config" #$ngircd.conf
>> + ;; Ensure stdin is not a TTY to avoid pausing for a key
>> + ;; during boot when a problem is detected.
>> + "<" "/dev/null"))))))
>
> I think you can do:
>
> (parameterize ((current-input-port (%make-void-port "r")))
> (system* #$(file-append …) "--configtest" …))
Neat! I seemed to remember a buggy Guile interaction between 'system'
and stdin/stdout redirections from Guile (e.g. bug#43364), but it
doesn't seem to be an issue with your suggestion above.
> But! if it’s about checking the configuration, I would do it in a
> derivation (instead of at activation time), similar to how this is done
> for mcron.
Hm, I have looked at the mcron service and others, but haven't found it.
Could you please point me to the exact file/line?
>> + (test-assert "ngircd listens on TCP port 6667"
>> + (wait-for-tcp-port 6667 marionette))
>
> Maybe try a /JOIN command or whatever?
Done, using the 'ii' minimalist IRC client, with something like this:
--8<---------------cut here---------------start------------->8---
2 files changed, 81 insertions(+), 28 deletions(-)
gnu/services/messaging.scm | 12 +++++++-----
gnu/tests/messaging.scm | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------
modified gnu/services/messaging.scm
@@ -1174,11 +1174,13 @@ (define-configuration ngircd-options ;[Options]
requests by non-chanops as if they were coming from the server. Only enable
this if you have ircd-irc2 servers in your IRC network.")
(pam?
- (maybe-boolean #t)
- "Set to @code{#f} to disable all calls to the PAM library at runtime; all
-users connecting without password are allowed to connect, all passwords given
-will fail. Users identified without PAM are registered with a
-tilde (@samp{~}) prepended to their user name.")
+ (boolean #f)
+ "Set to @code{#t} to enable calls to the PAM library at runtime; all users
+connecting without password are allowed to connect, all passwords given will
+fail. Users identified without PAM are registered with a tilde (@samp{~})
+prepended to their user name. This defaults to @code{#f} in Guix because the
+service runs as a unpriveleged user and thus cannot authenticate other users
+via the @code{pam_unix} PAM module.")
(pam-is-optional?
(maybe-boolean #f)
"Set to @code{#t} to make PAM authentication optional, causing clients not
modified gnu/tests/messaging.scm
@@ -24,9 +24,13 @@ (define-module (gnu tests messaging)
#:use-module (gnu system)
#:use-module (gnu system vm)
#:use-module (gnu services)
+ #:use-module (gnu services base)
#:use-module (gnu services messaging)
#:use-module (gnu services networking)
+ #:use-module (gnu services ssh)
+ #:use-module (gnu packages irc)
#:use-module (gnu packages messaging)
+ #:use-module (gnu packages screen)
#:use-module (guix gexp)
#:use-module (guix store)
#:use-module (guix modules)
@@ -225,32 +229,50 @@ (define %test-bitlbee
;;;
(define %ngircd-os
- (marionette-operating-system
- (simple-operating-system
- (service dhcp-client-service-type)
- (service ngircd-service-type
- (ngircd-configuration
- (debug? #t)
- (global
- (ngircd-global
- (server-uid 990)
- (server-gid 990)))
- ;; There is no need to serialize the following sections, which
- ;; are all optional, but include them anyway to test the
- ;; serializers.
- (limits (ngircd-limits))
- (options (ngircd-options))
- (ssl (ngircd-ssl))
- (operators (list (ngircd-operator
- (name "maxim")
- (password "1234"))))
- (channels (list (ngircd-channel
- (name "#guix")))))))
- #:imported-modules (source-module-closure '((gnu services herd)))))
+ (operating-system
+ (inherit %simple-os)
+ (packages (cons* ii screen %base-packages))
+ (services
+ (cons*
+ (service dhcp-client-service-type)
+ ;; For ease of debugging. Run the vm with:
+ ;; '-nic user,model=virtio-net-pci,hostfwd=tcp::10022-:22'
+ (service openssh-service-type ;for ease of debugging
+ (openssh-configuration
+ (permit-root-login #t)
+ (allow-empty-passwords? #t)))
+ (service ngircd-service-type
+ (ngircd-configuration
+ (debug? #t)
+ (global
+ (ngircd-global
+ (server-uid 990)
+ (server-gid 990)))
+ ;; There is no need to serialize the following sections, which
+ ;; are all optional, but include them anyway to test the
+ ;; serializers.
+ (limits (ngircd-limits))
+ (options (ngircd-options))
+ (ssl (ngircd-ssl))
+ (operators (list (ngircd-operator
+ (name "apteryx")
+ (password "1234"))))
+ (channels
+ (list (ngircd-channel
+ (name "#guix")
+ (topic "GNU Guix | https://guix.gnu.org"))))))
+ %base-services))))
(define (run-ngircd-test)
(define vm
- (virtual-machine (operating-system %ngircd-os)))
+ (virtual-machine
+ (operating-system
+ (marionette-operating-system
+ %ngircd-os
+ #:imported-modules (source-module-closure
+ '((gnu build dbus-service)
+ (guix build utils)
+ (gnu services herd)))))))
(define test
(with-imported-modules '((gnu build marionette))
@@ -274,6 +296,35 @@ (define (run-ngircd-test)
(test-assert "ngircd listens on TCP port 6667"
(wait-for-tcp-port 6667 marionette))
+ (test-assert "basic irc operations function as expected"
+ (marionette-eval
+ '(begin
+ (use-modules ((gnu build dbus-service) #:select (with-retries))
+ (ice-9 textual-ports))
+
+ (define (write-command command)
+ (call-with-output-file "in"
+ (lambda (port)
+ (display (string-append command "\n") port))))
+
+ (define (grep-output text)
+ (with-retries 5 1 ;retry for 5 seconds
+ (string-contains (call-with-input-file "out" get-string-all)
+ text)))
+
+ (unless (zero? (system "ii -s localhost -i /tmp &"))
+ (error "error connecting to irc server"))
+
+ (with-retries 5 1
+ (chdir "/tmp/localhost")) ;move to FIFO directory
+
+ (write-command "/join #guix")
+ (grep-output "GNU Guix | https://guix.gnu.org")
+
+ (write-command "/oper apteryx 1234")
+ (grep-output "+o"))
+ marionette))
+
(test-end))))
(gexp->derivation "ngircd-test" test))
--8<---------------cut here---------------end--------------->8---
I'll send a v2 with the above changes. The remaining points, pending my
above questions, are:
- Use dedicated log file?
- Move configuration check to a derivation
--
Thanks,
Maxim
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.