[bug#28960] [PATCH] services: Add murmur.

[Ludovic Courtès]

Hi nee,

nee <address@hidden> 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 <address@hidden>
> 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.

> address@hidden {Data Type} murmur-configuration
> +The service type for the murmur server. An example configuration can look 
> like this:
> address@hidden
> +(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")))
> address@hidden 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’.’