From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ricardo Wurmus Subject: Re: [PATCH] Add pam-limits-service. Date: Wed, 20 Jul 2016 07:28:57 +0200 Message-ID: <87wpkgzxly.fsf@elephly.net> References: <87bn5tyfrn.fsf@elephly.net> <87io01h9uc.fsf@gnu.org> <874mbkxymn.fsf@elephly.net> <87vb40f4t6.fsf@gnu.org> <87wpofzzgv.fsf@elephly.net> <87egam9xnl.fsf@gnu.org> <87inzwzecd.fsf@elephly.net> <87eg6snitm.fsf@elephly.net> <874m7nqgc9.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:46477) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPk4J-0000WL-Mf for guix-devel@gnu.org; Wed, 20 Jul 2016 01:29:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bPk4H-0001JT-Ka for guix-devel@gnu.org; Wed, 20 Jul 2016 01:29:14 -0400 In-reply-to: <874m7nqgc9.fsf@gnu.org> List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Ludovic =?utf-8?Q?Court=C3=A8s?= Cc: guix-devel Ludovic Courtès writes: > Ricardo Wurmus 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 >> Date: Mon, 12 Oct 2015 07:11:51 +0200 >> Subject: [PATCH] services: Add pam-limits-service. >> >> * gnu/system/pam.scm (): 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 > objects. To do that we’d simply need something like: > > (service-type > (name 'limits) > ;; … > (compose concatenate) ;concatenate lists of > (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 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 >> + (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