Subject: Re: bug#50882: [PATCH] gnu: services: Add darkhttpd service

From: jgart <[email-masked]>
Date: Thu, 7 Jul 2022 19:20:27 -0500

On Thu, 07 Jul 2022 14:02:36 -0400 Maxim Cournoyer <[email-masked]> wrote:
> tag 50882 moreinfo
> thanks
> 
> Hello jgart,
> 
> Ludovic Courtès <[email-masked]> writes:
> 
> > Hi,
> >
> > jgart <[email-masked]> skribis:
> >
> >> * gnu/services/web.scm (<darkhttpd-configuration>): New record type.
> >> (darkhttpd-accounts, darkhttpd-shepherd-service): New procedures.
> >> (darkhttpd-service-type): New variable.
> >> * doc/guix.texi (Web Services): Adds documentation for darkhttpd.
> >
> > Overall LGTM!  Some comments and suggestions below.
> >
> >> +@cindex darkhttpd
> >> +@uref{https://unix4lyfe.org/darkhttpd/, darkhttpd} is a web server with a 
> >> +focus on security and having a small memory footprint.
> >> +
> >> +Some security features are the following:
> >> +
> >> +@itemize
> >> +@item Logging accesses, including Referer and User-Agent.
> >> +@item Can chroot.
> >> +@item Can drop privileges.
> >> +@item Impervious to /../ sniffing.
> >> +@item Times out idle connections.
> >> +@item Drops overly long requests.
> >> +@end itemize 
> >
> > I’d replace the bullet list with a simple sentence like: “Among other
> > things, it can change root directories, drop privileges, it times out on
> > idle connections and can drop overly long requests.”
> >
> >> +@deffn {Scheme Variable} darkhttpd-service-type
> >> +This is the type of the darkhttpd service, whose value should be a
> >> +@code{darkhttpd-service-type} object, as in this example:
> >> +
> >> +@lisp
> >> +(service darkhttpd-service-type
> >> +	 (darkhttpd-configuration
> >
> > Please don’t use tabs.
> >
> >> +@end table
> >> +@end deftp
> >>  @node Certificate Services
> >
> > Missing newline before @node.  :-)
> >
> >> +  (mimetypes               darkhttpd-configuration-mimetypes
> >> +                           (default #f))
> >> +  (default-mimetype        darkhttpd-configuration-default-mimetype
> >
> > Rather ‘mime-type’ (two words).
> >
> >> +(define darkhttpd-shepherd-service
> >> +  (match-lambda
> >> +    (($ <darkhttpd-configuration> package content port address 
> >> +                                  maximum-connections log-file chroot? 
> >> +                                  daemonize? index-file do-not-serve-listing?
> >> +                                  mimetypes default-mimetype 
> >> +                                  drop-user-priviledges drop-group-priviledges 
> >> +                                  write-pid-file disable-keep-alive? 
> >> +                                  forward forward-all 
> >> +                                  no-server-id? enable-ipv6? 
> >> +                                  user group)
> >
> > Rather use ‘match-record’ here, to make sure we’re getting the right
> > fields.
> >
> >> +(define darkhttpd-accounts
> >> +  (match-lambda
> >> +    (($ <darkhttpd-configuration> _ _ _ _ _ _ _ _ 
> >> +                                  _ _ _ _ _ _ _ _ 
> >> +                                  _ _ user group)
> >
> > In such a case, simply call ‘darkhttpd-configuration-user’ and
> > ‘darkhttpd-configuration-group’; it’s much less error-prone!
> >
> >> +(define darkhttpd-service-type
> >> +  (service-type
> >> +   (name 'guix)
> >> +   (extensions
> >> +    (list (service-extension account-service-type
> >> +                             darkhttpd-accounts)
> >> +          (service-extension shepherd-root-service-type
> >> +                             darkhttpd-shepherd-service)))
> >> +   (default-value (darkhttpd-configuration))))
> >
> > Please add a ‘description’ field.
> >
> > Could you also add a system test, under (gnu tests web)?  You can start
> > by copying the nginx test; it should take around ~20 lines.
> 
> Friendly ping about the above requests from Ludovic :-).

Arun Isaac convinced me to not write a service for this one since it's
common usage is for quick serving by simply running `darkhttpd ...` from
the command line. I think that guile bindings for every CLI feature of
darkhttpd doesn't add to that aim. I might change my mind later on this.
Feel free to close this one for now.

It was a fun exercise though. 

Ludo, thanks for the review. It was much appreciated!

all best,

jgart

https://whereis.みんな/