GNU bug report logs - #28960
[PATCH] services: Add murmur.

Previous Next

Full log

Message #11 received at 28960 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: nee <nee <at> cock.li>
Cc: 28960 <at> debbugs.gnu.org
Subject: Re: [bug#28960] [PATCH] services: Add murmur.
Date: Mon, 23 Oct 2017 22:04:17 -0700

Hi nee,

nee <nee <at> cock.li> skribis:

> Hello, this patch adds a murmur service.
> Murmur is the biggest implementation of a mumble voice chat server. The
> murmur executable is already packaged in the mumble package.

Neat!

> From 74618e5a39198077327f14362d8d98538f4d39ab Mon Sep 17 00:00:00 2001
> From: nee <nee.git <at> cock.li>
> Date: Sat, 14 Oct 2017 11:27:50 +0200
> Subject: [PATCH] services: Add murmur.
>
> * gnu/services/telephony.scm: New file.
> * gnu/local.mk: Add it.
> * doc/guix.texi: Document it.

You can write:

  * doc/guix.texi (Telephony Services): New node.

> +@deftp {Data Type} murmur-configuration
> +The service type for the murmur server. An example configuration can look like this:
> +@example
> +(service murmur-service-type
> +         (murmur-configuration
> +	  (welcome-text "Welcome to this mumble server running on GuixSD!")
> +          (cert-required #t) ; disallow text password logins
> +          (ssl-cert "/etc/letsencrypt/live/mumble.example.com/fullchain.pem")
> +          (ssl-key "/etc/letsencrypt/live/mumble.example.com/privkey.pem")))
> +@end example

Please don’t use tabs.

> +After reconfiguring your system, you have to manually set the
> +SuperUser password with the command that is printed during the activation phase.

That sounds quite unusual.  Perhaps you need @code{SuperUser}, if you
literally mean the “SuperUser” account in Mumble?

> +Then you can use the @code{mumble} client to
> +login as new user, register, and logout.
> +For the next step login with the name "SuperUser" and the SuperUser password

Same here.

> +(define-record-type* <murmur-configuration> murmur-configuration
> +  make-murmur-configuration
> +  murmur-configuration?
> +  (package               murmur-configuration-package ;<package>
> +                         (default mumble))
> +  (user                  murmur-configuration-user
> +                         (default "murmur"))
> +  (group                 murmur-configuration-group
> +                         (default "murmur"))
> +  (port                  murmur-configuration-port
> +                         (default 64738))

[...]

> +  (allow-html            murmur-configuration-allow-html
> +                         (default #f))
> +  (allow-ping            murmur-configuration-allow-ping
> +                         (default #f))

Add a question mark since these are Boolean options.  So ‘allow-html?’
and ‘allow-ping?’.

> +(define (default-murmur-config
> +          package user group port welcome-text server-password
> +          max-users max-user-bandwidth database-file log-file pid-file
> +          autoban-attempts autoban-timeframe autoban-time
> +          opus-threshold channel-nesting-limit channelname-regex username-regex
> +          text-message-length image-message-length cert-required
> +          remember-channel allow-html allow-ping bonjour send-version log-days
> +          obfuscate-ips ssl-cert ssl-key ssl-dh-params ssl-ciphers
> +          public-registration)

This many positional parameters is not reasonable.  :-)  Just pass a
<murmur-configuration> directly, and use the accessor procedures.

> +(define murmur-activation
> +  (match-lambda
> +    (($ <murmur-configuration>
> +        package user group port welcome-text server-password
> +        max-users max-user-bandwidth database-file log-file pid-file
> +        autoban-attempts autoban-timeframe autoban-time
> +        opus-threshold channel-nesting-limit channelname-regex username-regex
> +        text-message-length image-message-length cert-required remember-channel
> +        allow-html allow-ping bonjour send-version log-days obfuscate-ips
> +        ssl-cert ssl-key ssl-dh-params ssl-ciphers public-registration file)

Likewise: use the accessor procedures instead of this.

> +(define murmur-accounts
> +  (match-lambda
> +    (($ <murmur-configuration> _ user group)
> +     (filter identity
> +             (list
> +              (and (equal? group "murmur")
> +                   (user-group
> +                    (name "murmur")
> +                    (system? #t)))
> +              (and (equal? user "murmur")
> +                   (user-account
> +                    (name "murmur")
> +                    (group group)
> +                    (system? #t)
> +                    (comment "Murmur Daemon")
> +                    (home-directory "/var/empty")
> +                    (shell (file-append shadow "/sbin/nologin")))))))))


Why not just

  (match-lambda
     (($ <murmur-configuration> _ user group)
      (list (user-group (name group) (system? #t))
            (user-account
              (name user)
              (group group)
              (system? #t)
              …
              ))))

?

> +(define murmur-shepherd-service
> +  (match-lambda
> +    (($ <murmur-configuration>
> +        package user group port welcome-text server-password
> +        max-users max-user-bandwidth database-file log-file pid-file
> +        autoban-attempts autoban-timeframe autoban-time
> +        opus-threshold channel-nesting-limit channelname-regex username-regex
> +        text-message-length image-message-length cert-required remember-channel
> +        allow-html allow-ping bonjour send-version log-days obfuscate-ips
> +        ssl-cert ssl-key ssl-dh-params ssl-ciphers public-registration file)

Use the accessors instead.

Could you send an updated patch?

Thanks,
Ludo’.’

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.