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

[nee]

Hello,
thanks to both ludo and ng0 looking at my patch.

24.10.2017 07:04 Ludovic Courtès:
>> 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.
> 
Okay, I changed this line in the commit message.
>> 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.
> 
Whoops, I untabified it.

>> +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.
> 
I reworded that part a little. It's about the mumble "SuperUser" who can
create channels and do moderator stuff like muting, banning, and
promoting users.

>> +(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?’.
> 
Okay, I'm just slightly confused whether the question mark is only used
for predicate procedures or everything that related to booleans.
I think there was discussion on the guile list about this, I'll read up
on it later.

>> +(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
>> …
> 
> Likewise: use the accessor procedures instead of this.
> 
>> +(define murmur-shepherd-service
>> …
> Use the accessors instead.
>
Right, that grew way too big. I removed most of the match blocks.
I like having the short names when it comes to stitching together the
actual config though, so I kept that one.
If that's still a no-go I'll make another update with accessors.

If the main problem here is the positional binding, is there a function
to match record fields by name that I could use instead?
It doesn't seem like it would be too complicated to write a macro for
this with the record-accessor procedure from srfi-9.

>> +(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)
>               …
>               ))))
> 
> ?
> 
Okay I changed it. I had copied this from the fcgiwrap service.

> Could you send an updated patch?
Here it is :-)

I also noticed a missing equal sign after rememberchannel in the
defaultconfig and added that.

0001-services-Add-murmur.patch
Description: Text Data