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 #31 received at 77396-done <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-done <at> debbugs.gnu.org Subject: Re: [bug#77396] [PATCH] services: Add ngircd-service-type. Date: Tue, 08 Apr 2025 10:52:54 +0900
Hi Ludovic, Ludovic Courtès <ludo <at> gnu.org> writes: [...] > >>> 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? > > Well, it’s a tradeoff; common practice is to err on the safe side by > not exposing these low-level details. OK. I've bitten the bullet and exported all the field accessors (no longer exporting the record types). [...] >>> 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. > > The feature is ‘herd status service’, which shows recent messages, and > yes, that’s the main reason I find it more convenient. :-) > But if you think otherwise, that’s fine too. I see! I've made it its own log file to ensure Shepherd can show the tail of the log in 'herd status ngircd', which seems useful. Here's the change, with the accessors exposed as well: --8<---------------cut here---------------start------------->8--- 1 file changed, 73 insertions(+), 13 deletions(-) gnu/services/messaging.scm | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------- modified gnu/services/messaging.scm @@ -63,31 +63,89 @@ (define-module (gnu services messaging) bitlbee-configuration? bitlbee-service-type - <ngircd-configuration> + ngircd-service-type ngircd-configuration ngircd-configuration? - <ngircd-global> + ngircd-configuration-ngircd + ngircd-configuration-debug? + ngircd-configuration-global + ngircd-configuration-limits + ngircd-configuration-options + ngircd-configuration-ssl + ngircd-configuration-operators + ngircd-configuration-servers + ngircd-configuration-channels ngircd-global ngircd-global? - <ngircd-limits> + ngircd-global-name + ngircd-global-admin-info-1 + ngircd-global-admin-info-2 + ngircd-global-admin-email + ngircd-global-help-file + ngircd-global-info + ngircd-global-listen + ngircd-global-motd-file + ngircd-global-motd-phrase + ngircd-global-network + ngircd-global-password + ngircd-global-pid-file + ngircd-global-ports + ngircd-global-server-uid + ngircd-global-server-gid ngircd-limits ngircd-limits? - <ngircd-options> + ngircd-limits-connect-retry + ngircd-limits-max-connections + ngircd-limits-max-connections-ip + ngircd-limits-max-joins + ngircd-limits-max-list-size + ngircd-limits-ping-timeout + ngircd-limits-pong-timeout ngircd-options ngircd-options? - <ngircd-ssl> + ngircd-options-allowed-channel-types + ngircd-options-allow-remote-oper? + ngircd-options-connect-ipv4? + ngircd-options-connect-ipv6? + ngircd-options-dns? + ngircd-options-more-privacy? + ngircd-options-notice-before-registration? + ngircd-options-oper-can-use-mode? + ngircd-options-oper-chan-p-auto-op? + ngircd-options-oper-server-mode? + ngircd-options-pam? + ngircd-options-pam-is-optional? + ngircd-options-require-auth-ping? ngircd-ssl ngircd-ssl? - <ngircd-operator> + ngircd-ssl-cert-file + ngircd-ssl-key-file + ngircd-ssl-ca-file + ngircd-ssl-ports + ngircd-ssl-cipher-list + ngircd-ssl-dh-file ngircd-operator ngircd-operator? - <ngircd-server> + ngircd-operator-name + ngircd-operator-password + ngircd-operator-mask ngircd-server ngircd-server? - <ngircd-channel> + ngircd-server-name + ngircd-server-host + ngircd-server-my-password + ngircd-server-peer-password + ngircd-server-bind + ngircd-server-port + ngircd-server-group + ngircd-server-passive? + ngircd-server-ssl-connect? ngircd-channel ngircd-channel? - ngircd-service-type + ngircd-channel-name + ngircd-channel-topic + ngircd-channel-modes + ngircd-channel-key-file quassel-configuration quassel-service-type @@ -1458,8 +1516,9 @@ (define (ngircd-wrapper config) #:mappings (append (list (file-system-mapping - (source "/dev/log") ;for syslog - (target source)) + (source "/var/log/ngircd.log") + (target source) + (writable? #t)) (file-system-mapping (source ngircd.conf) (target source))) @@ -1528,7 +1587,7 @@ (define (ngircd-shepherd-service config) (actions (list (shepherd-configuration-action ngircd.conf))) (start #~(make-systemd-constructor (append (list #$(ngircd-wrapper config) - "--nodaemon" "--syslog" + "--nodaemon" "--config" #$ngircd.conf) (if #$debug? '("--debug") @@ -1546,7 +1605,8 @@ (define (ngircd-shepherd-service config) (logior AI_NUMERICHOST AI_NUMERICSERV)))))) (list #$@addresses))) - (list #$@ports)))) + (list #$@ports)) + #:log-file "/var/log/ngircd.log")) (stop #~(make-systemd-destructor))))))) (define (ngircd-activation config) --8<---------------cut here---------------end--------------->8--- [...] >>> 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? > > See ‘validated-file’ in mcron.scm. Thanks. Done like so: --8<---------------cut here---------------start------------->8--- 1 file changed, 18 insertions(+), 17 deletions(-) gnu/services/messaging.scm | 35 ++++++++++++++++++----------------- modified gnu/services/messaging.scm @@ -1102,8 +1102,8 @@ (define-configuration ngircd-global ;[Global] "A list of IP address on which the server should listen. By default it 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. + ;; Provide an empty default file to avoid a warning when running + ;; --configtest to validate the configuration file. (file-like (plain-file "ngircd.motd" "")) "Text file with the @i{message of the day} (MOTD). This message will be shown to all users connecting to the server.") @@ -1490,9 +1490,21 @@ (define (ngircd-account config) (define (serialize-ngircd-configuration config) "Return a file-like object corresponding to the serialized <ngircd-configuration> record." - (mixed-text-file "ngircd.conf" - (serialize-configuration - config ngircd-configuration-fields))) + (let ((ngircd (file-append (ngircd-configuration-ngircd config) + "/sbin/ngircd")) + (ngircd.conf (mixed-text-file "unvalidated-ngircd.conf" + (serialize-configuration + config ngircd-configuration-fields)))) + (computed-file + "ngircd.conf" + (with-imported-modules '((guix build utils)) + #~(begin + (use-modules (guix build utils)) + ;; Ensure stdin is not connected to a TTY source to avoid ngircd + ;; configtest blocking with a confirmation prompt. + (parameterize ((current-input-port (%make-void-port "r"))) + (invoke #+ngircd "--config" #$ngircd.conf "--configtest" )) + (copy-file #$ngircd.conf #$output)))))) (define (ngircd-wrapper config) "Take CONFIG, a <ngircd-configuration> object, and provide a least-authority @@ -1609,15 +1621,6 @@ (define (ngircd-shepherd-service config) #:log-file "/var/log/ngircd.log")) (stop #~(make-systemd-destructor))))))) -(define (ngircd-activation config) - (let* ((ngircd (file-append (ngircd-configuration-ngircd config))) - (ngircd.conf (serialize-ngircd-configuration config))) - ;; Ensure stdin is not a TTY to avoid pausing for a key during boot - ;; when a problem is detected. - #~(parameterize ((current-input-port (%make-void-port "r"))) - (system* #$(file-append ngircd "/sbin/ngircd") - "--configtest" "--config" #$ngircd.conf)))) - (define ngircd-service-type (service-type (name 'ngircd) @@ -1627,9 +1630,7 @@ (define ngircd-service-type (service-extension profile-service-type (compose list ngircd-configuration-ngircd)) (service-extension account-service-type - ngircd-account) - (service-extension activation-service-type - ngircd-activation))) + ngircd-account))) (description "Run @url{https://ngircd.barton.de/, ngIRCd}, a lightweight @acronym{IRC, Internet Relay Chat} daemon."))) [back] --8<---------------cut here---------------end--------------->8--- The improved test suite still passes; I've pushed it as commit c9524b5841. Thanks for the review! -- Maxim
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.