Ludovic Courtès <l...@gnu.org> writes:
> Ricardo Wurmus <rek...@elephly.net> skribis:
[…]
>> One advantage of using “pam-limits-entry” instead of a plain string is
>> that values are validated according to the documentation in “man 5
>> limits.conf”.
>
> Nice!
>
> Eventually, we should probably use a constructor in the spirit of (rnrs
> enums) to provide expansion-time validation, as already done in (gnu
> system nss) (info "(guile) rnrs enums").
Oh, that’s new to me. I was a little disappointed that records don’t
support what Haskell calls “smart constructors”. I’ll check out enums
and see if I can improve this later. (Added to my growing org-mode
list of Guix things.)
Thanks for the pointer!
>> From 3f5d7b405ac7faadd753719fe4100d8f6605d191 Mon Sep 17 00:00:00 2001
>> From: Ricardo Wurmus <rek...@elephly.net>
>> Date: Mon, 12 Oct 2015 07:11:51 +0200
>> Subject: [PATCH] services: Add pam-limits-service.
>>
>> * gnu/system/pam.scm (<pam-limits-entry>): New record type.
>> (pam-limits-entry, pam-limits-entry->string): New procedures.
>> * gnu/services/base.scm (pam-limits-service-type): New variable.
>> (pam-limits-service): New procedure.
>> * doc/guix.texi (Base Services): Document it.
>
> [...]
>
>> +@deffn {Scheme Procedure} pam-limits-service [#:limits @var{limits}]
>> +
>> +Return a service that installs a configuration file for the
>> +@code{pam_limits} module. The procedure optionally takes a list of
> ^^^^^^^^^^^^^^^^^^
> It would be nice to add an @uref to the on-line manual of pam_limits, if
> it exists.
Added a link.
>> +(define pam-limits-service-type
>> + (let ((security-limits
>> + ;; Create /etc/security containing the provided "limits.conf" file.
>> + (lambda (limits-file)
>> + `(("security"
>> + ,(computed-file
>> + "security"
>> + #~(begin (mkdir #$output)
>> + (stat #$limits-file)
>> + (symlink #$limits-file
>> + (string-append #$output
>> "/limits.conf"))))))))
>
> Indentation, rather:
>
> (begin
> (mkdir #$output)
> …)
Okay.
>> + (service-type
>> + (name 'limits)
>> + (extensions
>> + (list (service-extension etc-service-type security-limits)
>> + (service-extension pam-root-service-type
>> + (lambda _ (list pam-extension))))))))
>
> It may be useful to allow users to extend this service with additional
> <pam-limits-entry> objects. To do that we’d simply need something like:
>
> (service-type
> (name 'limits)
> ;; …
> (compose concatenate) ;concatenate lists of <pam-limits-entry>
> (extend append)) ;append them
>
> WDYT?
>
> This shouldn’t block this patch, though.
Currently the default limits are an empty list, so there doesn’t seem to
be any advantage over simply passing a list of <pam-limits-entry>
values. Or is composition/extension here about e.g. enabling some
specialised sub-service to inherit from pam-limits-service and modify
the list of entries?
>> +(define-record-type <pam-limits-entry>
>> + (make-pam-limits-entry domain type item value)
>
> Maybe just add a comment above with the URL of the reference manual.
Done.
> Otherwise LGTM, thank you!
Yay, pushing this to master now. Thank you for the patient review!
~~ Ricardo
