Subject: Re: [bug#77396] [PATCH] services: Add ngircd-service-type.
From: Maxim Cournoyer <[email-masked]>
To: Ludovic Courtès <[email-masked]>
Cc: [email-masked]
Date: Tue, 08 Apr 2025 10:52:54 +0900
List-Id: <debbugs-submit.debbugs.gnu.org>

Hi Ludovic,

Ludovic Courtès <[email-masked]> 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