Charybdis: Read MOTD files from /etc/charybdis. Add option to configure MOTD. #25512
Conversation
| @@ -51,46 +51,55 @@ in | |||
| ''; | |||
| }; | |||
|
|
|||
| motd = mkOption { | |||
| type = types.nullOr types.string; | |||
There was a problem hiding this comment.
types.string is deprecated, is using it intentional? Usually, types.str or types.lines are better choices, depending on what kind of merge semantics you want.
There was a problem hiding this comment.
Not intentional, I just copied it from the extant option defs (not being aware of the deprecation; thanks!). "lines" sounds reasonable to me; I've changed it accordingly.
| motd = mkOption { | ||
| type = types.nullOr types.lines; | ||
| default = null; | ||
| description = "Charybdis MOTD."; |
There was a problem hiding this comment.
As we're not managing any of the settings in the charybdis-config, we should inform the user here where the motd will be placed so that we can put it in this config.
There was a problem hiding this comment.
I've expanded the description to include the path to the MOTD file we're using.
This isn't really useful for use in ircd.conf, since charybdis doesn't allow MOTD location to be configured there, but is still good to include for users who'd rather write the file themselves rather than letting nix manage it.
| BANDB_DBPATH = "${cfg.statedir}/ban.db"; | ||
| }; | ||
| serviceConfig = { | ||
| ExecStart = "${charybdis}/bin/charybdis-ircd -foreground -logfile /dev/stdout -configfile ${configFile}"; |
There was a problem hiding this comment.
I was thinking about adding service reload support on changes to the configuration file at some point which is similar to the problem you're trying to solve. For instance the prosody service is also doing this. This behavior makes a lot of sense for such services that are best used with long-running connections and service uptime.
I'm not sure though if charybdis supports config reloading by sending signals. One can at least manually reload the config via IRC commands as an oper.
There was a problem hiding this comment.
This doesn't really seem to be documented, but diving into the source shows that yes, charybdis supports this - to be precise, it will reload (only) ircd.conf on SIGHUP, (only) MOTD on SIGUSR1 and (only) ban files on SIGUSR2. Splitting it up like this seems kinda silly to me, but there it is.
I don't know how one would set up sending of those signals in the nix config for the service, though to do it for other files than MOTD would require also symlinking those to somewhere they can be changed (probably /etc/charybdis, next to the motd).
There was a problem hiding this comment.
Oh, thanks for looking into this. Anyway, I'll try that if I get around eventually. This pulll request looks good for now so let's merge this. :)
| BANDB_DBPATH = "${cfg.statedir}/ban.db"; | ||
| }; | ||
| serviceConfig = { | ||
| ExecStart = "${charybdis}/bin/charybdis-ircd -foreground -logfile /dev/stdout -configfile ${configFile}"; |
There was a problem hiding this comment.
Oh, thanks for looking into this. Anyway, I'll try that if I get around eventually. This pulll request looks good for now so let's merge this. :)
Motivation for this change
The current charybdis build hardcodes the path to the MOTD file to the default file produced by the install in the nix store.
Charybdis doesn't have any option to override this path via cmdline flags or ircd.conf entries; this makes it exceedingly hard to configure.
We point this path at /etc/charybdis/ircd.motd instead and add a config option to write this file, for convenient configurability.
Things done
(nix.useSandbox on NixOS,
or option
build-use-sandboxinnix.confon non-NixOS)
nix-shell -p nox --run "nox-review wip"./result/bin/)